From 8b72968dad8f39f6e91173c56fe42f70cce16418 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 16 Sep 2025 11:23:44 +0200 Subject: [PATCH 1/6] Relax webhook preconditions Signed-off-by: Per Goncalves da Silva --- .../render/registryv1/validators/validator.go | 23 +++++++----- .../registryv1/validators/validator_test.go | 36 +++---------------- 2 files changed, 18 insertions(+), 41 deletions(-) diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 61d0aad7c..910ba0dc7 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go +++ b/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)} } } diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go index 6c1d7491b..deae54057 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go +++ b/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) }) } From 1399b507f5771bc2129094543b9493c7234846a0 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Wed, 17 Sep 2025 10:50:39 +0200 Subject: [PATCH 2/6] Fix up WithInstallModeSupportFor to include all install modes Signed-off-by: Per G. da Silva --- .../rukpak/util/testing/testing.go | 18 +++++++++++++----- .../rukpak/util/testing/testing_test.go | 8 ++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/internal/operator-controller/rukpak/util/testing/testing.go b/internal/operator-controller/rukpak/util/testing/testing.go index f5c9b36a3..e544e546c 100644 --- a/internal/operator-controller/rukpak/util/testing/testing.go +++ b/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 } } diff --git a/internal/operator-controller/rukpak/util/testing/testing_test.go b/internal/operator-controller/rukpak/util/testing/testing_test.go index c8744cfa0..703cc0018 100644 --- a/internal/operator-controller/rukpak/util/testing/testing_test.go +++ b/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) From 9e9135670ca5324dcbf281a2883c8ed67a3bd936 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Wed, 17 Sep 2025 13:49:24 +0200 Subject: [PATCH 3/6] Add CheckConversionWebhookSupport to validator Signed-off-by: Per G. da Silva --- .../operator-controller/rukpak/render/registryv1/registryv1.go | 1 + .../rukpak/render/registryv1/registryv1_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1.go b/internal/operator-controller/rukpak/render/registryv1/registryv1.go index 6621a6ca4..c9caa4c5f 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1.go +++ b/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, diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go b/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go index c75f1d602..cf56863af 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1_test.go +++ b/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, From 42f01b55ca199543d0afb30387131d1eb373243b Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Wed, 17 Sep 2025 13:49:57 +0200 Subject: [PATCH 4/6] Add check for install mode support Signed-off-by: Per G. da Silva --- .../rukpak/render/registryv1/validators/validator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 910ba0dc7..5570359c0 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go @@ -116,7 +116,9 @@ func CheckConversionWebhookSupport(rv1 *bundle.RegistryV1) []error { if hasConversionWebhooks { 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) { sortedModes := slices.Sorted(slices.Values(supportedInstallModes.UnsortedList())) From a1a5a584a91b483191ca7a2f1a3a169f7e65d9d3 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 17 Sep 2025 14:31:53 +0200 Subject: [PATCH 5/6] Update error messages Signed-off-by: Per Goncalves da Silva --- .../render/registryv1/validators/validator.go | 15 ++++++++++----- .../registryv1/validators/validator_test.go | 15 ++++++++++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 5570359c0..29db33ceb 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go @@ -105,24 +105,29 @@ func CheckPackageNameNotEmpty(rv1 *bundle.RegistryV1) []error { // 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 func CheckConversionWebhookSupport(rv1 *bundle.RegistryV1) []error { - hasConversionWebhooks := false + var conversionWebhookNames []string for _, wh := range rv1.CSV.Spec.WebhookDefinitions { if wh.Type == v1alpha1.ConversionWebhook { - hasConversionWebhooks = true - break + conversionWebhookNames = append(conversionWebhookNames, wh.GenerateName) } } - if hasConversionWebhooks { + if len(conversionWebhookNames) > 0 { supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{} for _, mode := range rv1.CSV.Spec.InstallModes { if mode.Supported { supportedInstallModes.Insert(mode.Type) } } + slices.Sort(conversionWebhookNames) + if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) { 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)} + 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 } } diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go index deae54057..30b7c7c39 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go @@ -292,15 +292,24 @@ func Test_CheckWebhookSupport(t *testing.T) { 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 conversion webhooks and supports install modes [AllNamespaces SingleNamespace] - conversion webhooks are only supported for bundles that only support AllNamespaces install mode")}, + expectedErrs: []error{ + // Webhook names and install modes are sorted in alphanumerical order + 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"), + 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"), + }, }, } { t.Run(tc.name, func(t *testing.T) { From 362b58e580441a82b31170d3cd8913c78ecdadf4 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Thu, 18 Sep 2025 09:48:07 +0200 Subject: [PATCH 6/6] Remove error sorting by webhook name Signed-off-by: Per Goncalves da Silva --- .../rukpak/render/registryv1/validators/validator.go | 1 - .../rukpak/render/registryv1/validators/validator_test.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 29db33ceb..960b4ce73 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go @@ -119,7 +119,6 @@ func CheckConversionWebhookSupport(rv1 *bundle.RegistryV1) []error { supportedInstallModes.Insert(mode.Type) } } - slices.Sort(conversionWebhookNames) if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) { sortedModes := slices.Sorted(slices.Values(supportedInstallModes.UnsortedList())) diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go index 30b7c7c39..c33af4850 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go @@ -306,9 +306,8 @@ func Test_CheckWebhookSupport(t *testing.T) { ), }, expectedErrs: []error{ - // Webhook names and install modes are sorted in alphanumerical order - 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"), 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"), }, }, } {