Move install mode validation to renderer

Signed-off-by: Per G. da Silva <[email protected]>

Author: Per G. da Silva

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

@@ -191,29 +191,6 @@ func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS
 	return nil
 }
 
-func validateTargetNamespaces(supportedInstallModes sets.Set[string], installNamespace string, targetNamespaces []string) error {
-	set := sets.New[string](targetNamespaces...)
-	switch {
-	case set.Len() == 0 || (set.Len() == 1 && set.Has("")):
-		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) {
-			return nil
-		}
-		return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes))
-	case set.Len() == 1 && !set.Has(""):
-		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) {
-			return nil
-		}
-		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace {
-			return nil
-		}
-	default:
-		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") {
-			return nil
-		}
-	}
-	return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces)
-}
-
 var PlainConverter = Converter{
 	BundleRenderer: render.BundleRenderer{
 		BundleValidator: validators.RegistryV1BundleValidator,
@@ -255,10 +232,6 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe
 		}
 	}
 
-	if err := validateTargetNamespaces(supportedInstallModes, installNamespace, targetNamespaces); err != nil {
-		return nil, err
-	}
-
 	if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 {
 		return nil, fmt.Errorf("apiServiceDefintions are not supported")
 	}

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

@@ -244,138 +244,6 @@ func getBaseCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) {
 	return baseCSV, svc
 }
 
-func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) {
-	t.Log("RegistryV1 Suite Convert")
-	t.Log("It should generate objects successfully based on target namespaces")
-
-	t.Log("It should convert into plain manifests successfully with AllNamespaces")
-	baseCSV, svc := getBaseCsvAndService()
-	csv := baseCSV.DeepCopy()
-	csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}}
-
-	t.Log("By creating a registry v1 bundle")
-	watchNamespaces := []string{""}
-	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := render.RegistryV1{
-		PackageName: "testPkg",
-		CSV:         *csv,
-		Others:      []unstructured.Unstructured{unstructuredSvc},
-	}
-
-	t.Log("By converting to plain")
-	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
-	require.NoError(t, err)
-
-	t.Log("By verifying if plain bundle has required objects")
-	require.NotNil(t, plainBundle)
-	require.Len(t, plainBundle.Objects, 5)
-
-	t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template")
-	dep := findObjectByName("test-deployment", plainBundle.Objects)
-	require.NotNil(t, dep)
-	require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces)
-	require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces])
-}
-
-func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) {
-	t.Log("RegistryV1 Suite Convert")
-	t.Log("It should generate objects successfully based on target namespaces")
-
-	t.Log("It should convert into plain manifests successfully with MultiNamespace")
-	baseCSV, svc := getBaseCsvAndService()
-	csv := baseCSV.DeepCopy()
-	csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}}
-
-	t.Log("By creating a registry v1 bundle")
-	watchNamespaces := []string{"testWatchNs1", "testWatchNs2"}
-	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := render.RegistryV1{
-		PackageName: "testPkg",
-		CSV:         *csv,
-		Others:      []unstructured.Unstructured{unstructuredSvc},
-	}
-
-	t.Log("By converting to plain")
-	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
-	require.NoError(t, err)
-
-	t.Log("By verifying if plain bundle has required objects")
-	require.NotNil(t, plainBundle)
-	require.Len(t, plainBundle.Objects, 7)
-
-	t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template")
-	dep := findObjectByName("test-deployment", plainBundle.Objects)
-	require.NotNil(t, dep)
-	require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces)
-	require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces])
-}
-
-func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) {
-	t.Log("RegistryV1 Suite Convert")
-	t.Log("It should generate objects successfully based on target namespaces")
-
-	t.Log("It should convert into plain manifests successfully with SingleNamespace")
-	baseCSV, svc := getBaseCsvAndService()
-	csv := baseCSV.DeepCopy()
-	csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}}
-
-	t.Log("By creating a registry v1 bundle")
-	watchNamespaces := []string{"testWatchNs1"}
-	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := render.RegistryV1{
-		PackageName: "testPkg",
-		CSV:         *csv,
-		Others:      []unstructured.Unstructured{unstructuredSvc},
-	}
-
-	t.Log("By converting to plain")
-	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
-	require.NoError(t, err)
-
-	t.Log("By verifying if plain bundle has required objects")
-	require.NotNil(t, plainBundle)
-	require.Len(t, plainBundle.Objects, 5)
-
-	t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template")
-	dep := findObjectByName("test-deployment", plainBundle.Objects)
-	require.NotNil(t, dep)
-	require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces)
-	require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces])
-}
-
-func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) {
-	t.Log("RegistryV1 Suite Convert")
-	t.Log("It should generate objects successfully based on target namespaces")
-
-	t.Log("It should convert into plain manifests successfully with own namespace")
-	baseCSV, svc := getBaseCsvAndService()
-	csv := baseCSV.DeepCopy()
-	csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}}
-
-	t.Log("By creating a registry v1 bundle")
-	watchNamespaces := []string{installNamespace}
-	unstructuredSvc := convertToUnstructured(t, svc)
-	registryv1Bundle := render.RegistryV1{
-		PackageName: "testPkg",
-		CSV:         *csv,
-		Others:      []unstructured.Unstructured{unstructuredSvc},
-	}
-
-	t.Log("By converting to plain")
-	plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
-	require.NoError(t, err)
-
-	t.Log("By verifying if plain bundle has required objects")
-	require.NotNil(t, plainBundle)
-	require.Len(t, plainBundle.Objects, 5)
-
-	t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template")
-	dep := findObjectByName("test-deployment", plainBundle.Objects)
-	require.NotNil(t, dep)
-	require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces)
-	require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces])
-}
-
 func TestConvertInstallModeValidation(t *testing.T) {
 	for _, tc := range []struct {
 		description      string
@@ -609,7 +477,7 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) {
 	require.NotNil(t, plainBundle)
 }
 
-func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) {
+func TestRegistryV1SuiteGenerateNoAPIServiceDefinitions(t *testing.T) {
 	t.Log("RegistryV1 Suite Convert")
 	t.Log("It should generate objects successfully based on target namespaces")
 

internal/operator-controller/rukpak/render/render.go

@@ -2,10 +2,12 @@ package render
 
 import (
 	"errors"
+	"fmt"
 
 	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/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -78,6 +80,20 @@ func (o *Options) apply(opts ...Option) *Options {
 	return o
 }
 
+func (o *Options) validate(rv1 *RegistryV1) (*Options, []error) {
+	var errs []error
+	if len(o.TargetNamespaces) == 0 {
+		errs = append(errs, errors.New("at least one target namespace must be specified"))
+	}
+	if o.UniqueNameGenerator == nil {
+		errs = append(errs, errors.New("unique name generator must be specified"))
+	}
+	if err := validateTargetNamespaces(rv1, o.InstallNamespace, o.TargetNamespaces); err != nil {
+		errs = append(errs, fmt.Errorf("invalid target namespaces %v: %w", o.TargetNamespaces, err))
+	}
+	return o, errs
+}
+
 type Option func(*Options)
 
 func WithTargetNamespaces(namespaces ...string) Option {
@@ -109,13 +125,19 @@ func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, opts ...
 		return nil, err
 	}
 
-	genOpts := (&Options{
+	// generate bundle objects
+	genOpts, errs := (&Options{
+		// default options
 		InstallNamespace:    installNamespace,
 		TargetNamespaces:    []string{metav1.NamespaceAll},
 		UniqueNameGenerator: DefaultUniqueNameGenerator,
-	}).apply(opts...)
+		CertificateProvider: nil,
+	}).apply(opts...).validate(&rv1)
+
+	if len(errs) > 0 {
+		return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...))
+	}
 
-	// generate bundle objects
 	objs, err := ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts)
 	if err != nil {
 		return nil, err
@@ -131,3 +153,33 @@ func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) {
 	}
 	return util.ObjectNameForBaseAndSuffix(base, hashStr), nil
 }
+
+func validateTargetNamespaces(rv1 *RegistryV1, installNamespace string, targetNamespaces []string) error {
+	supportedInstallModes := sets.New[string]()
+	for _, im := range rv1.CSV.Spec.InstallModes {
+		if im.Supported {
+			supportedInstallModes.Insert(string(im.Type))
+		}
+	}
+
+	set := sets.New[string](targetNamespaces...)
+	switch {
+	case set.Len() == 0 || (set.Len() == 1 && set.Has("")):
+		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) {
+			return nil
+		}
+		return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes))
+	case set.Len() == 1 && !set.Has(""):
+		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) {
+			return nil
+		}
+		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace {
+			return nil
+		}
+	default:
+		if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") {
+			return nil
+		}
+	}
+	return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces)
+}