From 55a83aff233d987348d74061b11f1f64570b0a9e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sat, 1 Mar 2025 15:30:30 +0100 Subject: [PATCH] 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.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 --- cli/command/manifest/create_list.go | 6 ------ cli/command/manifest/inspect.go | 6 +----- cli/command/manifest/push.go | 28 +++++++--------------------- 3 files changed, 8 insertions(+), 32 deletions(-) diff --git a/cli/command/manifest/create_list.go b/cli/command/manifest/create_list.go index d6ca591ab2..5a399d5758 100644 --- a/cli/command/manifest/create_list.go +++ b/cli/command/manifest/create_list.go @@ -7,7 +7,6 @@ import ( "github.com/docker/cli/cli" "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/manifest/store" - "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -42,11 +41,6 @@ func createManifestList(ctx context.Context, dockerCLI command.Cli, args []strin return errors.Wrapf(err, "error parsing name for manifest list %s", newRef) } - _, err = registry.ParseRepositoryInfo(targetRef) - if err != nil { - return errors.Wrapf(err, "error parsing repository name for manifest list %s", newRef) - } - manifestStore := dockerCLI.ManifestStore() _, err = manifestStore.GetList(targetRef) switch { diff --git a/cli/command/manifest/inspect.go b/cli/command/manifest/inspect.go index 75acb8b604..23ea2e69c6 100644 --- a/cli/command/manifest/inspect.go +++ b/cli/command/manifest/inspect.go @@ -11,7 +11,6 @@ import ( "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/manifest/types" "github.com/docker/distribution/manifest/manifestlist" - "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -113,10 +112,7 @@ func printManifest(dockerCli command.Cli, manifest types.ImageManifest, opts ins func printManifestList(dockerCli command.Cli, namedRef reference.Named, list []types.ImageManifest, opts inspectOptions) error { if !opts.verbose { - targetRepo, err := registry.ParseRepositoryInfo(namedRef) - if err != nil { - return err - } + targetRepo := reference.TrimNamed(namedRef) manifests := []manifestlist.ManifestDescriptor{} // More than one response. This is a manifest list. diff --git a/cli/command/manifest/push.go b/cli/command/manifest/push.go index 67c370ec78..2e1efd15e5 100644 --- a/cli/command/manifest/push.go +++ b/cli/command/manifest/push.go @@ -15,7 +15,6 @@ import ( "github.com/docker/distribution/manifest/manifestlist" "github.com/docker/distribution/manifest/ocischema" "github.com/docker/distribution/manifest/schema2" - "github.com/docker/docker/registry" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -100,11 +99,7 @@ func buildPushRequest(manifests []types.ImageManifest, targetRef reference.Named return req, err } - targetRepo, err := registry.ParseRepositoryInfo(targetRef) - if err != nil { - return req, err - } - targetRepoName, err := registryclient.RepoNameForReference(targetRepo.Name) + targetRepoName, err := registryclient.RepoNameForReference(targetRef) if err != nil { return req, err } @@ -134,11 +129,7 @@ func buildPushRequest(manifests []types.ImageManifest, targetRef reference.Named } func buildManifestList(manifests []types.ImageManifest, targetRef reference.Named) (*manifestlist.DeserializedManifestList, error) { - targetRepoInfo, err := registry.ParseRepositoryInfo(targetRef) - if err != nil { - return nil, err - } - + targetRepo := reference.TrimNamed(targetRef) descriptors := []manifestlist.ManifestDescriptor{} for _, imageManifest := range manifests { if imageManifest.Descriptor.Platform == nil || @@ -147,7 +138,7 @@ func buildManifestList(manifests []types.ImageManifest, targetRef reference.Name return nil, errors.Errorf( "manifest %s must have an OS and Architecture to be pushed to a registry", imageManifest.Ref) } - descriptor, err := buildManifestDescriptor(targetRepoInfo, imageManifest) + descriptor, err := buildManifestDescriptor(targetRepo, imageManifest) if err != nil { return nil, err } @@ -157,14 +148,9 @@ func buildManifestList(manifests []types.ImageManifest, targetRef reference.Name return manifestlist.FromDescriptors(descriptors) } -func buildManifestDescriptor(targetRepo *registry.RepositoryInfo, imageManifest types.ImageManifest) (manifestlist.ManifestDescriptor, error) { - repoInfo, err := registry.ParseRepositoryInfo(imageManifest.Ref) - if err != nil { - return manifestlist.ManifestDescriptor{}, err - } - - manifestRepoHostname := reference.Domain(repoInfo.Name) - targetRepoHostname := reference.Domain(targetRepo.Name) +func buildManifestDescriptor(targetRepo reference.Named, imageManifest types.ImageManifest) (manifestlist.ManifestDescriptor, error) { + manifestRepoHostname := reference.Domain(reference.TrimNamed(imageManifest.Ref)) + targetRepoHostname := reference.Domain(reference.TrimNamed(targetRepo)) if manifestRepoHostname != targetRepoHostname { return manifestlist.ManifestDescriptor{}, errors.Errorf("cannot use source images from a different registry than the target image: %s != %s", manifestRepoHostname, targetRepoHostname) } @@ -182,7 +168,7 @@ func buildManifestDescriptor(targetRepo *registry.RepositoryInfo, imageManifest manifest.Platform = *platform } - if err = manifest.Descriptor.Digest.Validate(); err != nil { + if err := manifest.Descriptor.Digest.Validate(); err != nil { return manifestlist.ManifestDescriptor{}, errors.Wrapf(err, "digest parse of image %q failed", imageManifest.Ref) }