Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var BundleValidator = render.BundleValidator{
validators.CheckWebhookNameIsDNS1123SubDomain,
validators.CheckConversionWebhookCRDReferenceUniqueness,
validators.CheckConversionWebhooksReferenceOwnedCRDs,
validators.CheckWebhookRules,
}

// ResourceGenerators a slice of ResourceGenerators required to generate plain resource manifests for
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) {
validators.CheckWebhookNameIsDNS1123SubDomain,
validators.CheckConversionWebhookCRDReferenceUniqueness,
validators.CheckConversionWebhooksReferenceOwnedCRDs,
validators.CheckWebhookRules,
}
actualValidationFns := registryv1.BundleValidator

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,55 @@ func CheckWebhookNameIsDNS1123SubDomain(rv1 *bundle.RegistryV1) []error {
}
return errs
}

// forbiddenWebhookRuleAPIGroups contain the API groups that are forbidden for webhook configuration rules in OLMv1
var forbiddenWebhookRuleAPIGroups = sets.New("olm.operatorframework.io", "*")

// forbiddenAdmissionRegistrationResources contain the resources that are forbidden for webhook configuration rules
// for the admissionregistration.k8s.io api group
var forbiddenAdmissionRegistrationResources = sets.New(
"*",
"mutatingwebhookconfiguration",
"mutatingwebhookconfigurations",
"validatingwebhookconfiguration",
"validatingwebhookconfigurations",
)

// CheckWebhookRules ensures webhook rules do not reference forbidden API groups or resources in line with OLMv0 behavior
// The following are forbidden, rules targeting:
// - all API groups (i.e. '*')
// - OLMv1 API group (i.e. 'olm.operatorframework.io')
// - all resources under the 'admissionregistration.k8s.io' API group
// - the 'ValidatingWebhookConfiguration' resource under the 'admissionregistration.k8s.io' API group
// - the 'MutatingWebhookConfiguration' resource under the 'admissionregistration.k8s.io' API group
//
// These boundaries attempt to reduce the blast radius of faulty webhooks and avoid deadlocks preventing the user
// from deleting OLMv1 resources installing and managing the faulty webhook, or deleting faulty admission webhook
// configurations.
// See https://github.com/operator-framework/operator-lifecycle-manager/blob/ccf0c4c91f1e7673e87f3a18947f9a1f88d48438/pkg/controller/install/webhook.go#L19
// for more details
func CheckWebhookRules(rv1 *bundle.RegistryV1) []error {
var errs []error
for _, wh := range rv1.CSV.Spec.WebhookDefinitions {
// Rules are not used for conversion webhooks
if wh.Type == v1alpha1.ConversionWebhook {
continue
}
webhookName := wh.GenerateName
for _, rule := range wh.Rules {
for _, apiGroup := range rule.APIGroups {
if forbiddenWebhookRuleAPIGroups.Has(apiGroup) {
errs = append(errs, fmt.Errorf("webhook %q contains forbidden rule: admission webhook rules cannot reference API group %q", webhookName, apiGroup))
}
if apiGroup == "admissionregistration.k8s.io" {
for _, resource := range rule.Resources {
if forbiddenAdmissionRegistrationResources.Has(strings.ToLower(resource)) {
errs = append(errs, fmt.Errorf("webhook %q contains forbidden rule: admission webhook rules cannot reference resource %q for API group %q", webhookName, resource, apiGroup))
}
}
}
}
}
}
return errs
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand Down Expand Up @@ -338,6 +339,315 @@ func Test_CheckWebhookSupport(t *testing.T) {
}
}

func Test_CheckWebhookRules(t *testing.T) {
for _, tc := range []struct {
name string
bundle *bundle.RegistryV1
expectedErrs []error
}{
{
name: "accepts bundles with webhook definitions without rules",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
},
v1alpha1.WebhookDescription{
Type: v1alpha1.MutatingAdmissionWebhook,
},
),
),
},
},
{
name: "accepts bundles with webhook definitions with supported rules",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"appsv1"},
Resources: []string{"deployments", "replicasets", "statefulsets"},
},
},
},
},
v1alpha1.WebhookDescription{
Type: v1alpha1.MutatingAdmissionWebhook,
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{""},
Resources: []string{"services"},
},
},
},
},
),
),
},
},
{
name: "reject bundles with webhook definitions with rules containing '*' api group",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-z",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"*"},
},
},
},
},
v1alpha1.WebhookDescription{
Type: v1alpha1.MutatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"*"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-z\" contains forbidden rule: admission webhook rules cannot reference API group \"*\""),
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference API group \"*\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'olm.operatorframework.io' api group",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-z",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"olm.operatorframework.io"},
},
},
},
},
v1alpha1.WebhookDescription{
Type: v1alpha1.MutatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"olm.operatorframework.io"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-z\" contains forbidden rule: admission webhook rules cannot reference API group \"olm.operatorframework.io\""),
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference API group \"olm.operatorframework.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and '*' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"*"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"*\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'MutatingWebhookConfiguration' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"MutatingWebhookConfiguration"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"MutatingWebhookConfiguration\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'mutatingwebhookconfiguration' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"mutatingwebhookconfiguration"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"mutatingwebhookconfiguration\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'mutatingwebhookconfigurations' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"mutatingwebhookconfigurations"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"mutatingwebhookconfigurations\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'ValidatingWebhookConfiguration' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"ValidatingWebhookConfiguration"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"ValidatingWebhookConfiguration\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'validatingwebhookconfiguration' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"validatingwebhookconfiguration"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"validatingwebhookconfiguration\" for API group \"admissionregistration.k8s.io\""),
},
},
{
name: "reject bundles with webhook definitions with rules containing 'admissionregistration.k8s.io' api group and 'validatingwebhookconfigurations' resource",
bundle: &bundle.RegistryV1{
CSV: MakeCSV(
WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces),
WithWebhookDefinitions(
v1alpha1.WebhookDescription{
Type: v1alpha1.ValidatingAdmissionWebhook,
GenerateName: "webhook-a",
Rules: []admissionregistrationv1.RuleWithOperations{
{
Rule: admissionregistrationv1.Rule{
APIGroups: []string{"admissionregistration.k8s.io"},
Resources: []string{"validatingwebhookconfigurations"},
},
},
},
},
),
),
},
expectedErrs: []error{
errors.New("webhook \"webhook-a\" contains forbidden rule: admission webhook rules cannot reference resource \"validatingwebhookconfigurations\" for API group \"admissionregistration.k8s.io\""),
},
},
} {
t.Run(tc.name, func(t *testing.T) {
errs := validators.CheckWebhookRules(tc.bundle)
require.Equal(t, tc.expectedErrs, errs)
})
}
}

func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down
Loading