diff --git a/pkg/image/remote/remote.go b/pkg/image/remote/remote.go index 2baa578b80..787c5381d0 100644 --- a/pkg/image/remote/remote.go +++ b/pkg/image/remote/remote.go @@ -52,37 +52,27 @@ func RetrieveRemoteImage(image string, opts config.RegistryOptions, customPlatfo } if newRegURLs, found := opts.RegistryMaps[ref.Context().RegistryStr()]; found { - for _, regToMapTo := range newRegURLs { + for _, registryMapping := range newRegURLs { - //extract custom path if any in all registry map and clean regToMapTo to only the registry without the path - custompath, regToMapTo := extractPathFromRegistryURL(regToMapTo) - //normalize reference is call in every fallback to ensure that the image is normalized to the new registry include the image prefix - ref, err = normalizeReference(ref, image, custompath) + regToMapTo, repositoryPrefix := parseRegistryMapping(registryMapping) - if err != nil { - return nil, err - } + insecurePull := opts.InsecurePull || opts.InsecureRegistries.Contains(regToMapTo) - var newReg name.Registry - if opts.InsecurePull || opts.InsecureRegistries.Contains(regToMapTo) { - newReg, err = name.NewRegistry(regToMapTo, name.WeakValidation, name.Insecure) - } else { - newReg, err = name.NewRegistry(regToMapTo, name.StrictValidation) - } + remappedRepository, err := remapRepository(ref.Context(), regToMapTo, repositoryPrefix, insecurePull) if err != nil { return nil, err } - //ref will be already use library/ or the custom path in registry map suffix - ref := setNewRegistry(ref, newReg) - logrus.Infof("Retrieving image %s from mapped registry %s", ref, regToMapTo) + + remappedRef := setNewRepository(ref, remappedRepository) + + logrus.Infof("Retrieving image %s from mapped registry %s", remappedRef, regToMapTo) retryFunc := func() (v1.Image, error) { - return remoteImageFunc(ref, remoteOptions(regToMapTo, opts, customPlatform)...) + return remoteImageFunc(remappedRef, remoteOptions(regToMapTo, opts, customPlatform)...) } var remoteImage v1.Image - var err error if remoteImage, err = util.RetryWithResult(retryFunc, opts.ImageDownloadRetry, 1000); err != nil { - logrus.Warnf("Failed to retrieve image %s from remapped registry %s: %s. Will try with the next registry, or fallback to the original registry.", ref, regToMapTo, err) + logrus.Warnf("Failed to retrieve image %s from remapped registry %s: %s. Will try with the next registry, or fallback to the original registry.", remappedRef, regToMapTo, err) continue } @@ -119,19 +109,26 @@ func RetrieveRemoteImage(image string, opts config.RegistryOptions, customPlatfo return remoteImage, err } -// normalizeReference adds the library/ or the {path}/ in registry map suffix to images without it. -// -// It is mostly useful when using a registry maps that is not able to perform -// this fix automatically add library or the custom path on registry map. -func normalizeReference(ref name.Reference, image string, custompath string) (name.Reference, error) { - if custompath == "" { - custompath = "library" - } - if !strings.ContainsRune(image, '/') { - return name.ParseReference(custompath+"/"+image, name.WeakValidation) +// remapRepository adds the {repositoryPrefix}/ to the original repo, and normalizes with an additional library/ if necessary +func remapRepository(repo name.Repository, regToMapTo string, repositoryPrefix string, insecurePull bool) (name.Repository, error) { + if insecurePull { + return name.NewRepository(repositoryPrefix+repo.RepositoryStr(), name.WithDefaultRegistry(regToMapTo), name.WeakValidation, name.Insecure) + } else { + return name.NewRepository(repositoryPrefix+repo.RepositoryStr(), name.WithDefaultRegistry(regToMapTo), name.WeakValidation) } +} - return ref, nil +func setNewRepository(ref name.Reference, newRepo name.Repository) name.Reference { + switch r := ref.(type) { + case name.Tag: + r.Repository = newRepo + return r + case name.Digest: + r.Repository = newRepo + return r + default: + return ref + } } func setNewRegistry(ref name.Reference, newReg name.Registry) name.Reference { @@ -165,16 +162,16 @@ func remoteOptions(registryName string, opts config.RegistryOptions, customPlatf return []remote.Option{remote.WithTransport(tr), remote.WithAuthFromKeychain(creds.GetKeychain()), remote.WithPlatform(*platform)} } -// Parse the registry URL -// example: regURL = "registry.example.com/namespace/repo:tag" will return namespace/repo -func extractPathFromRegistryURL(regFullURL string) (string, string) { - // Split the regURL by slashes - // becaues the registry url is write without scheme, we just need to remove the first one - segments := strings.Split(regFullURL, "/") - // Join all segments except the first one (which is typically empty) - path := strings.Join(segments[1:], "/") - // get the fist segment to get the registry url - regURL := segments[0] - - return path, regURL +// Parse the registry mapping +// example: regMapping = "registry.example.com/subdir1/subdir2" will return registry.example.com and subdir1/subdir2/ +func parseRegistryMapping(regMapping string) (string, string) { + // Split the registry mapping by first slash + regURL, repositoryPrefix, _ := strings.Cut(regMapping, "/") + + // Normalize with a trailing slash if not empty + if repositoryPrefix != "" && !strings.HasSuffix(repositoryPrefix, "/") { + repositoryPrefix = repositoryPrefix + "/" + } + + return regURL, repositoryPrefix } diff --git a/pkg/image/remote/remote_test.go b/pkg/image/remote/remote_test.go index 4d3712a967..c1d2300ff6 100644 --- a/pkg/image/remote/remote_test.go +++ b/pkg/image/remote/remote_test.go @@ -81,21 +81,95 @@ func (m *mockImage) Size() (int64, error) { return 0, nil } -func Test_normalizeReference(t *testing.T) { - expected := "index.docker.io/library/debian:latest" - - ref, err := name.ParseReference(image) - if err != nil { - t.Fatal(err) +func Test_remapRepository(t *testing.T) { + tests := []struct { + name string + repository string + newRegistry string + newRepositoryPrefix string + expectedRepository string + }{ + { + name: "Test case 1", + repository: "debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/library/debian", + }, + { + name: "Test case 2", + repository: "docker.io/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/library/debian", + }, + { + name: "Test case 3", + repository: "index.docker.io/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/library/debian", + }, + { + name: "Test case 4", + repository: "oldreg.io/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/debian", + }, + { + name: "Test case 5", + repository: "debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "subdir1/subdir2/", + expectedRepository: "newreg.io/subdir1/subdir2/library/debian", + }, + { + name: "Test case 6", + repository: "library/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/library/debian", + }, + { + name: "Test case 7", + repository: "library/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "subdir1/subdir2/", + expectedRepository: "newreg.io/subdir1/subdir2/library/debian", + }, + { + name: "Test case 8", + repository: "namespace/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "", + expectedRepository: "newreg.io/namespace/debian", + }, + { + name: "Test case 9", + repository: "namespace/debian", + newRegistry: "newreg.io", + newRepositoryPrefix: "subdir1/subdir2/", + expectedRepository: "newreg.io/subdir1/subdir2/namespace/debian", + }, + // Add more test cases here } - ref2, err := normalizeReference(ref, image, "library") - if err != nil { - t.Fatal(err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo, err := name.NewRepository(tt.repository) + if err != nil { + t.Fatal(err) + } + repo2, err := remapRepository(repo, tt.newRegistry, tt.newRepositoryPrefix, false) + if err != nil { + t.Fatal(err) + } - if ref2.Name() != ref.Name() || ref2.Name() != expected { - t.Errorf("%s should have been normalized to %s, got %s", ref2.Name(), expected, ref.Name()) + if repo2.Name() != tt.expectedRepository { + t.Errorf("%s should have been normalized to %s, got %s", repo.Name(), tt.expectedRepository, repo2.Name()) + } + }) } } @@ -130,7 +204,7 @@ func Test_RetrieveRemoteImage_skipFallback(t *testing.T) { } if _, err := RetrieveRemoteImage(image, opts, ""); err != nil { - t.Fatal("Expected call to succeed because fallback to default registry") + t.Fatalf("Expected call to succeed because fallback to default registry") } opts.SkipDefaultRegistryFallback = true @@ -184,36 +258,54 @@ func Test_NoRetryRetrieveRemoteImageFails(t *testing.T) { } } -func Test_ExtractPathFromRegistryURL(t *testing.T) { +func Test_ParseRegistryMapping(t *testing.T) { tests := []struct { - name string - regFullURL string - expectedRegistry string - expectedImagePrefix string + name string + registryMapping string + expectedRegistry string + expectedRepositoryPrefix string }{ { - name: "Test case 1", - regFullURL: "registry.example.com/namespace", - expectedRegistry: "registry.example.com", - expectedImagePrefix: "namespace", + name: "Test case 1", + registryMapping: "registry.example.com/subdir", + expectedRegistry: "registry.example.com", + expectedRepositoryPrefix: "subdir/", }, { - name: "Test case 2", - regFullURL: "registry.example.com", - expectedRegistry: "registry.example.com", - expectedImagePrefix: "", + name: "Test case 2", + registryMapping: "registry.example.com/subdir/", + expectedRegistry: "registry.example.com", + expectedRepositoryPrefix: "subdir/", + }, + { + name: "Test case 3", + registryMapping: "registry.example.com/subdir1/subdir2", + expectedRegistry: "registry.example.com", + expectedRepositoryPrefix: "subdir1/subdir2/", + }, + { + name: "Test case 4", + registryMapping: "registry.example.com", + expectedRegistry: "registry.example.com", + expectedRepositoryPrefix: "", + }, + { + name: "Test case 5", + registryMapping: "registry.example.com/", + expectedRegistry: "registry.example.com", + expectedRepositoryPrefix: "", }, // Add more test cases here } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - path, registry := extractPathFromRegistryURL(tt.regFullURL) + registry, repositoryPrefix := parseRegistryMapping(tt.registryMapping) if registry != tt.expectedRegistry { - t.Errorf("Expected path: %s, but got: %s", tt.expectedRegistry, registry) + t.Errorf("Expected registry: %s, but got: %s", tt.expectedRegistry, registry) } - if path != tt.expectedImagePrefix { - t.Errorf("Expected repo: %s, but got: %s", tt.expectedImagePrefix, path) + if repositoryPrefix != tt.expectedRepositoryPrefix { + t.Errorf("Expected repoPrefix: %s, but got: %s", tt.expectedRepositoryPrefix, repositoryPrefix) } }) }