Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release-4.13] OCPBUGS-13591,OCPBUGS-13592: Limit the nested repository path while mirroring the images #632

Closed
wants to merge 2 commits into from
Closed

[release-4.13] OCPBUGS-13591,OCPBUGS-13592: Limit the nested repository path while mirroring the images #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2605ec7
OCPBUGS-11922: Limit the nested repository path while mirroring the i…
lmzuccarelli Apr 21, 2023
184fa70
Fix issues with ICSP not containing the modified mirrors based on --m…
sherine-k May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions pkg/cli/mirror/manifests.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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 @@ -869,7 +883,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 @@ -880,3 +894,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}
}