OCPBUGS-31536: Change default behavior to not rebuild catalogs for V1 (…

…openshift#844)

pkg/cli/mirror/catalog_images.go

@@ -183,6 +183,80 @@ func (o *MirrorOptions) rebuildCatalogs(ctx context.Context, dstDir string) (ima
 	return refs, nil
 }
 
+func (o *MirrorOptions) copyCatalogs(ctx context.Context, dstDir string) error {
+	var err error
+
+	mirrorRef := imagesource.TypedImageReference{Type: imagesource.DestinationRegistry}
+	mirrorRef.Ref, err = reference.Parse(o.ToMirror)
+	if err != nil {
+		return err
+	}
+
+	dstDir = filepath.Clean(dstDir)
+	if err := filepath.Walk(dstDir, func(fpath string, info fs.FileInfo, err error) error {
+		if err != nil || info == nil {
+			return err
+		}
+		slashPath := filepath.ToSlash(fpath)
+
+		if filepath.Base(fpath) == config.LayoutsDir {
+
+			// results in <some path>/src/catalogs/<repoPath>/layout
+			slashPath = path.Dir(slashPath)
+
+			// remove the <some path>/src/catalogs from the path to arrive at <repoPath>
+			repoPath := strings.TrimPrefix(slashPath, fmt.Sprintf("%s/%s/", dstDir, config.CatalogsDir))
+			// get the repo namespace and id (where ID is a SHA or tag)
+			// example: foo.com/foo/bar/<id>
+			regRepoNs, id := path.Split(repoPath)
+			regRepoNs = path.Clean(regRepoNs)
+			// reconstitute the path into a valid docker ref
+			var img string
+			if strings.Contains(id, ":") {
+				// Digest.
+				img = fmt.Sprintf("%s@%s", regRepoNs, id)
+			} else {
+				// Tag.
+				img = fmt.Sprintf("%s:%s", regRepoNs, id)
+			}
+
+			ctlgRef := image.TypedImage{}
+			ctlgRef.Type = imagesource.DestinationRegistry
+			originRef, err := image.ParseReference(img)
+			// since we can't really tell if the "img" reference originated from an actual docker
+			// reference or from an OCI file path that approximates a docker reference, ParseReference
+			// might not lowercase the name and namespace values which is required by the
+			// docker reference spec (see https://github.com/distribution/distribution/blob/main/reference/reference.go).
+			// Therefore we lower case name and namespace here to make sure it's done.
+			originRef.Ref.Name = strings.ToLower(originRef.Ref.Name)
+			originRef.Ref.Namespace = strings.ToLower(originRef.Ref.Namespace)
+
+			if err != nil {
+				return fmt.Errorf("error parsing index dir path %q as image %q: %v", fpath, img, err)
+			}
+			ctlgRef.Ref = originRef.Ref
+			// Update registry so the existing catalog image can be pulled.
+			ctlgRef.Ref.Registry = mirrorRef.Ref.Registry
+			ctlgRef.Ref.Namespace = path.Join(o.UserNamespace, ctlgRef.Ref.Namespace)
+			ctlgRef = ctlgRef.SetDefaults()
+			// Unset the ID when passing to the image builder.
+			// Tags are needed here since the digest will be recalculated.
+			ctlgRef.Ref.ID = ""
+
+			// Add to mapping for ICSP generation
+			_, err = o.copyImage(ctx, "oci://"+fpath, "docker://"+ctlgRef.Ref.String(), o.remoteRegFuncs)
+			if err != nil {
+				return fmt.Errorf("error copying image %s to %s: %v", "oci://"+fpath, "docker://"+ctlgRef.Ref.String(), err)
+			}
+		}
+		return nil
+	}); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 /*
 processCatalogRefs uses the image builder to update a given image using the data provided in catalogRefs.
 
@@ -240,12 +314,17 @@ func (o *MirrorOptions) processCatalogRefs(ctx context.Context, catalogsByImage
 		layersToDelete = append(layersToDelete, deletedConfigLayer)
 
 		if withCacheRegeneration {
-
-			opmCmdPath := filepath.Join(artifactDir, config.OpmBinDir, "opm")
-			_, err = os.Stat(opmCmdPath)
-			if err != nil {
-				return fmt.Errorf("cannot find opm in the extracted catalog %v for %s on %s: %v", ctlgRef, runtime.GOOS, runtime.GOARCH, err)
+			opmCmdPath := ""
+			if opmBinary := os.Getenv("OPM_BINARY"); opmBinary != "" {
+				opmCmdPath = opmBinary
+			} else {
+				opmCmdPath = filepath.Join(artifactDir, config.OpmBinDir, "opm")
+				_, err = os.Stat(opmCmdPath)
+				if err != nil {
+					return fmt.Errorf("cannot find opm in the extracted catalog %v for %s on %s: %v", ctlgRef, runtime.GOOS, runtime.GOARCH, err)
+				}
 			}
+
 			absConfigPath, err := filepath.Abs(filepath.Join(artifactDir, config.IndexDir))
 			if err != nil {
 				return fmt.Errorf("error getting absolute path for catalog's index %v: %v", filepath.Join(artifactDir, config.IndexDir), err)
@@ -354,12 +433,16 @@ func extractOPMAndCache(ctx context.Context, srcRef image.TypedImageReference, c
 	}
 	// cachePath exists, opm binary will be needed to regenerate it
 	opmBinaryFileName, err := copyOPMBinary(img, ctlgSrcDir)
-	if err != nil {
+	opmBinary := os.Getenv("OPM_BINARY")
+	if err != nil && opmBinary == "" {
 		return err
+	} else {
+		klog.Warning("unable to extract opm binary from the catalog %s: %v", img, err)
+		klog.Warning("ignoring, since $OPM_BINARY is provided")
 	}
 	// check for the extracted opm file (it should exist if we found something)
 	_, err = os.Stat(opmBinaryFileName)
-	if errors.Is(err, os.ErrNotExist) {
+	if errors.Is(err, os.ErrNotExist) && opmBinary == "" {
 		return fmt.Errorf("opm binary not found after extracting opm from catalog image %v", srcRef)
 	}
 	err = os.Chmod(opmBinaryFileName, 0744)

pkg/cli/mirror/fbc_operators.go

@@ -523,7 +523,7 @@ func (o *MirrorOptions) copyImage(ctx context.Context, from, to string, funcs Re
 		SourceCtx:             sourceCtx,
 		DestinationCtx:        destinationCtx,
 		ForceManifestMIMEType: "",
-		ImageListSelection:    imagecopy.CopySystemImage,
+		ImageListSelection:    imagecopy.CopyAllImages,
 		OciDecryptConfig:      nil,
 		OciEncryptLayers:      nil,
 		OciEncryptConfig:      nil,

pkg/cli/mirror/mirror.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"strings"
 
+	imagecopy "github.com/containers/image/v5/copy"
+	"github.com/containers/image/v5/signature"
 	"github.com/containers/image/v5/types"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -340,6 +342,9 @@ func (o *MirrorOptions) Run(cmd *cobra.Command, f kcmdutil.Factory) (err error)
 func (o *MirrorOptions) mirrorImages(ctx context.Context, cleanup cleanupFunc) error {
 
 	o.remoteRegFuncs = RemoteRegFuncs{
+		copy: func(ctx context.Context, policyContext *signature.PolicyContext, destRef types.ImageReference, srcRef types.ImageReference, options *imagecopy.Options) (copiedManifest []byte, retErr error) {
+			return imagecopy.Image(ctx, policyContext, destRef, srcRef, options)
+		},
 		newImageSource: func(ctx context.Context, sys *types.SystemContext, imgRef types.ImageReference) (types.ImageSource, error) {
 			return imgRef.NewImageSource(ctx, sys)
 		},
@@ -746,7 +751,7 @@ func (o *MirrorOptions) mirrorToMirrorWrapper(ctx context.Context, cfg v1alpha2.
 		return err
 	}
 	// process catalog FBC images
-	if len(cfg.Mirror.Operators) > 0 {
+	if o.RebuildCatalogs && len(cfg.Mirror.Operators) > 0 {
 		ctlgRefs, err := o.rebuildCatalogs(ctx, filepath.Join(o.Dir, config.SourceDir))
 		if err != nil {
 			return fmt.Errorf("error rebuilding catalog images from file-based catalogs: %v", err)

pkg/cli/mirror/operator.go

@@ -194,17 +194,34 @@ func (o *OperatorOptions) run(
 		} else if targetCtlg.Ref.Tag != "" {
 			ctlgSrcDir = filepath.Join(ctlgSrcDir, targetCtlg.Ref.Tag)
 		}
-		err = extractOPMAndCache(ctx, ctlgRef, ctlgSrcDir, o.SourceSkipTLS)
-		if err != nil {
-			reg.Destroy()
-			return nil, fmt.Errorf("unable to extract OPM binary from catalog %s: %v", targetName, err)
+		if o.RebuildCatalogs {
+			err = extractOPMAndCache(ctx, ctlgRef, ctlgSrcDir, o.SourceSkipTLS)
+			if err != nil {
+				reg.Destroy()
+				return nil, fmt.Errorf("unable to extract OPM binary from catalog %s: %v", targetName, err)
+			}
 		}
 
 		mappings, err := o.plan(ctx, dc, ic, ctlgRef, targetCtlg)
 		if err != nil {
 			reg.Destroy()
 			return nil, err
 		}
+		if !o.RebuildCatalogs {
+			var destCatalogRef string
+			if o.ToMirror != "" { // mirror to mirror
+				destCatalogRef, err = prepareDestCatalogRef(ctlg, o.ToMirror, o.UserNamespace)
+				if err != nil {
+					return nil, err
+				}
+			} else { // mirror to disk
+				destCatalogRef = "oci://" + ctlgSrcDir + "/layout"
+			}
+			_, err = o.copyImage(ctx, ctlg.Catalog, destCatalogRef, o.remoteRegFuncs)
+			if err != nil {
+				return nil, err
+			}
+		}
 		mmapping.Merge(mappings)
 		reg.Destroy()
 	}
@@ -604,21 +621,23 @@ func (o *OperatorOptions) plan(ctx context.Context, dc *declcfg.DeclarativeConfi
 		return nil, err
 	}
 
-	// Remove the catalog image from mappings we are going to transfer this
-	// using an OCI layout.
-	var ctlgImg image.TypedImage
-	if ctlgRef.Type == "oci" {
-		ctlgImg, err = image.ParseTypedImage(ctlgRef.OCIFBCPath, v1alpha2.TypeOperatorBundle)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		ctlgImg, err = image.ParseTypedImage(ctlgRef.Ref.Exact(), v1alpha2.TypeOperatorBundle)
-		if err != nil {
-			return nil, err
+	if o.RebuildCatalogs {
+		// Remove the catalog image from mappings we are going to transfer this
+		// using an OCI layout.
+		var ctlgImg image.TypedImage
+		if ctlgRef.Type == "oci" {
+			ctlgImg, err = image.ParseTypedImage(ctlgRef.OCIFBCPath, v1alpha2.TypeOperatorBundle)
+			if err != nil {
+				return nil, err
+			}
+		} else {
+			ctlgImg, err = image.ParseTypedImage(ctlgRef.Ref.Exact(), v1alpha2.TypeOperatorBundle)
+			if err != nil {
+				return nil, err
+			}
 		}
+		mappings.Remove(ctlgImg)
 	}
-	mappings.Remove(ctlgImg)
 	// Write catalog OCI layout file to src so it is included in the archive
 	// at a path unique to the image.
 	if ctlgRef.Type != image.DestinationOCI {
@@ -655,15 +674,17 @@ func (o *OperatorOptions) plan(ctx context.Context, dc *declcfg.DeclarativeConfi
 	}
 	// Remove catalog namespace prefix from each mapping's destination, which is added by opts.Run().
 	for srcRef, dstRef := range mappings {
-		newRepoName := strings.TrimPrefix(dstRef.Ref.RepositoryName(), ctlgRef.Ref.RepositoryName())
-		newRepoName = strings.TrimPrefix(newRepoName, "/")
-		tmpRef, err := imgreference.Parse(newRepoName)
-		if err != nil {
-			return nil, err
+		if srcRef.Ref.String() != ctlgRef.Ref.String() || o.RebuildCatalogs { // OCPBUGS-31536: don't do this for catalog images unless they will be rebuilt
+			newRepoName := strings.TrimPrefix(dstRef.Ref.RepositoryName(), ctlgRef.Ref.RepositoryName())
+			newRepoName = strings.TrimPrefix(newRepoName, "/")
+			tmpRef, err := imgreference.Parse(newRepoName)
+			if err != nil {
+				return nil, err
+			}
+			dstRef.Ref.Namespace = tmpRef.Namespace
+			dstRef.Ref.Name = tmpRef.Name
+			mappings[srcRef] = dstRef
 		}
-		dstRef.Ref.Namespace = tmpRef.Namespace
-		dstRef.Ref.Name = tmpRef.Name
-		mappings[srcRef] = dstRef
 	}
 	return mappings, validateMapping(*dc, mappings)
 }

pkg/cli/mirror/options.go

@@ -39,6 +39,7 @@ type MirrorOptions struct {
 	OCIRegistriesConfig        string // Registries config file location (it works only with local oci catalogs)
 	OCIInsecureSignaturePolicy bool   // If set, OCI catalog push will not try to push signatures
 	MaxNestedPaths             int
+	RebuildCatalogs            bool // If set, rebuilds catalogs based on filtered declarative config, and regenerates the cache of that catalog
 	// cancelCh is a channel listening for command cancellations
 	cancelCh                          <-chan struct{}
 	once                              sync.Once
@@ -76,6 +77,7 @@ func (o *MirrorOptions) BindFlags(fs *pflag.FlagSet) {
 	fs.BoolVar(&o.OCIInsecureSignaturePolicy, "oci-insecure-signature-policy", o.OCIInsecureSignaturePolicy, "If set, OCI catalog push will not try to push signatures")
 	fs.BoolVar(&o.SkipPruning, "skip-pruning", o.SkipPruning, "If set, will disable pruning globally")
 	fs.IntVar(&o.MaxNestedPaths, "max-nested-paths", 0, "Number of nested paths, for destination registries that limit nested paths")
+	fs.BoolVar(&o.RebuildCatalogs, "rebuild-catalogs", o.RebuildCatalogs, "If set (defaults to false), rebuilds catalogs based on filtered declarative config, and regenerates the cache of that catalog")
 }
 
 func (o *MirrorOptions) init() {

pkg/cli/mirror/publish.go

@@ -336,19 +336,26 @@ func (o *MirrorOptions) processMirroredImages(ctx context.Context, assocs image.
 // processCustomImages builds custom images for operator catalogs or Cincinnati graph data if data is present in the archive
 func (o *MirrorOptions) processCustomImages(ctx context.Context, dir string, filesInArchive map[string]string) (image.TypedImageMapping, error) {
 	allMappings := image.TypedImageMapping{}
-	// process catalogs
+	// process catalogs only when --rebuild-catalogs flag is on
 	klog.V(2).Infof("rebuilding catalog images")
 	found, err := o.unpackCatalog(dir, filesInArchive)
 	if err != nil {
 		return allMappings, err
 	}
 
 	if found {
-		ctlgRefs, err := o.rebuildCatalogs(ctx, dir)
-		if err != nil {
-			return allMappings, fmt.Errorf("error rebuilding catalog images from file-based catalogs: %v", err)
+		if o.RebuildCatalogs {
+			ctlgRefs, err := o.rebuildCatalogs(ctx, dir)
+			if err != nil {
+				return allMappings, fmt.Errorf("error rebuilding catalog images from file-based catalogs: %v", err)
+			}
+			allMappings.Merge(ctlgRefs)
+		} else {
+			err := o.copyCatalogs(ctx, dir)
+			if err != nil {
+				return allMappings, fmt.Errorf("error rebuilding catalog images from file-based catalogs: %v", err)
+			}
 		}
-		allMappings.Merge(ctlgRefs)
 	}
 
 	klog.V(2).Infof("building cincinnati graph data image")

test/e2e/lib/workflow.sh

@@ -140,7 +140,7 @@ function workflow_m2d2m_oci_catalog() {
   if !$DIFF; then
     cleanup_conn
   fi
-  run_cmd --from "${CREATE_FULL_DIR}/mirror_seq1_000000.tar" "docker://${remote_image}"
+  run_cmd --from "${CREATE_FULL_DIR}/mirror_seq1_000000.tar" "docker://${remote_image}" $PUBLISH_FLAGS
 
   popd
 }