Skip to content

Commit

Permalink
OCPBUGS-11922,OCPBUGS-11910: Limit the nested repository path while m…
Browse files Browse the repository at this point in the history 
…irroring the images (openshift#623)

* OCPBUGS-11922: Limit the nested repository path while mirroring the images

* Fix issues with ICSP not containing the modified mirrors based on --max-nested-paths

---------

Co-authored-by: Sherine Khoury <skhoury@redhat.com>
  • Loading branch information
@lmzuccarelli @sherine-k @prb112
2 people authored and prb112 committed May 24, 2023
1 parent c1ff096 commit 5aa2400
Show file tree
Hide file tree
Showing 8 changed files with 188 additions and 48 deletions.
12 changes: 6 additions & 6 deletions pkg/cli/mirror/manifests.go
View file
Expand Up @@ -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{}
Expand All @@ -58,7 +58,7 @@ 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
Expand All @@ -83,7 +83,7 @@ func (b *OperatorBuilder) New(icspName string, icspCount int) operatorv1alpha1.I
}
}

func (b *OperatorBuilder) GetMapping(icspScope string, mapping image.TypedImageMapping) (map[string]string, error) {
func (b *OperatorBuilder) GetMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) {
return getRegistryMapping(icspScope, mapping)
}

Expand All @@ -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) {
func (b *GenericBuilder) GetMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) {
return getRegistryMapping(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
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/cli/mirror/manifests_test.go
View file
Expand Up @@ -443,14 +443,21 @@ 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{}
for ind, sourceImage := range test.sourceImages {
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 {
Expand Down
75 changes: 56 additions & 19 deletions pkg/cli/mirror/mirror.go
View file
Expand Up @@ -202,10 +202,9 @@ 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)
depth := strings.Split(mirror.Ref.RepositoryName(), "/")
if nested > 0 && (len(depth) >= nested) {
return fmt.Errorf("the max-nested-paths value (%d) must be strictly higher than the number of path-components in the destination %s - try increasing the value", nested, mirror.Ref.RepositoryName())
}
return nil
}
Expand Down Expand Up @@ -445,10 +444,19 @@ 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)
// Updating the original map - which will later be used to generate ICSP
images[srcRef] = image.TypedImage{
Category: dstRef.Category,
ImageFormat: dstRef.ImageFormat,
TypedImageReference: image.TypedImageReference{
Type: dstRef.Type,
Ref: dstTIR.Ref,
OCIFBCPath: dstRef.OCIFBCPath,
},
}
// Updating mappings which will be used for mirroring the images
mappings = append(mappings, mirror.Mapping{
Source: srcTIR,
Destination: dstTIR,
Expand Down Expand Up @@ -489,7 +497,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
}

Expand All @@ -499,8 +507,8 @@ func (o *MirrorOptions) generateResults(mapping image.TypedImageMapping, dir str
generic := image.ByCategory(mapping, v1alpha2.TypeGeneric)
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)
getICSP := func(mapping image.TypedImageMapping, name string, scope string, builder ICSPBuilder) error {
icsps, err := o.GenerateICSP(name, scope, icspSizeLimit, mapping, builder)
if err != nil {
return fmt.Errorf("error generating ICSP manifests")
}
Expand Down Expand Up @@ -535,14 +543,20 @@ func (o *MirrorOptions) generateResults(mapping image.TypedImageMapping, dir str
}
}

if err := getICSP(releases, "release", &ReleaseBuilder{}); err != nil {
if err := getICSP(releases, "release", namespaceICSPScope, &ReleaseBuilder{}); err != nil {
return err
}
if err := getICSP(generic, "generic", &GenericBuilder{}); err != nil {
if err := getICSP(generic, "generic", namespaceICSPScope, &GenericBuilder{}); err != nil {
return err
}
if err := getICSP(operator, "operator", &OperatorBuilder{}); err != nil {
return err
if o.MaxNestedPaths > 0 {
if err := getICSP(operator, "operator", repositoryICSPScope, &OperatorBuilder{}); err != nil {
return err
}
} else {
if err := getICSP(operator, "operator", namespaceICSPScope, &OperatorBuilder{}); err != nil {
return err
}
}

return WriteICSPs(dir, allICSPs)
Expand Down Expand Up @@ -594,15 +608,15 @@ 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 {
return err
}
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()
Expand Down Expand Up @@ -666,7 +680,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 {
Expand Down Expand Up @@ -816,7 +830,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()
Expand Down Expand Up @@ -877,7 +891,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()
Expand All @@ -888,3 +902,26 @@ 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 := dir.RepositoryName()

pathComponents := strings.Split(full, "/")
if o.MaxNestedPaths > 0 && len(pathComponents) > o.MaxNestedPaths {
lastPathComponent := strings.Join(pathComponents[o.MaxNestedPaths-1:], "-")
newPathComponents := pathComponents[:o.MaxNestedPaths-1]
newRef := dir // reinitializing newRef from dir (so that we don't loose id and tag)
newRef.Namespace = strings.Join(newPathComponents, "/")
newRef.Name = lastPathComponent
return imagesource.TypedImageReference{Ref: newRef, 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}
}

0 comments on commit 5aa2400

Please sign in to comment.