NO-ISSUE: Synchronize From Upstream Repositories

Makefile

@@ -151,9 +151,15 @@ generate: $(CONTROLLER_GEN) #EXHELP Generate code containing DeepCopy, DeepCopyI
 	$(CONTROLLER_GEN) --load-build-tags=$(GO_BUILD_TAGS) object:headerFile="hack/boilerplate.go.txt" paths="./..."
 
 .PHONY: verify
-verify: tidy fmt generate manifests crd-ref-docs #HELP Verify all generated code is up-to-date.
+verify: tidy fmt generate manifests crd-ref-docs generate-test-data #HELP Verify all generated code is up-to-date.
 	git diff --exit-code
 
+# Renders registry+v1 bundles in test/convert
+# Used by CI in verify to catch regressions in the registry+v1 -> plain conversion code
+.PHONY: generate-test-data
+generate-test-data:
+	go run test/convert/generate-manifests.go
+
 .PHONY: fix-lint
 fix-lint: $(GOLANGCI_LINT) #EXHELP Fix lint issues
 	$(GOLANGCI_LINT) run --fix --build-tags $(GO_BUILD_TAGS) $(GOLANGCI_LINT_ARGS)

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: d3aec37756e449b47c0c5ed96d9d09c3c709c291
+expectedMergeBase: 1f0b4f240005af30bef7a545cde5ab82dbf5a65c
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

internal/operator-controller/applier/helm_test.go

@@ -101,7 +101,8 @@ func (mag *mockActionGetter) Reconcile(rel *release.Release) error {
 var (
 	// required for unmockable call to convert.RegistryV1ToHelmChart
 	validFS = fstest.MapFS{
-		"metadata/annotations.yaml": &fstest.MapFile{Data: []byte("{}")},
+		"metadata/annotations.yaml": &fstest.MapFile{Data: []byte(`annotations:
+  operators.operatorframework.io.bundle.package.v1: my-package`)},
 		"manifests": &fstest.MapFile{Data: []byte(`apiVersion: operators.operatorframework.io/v1alpha1
 kind: ClusterServiceVersion
 metadata:

internal/operator-controller/rukpak/convert/registryv1.go

@@ -13,6 +13,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -32,6 +33,7 @@ import (
 type RegistryV1 struct {
 	PackageName string
 	CSV         v1alpha1.ClusterServiceVersion
+	CRDs        []apiextensionsv1.CustomResourceDefinition
 	Others      []unstructured.Unstructured
 }
 
@@ -45,7 +47,7 @@ func RegistryV1ToHelmChart(rv1 fs.FS, installNamespace string, watchNamespace st
 		return nil, err
 	}
 
-	plain, err := Convert(reg, installNamespace, []string{watchNamespace})
+	plain, err := PlainConverter.Convert(reg, installNamespace, []string{watchNamespace})
 	if err != nil {
 		return nil, err
 	}
@@ -121,6 +123,12 @@ func ParseFS(rv1 fs.FS) (RegistryV1, error) {
 				}
 				reg.CSV = csv
 				foundCSV = true
+			case "CustomResourceDefinition":
+				crd := apiextensionsv1.CustomResourceDefinition{}
+				if err := runtime.DefaultUnstructuredConverter.FromUnstructured(info.Object.(*unstructured.Unstructured).Object, &crd); err != nil {
+					return err
+				}
+				reg.CRDs = append(reg.CRDs, crd)
 			default:
 				reg.Others = append(reg.Others, *info.Object.(*unstructured.Unstructured))
 			}
@@ -222,15 +230,23 @@ func saNameOrDefault(saName string) string {
 	return saName
 }
 
-func Convert(in RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) {
+type Converter struct {
+	BundleValidator BundleValidator
+}
+
+func (c Converter) Convert(rv1 RegistryV1, installNamespace string, targetNamespaces []string) (*Plain, error) {
+	if err := c.BundleValidator.Validate(&rv1); err != nil {
+		return nil, err
+	}
+
 	if installNamespace == "" {
-		installNamespace = in.CSV.Annotations["operatorframework.io/suggested-namespace"]
+		installNamespace = rv1.CSV.Annotations["operatorframework.io/suggested-namespace"]
 	}
 	if installNamespace == "" {
-		installNamespace = fmt.Sprintf("%s-system", in.PackageName)
+		installNamespace = fmt.Sprintf("%s-system", rv1.PackageName)
 	}
 	supportedInstallModes := sets.New[string]()
-	for _, im := range in.CSV.Spec.InstallModes {
+	for _, im := range rv1.CSV.Spec.InstallModes {
 		if im.Supported {
 			supportedInstallModes.Insert(string(im.Type))
 		}
@@ -247,18 +263,18 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 		return nil, err
 	}
 
-	if len(in.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
+	if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
 		return nil, fmt.Errorf("apiServiceDefintions are not supported")
 	}
 
-	if len(in.CSV.Spec.WebhookDefinitions) > 0 {
+	if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
 		return nil, fmt.Errorf("webhookDefinitions are not supported")
 	}
 
 	deployments := []appsv1.Deployment{}
 	serviceAccounts := map[string]corev1.ServiceAccount{}
-	for _, depSpec := range in.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
-		annotations := util.MergeMaps(in.CSV.Annotations, depSpec.Spec.Template.Annotations)
+	for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
+		annotations := util.MergeMaps(rv1.CSV.Annotations, depSpec.Spec.Template.Annotations)
 		annotations["olm.targetNamespaces"] = strings.Join(targetNamespaces, ",")
 		depSpec.Spec.Template.Annotations = annotations
 		deployments = append(deployments, appsv1.Deployment{
@@ -292,8 +308,8 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 	clusterRoles := []rbacv1.ClusterRole{}
 	clusterRoleBindings := []rbacv1.ClusterRoleBinding{}
 
-	permissions := in.CSV.Spec.InstallStrategy.StrategySpec.Permissions
-	clusterPermissions := in.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
+	permissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.Permissions
+	clusterPermissions := rv1.CSV.Spec.InstallStrategy.StrategySpec.ClusterPermissions
 	allPermissions := append(permissions, clusterPermissions...)
 
 	// Create all the service accounts
@@ -320,7 +336,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 	for _, ns := range targetNamespaces {
 		for _, permission := range permissions {
 			saName := saNameOrDefault(permission.ServiceAccountName)
-			name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
+			name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
 			if err != nil {
 				return nil, err
 			}
@@ -331,7 +347,7 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 
 	for _, permission := range clusterPermissions {
 		saName := saNameOrDefault(permission.ServiceAccountName)
-		name, err := generateName(fmt.Sprintf("%s-%s", in.CSV.Name, saName), permission)
+		name, err := generateName(fmt.Sprintf("%s-%s", rv1.CSV.Name, saName), permission)
 		if err != nil {
 			return nil, err
 		}
@@ -362,7 +378,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 		obj := obj
 		objs = append(objs, &obj)
 	}
-	for _, obj := range in.Others {
+	for _, obj := range rv1.CRDs {
+		objs = append(objs, &obj)
+	}
+	for _, obj := range rv1.Others {
 		obj := obj
 		supported, namespaced := registrybundle.IsSupported(obj.GetKind())
 		if !supported {
@@ -380,6 +399,10 @@ func Convert(in RegistryV1, installNamespace string, targetNamespaces []string)
 	return &Plain{Objects: objs}, nil
 }
 
+var PlainConverter = Converter{
+	BundleValidator: RegistryV1BundleValidator,
+}
+
 const maxNameLength = 63
 
 func generateName(base string, o interface{}) (string, error) {

internal/operator-controller/rukpak/convert/registryv1_test.go

@@ -1,6 +1,7 @@
 package convert_test
 
 import (
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -52,6 +53,24 @@ func getCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) {
 	return csv, svc
 }
 
+func TestConverterValidatesBundle(t *testing.T) {
+	converter := convert.Converter{
+		BundleValidator: []func(rv1 *convert.RegistryV1) []error{
+			func(rv1 *convert.RegistryV1) []error {
+				return []error{errors.New("test error")}
+			},
+		},
+	}
+
+	_, err := converter.Convert(convert.RegistryV1{}, "installNamespace", []string{"watchNamespace"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "test error")
+}
+
+func TestPlainConverterUsedRegV1Validator(t *testing.T) {
+	require.Equal(t, convert.RegistryV1BundleValidator, convert.PlainConverter.BundleValidator)
+}
+
 func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) {
 	var targetNamespaces []string
 
@@ -70,7 +89,7 @@ func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -104,7 +123,7 @@ func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -145,7 +164,7 @@ func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "bundle contains unsupported resource")
 	require.Nil(t, plainBundle)
@@ -179,7 +198,7 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, targetNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, targetNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -266,7 +285,7 @@ func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -299,7 +318,7 @@ func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -332,7 +351,7 @@ func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -365,7 +384,7 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.NoError(t, err)
 
 	t.Log("By verifying if plain bundle has required objects")
@@ -470,7 +489,7 @@ func TestConvertInstallModeValidation(t *testing.T) {
 			}
 
 			t.Log("By converting to plain")
-			plainBundle, err := convert.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces)
+			plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces)
 			require.Error(t, err)
 			require.Nil(t, plainBundle)
 		})
@@ -559,7 +578,7 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "webhookDefinitions are not supported")
 	require.Nil(t, plainBundle)
@@ -590,7 +609,7 @@ func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) {
 	}
 
 	t.Log("By converting to plain")
-	plainBundle, err := convert.Convert(registryv1Bundle, installNamespace, watchNamespaces)
+	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
 	require.Error(t, err)
 	require.ErrorContains(t, err, "apiServiceDefintions are not supported")
 	require.Nil(t, plainBundle)