Skip to content

Commit 5478ac6

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
✨ OPRUN-4150: Relax webhook support preconditions (#2222)
* Relax webhook preconditions Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> * Fix up WithInstallModeSupportFor to include all install modes Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Add CheckConversionWebhookSupport to validator Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Add check for install mode support Signed-off-by: Per G. da Silva <pegoncal@redhat.com> * Update error messages Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> * Remove error sorting by webhook name Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> --------- Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Signed-off-by: Per G. da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent ee3456a commit 5478ac6

File tree

6 files changed

+58
-49
lines changed

6 files changed

+58
-49
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ var BundleValidator = render.BundleValidator{
2222
validators.CheckCRDResourceUniqueness,
2323
validators.CheckOwnedCRDExistence,
2424
validators.CheckPackageNameNotEmpty,
25+
validators.CheckConversionWebhookSupport,
2526
validators.CheckWebhookDeploymentReferentialIntegrity,
2627
validators.CheckWebhookNameUniqueness,
2728
validators.CheckWebhookNameIsDNS1123SubDomain,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
2626
validators.CheckCRDResourceUniqueness,
2727
validators.CheckOwnedCRDExistence,
2828
validators.CheckPackageNameNotEmpty,
29+
validators.CheckConversionWebhookSupport,
2930
validators.CheckWebhookDeploymentReferentialIntegrity,
3031
validators.CheckWebhookNameUniqueness,
3132
validators.CheckWebhookNameIsDNS1123SubDomain,

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

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,32 @@ func CheckPackageNameNotEmpty(rv1 *bundle.RegistryV1) []error {
101101
return nil
102102
}
103103

104-
// CheckWebhookSupport checks that if the bundle cluster service version declares webhook definitions
105-
// that it is a singleton operator, i.e. that it only supports AllNamespaces mode. This keeps parity
106-
// with OLMv0 behavior for conversion webhooks,
104+
// CheckConversionWebhookSupport checks that if the bundle cluster service version declares conversion webhook definitions,
105+
// that the bundle also only supports AllNamespaces install mode. This keeps parity with OLMv0 behavior for conversion webhooks,
107106
// https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L193
108-
// Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks.
109-
// While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn
110-
// after getting the webhook support working.
111-
func CheckWebhookSupport(rv1 *bundle.RegistryV1) []error {
112-
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
107+
func CheckConversionWebhookSupport(rv1 *bundle.RegistryV1) []error {
108+
var conversionWebhookNames []string
109+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
110+
if wh.Type == v1alpha1.ConversionWebhook {
111+
conversionWebhookNames = append(conversionWebhookNames, wh.GenerateName)
112+
}
113+
}
114+
115+
if len(conversionWebhookNames) > 0 {
113116
supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{}
114117
for _, mode := range rv1.CSV.Spec.InstallModes {
115-
supportedInstallModes.Insert(mode.Type)
118+
if mode.Supported {
119+
supportedInstallModes.Insert(mode.Type)
120+
}
116121
}
122+
117123
if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
118-
return []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}
124+
sortedModes := slices.Sorted(slices.Values(supportedInstallModes.UnsortedList()))
125+
errs := make([]error, len(conversionWebhookNames))
126+
for i, webhookName := range conversionWebhookNames {
127+
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)
128+
}
129+
return errs
119130
}
120131
}
121132

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

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -249,32 +249,6 @@ func Test_CheckWebhookSupport(t *testing.T) {
249249
bundle *bundle.RegistryV1
250250
expectedErrs []error
251251
}{
252-
{
253-
name: "accepts bundles with validating webhook definitions when they only support AllNamespaces install mode",
254-
bundle: &bundle.RegistryV1{
255-
CSV: MakeCSV(
256-
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
257-
WithWebhookDefinitions(
258-
v1alpha1.WebhookDescription{
259-
Type: v1alpha1.ValidatingAdmissionWebhook,
260-
},
261-
),
262-
),
263-
},
264-
},
265-
{
266-
name: "accepts bundles with mutating webhook definitions when they only support AllNamespaces install mode",
267-
bundle: &bundle.RegistryV1{
268-
CSV: MakeCSV(
269-
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
270-
WithWebhookDefinitions(
271-
v1alpha1.WebhookDescription{
272-
Type: v1alpha1.MutatingAdmissionWebhook,
273-
},
274-
),
275-
),
276-
},
277-
},
278252
{
279253
name: "accepts bundles with conversion webhook definitions when they only support AllNamespaces install mode",
280254
bundle: &bundle.RegistryV1{
@@ -289,7 +263,7 @@ func Test_CheckWebhookSupport(t *testing.T) {
289263
},
290264
},
291265
{
292-
name: "rejects bundles with validating webhook definitions when they support more modes than AllNamespaces install mode",
266+
name: "accepts bundles with validating webhook definitions when they support more modes than AllNamespaces install mode",
293267
bundle: &bundle.RegistryV1{
294268
CSV: MakeCSV(
295269
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
@@ -300,7 +274,6 @@ func Test_CheckWebhookSupport(t *testing.T) {
300274
),
301275
),
302276
},
303-
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
304277
},
305278
{
306279
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) {
314287
),
315288
),
316289
},
317-
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
318290
},
319291
{
320-
name: "accepts bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode",
292+
name: "rejects bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode",
321293
bundle: &bundle.RegistryV1{
322294
CSV: MakeCSV(
323-
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
295+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces),
324296
WithWebhookDefinitions(
325297
v1alpha1.WebhookDescription{
326-
Type: v1alpha1.ConversionWebhook,
298+
GenerateName: "webhook-b",
299+
Type: v1alpha1.ConversionWebhook,
300+
},
301+
v1alpha1.WebhookDescription{
302+
GenerateName: "webhook-a",
303+
Type: v1alpha1.ConversionWebhook,
327304
},
328305
),
329306
),
330307
},
331-
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
308+
expectedErrs: []error{
309+
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"),
310+
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"),
311+
},
332312
},
333313
} {
334314
t.Run(tc.name, func(t *testing.T) {
335-
errs := validators.CheckWebhookSupport(tc.bundle)
315+
errs := validators.CheckConversionWebhookSupport(tc.bundle)
336316
require.Equal(t, tc.expectedErrs, errs)
337317
})
338318
}

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/stretchr/testify/require"
77
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
88
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
9+
"k8s.io/apimachinery/pkg/util/sets"
910
"sigs.k8s.io/controller-runtime/pkg/client"
1011

1112
"github.com/operator-framework/api/pkg/operators/v1alpha1"
@@ -55,15 +56,22 @@ func WithOwnedCRDs(crdDesc ...v1alpha1.CRDDescription) CSVOption {
5556
}
5657

5758
func WithInstallModeSupportFor(installModeType ...v1alpha1.InstallModeType) CSVOption {
59+
var installModes = []v1alpha1.InstallModeType{
60+
v1alpha1.InstallModeTypeAllNamespaces,
61+
v1alpha1.InstallModeTypeSingleNamespace,
62+
v1alpha1.InstallModeTypeMultiNamespace,
63+
v1alpha1.InstallModeTypeOwnNamespace,
64+
}
5865
return func(csv *v1alpha1.ClusterServiceVersion) {
59-
installModes := make([]v1alpha1.InstallMode, 0, len(installModeType))
60-
for _, t := range installModeType {
61-
installModes = append(installModes, v1alpha1.InstallMode{
66+
supportedInstallModes := sets.New(installModeType...)
67+
csvInstallModes := make([]v1alpha1.InstallMode, 0, len(installModeType))
68+
for _, t := range installModes {
69+
csvInstallModes = append(csvInstallModes, v1alpha1.InstallMode{
6270
Type: t,
63-
Supported: true,
71+
Supported: supportedInstallModes.Has(t),
6472
})
6573
}
66-
csv.Spec.InstallModes = installModes
74+
csv.Spec.InstallModes = csvInstallModes
6775
}
6876
}
6977

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,14 @@ func Test_MakeCSV_WithInstallModeSupportFor(t *testing.T) {
209209
Type: v1alpha1.InstallModeTypeSingleNamespace,
210210
Supported: true,
211211
},
212+
{
213+
Type: v1alpha1.InstallModeTypeMultiNamespace,
214+
Supported: false,
215+
},
216+
{
217+
Type: v1alpha1.InstallModeTypeOwnNamespace,
218+
Supported: false,
219+
},
212220
},
213221
},
214222
}, csv)

0 commit comments

Comments
 (0)