✨ OPRUN-4150: Relax webhook support preconditions

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

@@ -22,6 +22,7 @@ var BundleValidator = render.BundleValidator{
 	validators.CheckCRDResourceUniqueness,
 	validators.CheckOwnedCRDExistence,
 	validators.CheckPackageNameNotEmpty,
+	validators.CheckConversionWebhookSupport,
 	validators.CheckWebhookDeploymentReferentialIntegrity,
 	validators.CheckWebhookNameUniqueness,
 	validators.CheckWebhookNameIsDNS1123SubDomain,

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

@@ -26,6 +26,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
 		validators.CheckCRDResourceUniqueness,
 		validators.CheckOwnedCRDExistence,
 		validators.CheckPackageNameNotEmpty,
+		validators.CheckConversionWebhookSupport,
 		validators.CheckWebhookDeploymentReferentialIntegrity,
 		validators.CheckWebhookNameUniqueness,
 		validators.CheckWebhookNameIsDNS1123SubDomain,

internal/operator-controller/rukpak/render/registryv1/validators/validator.go

@@ -101,21 +101,32 @@ func CheckPackageNameNotEmpty(rv1 *bundle.RegistryV1) []error {
 	return nil
 }
 
-// CheckWebhookSupport checks that if the bundle cluster service version declares webhook definitions
-// that it is a singleton operator, i.e. that it only supports AllNamespaces mode. This keeps parity
-// with OLMv0 behavior for conversion webhooks,
+// CheckConversionWebhookSupport checks that if the bundle cluster service version declares conversion webhook definitions,
+// that the bundle also only supports AllNamespaces install mode. This keeps parity with OLMv0 behavior for conversion webhooks,
 // https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L193
-// Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks.
-// While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn
-// after getting the webhook support working.
-func CheckWebhookSupport(rv1 *bundle.RegistryV1) []error {
-	if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
+func CheckConversionWebhookSupport(rv1 *bundle.RegistryV1) []error {
+	var conversionWebhookNames []string
+	for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
+		if wh.Type == v1alpha1.ConversionWebhook {
+			conversionWebhookNames = append(conversionWebhookNames, wh.GenerateName)
+		}
+	}
+
+	if len(conversionWebhookNames) > 0 {
 		supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{}
 		for _, mode := range rv1.CSV.Spec.InstallModes {
-			supportedInstallModes.Insert(mode.Type)
+			if mode.Supported {
+				supportedInstallModes.Insert(mode.Type)
+			}
 		}
+
 		if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
-			return []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}
+			sortedModes := slices.Sorted(slices.Values(supportedInstallModes.UnsortedList()))
+			errs := make([]error, len(conversionWebhookNames))
+			for i, webhookName := range conversionWebhookNames {
+				errs[i] = fmt.Errorf("bundle contains conversion webhook %q and supports install modes %v - conversion webhooks are only supported for bundles that only support AllNamespaces install mode", webhookName, sortedModes)
+			}
+			return errs
 		}
 	}
 

internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go

@@ -249,32 +249,6 @@ func Test_CheckWebhookSupport(t *testing.T) {
 		bundle       *bundle.RegistryV1
 		expectedErrs []error
 	}{
-		{
-			name: "accepts bundles with validating webhook definitions when they only support AllNamespaces install mode",
-			bundle: &bundle.RegistryV1{
-				CSV: MakeCSV(
-					WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
-					WithWebhookDefinitions(
-						v1alpha1.WebhookDescription{
-							Type: v1alpha1.ValidatingAdmissionWebhook,
-						},
-					),
-				),
-			},
-		},
-		{
-			name: "accepts bundles with mutating webhook definitions when they only support AllNamespaces install mode",
-			bundle: &bundle.RegistryV1{
-				CSV: MakeCSV(
-					WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
-					WithWebhookDefinitions(
-						v1alpha1.WebhookDescription{
-							Type: v1alpha1.MutatingAdmissionWebhook,
-						},
-					),
-				),
-			},
-		},
 		{
 			name: "accepts bundles with conversion webhook definitions when they only support AllNamespaces install mode",
 			bundle: &bundle.RegistryV1{
@@ -289,7 +263,7 @@ func Test_CheckWebhookSupport(t *testing.T) {
 			},
 		},
 		{
-			name: "rejects bundles with validating webhook definitions when they support more modes than AllNamespaces install mode",
+			name: "accepts bundles with validating webhook definitions when they support more modes than AllNamespaces install mode",
 			bundle: &bundle.RegistryV1{
 				CSV: MakeCSV(
 					WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
@@ -300,7 +274,6 @@ func Test_CheckWebhookSupport(t *testing.T) {
 					),
 				),
 			},
-			expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
 		},
 		{
 			name: "accepts bundles with mutating webhook definitions when they support more modes than AllNamespaces install mode",
@@ -314,25 +287,32 @@ func Test_CheckWebhookSupport(t *testing.T) {
 					),
 				),
 			},
-			expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
 		},
 		{
-			name: "accepts bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode",
+			name: "rejects bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode",
 			bundle: &bundle.RegistryV1{
 				CSV: MakeCSV(
-					WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
+					WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces),
 					WithWebhookDefinitions(
 						v1alpha1.WebhookDescription{
-							Type: v1alpha1.ConversionWebhook,
+							GenerateName: "webhook-b",
+							Type:         v1alpha1.ConversionWebhook,
+						},
+						v1alpha1.WebhookDescription{
+							GenerateName: "webhook-a",
+							Type:         v1alpha1.ConversionWebhook,
 						},
 					),
 				),
 			},
-			expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
+			expectedErrs: []error{
+				errors.New("bundle contains conversion webhook \"webhook-b\" and supports install modes [AllNamespaces SingleNamespace] - conversion webhooks are only supported for bundles that only support AllNamespaces install mode"),
+				errors.New("bundle contains conversion webhook \"webhook-a\" and supports install modes [AllNamespaces SingleNamespace] - conversion webhooks are only supported for bundles that only support AllNamespaces install mode"),
+			},
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			errs := validators.CheckWebhookSupport(tc.bundle)
+			errs := validators.CheckConversionWebhookSupport(tc.bundle)
 			require.Equal(t, tc.expectedErrs, errs)
 		})
 	}

internal/operator-controller/rukpak/util/testing/testing.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 	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"
@@ -55,15 +56,22 @@ func WithOwnedCRDs(crdDesc ...v1alpha1.CRDDescription) CSVOption {
 }
 
 func WithInstallModeSupportFor(installModeType ...v1alpha1.InstallModeType) CSVOption {
+	var installModes = []v1alpha1.InstallModeType{
+		v1alpha1.InstallModeTypeAllNamespaces,
+		v1alpha1.InstallModeTypeSingleNamespace,
+		v1alpha1.InstallModeTypeMultiNamespace,
+		v1alpha1.InstallModeTypeOwnNamespace,
+	}
 	return func(csv *v1alpha1.ClusterServiceVersion) {
-		installModes := make([]v1alpha1.InstallMode, 0, len(installModeType))
-		for _, t := range installModeType {
-			installModes = append(installModes, v1alpha1.InstallMode{
+		supportedInstallModes := sets.New(installModeType...)
+		csvInstallModes := make([]v1alpha1.InstallMode, 0, len(installModeType))
+		for _, t := range installModes {
+			csvInstallModes = append(csvInstallModes, v1alpha1.InstallMode{
 				Type:      t,
-				Supported: true,
+				Supported: supportedInstallModes.Has(t),
 			})
 		}
-		csv.Spec.InstallModes = installModes
+		csv.Spec.InstallModes = csvInstallModes
 	}
 }
 

internal/operator-controller/rukpak/util/testing/testing_test.go

@@ -209,6 +209,14 @@ func Test_MakeCSV_WithInstallModeSupportFor(t *testing.T) {
 					Type:      v1alpha1.InstallModeTypeSingleNamespace,
 					Supported: true,
 				},
+				{
+					Type:      v1alpha1.InstallModeTypeMultiNamespace,
+					Supported: false,
+				},
+				{
+					Type:      v1alpha1.InstallModeTypeOwnNamespace,
+					Supported: false,
+				},
 			},
 		},
 	}, csv)