Skip to content

Commit

Permalink
OCPBUGS-11922: Limit the nested repository path while mirroring the i…
Browse files Browse the repository at this point in the history 
…mages
  • Loading branch information
@lmzuccarelli
lmzuccarelli committed Apr 28, 2023
1 parent 7210f5c commit da0a36e
Show file tree
Hide file tree
Showing 8 changed files with 204 additions and 52 deletions.
36 changes: 22 additions & 14 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,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{}
Expand All @@ -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{}
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) {
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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
}
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
71 changes: 56 additions & 15 deletions pkg/cli/mirror/mirror.go
View file
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand All @@ -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")
}
Expand Down Expand Up @@ -594,15 +593,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 +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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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}
}

0 comments on commit da0a36e

Please sign in to comment.