Skip to content

Commit d3e8883

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
✨ OPRUN-4151: Add webhook rule validation (#2226)
* Add webhook rule checker Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> * fix ups Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> * Add webhook rule checker to validator Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> --------- Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent 5478ac6 commit d3e8883

File tree

4 files changed

+364
-0
lines changed

4 files changed

+364
-0
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ var BundleValidator = render.BundleValidator{
2828
validators.CheckWebhookNameIsDNS1123SubDomain,
2929
validators.CheckConversionWebhookCRDReferenceUniqueness,
3030
validators.CheckConversionWebhooksReferenceOwnedCRDs,
31+
validators.CheckWebhookRules,
3132
}
3233

3334
// ResourceGenerators a slice of ResourceGenerators required to generate plain resource manifests for

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
3232
validators.CheckWebhookNameIsDNS1123SubDomain,
3333
validators.CheckConversionWebhookCRDReferenceUniqueness,
3434
validators.CheckConversionWebhooksReferenceOwnedCRDs,
35+
validators.CheckWebhookRules,
3536
}
3637
actualValidationFns := registryv1.BundleValidator
3738

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,3 +275,55 @@ func CheckWebhookNameIsDNS1123SubDomain(rv1 *bundle.RegistryV1) []error {
275275
}
276276
return errs
277277
}
278+
279+
// forbiddenWebhookRuleAPIGroups contain the API groups that are forbidden for webhook configuration rules in OLMv1
280+
var forbiddenWebhookRuleAPIGroups = sets.New("olm.operatorframework.io", "*")
281+
282+
// forbiddenAdmissionRegistrationResources contain the resources that are forbidden for webhook configuration rules
283+
// for the admissionregistration.k8s.io api group
284+
var forbiddenAdmissionRegistrationResources = sets.New(
285+
"*",
286+
"mutatingwebhookconfiguration",
287+
"mutatingwebhookconfigurations",
288+
"validatingwebhookconfiguration",
289+
"validatingwebhookconfigurations",
290+
)
291+
292+
// CheckWebhookRules ensures webhook rules do not reference forbidden API groups or resources in line with OLMv0 behavior
293+
// The following are forbidden, rules targeting:
294+
// - all API groups (i.e. '*')
295+
// - OLMv1 API group (i.e. 'olm.operatorframework.io')
296+
// - all resources under the 'admissionregistration.k8s.io' API group
297+
// - the 'ValidatingWebhookConfiguration' resource under the 'admissionregistration.k8s.io' API group
298+
// - the 'MutatingWebhookConfiguration' resource under the 'admissionregistration.k8s.io' API group
299+
//
300+
// These boundaries attempt to reduce the blast radius of faulty webhooks and avoid deadlocks preventing the user
301+
// from deleting OLMv1 resources installing and managing the faulty webhook, or deleting faulty admission webhook
302+
// configurations.
303+
// See https://github.com/operator-framework/operator-lifecycle-manager/blob/ccf0c4c91f1e7673e87f3a18947f9a1f88d48438/pkg/controller/install/webhook.go#L19
304+
// for more details
305+
func CheckWebhookRules(rv1 *bundle.RegistryV1) []error {
306+
var errs []error
307+
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
308+
// Rules are not used for conversion webhooks
309+
if wh.Type == v1alpha1.ConversionWebhook {
310+
continue
311+
}
312+
webhookName := wh.GenerateName
313+
for _, rule := range wh.Rules {
314+
for _, apiGroup := range rule.APIGroups {
315+
if forbiddenWebhookRuleAPIGroups.Has(apiGroup) {
316+
errs = append(errs, fmt.Errorf("webhook %q contains forbidden rule: admission webhook rules cannot reference API group %q", webhookName, apiGroup))
317+
}
318+
if apiGroup == "admissionregistration.k8s.io" {
319+
for _, resource := range rule.Resources {
320+
if forbiddenAdmissionRegistrationResources.Has(strings.ToLower(resource)) {
321+
errs = append(errs, fmt.Errorf("webhook %q contains forbidden rule: admission webhook rules cannot reference resource %q for API group %q", webhookName, resource, apiGroup))
322+
}
323+
}
324+
}
325+
}
326+
}
327+
}
328+
return errs
329+
}

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

Lines changed: 310 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/stretchr/testify/require"
8+
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
89
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1011

@@ -318,6 +319,315 @@ func Test_CheckWebhookSupport(t *testing.T) {
318319
}
319320
}
320321

322+
func Test_CheckWebhookRules(t *testing.T) {
323+
for _, tc := range []struct {
324+
name string
325+
bundle *bundle.RegistryV1
326+
expectedErrs []error
327+
}{
328+
{
329+
name: "accepts bundles with webhook definitions without rules",
330+
bundle: &bundle.RegistryV1{
331+
CSV: MakeCSV(
332+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
333+
WithWebhookDefinitions(
334+
v1alpha1.WebhookDescription{
335+
Type: v1alpha1.ValidatingAdmissionWebhook,
336+
},
337+
v1alpha1.WebhookDescription{
338+
Type: v1alpha1.MutatingAdmissionWebhook,
339+
},
340+
),
341+
),
342+
},
343+
},
344+
{
345+
name: "accepts bundles with webhook definitions with supported rules",
346+
bundle: &bundle.RegistryV1{
347+
CSV: MakeCSV(
348+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
349+
WithWebhookDefinitions(
350+
v1alpha1.WebhookDescription{
351+
Type: v1alpha1.ValidatingAdmissionWebhook,
352+
Rules: []admissionregistrationv1.RuleWithOperations{
353+
{
354+
Rule: admissionregistrationv1.Rule{
355+
APIGroups: []string{"appsv1"},
356+
Resources: []string{"deployments", "replicasets", "statefulsets"},
357+
},
358+
},
359+
},
360+
},
361+
v1alpha1.WebhookDescription{
362+
Type: v1alpha1.MutatingAdmissionWebhook,
363+
Rules: []admissionregistrationv1.RuleWithOperations{
364+
{
365+
Rule: admissionregistrationv1.Rule{
366+
APIGroups: []string{""},
367+
Resources: []string{"services"},
368+
},
369+
},
370+
},
371+
},
372+
),
373+
),
374+
},
375+
},
376+
{
377+
name: "reject bundles with webhook definitions with rules containing '*' api group",
378+
bundle: &bundle.RegistryV1{
379+
CSV: MakeCSV(
380+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
381+
WithWebhookDefinitions(
382+
v1alpha1.WebhookDescription{
383+
Type: v1alpha1.ValidatingAdmissionWebhook,
384+
GenerateName: "webhook-z",
385+
Rules: []admissionregistrationv1.RuleWithOperations{
386+
{
387+
Rule: admissionregistrationv1.Rule{
388+
APIGroups: []string{"*"},
389+
},
390+
},
391+
},
392+
},
393+
v1alpha1.WebhookDescription{
394+
Type: v1alpha1.MutatingAdmissionWebhook,
395+
GenerateName: "webhook-a",
396+
Rules: []admissionregistrationv1.RuleWithOperations{
397+
{
398+
Rule: admissionregistrationv1.Rule{
399+
APIGroups: []string{"*"},
400+
},
401+
},
402+
},
403+
},
404+
),
405+
),
406+
},
407+
expectedErrs: []error{
408+
errors.New("webhook \"webhook-z\" contains forbidden rule: admission webhook rules cannot reference API group \"*\""),
409+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference API group \"*\""),
410+
},
411+
},
412+
{
413+
name: "reject bundles with webhook definitions with rules containing 'olm.operatorframework.io' api group",
414+
bundle: &bundle.RegistryV1{
415+
CSV: MakeCSV(
416+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
417+
WithWebhookDefinitions(
418+
v1alpha1.WebhookDescription{
419+
Type: v1alpha1.ValidatingAdmissionWebhook,
420+
GenerateName: "webhook-z",
421+
Rules: []admissionregistrationv1.RuleWithOperations{
422+
{
423+
Rule: admissionregistrationv1.Rule{
424+
APIGroups: []string{"olm.operatorframework.io"},
425+
},
426+
},
427+
},
428+
},
429+
v1alpha1.WebhookDescription{
430+
Type: v1alpha1.MutatingAdmissionWebhook,
431+
GenerateName: "webhook-a",
432+
Rules: []admissionregistrationv1.RuleWithOperations{
433+
{
434+
Rule: admissionregistrationv1.Rule{
435+
APIGroups: []string{"olm.operatorframework.io"},
436+
},
437+
},
438+
},
439+
},
440+
),
441+
),
442+
},
443+
expectedErrs: []error{
444+
errors.New("webhook \"webhook-z\" contains forbidden rule: admission webhook rules cannot reference API group \"olm.operatorframework.io\""),
445+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference API group \"olm.operatorframework.io\""),
446+
},
447+
},
448+
{
449+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and '*' resource",
450+
bundle: &bundle.RegistryV1{
451+
CSV: MakeCSV(
452+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
453+
WithWebhookDefinitions(
454+
v1alpha1.WebhookDescription{
455+
Type: v1alpha1.ValidatingAdmissionWebhook,
456+
GenerateName: "webhook-a",
457+
Rules: []admissionregistrationv1.RuleWithOperations{
458+
{
459+
Rule: admissionregistrationv1.Rule{
460+
APIGroups: []string{"admissionregistration.k8s.io"},
461+
Resources: []string{"*"},
462+
},
463+
},
464+
},
465+
},
466+
),
467+
),
468+
},
469+
expectedErrs: []error{
470+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"*\" for API group \"admissionregistration.k8s.io\""),
471+
},
472+
},
473+
{
474+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'MutatingWebhookConfiguration' resource",
475+
bundle: &bundle.RegistryV1{
476+
CSV: MakeCSV(
477+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
478+
WithWebhookDefinitions(
479+
v1alpha1.WebhookDescription{
480+
Type: v1alpha1.ValidatingAdmissionWebhook,
481+
GenerateName: "webhook-a",
482+
Rules: []admissionregistrationv1.RuleWithOperations{
483+
{
484+
Rule: admissionregistrationv1.Rule{
485+
APIGroups: []string{"admissionregistration.k8s.io"},
486+
Resources: []string{"MutatingWebhookConfiguration"},
487+
},
488+
},
489+
},
490+
},
491+
),
492+
),
493+
},
494+
expectedErrs: []error{
495+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"MutatingWebhookConfiguration\" for API group \"admissionregistration.k8s.io\""),
496+
},
497+
},
498+
{
499+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'mutatingwebhookconfiguration' resource",
500+
bundle: &bundle.RegistryV1{
501+
CSV: MakeCSV(
502+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
503+
WithWebhookDefinitions(
504+
v1alpha1.WebhookDescription{
505+
Type: v1alpha1.ValidatingAdmissionWebhook,
506+
GenerateName: "webhook-a",
507+
Rules: []admissionregistrationv1.RuleWithOperations{
508+
{
509+
Rule: admissionregistrationv1.Rule{
510+
APIGroups: []string{"admissionregistration.k8s.io"},
511+
Resources: []string{"mutatingwebhookconfiguration"},
512+
},
513+
},
514+
},
515+
},
516+
),
517+
),
518+
},
519+
expectedErrs: []error{
520+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"mutatingwebhookconfiguration\" for API group \"admissionregistration.k8s.io\""),
521+
},
522+
},
523+
{
524+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'mutatingwebhookconfigurations' resource",
525+
bundle: &bundle.RegistryV1{
526+
CSV: MakeCSV(
527+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
528+
WithWebhookDefinitions(
529+
v1alpha1.WebhookDescription{
530+
Type: v1alpha1.ValidatingAdmissionWebhook,
531+
GenerateName: "webhook-a",
532+
Rules: []admissionregistrationv1.RuleWithOperations{
533+
{
534+
Rule: admissionregistrationv1.Rule{
535+
APIGroups: []string{"admissionregistration.k8s.io"},
536+
Resources: []string{"mutatingwebhookconfigurations"},
537+
},
538+
},
539+
},
540+
},
541+
),
542+
),
543+
},
544+
expectedErrs: []error{
545+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"mutatingwebhookconfigurations\" for API group \"admissionregistration.k8s.io\""),
546+
},
547+
},
548+
{
549+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'ValidatingWebhookConfiguration' resource",
550+
bundle: &bundle.RegistryV1{
551+
CSV: MakeCSV(
552+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
553+
WithWebhookDefinitions(
554+
v1alpha1.WebhookDescription{
555+
Type: v1alpha1.ValidatingAdmissionWebhook,
556+
GenerateName: "webhook-a",
557+
Rules: []admissionregistrationv1.RuleWithOperations{
558+
{
559+
Rule: admissionregistrationv1.Rule{
560+
APIGroups: []string{"admissionregistration.k8s.io"},
561+
Resources: []string{"ValidatingWebhookConfiguration"},
562+
},
563+
},
564+
},
565+
},
566+
),
567+
),
568+
},
569+
expectedErrs: []error{
570+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"ValidatingWebhookConfiguration\" for API group \"admissionregistration.k8s.io\""),
571+
},
572+
},
573+
{
574+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'validatingwebhookconfiguration' resource",
575+
bundle: &bundle.RegistryV1{
576+
CSV: MakeCSV(
577+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
578+
WithWebhookDefinitions(
579+
v1alpha1.WebhookDescription{
580+
Type: v1alpha1.ValidatingAdmissionWebhook,
581+
GenerateName: "webhook-a",
582+
Rules: []admissionregistrationv1.RuleWithOperations{
583+
{
584+
Rule: admissionregistrationv1.Rule{
585+
APIGroups: []string{"admissionregistration.k8s.io"},
586+
Resources: []string{"validatingwebhookconfiguration"},
587+
},
588+
},
589+
},
590+
},
591+
),
592+
),
593+
},
594+
expectedErrs: []error{
595+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"validatingwebhookconfiguration\" for API group \"admissionregistration.k8s.io\""),
596+
},
597+
},
598+
{
599+
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'validatingwebhookconfigurations' resource",
600+
bundle: &bundle.RegistryV1{
601+
CSV: MakeCSV(
602+
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
603+
WithWebhookDefinitions(
604+
v1alpha1.WebhookDescription{
605+
Type: v1alpha1.ValidatingAdmissionWebhook,
606+
GenerateName: "webhook-a",
607+
Rules: []admissionregistrationv1.RuleWithOperations{
608+
{
609+
Rule: admissionregistrationv1.Rule{
610+
APIGroups: []string{"admissionregistration.k8s.io"},
611+
Resources: []string{"validatingwebhookconfigurations"},
612+
},
613+
},
614+
},
615+
},
616+
),
617+
),
618+
},
619+
expectedErrs: []error{
620+
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"validatingwebhookconfigurations\" for API group \"admissionregistration.k8s.io\""),
621+
},
622+
},
623+
} {
624+
t.Run(tc.name, func(t *testing.T) {
625+
errs := validators.CheckWebhookRules(tc.bundle)
626+
require.Equal(t, tc.expectedErrs, errs)
627+
})
628+
}
629+
}
630+
321631
func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) {
322632
for _, tc := range []struct {
323633
name string

0 commit comments

Comments
 (0)