fix: support docker:// URIs with port numbers in registry references#2543
Open
Onyx2406 wants to merge 1 commit intobuildpacks:mainfrom
Open
fix: support docker:// URIs with port numbers in registry references#2543Onyx2406 wants to merge 1 commit intobuildpacks:mainfrom
Onyx2406 wants to merge 1 commit intobuildpacks:mainfrom
Conversation
Strip the docker:// scheme prefix before passing the reference to name.ParseReference, which expects bare Docker references like localhost:5000/foo/bar rather than docker://localhost:5000/foo/bar. Previously, docker:// URIs with port numbers (e.g. docker://localhost:5000/foo/bar) were not recognized as valid PackageLocator references because name.ParseReference failed to parse the full URI including the scheme prefix. Fixes buildpacks#2536
github-actions
bot
added
the
type/enhancement
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
1 participant
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.
Summary
docker://scheme prefix before passing references toname.ParseReferencedocker://localhost:5000/foo/barbeing rejected as an invalid locatorMotivation
Fixes #2536 — Docker URIs with port numbers (e.g.
docker://localhost:5000/foo/bar) were not recognized as validPackageLocatorreferences. Thego-containerregistrylibrary'sname.ParseReferenceexpects bare Docker references without the URI scheme prefix.Root Cause
In
GetLocatorType, when adocker://URI is detected, the full string including the scheme was passed toname.ParseReference. This works for simple refs likedocker://cnbs/some-bp(wheredockeris interpreted as a registry host), but fails for refs with explicit ports likedocker://localhost:5000/foo/bar.Changes
pkg/buildpack/locator_type.go:strings.TrimPrefix(locator, "docker://")beforename.ParseReferencepkg/buildpack/locator_type_test.go: Add 3 test cases for docker:// URIs with port numbersTest plan
docker://localhost:5000/foo/bar→PackageLocatordocker://localhost:5000/foo/bar:latest→PackageLocatordocker://myregistry:8080/cnbs/some-bp→PackageLocator