Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 48a8e6f

Browse files
author
Per Goncalves da Silva
committedApr 29, 2025
add webhook bundle validators
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 1171691 commit 48a8e6f

File tree

3 files changed

+778
-1
lines changed

3 files changed

+778
-1
lines changed
 

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
package validators
22

33
import (
4+
"cmp"
45
"errors"
56
"fmt"
7+
"maps"
68
"slices"
9+
"strings"
710

811
"k8s.io/apimachinery/pkg/util/sets"
912

13+
"github.com/operator-framework/api/pkg/operators/v1alpha1"
14+
1015
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
1116
)
1217

@@ -19,6 +24,10 @@ var RegistryV1BundleValidator = render.BundleValidator{
1924
CheckCRDResourceUniqueness,
2025
CheckOwnedCRDExistence,
2126
CheckPackageNameNotEmpty,
27+
CheckWebhookDeploymentReferentialIntegrity,
28+
CheckWebhookNameUniqueness,
29+
CheckConversionWebhookCRDReferenceUniqueness,
30+
CheckConversionWebhooksReferenceOwnedCRDs,
2231
}
2332

2433
// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.
@@ -86,3 +95,144 @@ func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error {
8695
}
8796
return nil
8897
}
98+
99+
// CheckWebhookSupport checks that if the bundle cluster service version declares webhook definitions
100+
// that it is a singleton operator, i.e. that it only supports AllNamespaces mode. This keeps parity
101+
// with OLMv0 behavior for conversion webhooks,
102+
// https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L193
103+
// Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks.
104+
// While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn
105+
// after getting the webhook support working.
106+
func CheckWebhookSupport(rv1 *render.RegistryV1) []error {
107+
if len(rv1.CSV.Spec.WebhookDefinitions) > 0 {
108+
supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{}
109+
for _, mode := range rv1.CSV.Spec.InstallModes {
110+
supportedInstallModes.Insert(mode.Type)
111+
}
112+
if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) {
113+
return []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}
114+
}
115+
}
116+
117+
return nil
118+
}
119+
120+
// CheckWebhookDeploymentReferentialIntegrity checks that each webhook definition in the csv
121+
// references an existing strategy deployment spec. Errors are sorted by strategy deployment spec name,
122+
// webhook type, and webhook name.
123+
func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error {
124+
webhooksByDeployment := map[string][]v1alpha1.WebhookDescription{}
125+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
126+
webhooksByDeployment[wh.DeploymentName] = append(webhooksByDeployment[wh.DeploymentName], wh)
127+
}
128+
129+
for _, depl := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs {
130+
delete(webhooksByDeployment, depl.Name)
131+
}
132+
133+
var errs []error
134+
// Loop through sorted keys to keep error messages ordered by deployment name
135+
for _, deploymentName := range slices.Sorted(maps.Keys(webhooksByDeployment)) {
136+
webhookDefns := webhooksByDeployment[deploymentName]
137+
slices.SortFunc(webhookDefns, func(a, b v1alpha1.WebhookDescription) int {
138+
return cmp.Or(cmp.Compare(a.Type, b.Type), cmp.Compare(a.GenerateName, b.GenerateName))
139+
})
140+
for _, webhookDef := range webhookDefns {
141+
errs = append(errs, fmt.Errorf("webhook '%s' of type '%s' references non-existent deployment '%s'", webhookDef.GenerateName, webhookDef.Type, webhookDef.DeploymentName))
142+
}
143+
}
144+
return errs
145+
}
146+
147+
// CheckWebhookNameUniqueness checks that each webhook definition of each type (validating, mutating, or conversion)
148+
// has a unique name. Webhooks of different types can have the same name. Errors are sorted by webhook type
149+
// and name.
150+
func CheckWebhookNameUniqueness(rv1 *render.RegistryV1) []error {
151+
webhookNameSetByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{}
152+
duplicateWebhooksByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{}
153+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
154+
if _, ok := webhookNameSetByType[wh.Type]; !ok {
155+
webhookNameSetByType[wh.Type] = sets.Set[string]{}
156+
}
157+
if webhookNameSetByType[wh.Type].Has(wh.GenerateName) {
158+
if _, ok := duplicateWebhooksByType[wh.Type]; !ok {
159+
duplicateWebhooksByType[wh.Type] = sets.Set[string]{}
160+
}
161+
duplicateWebhooksByType[wh.Type].Insert(wh.GenerateName)
162+
}
163+
webhookNameSetByType[wh.Type].Insert(wh.GenerateName)
164+
}
165+
166+
var errs []error
167+
for _, whType := range slices.Sorted(maps.Keys(duplicateWebhooksByType)) {
168+
for _, webhookName := range slices.Sorted(slices.Values(duplicateWebhooksByType[whType].UnsortedList())) {
169+
errs = append(errs, fmt.Errorf("duplicate webhook '%s' of type '%s'", webhookName, whType))
170+
}
171+
}
172+
return errs
173+
}
174+
175+
// CheckConversionWebhooksReferenceOwnedCRDs checks defined conversion webhooks reference bundle owned CRDs.
176+
// Errors are sorted by webhook name and CRD name.
177+
func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *render.RegistryV1) []error {
178+
//nolint:prealloc
179+
var conversionWebhooks []v1alpha1.WebhookDescription
180+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
181+
if wh.Type != v1alpha1.ConversionWebhook {
182+
continue
183+
}
184+
conversionWebhooks = append(conversionWebhooks, wh)
185+
}
186+
187+
if len(conversionWebhooks) == 0 {
188+
return nil
189+
}
190+
191+
ownedCRDNames := sets.Set[string]{}
192+
for _, crd := range rv1.CSV.Spec.CustomResourceDefinitions.Owned {
193+
ownedCRDNames.Insert(crd.Name)
194+
}
195+
196+
slices.SortFunc(conversionWebhooks, func(a, b v1alpha1.WebhookDescription) int {
197+
return cmp.Compare(a.GenerateName, b.GenerateName)
198+
})
199+
200+
var errs []error
201+
for _, webhook := range conversionWebhooks {
202+
webhookCRDs := webhook.ConversionCRDs
203+
slices.Sort(webhookCRDs)
204+
for _, crd := range webhookCRDs {
205+
if !ownedCRDNames.Has(crd) {
206+
errs = append(errs, fmt.Errorf("conversion webhook '%s' references custom resource definition '%s' not owned bundle", webhook.GenerateName, crd))
207+
}
208+
}
209+
}
210+
return errs
211+
}
212+
213+
// CheckConversionWebhookCRDReferenceUniqueness checks no two (or more) conversion webhooks reference the same CRD.
214+
func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []error {
215+
// collect webhooks by crd
216+
crdToWh := map[string][]string{}
217+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
218+
if wh.Type != v1alpha1.ConversionWebhook {
219+
continue
220+
}
221+
for _, crd := range wh.ConversionCRDs {
222+
crdToWh[crd] = append(crdToWh[crd], wh.GenerateName)
223+
}
224+
}
225+
226+
// remove crds with single webhook
227+
maps.DeleteFunc(crdToWh, func(crd string, whs []string) bool {
228+
return len(whs) == 1
229+
})
230+
231+
errs := make([]error, 0, len(crdToWh))
232+
orderedCRDs := slices.Sorted(maps.Keys(crdToWh))
233+
for _, crd := range orderedCRDs {
234+
orderedWhs := strings.Join(slices.Sorted(slices.Values(crdToWh[crd])), ",")
235+
errs = append(errs, fmt.Errorf("conversion webhooks [%s] reference same custom resource definition '%s'", orderedWhs, crd))
236+
}
237+
return errs
238+
}

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

Lines changed: 622 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
1515
"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators"
16-
. "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util"
16+
. "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing"
1717
)
1818

1919
func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
@@ -22,6 +22,10 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
2222
validators.CheckCRDResourceUniqueness,
2323
validators.CheckOwnedCRDExistence,
2424
validators.CheckPackageNameNotEmpty,
25+
validators.CheckWebhookDeploymentReferentialIntegrity,
26+
validators.CheckWebhookNameUniqueness,
27+
validators.CheckConversionWebhookCRDReferenceUniqueness,
28+
validators.CheckConversionWebhooksReferenceOwnedCRDs,
2529
}
2630
actualValidationFns := validators.RegistryV1BundleValidator
2731

@@ -216,3 +220,620 @@ func Test_CheckPackageNameNotEmpty(t *testing.T) {
216220
})
217221
}
218222
}
223+
224+
func Test_CheckWebhookSupport(t *testing.T) {
225+
for _, tc := range []struct {
226+
name string
227+
bundle *render.RegistryV1
228+
expectedErrs []error
229+
}{
230+
{
231+
name: "accepts bundles with validating webhook definitions when they only support AllNamespaces install mode",
232+
bundle: &render.RegistryV1{
233+
CSV: MakeCSV(
234+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
235+
WithWebhookDefinitions(
236+
v1alpha1.WebhookDescription{
237+
Type: v1alpha1.ValidatingAdmissionWebhook,
238+
},
239+
),
240+
),
241+
},
242+
},
243+
{
244+
name: "accepts bundles with mutating webhook definitions when they only support AllNamespaces install mode",
245+
bundle: &render.RegistryV1{
246+
CSV: MakeCSV(
247+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
248+
WithWebhookDefinitions(
249+
v1alpha1.WebhookDescription{
250+
Type: v1alpha1.MutatingAdmissionWebhook,
251+
},
252+
),
253+
),
254+
},
255+
},
256+
{
257+
name: "accepts bundles with conversion webhook definitions when they only support AllNamespaces install mode",
258+
bundle: &render.RegistryV1{
259+
CSV: MakeCSV(
260+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
261+
WithWebhookDefinitions(
262+
v1alpha1.WebhookDescription{
263+
Type: v1alpha1.ConversionWebhook,
264+
},
265+
),
266+
),
267+
},
268+
},
269+
{
270+
name: "rejects bundles with validating webhook definitions when they support more modes than AllNamespaces install mode",
271+
bundle: &render.RegistryV1{
272+
CSV: MakeCSV(
273+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
274+
WithWebhookDefinitions(
275+
v1alpha1.WebhookDescription{
276+
Type: v1alpha1.ValidatingAdmissionWebhook,
277+
},
278+
),
279+
),
280+
},
281+
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
282+
},
283+
{
284+
name: "accepts bundles with mutating webhook definitions when they support more modes than AllNamespaces install mode",
285+
bundle: &render.RegistryV1{
286+
CSV: MakeCSV(
287+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
288+
WithWebhookDefinitions(
289+
v1alpha1.WebhookDescription{
290+
Type: v1alpha1.MutatingAdmissionWebhook,
291+
},
292+
),
293+
),
294+
},
295+
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
296+
},
297+
{
298+
name: "accepts bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode",
299+
bundle: &render.RegistryV1{
300+
CSV: MakeCSV(
301+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace),
302+
WithWebhookDefinitions(
303+
v1alpha1.WebhookDescription{
304+
Type: v1alpha1.ConversionWebhook,
305+
},
306+
),
307+
),
308+
},
309+
expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")},
310+
},
311+
} {
312+
t.Run(tc.name, func(t *testing.T) {
313+
errs := validators.CheckWebhookSupport(tc.bundle)
314+
require.Equal(t, tc.expectedErrs, errs)
315+
})
316+
}
317+
}
318+
319+
func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) {
320+
for _, tc := range []struct {
321+
name string
322+
bundle *render.RegistryV1
323+
expectedErrs []error
324+
}{
325+
{
326+
name: "accepts bundles where webhook definitions reference existing strategy deployment specs",
327+
bundle: &render.RegistryV1{
328+
CSV: MakeCSV(
329+
WithStrategyDeploymentSpecs(
330+
v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"},
331+
v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-two"},
332+
),
333+
WithWebhookDefinitions(
334+
v1alpha1.WebhookDescription{
335+
Type: v1alpha1.MutatingAdmissionWebhook,
336+
GenerateName: "test-webhook",
337+
DeploymentName: "test-deployment-one",
338+
},
339+
),
340+
),
341+
},
342+
}, {
343+
name: "rejects bundles with webhook definitions that reference non-existing strategy deployment specs",
344+
bundle: &render.RegistryV1{
345+
CSV: MakeCSV(
346+
WithStrategyDeploymentSpecs(
347+
v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"},
348+
),
349+
WithWebhookDefinitions(
350+
v1alpha1.WebhookDescription{
351+
Type: v1alpha1.ValidatingAdmissionWebhook,
352+
GenerateName: "test-webhook",
353+
DeploymentName: "test-deployment-two",
354+
},
355+
),
356+
),
357+
},
358+
expectedErrs: []error{
359+
errors.New("webhook 'test-webhook' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-two'"),
360+
},
361+
}, {
362+
name: "errors are ordered by deployment strategy spec name, webhook type, and webhook name",
363+
bundle: &render.RegistryV1{
364+
CSV: MakeCSV(
365+
WithStrategyDeploymentSpecs(
366+
v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"},
367+
),
368+
WithWebhookDefinitions(
369+
v1alpha1.WebhookDescription{
370+
Type: v1alpha1.ValidatingAdmissionWebhook,
371+
GenerateName: "test-val-webhook-c",
372+
DeploymentName: "test-deployment-c",
373+
},
374+
v1alpha1.WebhookDescription{
375+
Type: v1alpha1.MutatingAdmissionWebhook,
376+
GenerateName: "test-mute-webhook-a",
377+
DeploymentName: "test-deployment-a",
378+
},
379+
v1alpha1.WebhookDescription{
380+
Type: v1alpha1.ConversionWebhook,
381+
GenerateName: "test-conv-webhook-b",
382+
DeploymentName: "test-deployment-b",
383+
}, v1alpha1.WebhookDescription{
384+
Type: v1alpha1.MutatingAdmissionWebhook,
385+
GenerateName: "test-mute-webhook-c",
386+
DeploymentName: "test-deployment-c",
387+
},
388+
v1alpha1.WebhookDescription{
389+
Type: v1alpha1.ConversionWebhook,
390+
GenerateName: "test-conv-webhook-c-b",
391+
DeploymentName: "test-deployment-c",
392+
}, v1alpha1.WebhookDescription{
393+
Type: v1alpha1.ConversionWebhook,
394+
GenerateName: "test-conv-webhook-c-a",
395+
DeploymentName: "test-deployment-c",
396+
},
397+
),
398+
),
399+
},
400+
expectedErrs: []error{
401+
errors.New("webhook 'test-mute-webhook-a' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-a'"),
402+
errors.New("webhook 'test-conv-webhook-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-b'"),
403+
errors.New("webhook 'test-conv-webhook-c-a' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"),
404+
errors.New("webhook 'test-conv-webhook-c-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"),
405+
errors.New("webhook 'test-mute-webhook-c' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"),
406+
errors.New("webhook 'test-val-webhook-c' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"),
407+
},
408+
},
409+
} {
410+
t.Run(tc.name, func(t *testing.T) {
411+
errs := validators.CheckWebhookDeploymentReferentialIntegrity(tc.bundle)
412+
require.Equal(t, tc.expectedErrs, errs)
413+
})
414+
}
415+
}
416+
417+
func Test_CheckWebhookNameUniqueness(t *testing.T) {
418+
for _, tc := range []struct {
419+
name string
420+
bundle *render.RegistryV1
421+
expectedErrs []error
422+
}{
423+
{
424+
name: "accepts bundles without webhook definitions",
425+
bundle: &render.RegistryV1{
426+
CSV: MakeCSV(),
427+
},
428+
}, {
429+
name: "accepts bundles with unique webhook names",
430+
bundle: &render.RegistryV1{
431+
CSV: MakeCSV(
432+
WithWebhookDefinitions(
433+
v1alpha1.WebhookDescription{
434+
Type: v1alpha1.MutatingAdmissionWebhook,
435+
GenerateName: "test-webhook-one",
436+
}, v1alpha1.WebhookDescription{
437+
Type: v1alpha1.ValidatingAdmissionWebhook,
438+
GenerateName: "test-webhook-two",
439+
}, v1alpha1.WebhookDescription{
440+
Type: v1alpha1.ConversionWebhook,
441+
GenerateName: "test-webhook-three",
442+
}, v1alpha1.WebhookDescription{
443+
Type: v1alpha1.MutatingAdmissionWebhook,
444+
GenerateName: "test-webhook-four",
445+
}, v1alpha1.WebhookDescription{
446+
Type: v1alpha1.ValidatingAdmissionWebhook,
447+
GenerateName: "test-webhook-five",
448+
}, v1alpha1.WebhookDescription{
449+
Type: v1alpha1.ConversionWebhook,
450+
GenerateName: "test-webhook-six",
451+
},
452+
),
453+
),
454+
},
455+
}, {
456+
name: "accepts bundles with webhooks with the same name but different types",
457+
bundle: &render.RegistryV1{
458+
CSV: MakeCSV(
459+
WithWebhookDefinitions(
460+
v1alpha1.WebhookDescription{
461+
Type: v1alpha1.MutatingAdmissionWebhook,
462+
GenerateName: "test-webhook",
463+
}, v1alpha1.WebhookDescription{
464+
Type: v1alpha1.ValidatingAdmissionWebhook,
465+
GenerateName: "test-webhook",
466+
}, v1alpha1.WebhookDescription{
467+
Type: v1alpha1.ConversionWebhook,
468+
GenerateName: "test-webhook",
469+
},
470+
),
471+
),
472+
},
473+
}, {
474+
name: "rejects bundles with duplicate validating webhook definitions",
475+
bundle: &render.RegistryV1{
476+
CSV: MakeCSV(
477+
WithWebhookDefinitions(
478+
v1alpha1.WebhookDescription{
479+
Type: v1alpha1.ValidatingAdmissionWebhook,
480+
GenerateName: "test-webhook",
481+
}, v1alpha1.WebhookDescription{
482+
Type: v1alpha1.ValidatingAdmissionWebhook,
483+
GenerateName: "test-webhook",
484+
},
485+
),
486+
),
487+
},
488+
expectedErrs: []error{
489+
errors.New("duplicate webhook 'test-webhook' of type 'ValidatingAdmissionWebhook'"),
490+
},
491+
}, {
492+
name: "rejects bundles with duplicate mutating webhook definitions",
493+
bundle: &render.RegistryV1{
494+
CSV: MakeCSV(
495+
WithWebhookDefinitions(
496+
v1alpha1.WebhookDescription{
497+
Type: v1alpha1.MutatingAdmissionWebhook,
498+
GenerateName: "test-webhook",
499+
}, v1alpha1.WebhookDescription{
500+
Type: v1alpha1.MutatingAdmissionWebhook,
501+
GenerateName: "test-webhook",
502+
},
503+
),
504+
),
505+
},
506+
expectedErrs: []error{
507+
errors.New("duplicate webhook 'test-webhook' of type 'MutatingAdmissionWebhook'"),
508+
},
509+
}, {
510+
name: "rejects bundles with duplicate conversion webhook definitions",
511+
bundle: &render.RegistryV1{
512+
CSV: MakeCSV(
513+
WithWebhookDefinitions(
514+
v1alpha1.WebhookDescription{
515+
Type: v1alpha1.ConversionWebhook,
516+
GenerateName: "test-webhook",
517+
}, v1alpha1.WebhookDescription{
518+
Type: v1alpha1.ConversionWebhook,
519+
GenerateName: "test-webhook",
520+
},
521+
),
522+
),
523+
},
524+
expectedErrs: []error{
525+
errors.New("duplicate webhook 'test-webhook' of type 'ConversionWebhook'"),
526+
},
527+
}, {
528+
name: "orders errors by webhook type and name",
529+
bundle: &render.RegistryV1{
530+
CSV: MakeCSV(
531+
WithWebhookDefinitions(
532+
v1alpha1.WebhookDescription{
533+
Type: v1alpha1.ValidatingAdmissionWebhook,
534+
GenerateName: "test-val-webhook-b",
535+
}, v1alpha1.WebhookDescription{
536+
Type: v1alpha1.ValidatingAdmissionWebhook,
537+
GenerateName: "test-val-webhook-a",
538+
},
539+
v1alpha1.WebhookDescription{
540+
Type: v1alpha1.ValidatingAdmissionWebhook,
541+
GenerateName: "test-val-webhook-a",
542+
}, v1alpha1.WebhookDescription{
543+
Type: v1alpha1.ValidatingAdmissionWebhook,
544+
GenerateName: "test-val-webhook-b",
545+
},
546+
v1alpha1.WebhookDescription{
547+
Type: v1alpha1.ConversionWebhook,
548+
GenerateName: "test-conv-webhook-b",
549+
}, v1alpha1.WebhookDescription{
550+
Type: v1alpha1.ConversionWebhook,
551+
GenerateName: "test-conv-webhook-a",
552+
},
553+
v1alpha1.WebhookDescription{
554+
Type: v1alpha1.ConversionWebhook,
555+
GenerateName: "test-conv-webhook-a",
556+
}, v1alpha1.WebhookDescription{
557+
Type: v1alpha1.ConversionWebhook,
558+
GenerateName: "test-conv-webhook-b",
559+
}, v1alpha1.WebhookDescription{
560+
Type: v1alpha1.MutatingAdmissionWebhook,
561+
GenerateName: "test-mute-webhook-b",
562+
}, v1alpha1.WebhookDescription{
563+
Type: v1alpha1.MutatingAdmissionWebhook,
564+
GenerateName: "test-mute-webhook-a",
565+
},
566+
v1alpha1.WebhookDescription{
567+
Type: v1alpha1.MutatingAdmissionWebhook,
568+
GenerateName: "test-mute-webhook-a",
569+
}, v1alpha1.WebhookDescription{
570+
Type: v1alpha1.MutatingAdmissionWebhook,
571+
GenerateName: "test-mute-webhook-b",
572+
},
573+
),
574+
),
575+
},
576+
expectedErrs: []error{
577+
errors.New("duplicate webhook 'test-conv-webhook-a' of type 'ConversionWebhook'"),
578+
errors.New("duplicate webhook 'test-conv-webhook-b' of type 'ConversionWebhook'"),
579+
errors.New("duplicate webhook 'test-mute-webhook-a' of type 'MutatingAdmissionWebhook'"),
580+
errors.New("duplicate webhook 'test-mute-webhook-b' of type 'MutatingAdmissionWebhook'"),
581+
errors.New("duplicate webhook 'test-val-webhook-a' of type 'ValidatingAdmissionWebhook'"),
582+
errors.New("duplicate webhook 'test-val-webhook-b' of type 'ValidatingAdmissionWebhook'"),
583+
},
584+
},
585+
} {
586+
t.Run(tc.name, func(t *testing.T) {
587+
errs := validators.CheckWebhookNameUniqueness(tc.bundle)
588+
require.Equal(t, tc.expectedErrs, errs)
589+
})
590+
}
591+
}
592+
593+
func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) {
594+
for _, tc := range []struct {
595+
name string
596+
bundle *render.RegistryV1
597+
expectedErrs []error
598+
}{
599+
{
600+
name: "accepts bundles without webhook definitions",
601+
bundle: &render.RegistryV1{},
602+
}, {
603+
name: "accepts bundles without conversion webhook definitions",
604+
bundle: &render.RegistryV1{
605+
CSV: MakeCSV(
606+
WithWebhookDefinitions(
607+
v1alpha1.WebhookDescription{
608+
Type: v1alpha1.ValidatingAdmissionWebhook,
609+
GenerateName: "test-val-webhook",
610+
},
611+
v1alpha1.WebhookDescription{
612+
Type: v1alpha1.MutatingAdmissionWebhook,
613+
GenerateName: "test-mute-webhook",
614+
},
615+
),
616+
),
617+
},
618+
}, {
619+
name: "accepts bundles with conversion webhooks that reference owned CRDs",
620+
bundle: &render.RegistryV1{
621+
CSV: MakeCSV(
622+
WithOwnedCRDs(
623+
v1alpha1.CRDDescription{Name: "some.crd.something"},
624+
v1alpha1.CRDDescription{Name: "another.crd.something"},
625+
),
626+
WithWebhookDefinitions(
627+
v1alpha1.WebhookDescription{
628+
Type: v1alpha1.ConversionWebhook,
629+
GenerateName: "test-webhook",
630+
ConversionCRDs: []string{
631+
"some.crd.something",
632+
"another.crd.something",
633+
},
634+
},
635+
),
636+
),
637+
},
638+
}, {
639+
name: "rejects bundles with conversion webhooks that reference existing CRDs that are not owned",
640+
bundle: &render.RegistryV1{
641+
CSV: MakeCSV(
642+
WithOwnedCRDs(
643+
v1alpha1.CRDDescription{Name: "some.crd.something"},
644+
),
645+
WithWebhookDefinitions(
646+
v1alpha1.WebhookDescription{
647+
Type: v1alpha1.ConversionWebhook,
648+
GenerateName: "test-webhook",
649+
ConversionCRDs: []string{
650+
"some.crd.something",
651+
"another.crd.something",
652+
},
653+
},
654+
),
655+
),
656+
},
657+
expectedErrs: []error{
658+
errors.New("conversion webhook 'test-webhook' references custom resource definition 'another.crd.something' not owned bundle"),
659+
},
660+
}, {
661+
name: "errors are ordered by webhook name and CRD name",
662+
bundle: &render.RegistryV1{
663+
CSV: MakeCSV(
664+
WithOwnedCRDs(
665+
v1alpha1.CRDDescription{Name: "b.crd.something"},
666+
),
667+
WithWebhookDefinitions(
668+
v1alpha1.WebhookDescription{
669+
Type: v1alpha1.ConversionWebhook,
670+
GenerateName: "test-webhook-b",
671+
ConversionCRDs: []string{
672+
"b.crd.something",
673+
},
674+
}, v1alpha1.WebhookDescription{
675+
Type: v1alpha1.ConversionWebhook,
676+
GenerateName: "test-webhook-a",
677+
ConversionCRDs: []string{
678+
"c.crd.something",
679+
"a.crd.something",
680+
},
681+
}, v1alpha1.WebhookDescription{
682+
Type: v1alpha1.ConversionWebhook,
683+
GenerateName: "test-webhook-c",
684+
ConversionCRDs: []string{
685+
"a.crd.something",
686+
"d.crd.something",
687+
},
688+
},
689+
),
690+
),
691+
},
692+
expectedErrs: []error{
693+
errors.New("conversion webhook 'test-webhook-a' references custom resource definition 'a.crd.something' not owned bundle"),
694+
errors.New("conversion webhook 'test-webhook-a' references custom resource definition 'c.crd.something' not owned bundle"),
695+
errors.New("conversion webhook 'test-webhook-c' references custom resource definition 'a.crd.something' not owned bundle"),
696+
errors.New("conversion webhook 'test-webhook-c' references custom resource definition 'd.crd.something' not owned bundle"),
697+
},
698+
},
699+
} {
700+
t.Run(tc.name, func(t *testing.T) {
701+
errs := validators.CheckConversionWebhooksReferenceOwnedCRDs(tc.bundle)
702+
require.Equal(t, tc.expectedErrs, errs)
703+
})
704+
}
705+
}
706+
707+
func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) {
708+
for _, tc := range []struct {
709+
name string
710+
bundle *render.RegistryV1
711+
expectedErrs []error
712+
}{
713+
{
714+
name: "accepts bundles without webhook definitions",
715+
bundle: &render.RegistryV1{},
716+
expectedErrs: []error{},
717+
},
718+
{
719+
name: "accepts bundles without conversion webhook definitions",
720+
bundle: &render.RegistryV1{
721+
CSV: MakeCSV(
722+
WithWebhookDefinitions(
723+
v1alpha1.WebhookDescription{
724+
Type: v1alpha1.ValidatingAdmissionWebhook,
725+
GenerateName: "test-val-webhook",
726+
},
727+
v1alpha1.WebhookDescription{
728+
Type: v1alpha1.MutatingAdmissionWebhook,
729+
GenerateName: "test-mute-webhook",
730+
},
731+
),
732+
),
733+
},
734+
expectedErrs: []error{},
735+
},
736+
{
737+
name: "accepts bundles with conversion webhooks that reference different CRDs",
738+
bundle: &render.RegistryV1{
739+
CSV: MakeCSV(
740+
WithOwnedCRDs(
741+
v1alpha1.CRDDescription{Name: "some.crd.something"},
742+
v1alpha1.CRDDescription{Name: "another.crd.something"},
743+
),
744+
WithWebhookDefinitions(
745+
v1alpha1.WebhookDescription{
746+
Type: v1alpha1.ConversionWebhook,
747+
GenerateName: "test-webhook",
748+
ConversionCRDs: []string{
749+
"some.crd.something",
750+
},
751+
},
752+
v1alpha1.WebhookDescription{
753+
Type: v1alpha1.ConversionWebhook,
754+
GenerateName: "test-webhook-2",
755+
ConversionCRDs: []string{
756+
"another.crd.something",
757+
},
758+
},
759+
),
760+
),
761+
},
762+
expectedErrs: []error{},
763+
},
764+
{
765+
name: "rejects bundles with conversion webhooks that reference the same CRD",
766+
bundle: &render.RegistryV1{
767+
CSV: MakeCSV(
768+
WithOwnedCRDs(
769+
v1alpha1.CRDDescription{Name: "some.crd.something"},
770+
),
771+
WithWebhookDefinitions(
772+
v1alpha1.WebhookDescription{
773+
Type: v1alpha1.ConversionWebhook,
774+
GenerateName: "test-webhook",
775+
ConversionCRDs: []string{
776+
"some.crd.something",
777+
},
778+
},
779+
v1alpha1.WebhookDescription{
780+
Type: v1alpha1.ConversionWebhook,
781+
GenerateName: "test-webhook-two",
782+
ConversionCRDs: []string{
783+
"some.crd.something",
784+
},
785+
},
786+
),
787+
),
788+
},
789+
expectedErrs: []error{
790+
errors.New("conversion webhooks [test-webhook,test-webhook-two] reference same custom resource definition 'some.crd.something'"),
791+
},
792+
},
793+
{
794+
name: "errors are ordered by CRD name and webhook names",
795+
bundle: &render.RegistryV1{
796+
CSV: MakeCSV(
797+
WithOwnedCRDs(
798+
v1alpha1.CRDDescription{Name: "b.crd.something"},
799+
),
800+
WithWebhookDefinitions(
801+
v1alpha1.WebhookDescription{
802+
Type: v1alpha1.ConversionWebhook,
803+
GenerateName: "test-webhook-b",
804+
ConversionCRDs: []string{
805+
"b.crd.something",
806+
"a.crd.something",
807+
},
808+
}, v1alpha1.WebhookDescription{
809+
Type: v1alpha1.ConversionWebhook,
810+
GenerateName: "test-webhook-a",
811+
ConversionCRDs: []string{
812+
"d.crd.something",
813+
"a.crd.something",
814+
"b.crd.something",
815+
},
816+
}, v1alpha1.WebhookDescription{
817+
Type: v1alpha1.ConversionWebhook,
818+
GenerateName: "test-webhook-c",
819+
ConversionCRDs: []string{
820+
"b.crd.something",
821+
"d.crd.something",
822+
},
823+
},
824+
),
825+
),
826+
},
827+
expectedErrs: []error{
828+
errors.New("conversion webhooks [test-webhook-a,test-webhook-b] reference same custom resource definition 'a.crd.something'"),
829+
errors.New("conversion webhooks [test-webhook-a,test-webhook-b,test-webhook-c] reference same custom resource definition 'b.crd.something'"),
830+
errors.New("conversion webhooks [test-webhook-a,test-webhook-c] reference same custom resource definition 'd.crd.something'"),
831+
},
832+
},
833+
} {
834+
t.Run(tc.name, func(t *testing.T) {
835+
errs := validators.CheckConversionWebhookCRDReferenceUniqueness(tc.bundle)
836+
require.Equal(t, tc.expectedErrs, errs)
837+
})
838+
}
839+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ func WithInstallModeSupportFor(installModeType ...v1alpha1.InstallModeType) CSVO
5858
}
5959
}
6060

61+
func WithWebhookDefinitions(webhookDefinitions ...v1alpha1.WebhookDescription) CSVOption {
62+
return func(csv *v1alpha1.ClusterServiceVersion) {
63+
csv.Spec.WebhookDefinitions = webhookDefinitions
64+
}
65+
}
66+
6167
func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion {
6268
csv := v1alpha1.ClusterServiceVersion{
6369
TypeMeta: metav1.TypeMeta{

0 commit comments

Comments
 (0)
Please sign in to comment.