cli/command/manifest: remove redundant uses of ParseRepositoryInfo - #5877
Merged
thaJeztah merged 1 commit intoMar 3, 2025
Merged
Conversation
[ParseRepositoryInfo] parses an image reference and returns information about the Repository and the registry. As part of this, it validates if the registry's hostname is considered valid using [ValidateIndexName], as well as normalizing the image reference to strip tags and digests using [reference.TrimNamed]. ValidateIndexName only provides very limited value; the only validation happening is to check for the hostname to not start, or end with a hyphen. The cli/command/manifest package used ParseRepositoryInfo in various locations where only the repository name was used (i.e., the result of `reference.TrimNamed` on the given reference), and in one location only used it to validate the registry name. For buildPushRequest, the call was fully redundant, as [RepoNameForReference] was used on the result, calling [newDefaultRepositoryEndpoint], which uses ParseRepositoryInfo internally, so we were only repeating that work. This patch removes uses of ParseRepositoryInfo in those places, and instead calling [reference.TrimNamed] directly. [ParseRepositoryInfo]: https://github.com/moby/moby/blob/41f781fab3cae181cc9be3ec93cd91b99466fa84/registry/config.go#L375-L381 [ValidateIndexName]: https://github.com/moby/moby/blob/41f781fab3cae181cc9be3ec93cd91b99466fa84/registry/config.go#L288-L299 [reference.TrimNamed]: https://github.com/moby/moby/blob/41f781fab3cae181cc9be3ec93cd91b99466fa84/registry/config.go#L369 [RepoNameForReference]: https://github.com/docker/cli/blob/fe0a8d27912dc6fddc60cedcd35bbef27b776355/cli/registry/client/endpoint.go#L107-L110 [newDefaultRepositoryEndpoint]: https://github.com/docker/cli/blob/fe0a8d27912dc6fddc60cedcd35bbef27b776355/cli/registry/client/endpoint.go#L33-L38 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5877 +/- ##
==========================================
+ Coverage 59.32% 59.33% +0.01%
==========================================
Files 353 353
Lines 29748 29731 -17
==========================================
- Hits 17647 17640 -7
+ Misses 11113 11107 -6
+ Partials 988 984 -4 |
thaJeztah
marked this pull request as ready for review
March 1, 2025 14:50
vvoland
approved these changes
Mar 3, 2025
Benehiko
approved these changes
Mar 3, 2025
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
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.
cli/command/manifest: remove redundant uses of ParseRepositoryInfo
ParseRepositoryInfo parses an image reference and returns information
about the Repository and the registry. As part of this, it validates if
the registry's hostname is considered valid using ValidateIndexName,
as well as normalizing the image reference to strip tags and digests
using reference.TrimNamed.
ValidateIndexName only provides very limited value; the only validation
happening is to check for the hostname to not start, or end with a hyphen.
The cli/command/manifest package used ParseRepositoryInfo in various
locations where only the repository name was used (i.e., the result
of
reference.TrimNamedon the given reference), and in one locationonly used it to validate the registry name.
For buildPushRequest, the call was fully redundant, as RepoNameForReference
was used on the result, calling newDefaultRepositoryEndpoint, which
uses ParseRepositoryInfo internally, so we were only repeating that work.
This patch removes uses of ParseRepositoryInfo in those places, and instead
calling reference.TrimNamed directly.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)