Skip to content

Commit

Permalink
Fix issues with ICSP not containing the modified mirrors based on --m…
Browse files Browse the repository at this point in the history 
…ax-nested-paths
  • Loading branch information
@sherine-k
sherine-k committed May 10, 2023
1 parent 6d8e007 commit 73ff7b8
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 57 deletions.
24 changes: 8 additions & 16 deletions pkg/cli/mirror/manifests.go
View file
Expand Up @@ -62,7 +62,7 @@ func (b *ReleaseBuilder) GetMapping(paths int, _ string, mapping image.TypedImag
// Scope is set to repository for release because
// they are mirrored as different repo names by
// release planner
return getRegistryMapping(paths, repositoryICSPScope, mapping)
return getRegistryMapping(repositoryICSPScope, mapping)
}

var _ ICSPBuilder = &OperatorBuilder{}
Expand All @@ -84,7 +84,7 @@ func (b *OperatorBuilder) New(icspName string, icspCount int) operatorv1alpha1.I
}

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

var _ ICSPBuilder = &GenericBuilder{}
Expand All @@ -105,7 +105,7 @@ func (b *GenericBuilder) New(icspName string, icspCount int) operatorv1alpha1.Im
}

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

// GenerateICSP will generate ImageContentSourcePolicy objects based on image mapping and an ICSPBuilder
Expand Down Expand Up @@ -160,7 +160,7 @@ func aggregateICSPs(icsps [][]byte) []byte {
return aggregation
}

func getRegistryMapping(paths int, icspScope string, mapping image.TypedImageMapping) (map[string]string, error) {
func getRegistryMapping(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,18 +179,10 @@ func getRegistryMapping(paths int, icspScope string, mapping image.TypedImageMap
case icspScope == repositoryICSPScope:
registryMapping[k.Ref.AsRepository().String()] = v.Ref.AsRepository().String()
case icspScope == namespaceICSPScope:
// 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
}
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
74 changes: 35 additions & 39 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")
}

destination := strings.Join([]string{mirror.Ref.Registry, mirror.Ref.RepositoryName()}, "/")
depth := strings.Split(destination, "/")
depth := strings.Split(mirror.Ref.RepositoryName(), "/")
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 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 @@ -447,7 +446,17 @@ func (o *MirrorOptions) mirrorMappings(cfg v1alpha2.ImageSetConfiguration, image

// 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 @@ -498,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 := o.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 @@ -534,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 @@ -892,35 +907,16 @@ func (o *MirrorOptions) processNestedPaths(ref *image.TypedImage) imagesource.Ty

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}
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}
Expand Down
4 changes: 2 additions & 2 deletions pkg/cli/mirror/mirror_test.go
View file
Expand Up @@ -228,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 (4) for reg.com/foo/bar/baz must be higher than (registry + namespace) - try increasing the value",
expError: "the max-nested-paths value (2) must be strictly higher than the number of path-components in the destination foo/bar/baz - try increasing the value",
},
}

Expand Down Expand Up @@ -603,7 +603,7 @@ func TestNestedPaths(t *testing.T) {
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)
require.Equal(t, "localhost:5000/ocpbugs-11922/mirror-release-openshift4-ose-kube-rbac-proxy", dst)
})

o.MaxNestedPaths = 5
Expand Down

0 comments on commit 73ff7b8

Please sign in to comment.