Bug 1902299: catalog mirror improvements

contrib/completions/bash/oc

@@ -718,6 +718,10 @@ _oc_adm_catalog_mirror()
     two_word_flags+=("--icsp-scope")
     local_nonpersistent_flags+=("--icsp-scope")
     local_nonpersistent_flags+=("--icsp-scope=")
+    flags+=("--index-filter-by-os=")
+    two_word_flags+=("--index-filter-by-os")
+    local_nonpersistent_flags+=("--index-filter-by-os")
+    local_nonpersistent_flags+=("--index-filter-by-os=")
     flags+=("--insecure")
     local_nonpersistent_flags+=("--insecure")
     flags+=("--manifests-only")

contrib/completions/zsh/oc

@@ -818,6 +818,10 @@ _oc_adm_catalog_mirror()
     two_word_flags+=("--icsp-scope")
     local_nonpersistent_flags+=("--icsp-scope")
     local_nonpersistent_flags+=("--icsp-scope=")
+    flags+=("--index-filter-by-os=")
+    two_word_flags+=("--index-filter-by-os")
+    local_nonpersistent_flags+=("--index-filter-by-os")
+    local_nonpersistent_flags+=("--index-filter-by-os=")
     flags+=("--insecure")
     local_nonpersistent_flags+=("--insecure")
     flags+=("--manifests-only")

pkg/cli/admin/catalog/mirror.go

@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"text/tabwriter"
 	"time"
@@ -123,9 +124,18 @@ func NewMirrorCatalog(f kcmdutil.Factory, streams genericclioptions.IOStreams) *
 	flags := cmd.Flags()
 
 	o.SecurityOptions.Bind(flags)
-	o.FilterOptions.Bind(flags)
 	o.ParallelOptions.Bind(flags)
 
+	// Images referenced by catalogs must have all variants mirrored. FilterByOs will only apply to the initial index
+	// image, to indicate which arch should be used to extract the catalog db (the database inside should be the same
+	// for all arches, so this flag should never need to be set explicitly for standard workflows).
+	// this flag is renamed to make it clear that the underlying images are not filtered
+	flags.StringVar(&o.FilterOptions.FilterByOS, "index-filter-by-os", o.FilterOptions.FilterByOS, "A regular expression to control which index image is picked when multiple variants are available. Images will be passed as '<platform>/<architecture>[/<variant>]'. This does not apply to images referenced by the index.")
+
+	// the old flag name is kept for backwards-compatibility.
+	// if both old and new are specified, the value of the flag coming later will be used.
+	flags.StringVar(&o.FilterOptions.FilterByOS, "filter-by-os", o.FilterOptions.FilterByOS, "Use --index-filter-by-os instead. A regular expression to control which index image is picked when multiple variants are available. Images will be passed as '<platform>/<architecture>[/<variant>]'. This does not apply to images referenced by the index.")
+
 	flags.StringVar(&o.ManifestDir, "to-manifests", "", "Local path to store manifests.")
 	flags.StringVar(&o.DatabasePath, "path", "", "Specify an in-container to local path mapping for the database.")
 	flags.BoolVar(&o.DryRun, "dry-run", o.DryRun, "Print the actions that would be taken and exit without writing to the destinations.")
@@ -144,8 +154,10 @@ func (o *MirrorCatalogOptions) Complete(cmd *cobra.Command, args []string) error
 	src := args[0]
 	dest := args[1]
 
-	if err := o.FilterOptions.Complete(cmd.Flags()); err != nil {
-		return err
+	// default to linux/arm64 for index image, which we generally expect to exist
+	pattern := o.FilterOptions.FilterByOS
+	if len(pattern) == 0 {
+		o.FilterOptions.FilterByOS = regexp.QuoteMeta(fmt.Sprintf("%s/%s", "linux", "amd64"))
 	}
 	if err := o.FilterOptions.Validate(); err != nil {
 		return err
@@ -176,6 +188,11 @@ func (o *MirrorCatalogOptions) Complete(cmd *cobra.Command, args []string) error
 		o.ManifestDir = fmt.Sprintf("manifests-%s-%d", o.SourceRef.Ref.Name, time.Now().Unix())
 	}
 
+	allmanifests := imagemanifest.FilterOptions{FilterByOS: ".*"}
+	if err := allmanifests.Validate(); err != nil {
+		return err
+	}
+
 	if err := os.MkdirAll(o.ManifestDir, os.ModePerm); err != nil {
 		return err
 	}
@@ -260,7 +277,11 @@ func (o *MirrorCatalogOptions) Complete(cmd *cobra.Command, args []string) error
 		a.ContinueOnError = true
 		a.DryRun = o.DryRun
 		a.SecurityOptions = o.SecurityOptions
-		a.FilterOptions = o.FilterOptions
+		// because images in the catalog are statically referenced by digest,
+		// we do not allow filtering for mirroring. this may change if sparse manifestlists are allowed
+		// by registries, or if multi-arch management moves into images that can be rewritten on mirror (i.e. the bundle
+		// images themselves, not the images referenced inside of the bundle images).
+		a.FilterOptions = allmanifests
 		a.ParallelOptions = o.ParallelOptions
 		a.KeepManifestList = true
 		a.Mappings = mappings

pkg/cli/admin/catalog/mirror_options.go

@@ -27,7 +27,7 @@ func (o *IndexImageMirrorerOptions) Validate() error {
 		return fmt.Errorf("source image required")
 	}
 
-	if o.Dest.String() == "" {
+	if o.Dest.Ref.RegistryURL().Hostname() == "" && o.Dest.String() == "" {
 		return fmt.Errorf("destination registry required")
 	}
 

pkg/cli/admin/catalog/mirrorer.go

@@ -217,8 +217,12 @@ func mount(in, dest imagesource.TypedImageReference, maxComponents int) (out ima
 		out.Ref.Name = strings.Join(components[1:maxComponents-1], "/") + "/" + strings.Join(components[maxComponents-1:], "-")
 	} else if maxComponents == 0 {
 		out.Ref.Name = strings.Join(components[1:], "/")
-	} else {
+	} else if len(components) > 1 {
 		out.Ref.Name = strings.Join(components[1:maxComponents], "/")
+	} else {
+		// only one component, make it the name, not the namespace
+		out.Ref.Name = in.Ref.Name
+		out.Ref.Namespace = ""
 	}
 	out.Ref.Name = strings.TrimPrefix(out.Ref.Name, "/")
 	return

pkg/cli/admin/catalog/mirrorer_test.go

@@ -490,6 +490,29 @@ func TestMappingForImages(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "namespaceless image to registry",
+			args: args{
+				images: map[string]struct{}{
+					"registry.access.redhat.com/ubi8-minimal@sha256:9285da611437622492f9ef4229877efe302589f1401bbd4052e9bb261b3d4387": {},
+				},
+				src:           mustParseRef(t, "registry.access.redhat.com/ubi8-minimal@sha256:9285da611437622492f9ef4229877efe302589f1401bbd4052e9bb261b3d4387"),
+				dest:          mustParseRef(t, "quay.io"),
+				maxComponents: 2,
+			},
+			wantMapping: map[imagesource.TypedImageReference]imagesource.TypedImageReference{
+				mustParseRef(t, "registry.access.redhat.com/ubi8-minimal@sha256:9285da611437622492f9ef4229877efe302589f1401bbd4052e9bb261b3d4387"): {
+					Type: imagesource.DestinationRegistry,
+					Ref: reference.DockerImageReference{
+						Registry:  "quay.io",
+						Namespace: "",
+						Name:      "ubi8-minimal",
+						Tag:       "51f124fa",
+						ID:        "sha256:9285da611437622492f9ef4229877efe302589f1401bbd4052e9bb261b3d4387",
+					},
+				},
+			},
+		},
 		{
 			name: "untagged image to registry",
 			args: args{
@@ -932,7 +955,7 @@ func TestMappingForImages(t *testing.T) {
 					continue
 				}
 				if w != v {
-					t.Errorf("incorrect mapping for %s - have %s, want %s", k, w, v)
+					t.Errorf("incorrect mapping for %s - have %#v, want %#v", k, w, v)
 				}
 			}
 			for k, v := range gotMapping {