diff --git a/pkg/cli/mirror/manifests.go b/pkg/cli/mirror/manifests.go index 6707b788b..2abc85f2c 100644 --- a/pkg/cli/mirror/manifests.go +++ b/pkg/cli/mirror/manifests.go @@ -38,7 +38,7 @@ var icspTypeMeta = metav1.TypeMeta{ // ICSPBuilder defines methods for generating ICSPs type ICSPBuilder interface { New(string, int) operatorv1alpha1.ImageContentSourcePolicy - GetMapping(string, image.TypedImageMapping) (map[string]string, error) + GetMapping(int, string, image.TypedImageMapping) (map[string]string, error) } var _ ICSPBuilder = &ReleaseBuilder{} @@ -58,11 +58,11 @@ func (b *ReleaseBuilder) New(icspName string, icspCount int) operatorv1alpha1.Im } } -func (b *ReleaseBuilder) GetMapping(_ string, mapping image.TypedImageMapping) (map[string]string, error) { +func (b *ReleaseBuilder) GetMapping(paths int, _ string, mapping image.TypedImageMapping) (map[string]string, error) { // Scope is set to repository for release because // they are mirrored as different repo names by // release planner - return getRegistryMapping(repositoryICSPScope, mapping) + return getRegistryMapping(paths, repositoryICSPScope, mapping) } var _ ICSPBuilder = &OperatorBuilder{} @@ -83,8 +83,8 @@ func (b *OperatorBuilder) New(icspName string, icspCount int) operatorv1alpha1.I } } -func (b *OperatorBuilder) GetMapping(icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { - return getRegistryMapping(icspScope, mapping) +func (b *OperatorBuilder) GetMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { + return getRegistryMapping(paths, icspScope, mapping) } var _ ICSPBuilder = &GenericBuilder{} @@ -104,13 +104,13 @@ func (b *GenericBuilder) New(icspName string, icspCount int) operatorv1alpha1.Im } } -func (b *GenericBuilder) GetMapping(icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { - return getRegistryMapping(icspScope, mapping) +func (b *GenericBuilder) GetMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { + return getRegistryMapping(paths, icspScope, mapping) } // GenerateICSP will generate ImageContentSourcePolicy objects based on image mapping and an ICSPBuilder -func GenerateICSP(icspName, icspScope string, byteLimit int, mapping image.TypedImageMapping, builder ICSPBuilder) (icsps []operatorv1alpha1.ImageContentSourcePolicy, err error) { - registryMapping, err := builder.GetMapping(icspScope, mapping) +func (o *MirrorOptions) GenerateICSP(icspName, icspScope string, byteLimit int, mapping image.TypedImageMapping, builder ICSPBuilder) (icsps []operatorv1alpha1.ImageContentSourcePolicy, err error) { + registryMapping, err := builder.GetMapping(o.MaxNestedPaths, icspScope, mapping) if err != nil { return nil, err } @@ -160,7 +160,7 @@ func aggregateICSPs(icsps [][]byte) []byte { return aggregation } -func getRegistryMapping(icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { +func getRegistryMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) { registryMapping := map[string]string{} for k, v := range mapping { if len(v.Ref.ID) == 0 { @@ -179,10 +179,18 @@ func getRegistryMapping(icspScope string, mapping image.TypedImageMapping) (map[ case icspScope == repositoryICSPScope: registryMapping[k.Ref.AsRepository().String()] = v.Ref.AsRepository().String() case icspScope == namespaceICSPScope: - source := path.Join(imgRegistry, imgNamespace) - reg, org, _, _, _ := v1alpha2.ParseImageReference(v.Ref.AsRepository().Exact()) - dest := path.Join(reg, org) - registryMapping[source] = dest + // OCPBUGS-11922 + if paths > 0 { + source := path.Join(imgRegistry, imgNamespace, k.Ref.Name) + reg, org, repo, _, _ := v1alpha2.ParseImageReference(v.Ref.String()) + dest := path.Join(reg, org, repo) + registryMapping[source] = dest + } else { + source := path.Join(imgRegistry, imgNamespace) + reg, org, _, _, _ := v1alpha2.ParseImageReference(v.Ref.AsRepository().Exact()) + dest := path.Join(reg, org) + registryMapping[source] = dest + } default: return registryMapping, fmt.Errorf("invalid ICSP scope %s", icspScope) } diff --git a/pkg/cli/mirror/manifests_test.go b/pkg/cli/mirror/manifests_test.go index 2c766900d..b534e34a8 100644 --- a/pkg/cli/mirror/manifests_test.go +++ b/pkg/cli/mirror/manifests_test.go @@ -443,6 +443,13 @@ func TestICSPGeneration(t *testing.T) { }, }} + // test processNestedPaths + o := &MirrorOptions{ + MaxNestedPaths: 0, + ToMirror: "localhost:5000", + UserNamespace: "ocpbugs-11922/mirror-release", + } + for _, test := range tests { t.Run(test.name, func(t *testing.T) { mapping := image.TypedImageMapping{} @@ -450,7 +457,7 @@ func TestICSPGeneration(t *testing.T) { mapping[sourceImage] = test.destImages[ind] } - icsps, err := GenerateICSP("test", test.icspScope, test.icspSizeLimit, mapping, test.typ) + icsps, err := o.GenerateICSP("test", test.icspScope, test.icspSizeLimit, mapping, test.typ) if test.err != "" { require.EqualError(t, err, test.err) } else { diff --git a/pkg/cli/mirror/mirror.go b/pkg/cli/mirror/mirror.go index 3b804deae..4f6ed6dee 100644 --- a/pkg/cli/mirror/mirror.go +++ b/pkg/cli/mirror/mirror.go @@ -202,10 +202,10 @@ func checkDockerReference(mirror imagesource.TypedImageReference, nested int) er return fmt.Errorf("destination registry must consist of registry host and namespace(s) only, and must not include an image tag or ID") } - depth := strings.Split(strings.Join([]string{mirror.Ref.Namespace, mirror.Ref.Name}, "/"), "/") - if len(depth) > nested { - destination := strings.Join([]string{mirror.Ref.Registry, mirror.Ref.Namespace, mirror.Ref.Name}, "/") - return fmt.Errorf("the max-nested-paths value (%d) for %s exceeds the registry mirror paths setting (some registries limit the nested paths)", len(depth), destination) + destination := strings.Join([]string{mirror.Ref.Registry, mirror.Ref.RepositoryName()}, "/") + depth := strings.Split(destination, "/") + if nested > 0 && (len(depth) >= nested) { + return fmt.Errorf("the max-nested-paths value (%d) for %s must be higher than (registry + namespace) - try increasing the value", len(depth), destination) } return nil } @@ -445,10 +445,9 @@ func (o *MirrorOptions) mirrorMappings(cfg v1alpha2.ImageSetConfiguration, image Type: srcRef.Type, } - dstTIR := imagesource.TypedImageReference{ - Ref: dstRef.Ref, - Type: dstRef.Type, - } + // OCPBUGS-11922 + dstTIR := o.processNestedPaths(&dstRef) + mappings = append(mappings, mirror.Mapping{ Source: srcTIR, Destination: dstTIR, @@ -489,7 +488,7 @@ func (o *MirrorOptions) newMirrorImageOptions(insecure bool) (*mirror.MirrorImag func (o *MirrorOptions) generateResults(mapping image.TypedImageMapping, dir string) error { mappingResultsPath := filepath.Join(dir, mappingFile) - if err := writeMappingFile(mappingResultsPath, mapping); err != nil { + if err := o.writeMappingFile(mappingResultsPath, mapping); err != nil { return err } @@ -500,7 +499,7 @@ func (o *MirrorOptions) generateResults(mapping image.TypedImageMapping, dir str operator := image.ByCategory(mapping, v1alpha2.TypeOperatorBundle, v1alpha2.TypeOperatorRelatedImage) getICSP := func(mapping image.TypedImageMapping, name string, builder ICSPBuilder) error { - icsps, err := GenerateICSP(name, namespaceICSPScope, icspSizeLimit, mapping, builder) + icsps, err := o.GenerateICSP(name, namespaceICSPScope, icspSizeLimit, mapping, builder) if err != nil { return fmt.Errorf("error generating ICSP manifests") } @@ -594,7 +593,7 @@ func (o *MirrorOptions) processAssociationErrors(errs []error) error { return nil } -func writeMappingFile(mappingPath string, mapping image.TypedImageMapping) error { +func (o *MirrorOptions) writeMappingFile(mappingPath string, mapping image.TypedImageMapping) error { path := filepath.Clean(mappingPath) mappingFile, err := os.Create(path) if err != nil { @@ -602,7 +601,7 @@ func writeMappingFile(mappingPath string, mapping image.TypedImageMapping) error } defer mappingFile.Close() klog.Infof("Writing image mapping to %s", mappingPath) - if err := image.WriteImageMapping(mapping, mappingFile); err != nil { + if err := image.WriteImageMapping(o.MaxNestedPaths, mapping, mappingFile); err != nil { return err } return mappingFile.Sync() @@ -666,7 +665,7 @@ func (o *MirrorOptions) mirrorToMirrorWrapper(ctx context.Context, cfg v1alpha2. } if o.DryRun { - if err := writeMappingFile(mappingPath, mapping); err != nil { + if err := o.writeMappingFile(mappingPath, mapping); err != nil { return err } if err := o.outputPruneImagePlan(ctx, prevAssociations, prunedAssociations); err != nil { @@ -816,7 +815,7 @@ func (o *MirrorOptions) mirrorToDiskWrapper(ctx context.Context, cfg v1alpha2.Im mappingPath := filepath.Join(o.Dir, mappingFile) if o.DryRun { - if err := writeMappingFile(mappingPath, mapping); err != nil { + if err := o.writeMappingFile(mappingPath, mapping); err != nil { return err } return cleanup() @@ -877,7 +876,7 @@ func (o *MirrorOptions) diskToMirrorWrapper(ctx context.Context, cleanup cleanup mappingPath := filepath.Join(o.Dir, mappingFile) if o.DryRun { - if err := writeMappingFile(mappingPath, mapping); err != nil { + if err := o.writeMappingFile(mappingPath, mapping); err != nil { return err } return cleanup() @@ -888,3 +887,45 @@ func (o *MirrorOptions) diskToMirrorWrapper(ctx context.Context, cleanup cleanup } return nil } + +func (o *MirrorOptions) processNestedPaths(ref *image.TypedImage) imagesource.TypedImageReference { + + if o.MaxNestedPaths > 0 { + dir := ref.Ref + full := strings.Join([]string{dir.Registry, dir.Namespace, dir.Name}, "/") + splitUserNS := strings.Split(o.UserNamespace, "/") + // find the path length for registry & usernamespace + newIndex := o.MaxNestedPaths - (len(splitUserNS) + 1) + // if less than 0 will cause a panic + if newIndex >= 0 { + // restore the original usernamespace + dir.Namespace = o.UserNamespace + // split the full reference from the usernamespace + name := strings.Split(full, o.UserNamespace) + // use the second part to split the paths + newName := strings.Split(strings.Trim(name[1], "/"), "/") + // no need to replace + if newIndex >= len(newName) { + dir.Name = strings.Trim(name[1], "/") + return imagesource.TypedImageReference{Ref: dir, Type: ref.Type} + } + // replace all '/' with '-' from the index (MaxNestedPaths - (len(usernamespace) + 1)) + replacedName := strings.ReplaceAll(strings.Join(newName[newIndex:], "/"), "/", "-") + preFix := strings.Join(newName[:newIndex], "/") + // we should never really see this condition + // MaxNestedPaths should always be greater than len(split(registry + usernamespace) + // i.e o.MaxNestedPaths > strings.Split(registry + usernamespace) + if newIndex == 0 && len(preFix) == 0 { + dir.Name = strings.Trim(replacedName, "/") + } else { + dir.Name = strings.Join([]string{preFix, replacedName}, "/") + } + return imagesource.TypedImageReference{Ref: dir, Type: ref.Type} + } else { + // return original - no changes + return imagesource.TypedImageReference{Ref: ref.Ref, Type: ref.Type} + } + } + // return original - no changes + return imagesource.TypedImageReference{Ref: ref.Ref, Type: ref.Type} +} diff --git a/pkg/cli/mirror/mirror_test.go b/pkg/cli/mirror/mirror_test.go index a8d10b4db..54073df4e 100644 --- a/pkg/cli/mirror/mirror_test.go +++ b/pkg/cli/mirror/mirror_test.go @@ -3,6 +3,7 @@ package mirror import ( "net/http/httptest" "net/url" + "strings" "testing" "github.com/google/go-containerregistry/pkg/registry" @@ -75,77 +76,77 @@ func TestMirrorComplete(t *testing.T) { { name: "Valid/RegDest", args: []string{"docker://reg.com"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "reg.com", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/LocalhostRegDest", args: []string{"docker://localhost"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "localhost", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/FqdnRegPortDest", args: []string{"docker://reg.com:5000"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "reg.com:5000", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/LocalhostRegPortDest", args: []string{"docker://localhost:5000"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "localhost:5000", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/RegNamespace", args: []string{"docker://reg.com/foo/bar"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "reg.com", UserNamespace: "foo/bar", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/LocalhostRegNamespace", args: []string{"docker://localhost/foo/bar"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "localhost", UserNamespace: "foo/bar", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/NonFqdnRegPortNamespace", args: []string{"docker://reg:5000/foo"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "reg:5000", UserNamespace: "foo", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { name: "Valid/NonFqdnRegPortNamespaceName", args: []string{"docker://reg:5000/foo/bar"}, - opts: &MirrorOptions{MaxNestedPaths: 3}, + opts: &MirrorOptions{MaxNestedPaths: 0}, expOpts: &MirrorOptions{ ToMirror: "reg:5000", UserNamespace: "foo/bar", - MaxNestedPaths: 3, + MaxNestedPaths: 0, }, }, { @@ -227,7 +228,7 @@ func TestMirrorComplete(t *testing.T) { name: "Invalid/ExceedsNestedPathsLength", args: []string{"docker://reg.com/foo/bar/baz"}, opts: &MirrorOptions{MaxNestedPaths: 2}, - expError: "the max-nested-paths value (3) for reg.com/foo/bar/baz exceeds the registry mirror paths setting (some registries limit the nested paths)", + expError: "the max-nested-paths value (4) for reg.com/foo/bar/baz must be higher than (registry + namespace) - try increasing the value", }, } @@ -534,3 +535,89 @@ func TestRemovePreviouslyMirrored(t *testing.T) { }) } } + +func TestNestedPaths(t *testing.T) { + + // test processNestedPaths + o := &MirrorOptions{ + MaxNestedPaths: 2, + ToMirror: "localhost:5000", + UserNamespace: "ocpbugs-11922/mirror-release", + } + + img := &image.TypedImage{ + TypedImageReference: image.TypedImageReference{ + Ref: reference.DockerImageReference{ + Name: "ci-cd/gitlab-runner-ubi-images/gitlab-runner-helper-ocp", + Namespace: "ocpbugs-11922/mirror-release/gitlab-org", + Registry: "localhost:5000", + }, + }, + } + + t.Run("Testing processNestedPaths (2) : should fail", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.NotEqual(t, "localhost:5000/ocpbugs-11922/mirror-release/gitlab-org-ci-cd-gitlab-runner-ubi-images-gitlab-runner-helper-ocp", dst) + }) + o.MaxNestedPaths = 3 + t.Run("Testing processNestedPaths (3) : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/gitlab-org-ci-cd-gitlab-runner-ubi-images-gitlab-runner-helper-ocp", dst) + }) + + o.MaxNestedPaths = 4 + t.Run("Testing processNestedPaths (4) : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/gitlab-org/ci-cd-gitlab-runner-ubi-images-gitlab-runner-helper-ocp", dst) + }) + + o.MaxNestedPaths = 5 + t.Run("Testing processNestedPaths (5) : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/gitlab-org/ci-cd/gitlab-runner-ubi-images-gitlab-runner-helper-ocp", dst) + }) + + o.MaxNestedPaths = 6 + t.Run("Testing processNestedPaths (6) : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/gitlab-org/ci-cd/gitlab-runner-ubi-images/gitlab-runner-helper-ocp", dst) + }) + + // change image + img = &image.TypedImage{ + TypedImageReference: image.TypedImageReference{ + Ref: reference.DockerImageReference{ + Name: "openshift4/ose-kube-rbac-proxy", + Namespace: "ocpbugs-11922/mirror-release", + Registry: "localhost:5000", + }, + }, + } + + o.MaxNestedPaths = 2 + t.Run("Testing processNestedPaths (2) new image : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/openshift4/ose-kube-rbac-proxy", dst) + }) + + o.MaxNestedPaths = 5 + t.Run("Testing processNestedPaths (5) new image : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/openshift4/ose-kube-rbac-proxy", dst) + }) + + o.MaxNestedPaths = 10 + t.Run("Testing processNestedPaths (10) new image : should pass", func(t *testing.T) { + res := o.processNestedPaths(img) + dst := strings.Join([]string{res.Ref.Registry, res.Ref.Namespace, res.Ref.Name}, "/") + require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release/openshift4/ose-kube-rbac-proxy", dst) + }) + +} diff --git a/pkg/cli/mirror/operator.go b/pkg/cli/mirror/operator.go index fb882213a..c4ab2ed07 100644 --- a/pkg/cli/mirror/operator.go +++ b/pkg/cli/mirror/operator.go @@ -457,7 +457,7 @@ func (o *OperatorOptions) plan(ctx context.Context, dc *declcfg.DeclarativeConfi } } - if err := writeMappingFile(mappingFile, result); err != nil { + if err := o.writeMappingFile(mappingFile, result); err != nil { return nil, err } } diff --git a/pkg/cli/mirror/options.go b/pkg/cli/mirror/options.go index 16653757d..251ec269b 100644 --- a/pkg/cli/mirror/options.go +++ b/pkg/cli/mirror/options.go @@ -73,8 +73,8 @@ func (o *MirrorOptions) BindFlags(fs *pflag.FlagSet) { fs.StringVar(&o.OCIRegistriesConfig, "oci-registries-config", o.OCIRegistriesConfig, "Registries config file location (used only with --use-oci-feature flag)") fs.BoolVar(&o.OCIInsecureSignaturePolicy, "oci-insecure-signature-policy", o.OCIInsecureSignaturePolicy, "If set, OCI catalog push will not try to push signatures") fs.BoolVar(&o.SkipPruning, "skip-pruning", o.SkipPruning, "If set, will disable pruning globally") - fs.IntVar(&o.MaxNestedPaths, "max-nested-paths", 2, "Number of nested paths, for destination registries that limit nested paths") fs.MarkDeprecated("use-oci-feature", "Use --include-local-oci-catalogs instead") + fs.IntVar(&o.MaxNestedPaths, "max-nested-paths", 0, "Number of nested paths, for destination registries that limit nested paths") } func (o *MirrorOptions) init() { diff --git a/pkg/image/mapping.go b/pkg/image/mapping.go index c2a06f76b..751c429c8 100644 --- a/pkg/image/mapping.go +++ b/pkg/image/mapping.go @@ -161,14 +161,23 @@ func ReadImageMapping(mappingsPath, separator string, typ v1alpha2.ImageType) (T } // WriteImageMapping writes key map k/v to an io.Writer. -func WriteImageMapping(m TypedImageMapping, output io.Writer) error { +func WriteImageMapping(nestedPaths int, m TypedImageMapping, output io.Writer) error { + var strFrom, strTo string for fromStr, toStr := range m { // Prefer tag over id for mapping file for // compatability with `oc image mirror`. if toStr.Ref.Tag != "" { toStr.Ref.ID = "" } - _, err := output.Write([]byte(fmt.Sprintf("%s=%s\n", fromStr.Ref.Exact(), toStr.Ref.Exact()))) + // OCPBUGS-11922 + if nestedPaths > 0 { + strFrom = fromStr.Ref.String() + strTo = toStr.Ref.String() + } else { + strFrom = fromStr.Ref.Exact() + strTo = toStr.Ref.Exact() + } + _, err := output.Write([]byte(fmt.Sprintf("%s=%s\n", strFrom, strTo))) if err != nil { return err } diff --git a/pkg/image/mapping_test.go b/pkg/image/mapping_test.go index 37147f707..423f88c50 100644 --- a/pkg/image/mapping_test.go +++ b/pkg/image/mapping_test.go @@ -374,7 +374,7 @@ func TestWriteImageMapping(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { output := new(strings.Builder) - err := WriteImageMapping(test.mapping, output) + err := WriteImageMapping(0, test.mapping, output) if test.err != "" { require.EqualError(t, err, test.err) } else {