Relax webhook preconditions

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

Author: Per Goncalves da Silva

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

@@ -101,21 +101,26 @@ 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 {
+	hasConversionWebhooks := false
+	for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
+		if wh.Type == v1alpha1.ConversionWebhook {
+			hasConversionWebhooks = true
+			break
+		}
+	}
+
+	if hasConversionWebhooks {
 		supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{}
 		for _, mode := range rv1.CSV.Spec.InstallModes {
 			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()))
+			return []error{fmt.Errorf("bundle contains conversion webhooks and supports install modes %v - conversion webhooks are only supported for bundles that only support AllNamespaces install mode", sortedModes)}
 		}
 	}
 

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,10 +287,9 @@ 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),
@@ -328,11 +300,11 @@ func Test_CheckWebhookSupport(t *testing.T) {
 					),
 				),
 			},
-			expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
+			expectedErrs: []error{errors.New("bundle contains conversion webhooks 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)
 		})
 	}