Skip to content

Commit

Permalink
fix: make --registry-map compatible with namespaced images (#3138)
Browse files Browse the repository at this point in the history 
* For each registry mapping, represent it by a new instance of Repository and
  create a new Reference containing it.
* Improve registry mapping parser
* Add more unit tests to cover more use cases
  • Loading branch information
@mlallaouret
mlallaouret authored May 14, 2024
1 parent d65b9b5 commit 7f365a6
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 73 deletions.
83 changes: 40 additions & 43 deletions pkg/image/remote/remote.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
152 changes: 122 additions & 30 deletions pkg/image/remote/remote_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
})
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
})
}
Expand Down

0 comments on commit 7f365a6

Please sign in to comment.