Fixed CatalogMetadata syncing issues related to metadata.name and etc…

…d size constraints

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>

internal/k8sutil/k8sutil.go

@@ -0,0 +1,17 @@
+package k8sutil
+
+import (
+	"regexp"
+
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+var invalidDNSChars = regexp.MustCompile(`[^a-zA-Z0-9-]`)
+
+// MetadataName replaces all invalid DNS characters with a dash. If the result
+// is not a valid DNS subdomain, returns `result, false`. Otherwise, returns the
+// `result, true`.
+func MetadataName(name string) (string, bool) {
+	result := invalidDNSChars.ReplaceAllString(name, "-")
+	return result, validation.IsDNS1123Subdomain(result) == nil
+}

pkg/controllers/core/catalog_controller.go

@@ -18,14 +18,14 @@ package core
 
 import (
 	"context"
-	// #nosec
-	"crypto/sha1"
+	"crypto/sha1" // #nosec
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"io/fs"
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
+	"github.com/operator-framework/operator-registry/alpha/property"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -40,6 +40,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	"github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/catalogd/internal/k8sutil"
 	"github.com/operator-framework/catalogd/internal/source"
 	"github.com/operator-framework/catalogd/pkg/features"
 )
@@ -223,7 +224,7 @@ func (r *CatalogReconciler) syncBundleMetadata(ctx context.Context, declCfg *dec
 	newBundles := map[string]*v1alpha1.BundleMetadata{}
 
 	for _, bundle := range declCfg.Bundles {
-		bundleName := fmt.Sprintf("%s-%s", catalog.Name, bundle.Name)
+		bundleName, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, bundle.Name))
 
 		bundleMeta := v1alpha1.BundleMetadata{
 			TypeMeta: metav1.TypeMeta{
@@ -303,7 +304,7 @@ func (r *CatalogReconciler) syncPackages(ctx context.Context, declCfg *declcfg.D
 	newPkgs := map[string]*v1alpha1.Package{}
 
 	for _, pkg := range declCfg.Packages {
-		name := fmt.Sprintf("%s-%s", catalog.Name, pkg.Name)
+		name, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, pkg.Name))
 		var icon *v1alpha1.Icon
 		if pkg.Icon != nil {
 			icon = &v1alpha1.Icon{
@@ -342,7 +343,7 @@ func (r *CatalogReconciler) syncPackages(ctx context.Context, declCfg *declcfg.D
 	}
 
 	for _, ch := range declCfg.Channels {
-		pkgName := fmt.Sprintf("%s-%s", catalog.Name, ch.Package)
+		pkgName, _ := k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, ch.Package))
 		pkg, ok := newPkgs[pkgName]
 		if !ok {
 			return fmt.Errorf("channel %q references package %q which does not exist", ch.Name, ch.Package)
@@ -404,6 +405,26 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
 			return fmt.Errorf("error in generating catalog metadata name: %w", err)
 		}
 
+		blob := meta.Blob
+		if meta.Schema == declcfg.SchemaBundle {
+			var b declcfg.Bundle
+			if err := json.Unmarshal(blob, &b); err != nil {
+				return fmt.Errorf("error unmarshalling bundle: %w", err)
+			}
+			properties := b.Properties[:0]
+			for _, p := range b.Properties {
+				if p.Type == property.TypeBundleObject {
+					continue
+				}
+				properties = append(properties, p)
+			}
+			b.Properties = properties
+			blob, err = json.Marshal(b)
+			if err != nil {
+				return fmt.Errorf("error marshalling bundle: %w", err)
+			}
+		}
+
 		catalogMetadata := &v1alpha1.CatalogMetadata{
 			TypeMeta: metav1.TypeMeta{
 				APIVersion: v1alpha1.GroupVersion.String(),
@@ -432,7 +453,7 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
 				Name:    meta.Name,
 				Schema:  meta.Schema,
 				Package: meta.Package,
-				Content: meta.Blob,
+				Content: blob,
 			},
 		}
 
@@ -474,6 +495,7 @@ func (r *CatalogReconciler) syncCatalogMetadata(ctx context.Context, fsys fs.FS,
 // In the place of the empty `meta.Name`, it computes a hash of `meta.Blob` to prevent multiple FBC blobs colliding on the the object name.
 // Possible outcomes are: "{catalogName}-{meta.Schema}-{meta.Name}", "{catalogName}-{meta.Schema}-{meta.Package}-{meta.Name}",
 // "{catalogName}-{meta.Schema}-{hash{meta.Blob}}", "{catalogName}-{meta.Schema}-{meta.Package}-{hash{meta.Blob}}".
+// Characters that would result in an invalid DNS name are replaced with dashes.
 func generateCatalogMetadataName(_ context.Context, catalogName string, meta *declcfg.Meta) (string, error) {
 	objName := fmt.Sprintf("%s-%s", catalogName, meta.Schema)
 	if meta.Package != "" {
@@ -491,5 +513,6 @@ func generateCatalogMetadataName(_ context.Context, catalogName string, meta *de
 		h.Write(metaJSON)
 		objName = fmt.Sprintf("%s-%s", objName, hex.EncodeToString(h.Sum(nil)))
 	}
+	objName, _ = k8sutil.MetadataName(objName)
 	return objName, nil
 }

pkg/controllers/core/catalog_controller_test.go

@@ -19,6 +19,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
 	"github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/catalogd/internal/k8sutil"
 	"github.com/operator-framework/catalogd/internal/source"
 	"github.com/operator-framework/catalogd/pkg/controllers/core"
 	"github.com/operator-framework/catalogd/pkg/features"
@@ -222,14 +223,14 @@ var _ = Describe("Catalogd Controller Test", func() {
 
 			When("unpacker returns source.Result with state == 'Unpacked'", func() {
 				var (
-					testBundleName              = "webhook-operator.v0.0.1"
+					testBundleName              = "webhook_operator.v0.0.1"
 					testBundleImage             = "quay.io/olmtest/webhook-operator-bundle:0.0.3"
 					testBundleRelatedImageName  = "test"
 					testBundleRelatedImageImage = "testimage:latest"
 					testBundleObjectData        = "dW5pbXBvcnRhbnQK"
-					testPackageDefaultChannel   = "preview"
-					testPackageName             = "webhook-operator"
-					testChannelName             = "preview"
+					testPackageDefaultChannel   = "preview_test"
+					testPackageName             = "webhook_operator_test"
+					testChannelName             = "preview_test"
 					testPackage                 = fmt.Sprintf(testPackageTemplate, testPackageDefaultChannel, testPackageName)
 					testBundle                  = fmt.Sprintf(testBundleTemplate, testBundleImage, testBundleName, testPackageName, testBundleRelatedImageName, testBundleRelatedImageImage, testBundleObjectData)
 					testChannel                 = fmt.Sprintf(testChannelTemplate, testPackageName, testChannelName, testBundleName)
@@ -238,8 +239,8 @@ var _ = Describe("Catalogd Controller Test", func() {
 					testPackageMetaName string
 				)
 				BeforeEach(func() {
-					testBundleMetaName = fmt.Sprintf("%s-%s", catalog.Name, testBundleName)
-					testPackageMetaName = fmt.Sprintf("%s-%s", catalog.Name, testPackageName)
+					testBundleMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, testBundleName))
+					testPackageMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", catalog.Name, testPackageName))
 
 					filesys := &fstest.MapFS{
 						"bundle.yaml":  &fstest.MapFile{Data: []byte(testBundle), Mode: os.ModePerm},
@@ -286,21 +287,9 @@ var _ = Describe("Catalogd Controller Test", func() {
 					})
 
 					AfterEach(func() {
-						// clean up package
-						pkg := &v1alpha1.Package{
-							ObjectMeta: metav1.ObjectMeta{
-								Name: testPackageMetaName,
-							},
-						}
-						Expect(cl.Delete(ctx, pkg)).To(Succeed())
+						Expect(cl.DeleteAllOf(ctx, &v1alpha1.Package{})).To(Succeed())
+						Expect(cl.DeleteAllOf(ctx, &v1alpha1.BundleMetadata{})).To(Succeed())
 
-						// clean up bundlemetadata
-						bm := &v1alpha1.BundleMetadata{
-							ObjectMeta: metav1.ObjectMeta{
-								Name: testBundleMetaName,
-							},
-						}
-						Expect(cl.Delete(ctx, bm)).To(Succeed())
 						Expect(features.CatalogdFeatureGate.SetFromMap(map[string]bool{
 							string(features.PackagesBundleMetadataAPIs): false,
 							string(features.CatalogMetadataAPI):         false,
@@ -358,8 +347,8 @@ var _ = Describe("Catalogd Controller Test", func() {
 								},
 							}
 
-							tempTestBundleMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testBundleName)
-							tempTestPackageMetaName = fmt.Sprintf("%s-%s", tempCatalog.Name, testPackageName)
+							tempTestBundleMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", tempCatalog.Name, testBundleName))
+							tempTestPackageMetaName, _ = k8sutil.MetadataName(fmt.Sprintf("%s-%s", tempCatalog.Name, testPackageName))
 
 							Expect(cl.Create(ctx, tempCatalog)).To(Succeed())
 							res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "tempedout"}})