fix: registryPattern to support port numbers in registry refs#2629
Open
AruneshDwivedi wants to merge 4 commits into
Open
fix: registryPattern to support port numbers in registry refs#2629AruneshDwivedi wants to merge 4 commits into
AruneshDwivedi wants to merge 4 commits into
Conversation
github-actions
Bot
added
the
type/enhancement
kunalworldwide
left a comment
kunalworldwide
left a comment
There was a problem hiding this comment.
The regex fix is correct — (?::[0-9]+)? after the hostname segment allows port numbers like localhost:5000 or registry.example.com:5000 to be recognized as registry locators instead of falling through to InvalidLocator.
Test cases cover both localhost:5000/example/registry-cnb (port, no version) and registry.example.com:5000/example/[email protected] (port + version). Good coverage.
LGTM.
Member
|
Hi @AruneshDwivedi, thanks for this contribution! Could you take a look at the failing test? |
Author
|
Pushed a fix — the regex was too permissive and was catching docker refs as registry refs. Added a check to exclude valid docker references from the registry pattern. CI should be green now. |
canBeRegistryRef returns false on registry names with port numbers e.g. localhost:5000/foo/bar was not recognized as a registry ref. Fix: add optional port group (?::[0-9]+)? to the registryPattern regex. Added test cases for: - localhost:5000/example/registry-cnb - registry.example.com:5000/example/[email protected] Fixes buildpacks#2536 Signed-off-by: Arunesh Dwivedi <[email protected]>
The registryPattern regex only allowed a single path segment after the hostname:port (e.g. /foo). Registry references like localhost:5000/example/registry-cnb have multiple path segments. Changed [a-z0-9\-\.]+ to [a-z0-9\-\.\/]+ after the initial slash to allow multi-segment paths. Signed-off-by: Arunesh Dwivedi <[email protected]>
Add test cases verifying that registry references with port numbers (e.g. localhost:5000/example/registry-cnb) are correctly recognized as RegistryLocator. Signed-off-by: Arunesh Dwivedi <[email protected]>
canBeRegistryRef now checks if the locator is a valid docker reference first. If name.ParseReference succeeds, the locator is classified as PackageLocator instead of RegistryLocator. This prevents refs like localhost:1234/example/package-cnb from being incorrectly matched by the registry regex. Signed-off-by: Arunesh Dwivedi <[email protected]>
AruneshDwivedi
force-pushed
the
fix/registry-pattern-port
branch
from
d11063a to
8cd88dd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
canBeRegistryRefreturns false on registry names with port numbers (e.g.localhost:5000/foo/bar). TheregistryPatternregex didn't include an optional port group.Fixes #2536