diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index bb7b179d6610..62dec795dbe4 100644 --- a/pkg/controllers/webhook/controller.go +++ b/pkg/controllers/webhook/controller.go @@ -642,7 +642,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config } } rules := p.GetSpec().Rules - operationStatusMap = analyseOperationStatusMapMutatingWebhookConfiguration(rules, operationStatusMap) + operationStatusMap = computeOperationsForMutatingWebhookConf(rules, operationStatusMap) } operationReq := getMinimumOperations(operationStatusMap) webhookCfg := config.WebhookConfig{} @@ -744,6 +744,19 @@ func (c *controller) buildDefaultResourceValidatingWebhookConfiguration(cfg conf nil } +func scanResourceFilter(resFilter kyvernov1.ResourceFilters, operationStatusMap map[string]bool) (bool, map[string]bool) { + opFound := false + for _, rf := range resFilter { + if rf.ResourceDescription.Operations != nil { + for _, o := range rf.ResourceDescription.Operations { + opFound = true + operationStatusMap[string(o)] = true + } + } + } + return opFound, operationStatusMap +} + func getOperationStatusMap() map[string]bool { operationStatusMap := make(map[string]bool) operationStatusMap[webhookCreate] = false @@ -753,19 +766,34 @@ func getOperationStatusMap() map[string]bool { return operationStatusMap } -func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { +func computeOperationsForValidatingWebhookConf(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { for _, r := range rules { + opFound := false + if len(r.MatchResources.Any) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.MatchResources.Any, operationStatusMap) + } + if len(r.MatchResources.All) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.MatchResources.All, operationStatusMap) + } + if len(r.ExcludeResources.Any) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.Any, operationStatusMap) + } + if len(r.ExcludeResources.All) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.All, operationStatusMap) + } if r.MatchResources.ResourceDescription.Operations != nil { for _, o := range r.MatchResources.ResourceDescription.Operations { + opFound = true operationStatusMap[string(o)] = true } } if r.ExcludeResources.ResourceDescription.Operations != nil { - for _, o := range r.MatchResources.ResourceDescription.Operations { + for _, o := range r.ExcludeResources.ResourceDescription.Operations { + opFound = true operationStatusMap[string(o)] = true } } - if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { + if !opFound { operationStatusMap[webhookCreate] = true operationStatusMap[webhookUpdate] = true @@ -777,19 +805,34 @@ func analyseOperationStatusMapValidatingWebhookConfiguration(rules []kyvernov1.R return operationStatusMap } -func analyseOperationStatusMapMutatingWebhookConfiguration(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { +func computeOperationsForMutatingWebhookConf(rules []kyvernov1.Rule, operationStatusMap map[string]bool) map[string]bool { for _, r := range rules { + opFound := false + if len(r.MatchResources.Any) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.MatchResources.Any, operationStatusMap) + } + if len(r.MatchResources.All) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.MatchResources.All, operationStatusMap) + } + if len(r.ExcludeResources.Any) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.Any, operationStatusMap) + } + if len(r.ExcludeResources.All) != 0 { + opFound, operationStatusMap = scanResourceFilter(r.ExcludeResources.All, operationStatusMap) + } if r.MatchResources.ResourceDescription.Operations != nil { for _, o := range r.MatchResources.ResourceDescription.Operations { + opFound = true operationStatusMap[string(o)] = true } } if r.ExcludeResources.ResourceDescription.Operations != nil { - for _, o := range r.MatchResources.ResourceDescription.Operations { + for _, o := range r.ExcludeResources.ResourceDescription.Operations { + opFound = true operationStatusMap[string(o)] = true } } - if r.MatchResources.ResourceDescription.Operations == nil && r.ExcludeResources.ResourceDescription.Operations == nil { + if !opFound { operationStatusMap[webhookCreate] = true operationStatusMap[webhookUpdate] = true } @@ -832,7 +875,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf } } rules := p.GetSpec().Rules - operationStatusMap = analyseOperationStatusMapValidatingWebhookConfiguration(rules, operationStatusMap) + operationStatusMap = computeOperationsForValidatingWebhookConf(rules, operationStatusMap) } operationReq := getMinimumOperations(operationStatusMap) diff --git a/pkg/controllers/webhook/controller_test.go b/pkg/controllers/webhook/controller_test.go new file mode 100644 index 000000000000..19133fd9796e --- /dev/null +++ b/pkg/controllers/webhook/controller_test.go @@ -0,0 +1,176 @@ +package webhook + +import ( + "reflect" + "testing" + + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + v1 "github.com/kyverno/kyverno/api/kyverno/v1" +) + +func TestGetMinimumOperations(t *testing.T) { + testCases := []struct { + name string + inputMap map[string]bool + expectedResult []string + }{ + { + name: "Test Case 1", + inputMap: map[string]bool{ + "CREATE": true, + "UPDATE": false, + "DELETE": true, + }, + expectedResult: []string{"CREATE", "DELETE"}, + }, + { + name: "Test Case 2", + inputMap: map[string]bool{ + "CREATE": false, + "UPDATE": false, + "DELETE": false, + "CONNECT": true, + }, + expectedResult: []string{"CONNECT"}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := getMinimumOperations(testCase.inputMap) + if !reflect.DeepEqual(result, testCase.expectedResult) { + t.Errorf("Expected %v, but got %v", testCase.expectedResult, result) + } + }) + } +} + +func TestComputeOperationsForMutatingWebhookConf(t *testing.T) { + testCases := []struct { + name string + rules []kyvernov1.Rule + expectedResult map[string]bool + }{ + { + name: "Test Case 1", + rules: []kyvernov1.Rule{ + { + MatchResources: kyvernov1.MatchResources{ + ResourceDescription: kyvernov1.ResourceDescription{ + Operations: []v1.AdmissionOperation{"CREATE"}, + }, + }, + }, + }, + expectedResult: map[string]bool{ + "CREATE": true, + }, + }, + { + name: "Test Case 2", + rules: []kyvernov1.Rule{ + { + MatchResources: kyvernov1.MatchResources{}, + ExcludeResources: kyvernov1.MatchResources{}, + }, + { + MatchResources: kyvernov1.MatchResources{}, + ExcludeResources: kyvernov1.MatchResources{}, + }, + }, + expectedResult: map[string]bool{ + webhookCreate: true, + webhookUpdate: true, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := computeOperationsForMutatingWebhookConf(testCase.rules, make(map[string]bool)) + if !reflect.DeepEqual(result, testCase.expectedResult) { + t.Errorf("Expected %v, but got %v", testCase.expectedResult, result) + } + }) + } +} + +func TestComputeOperationsForValidatingWebhookConf(t *testing.T) { + testCases := []struct { + name string + rules []kyvernov1.Rule + expectedResult map[string]bool + }{ + { + name: "Test Case 1", + rules: []kyvernov1.Rule{ + { + MatchResources: kyvernov1.MatchResources{ + ResourceDescription: kyvernov1.ResourceDescription{ + Operations: []v1.AdmissionOperation{"CREATE"}, + }, + }, + }, + { + ExcludeResources: kyvernov1.MatchResources{ + ResourceDescription: kyvernov1.ResourceDescription{ + Operations: []v1.AdmissionOperation{"DELETE"}, + }, + }, + }, + }, + expectedResult: map[string]bool{ + "CREATE": true, + "DELETE": true, + }, + }, + { + name: "Test Case 2", + rules: []kyvernov1.Rule{ + { + MatchResources: kyvernov1.MatchResources{}, + ExcludeResources: kyvernov1.MatchResources{}, + }, + { + MatchResources: kyvernov1.MatchResources{}, + ExcludeResources: kyvernov1.MatchResources{}, + }, + }, + expectedResult: map[string]bool{ + webhookCreate: true, + webhookUpdate: true, + }, + }, + { + name: "Test Case 3", + rules: []kyvernov1.Rule{ + { + MatchResources: kyvernov1.MatchResources{ + ResourceDescription: kyvernov1.ResourceDescription{ + Operations: []v1.AdmissionOperation{"CREATE", "UPDATE"}, + }, + }, + ExcludeResources: kyvernov1.MatchResources{ + ResourceDescription: kyvernov1.ResourceDescription{ + Operations: []v1.AdmissionOperation{"DELETE"}, + }, + }, + }, + }, + expectedResult: map[string]bool{ + webhookCreate: true, + webhookUpdate: true, + "DELETE": true, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := computeOperationsForValidatingWebhookConf(testCase.rules, make(map[string]bool)) + if !reflect.DeepEqual(result, testCase.expectedResult) { + t.Errorf("Expected %v, but got %v", testCase.expectedResult, result) + } + }) + } +} diff --git a/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml b/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml index 07c43a20f7da..f0a7182c47ae 100644 --- a/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml +++ b/test/conformance/kuttl/webhooks/all-scale/webhooks.yaml @@ -13,8 +13,6 @@ webhooks: operations: - CREATE - UPDATE - - DELETE - - CONNECT resources: - '*/scale' scope: '*' diff --git a/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml b/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml index 13b2b04673d5..2aa731edad0e 100644 --- a/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml +++ b/test/conformance/kuttl/webhooks/double-wildcard/webhooks.yaml @@ -14,8 +14,6 @@ webhooks: operations: - CREATE - UPDATE - - DELETE - - CONNECT resources: - '*/*' scope: '*' diff --git a/test/conformance/kuttl/webhooks/dynamic-op/01-policy.yaml b/test/conformance/kuttl/webhooks/dynamic-op/01-policy.yaml new file mode 100644 index 000000000000..b088ed7601b5 --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/01-policy.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- policy.yaml +assert: +- policy-assert.yaml diff --git a/test/conformance/kuttl/webhooks/dynamic-op/02-webhooks.yaml b/test/conformance/kuttl/webhooks/dynamic-op/02-webhooks.yaml new file mode 100644 index 000000000000..7048b639a6cd --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/02-webhooks.yaml @@ -0,0 +1,4 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +assert: +- webhooks.yaml diff --git a/test/conformance/kuttl/webhooks/dynamic-op/README.md b/test/conformance/kuttl/webhooks/dynamic-op/README.md new file mode 100644 index 000000000000..4d4e5767300c --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/README.md @@ -0,0 +1,3 @@ +## Description + +This test verifies that operations configured dynmically are correct diff --git a/test/conformance/kuttl/webhooks/dynamic-op/policy-assert.yaml b/test/conformance/kuttl/webhooks/dynamic-op/policy-assert.yaml new file mode 100644 index 000000000000..2993bbaa6e6b --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/policy-assert.yaml @@ -0,0 +1,9 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: require-labels +status: + conditions: + - reason: Succeeded + status: "True" + type: Ready diff --git a/test/conformance/kuttl/webhooks/dynamic-op/policy.yaml b/test/conformance/kuttl/webhooks/dynamic-op/policy.yaml new file mode 100644 index 000000000000..8e4bb6e9962f --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/policy.yaml @@ -0,0 +1,24 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: require-labels + annotations: + pod-policies.kyverno.io/autogen-controllers: none +spec: + validationFailureAction: Audit + background: false + rules: + - name: require-team + match: + any: + - resources: + kinds: + - '*/scale' + operations: + - CREATE + validate: + message: 'The label `team` is required.' + pattern: + metadata: + labels: + team: '?*' diff --git a/test/conformance/kuttl/webhooks/dynamic-op/webhooks.yaml b/test/conformance/kuttl/webhooks/dynamic-op/webhooks.yaml new file mode 100644 index 000000000000..8c59f7e6b94e --- /dev/null +++ b/test/conformance/kuttl/webhooks/dynamic-op/webhooks.yaml @@ -0,0 +1,17 @@ +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + labels: + webhook.kyverno.io/managed-by: kyverno + name: kyverno-resource-validating-webhook-cfg +webhooks: +- rules: + - apiGroups: + - '*' + apiVersions: + - '*' + operations: + - CREATE + resources: + - '*/scale' + scope: '*' diff --git a/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml b/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml index 49a26e89be8d..d6124dcdb1da 100644 --- a/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml +++ b/test/conformance/kuttl/webhooks/only-pod/webhooks.yaml @@ -13,8 +13,6 @@ webhooks: operations: - CREATE - UPDATE - - DELETE - - CONNECT resources: - pods - pods/ephemeralcontainers diff --git a/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml b/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml index 9766dc5d3456..b44d5c57c2de 100644 --- a/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml +++ b/test/conformance/kuttl/webhooks/pod-all-subresources/webhooks.yaml @@ -13,8 +13,6 @@ webhooks: operations: - CREATE - UPDATE - - DELETE - - CONNECT resources: - pods/attach - pods/binding diff --git a/test/conformance/kuttl/webhooks/wildcard/webhooks.yaml b/test/conformance/kuttl/webhooks/wildcard/webhooks.yaml index 281adc9a9f39..6e08ec3b155f 100644 --- a/test/conformance/kuttl/webhooks/wildcard/webhooks.yaml +++ b/test/conformance/kuttl/webhooks/wildcard/webhooks.yaml @@ -14,8 +14,6 @@ webhooks: operations: - CREATE - UPDATE - - DELETE - - CONNECT resources: - '*' - pods/ephemeralcontainers