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 94e5940

Browse files
author
Per Goncalves da Silva
committedApr 11, 2025·
add webhook bundle validators
Signed-off-by: Per Goncalves da Silva <[email protected]>
1 parent 08cf766 commit 94e5940

File tree

3 files changed

+682
-12
lines changed

3 files changed

+682
-12
lines changed
 

‎internal/operator-controller/rukpak/convert/registryv1_test.go

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -570,23 +570,40 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) {
570570
t.Log("It should enforce limitations")
571571
t.Log("It should allow bundles with webhooks")
572572
t.Log("By creating a registry v1 bundle")
573-
csv := v1alpha1.ClusterServiceVersion{
574-
ObjectMeta: metav1.ObjectMeta{
575-
Name: "testCSV",
576-
},
577-
Spec: v1alpha1.ClusterServiceVersionSpec{
578-
InstallModes: []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}},
579-
WebhookDefinitions: []v1alpha1.WebhookDescription{{ConversionCRDs: []string{"fake-webhook.package-with-webhooks.io"}}},
580-
},
581-
}
582-
watchNamespaces := []string{metav1.NamespaceAll}
583573
registryv1Bundle := render.RegistryV1{
584574
PackageName: "testPkg",
585-
CSV: csv,
575+
CRDs: []apiextensionsv1.CustomResourceDefinition{
576+
{
577+
ObjectMeta: metav1.ObjectMeta{
578+
Name: "fake-webhook.package-with-webhooks.io",
579+
},
580+
},
581+
},
582+
CSV: MakeCSV(
583+
WithName("testCSV"),
584+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
585+
WithOwnedCRDs(
586+
v1alpha1.CRDDescription{
587+
Name: "fake-webhook.package-with-webhooks.io",
588+
},
589+
),
590+
WithStrategyDeploymentSpecs(
591+
v1alpha1.StrategyDeploymentSpec{
592+
Name: "some-deployment",
593+
},
594+
),
595+
WithWebhookDefinitions(
596+
v1alpha1.WebhookDescription{
597+
Type: v1alpha1.ConversionWebhook,
598+
ConversionCRDs: []string{"fake-webhook.package-with-webhooks.io"},
599+
DeploymentName: "some-deployment",
600+
},
601+
),
602+
),
586603
}
587604

588605
t.Log("By converting to plain")
589-
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces)
606+
plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, []string{metav1.NamespaceAll})
590607
require.NoError(t, err)
591608
require.NotNil(t, plainBundle)
592609
}

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

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
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

@@ -21,6 +24,10 @@ var RegistryV1BundleValidator = render.BundleValidator{
2124
CheckCRDResourceUniqueness,
2225
CheckOwnedCRDExistence,
2326
CheckPackageNameNotEmpty,
27+
CheckWebhookDeploymentReferentialIntegrity,
28+
CheckWebhookNameUniqueness,
29+
CheckConversionWebhookCRDReferenceUniqueness,
30+
CheckConversionWebhooksReferenceOwnedCRDs,
2431
}
2532

2633
// CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name.
@@ -109,3 +116,123 @@ func CheckWebhookSupport(rv1 *render.RegistryV1) []error {
109116

110117
return nil
111118
}
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: 526 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

@@ -311,3 +315,525 @@ func Test_CheckWebhookSupport(t *testing.T) {
311315
})
312316
}
313317
}
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+
}

0 commit comments

Comments
 (0)
Please sign in to comment.