Enable CatalogMetadataAPI via explicit flag, fix syncing issues

config/default/manager_auth_proxy_patch.yaml

@@ -50,5 +50,5 @@ spec:
         - "--health-probe-bind-address=:8081"
         - "--metrics-bind-address=127.0.0.1:8080"
         - "--leader-elect"
-        - "--feature-gates=PackagesBundleMetadataAPIs=true"
+        - "--feature-gates=PackagesBundleMetadataAPIs=true,CatalogMetadataAPI=true"
 

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/onsi/gomega v1.27.7
 	github.com/operator-framework/operator-registry v1.27.1
 	github.com/spf13/pflag v1.0.5
+	github.com/stretchr/testify v1.8.0
 	k8s.io/api v0.26.1
 	k8s.io/apimachinery v0.26.1
 	k8s.io/client-go v0.26.1
@@ -55,6 +56,7 @@ require (
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/operator-framework/api v0.17.4-0.20230223191600-0131a6301e42 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_golang v1.14.0 // indirect
 	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/common v0.37.0 // indirect

go.sum

@@ -300,6 +300,7 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
@@ -308,6 +309,7 @@ github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk=
+github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/xanzy/ssh-agent v0.3.0/go.mod h1:3s9xbODqPuuhK9JV1R321M/FlMZSBvE5aY6eAcqrDh0=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

internal/k8sutil/k8sutil.go

@@ -0,0 +1,17 @@
+package k8sutil
+
+import (
+	"regexp"
+
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+var invalidNameChars = 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 := invalidNameChars.ReplaceAllString(name, "-")
+	return result, validation.IsDNS1123Subdomain(result) == nil
+}

internal/k8sutil/k8sutil_test.go

@@ -0,0 +1,62 @@
+package k8sutil
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMetadataName(t *testing.T) {
+	type testCase struct {
+		name           string
+		in             string
+		expectedResult string
+		expectedValid  bool
+	}
+	for _, tc := range []testCase{
+		{
+			name:           "empty",
+			in:             "",
+			expectedResult: "",
+			expectedValid:  false,
+		},
+		{
+			name:           "invalid",
+			in:             "foo-bar.123!",
+			expectedResult: "foo-bar.123-",
+			expectedValid:  false,
+		},
+		{
+			name:           "too long",
+			in:             fmt.Sprintf("foo-bar_%s", strings.Repeat("1234567890", 50)),
+			expectedResult: fmt.Sprintf("foo-bar-%s", strings.Repeat("1234567890", 50)),
+			expectedValid:  false,
+		},
+		{
+			name:           "valid",
+			in:             "foo-bar.123",
+			expectedResult: "foo-bar.123",
+			expectedValid:  true,
+		},
+		{
+			name:           "valid with underscore",
+			in:             "foo-bar_123",
+			expectedResult: "foo-bar-123",
+			expectedValid:  true,
+		},
+		{
+			name:           "valid with colon",
+			in:             "foo-bar:123",
+			expectedResult: "foo-bar-123",
+			expectedValid:  true,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			actualResult, actualValid := MetadataName(tc.in)
+			assert.Equal(t, tc.expectedResult, actualResult)
+			assert.Equal(t, tc.expectedValid, actualValid)
+		})
+	}
+}

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"}})