diff --git a/pkg/controllers/webhook/controller.go b/pkg/controllers/webhook/controller.go index 2d7e880adae9..c04700b70a12 100644 --- a/pkg/controllers/webhook/controller.go +++ b/pkg/controllers/webhook/controller.go @@ -51,6 +51,10 @@ const ( IdleDeadline = tickerInterval * 10 maxRetries = 10 tickerInterval = 10 * time.Second + webhookCreate = "CREATE" + webhookUpdate = "UPDATE" + webhookDelete = "DELETE" + webhookConnect = "CONNECT" ) var ( @@ -627,6 +631,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config return nil, err } c.recordPolicyState(config.MutatingWebhookConfigurationName, policies...) + operationStatusMap := getOperationStatusMap() for _, p := range policies { spec := p.GetSpec() if spec.HasMutate() || spec.HasVerifyImages() { @@ -636,7 +641,10 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config c.mergeWebhook(fail, p, false) } } + rules := p.GetSpec().Rules + operationStatusMap = computeOperationsForMutatingWebhookConf(rules, operationStatusMap) } + operationReq := getMinimumOperations(operationStatusMap) webhookCfg := config.WebhookConfig{} webhookCfgs := cfg.GetWebhooks() if len(webhookCfgs) > 0 { @@ -649,7 +657,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config admissionregistrationv1.MutatingWebhook{ Name: config.MutatingWebhookName + "-ignore", ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/ignore"), - Rules: ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update), + Rules: ignore.buildRulesWithOperations(operationReq...), FailurePolicy: &ignore.failurePolicy, SideEffects: &noneOnDryRun, AdmissionReviewVersions: []string{"v1"}, @@ -667,7 +675,7 @@ func (c *controller) buildResourceMutatingWebhookConfiguration(cfg config.Config admissionregistrationv1.MutatingWebhook{ Name: config.MutatingWebhookName + "-fail", ClientConfig: c.clientConfig(caBundle, config.MutatingWebhookServicePath+"/fail"), - Rules: fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update), + Rules: fail.buildRulesWithOperations(operationReq...), FailurePolicy: &fail.failurePolicy, SideEffects: &noneOnDryRun, AdmissionReviewVersions: []string{"v1"}, @@ -736,6 +744,112 @@ 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 + operationStatusMap[webhookUpdate] = false + operationStatusMap[webhookDelete] = false + operationStatusMap[webhookConnect] = false + return operationStatusMap +} + +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.ExcludeResources.ResourceDescription.Operations { + opFound = true + operationStatusMap[string(o)] = true + } + } + if !opFound { + operationStatusMap[webhookCreate] = true + operationStatusMap[webhookUpdate] = true + operationStatusMap[webhookConnect] = true + operationStatusMap[webhookDelete] = true + } + } + return operationStatusMap +} + +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.ExcludeResources.ResourceDescription.Operations { + opFound = true + operationStatusMap[string(o)] = true + } + } + if !opFound { + operationStatusMap[webhookCreate] = true + operationStatusMap[webhookUpdate] = true + } + operationStatusMap[webhookDelete] = true + } + return operationStatusMap +} + +func getMinimumOperations(operationStatusMap map[string]bool) []admissionregistrationv1.OperationType { + operationReq := make([]admissionregistrationv1.OperationType, 0, 4) + for k, v := range operationStatusMap { + if v { + var oper admissionregistrationv1.OperationType = admissionregistrationv1.OperationType(k) + operationReq = append(operationReq, oper) + } + } + return operationReq +} + func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Configuration, caBundle []byte) (*admissionregistrationv1.ValidatingWebhookConfiguration, error) { result := admissionregistrationv1.ValidatingWebhookConfiguration{ ObjectMeta: objectMeta(config.ValidatingWebhookConfigurationName, cfg.GetWebhookAnnotations(), c.buildOwner()...), @@ -749,6 +863,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf return nil, err } c.recordPolicyState(config.ValidatingWebhookConfigurationName, policies...) + operationStatusMap := getOperationStatusMap() for _, p := range policies { spec := p.GetSpec() if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasVerifyImageChecks() || spec.HasVerifyManifests() { @@ -758,7 +873,11 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf c.mergeWebhook(fail, p, true) } } + rules := p.GetSpec().Rules + operationStatusMap = computeOperationsForValidatingWebhookConf(rules, operationStatusMap) } + operationsNeeded := getMinimumOperations(operationStatusMap) + webhookCfg := config.WebhookConfig{} webhookCfgs := cfg.GetWebhooks() if len(webhookCfgs) > 0 { @@ -775,7 +894,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf admissionregistrationv1.ValidatingWebhook{ Name: config.ValidatingWebhookName + "-ignore", ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/ignore"), - Rules: ignore.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect), + Rules: ignore.buildRulesWithOperations(operationsNeeded...), FailurePolicy: &ignore.failurePolicy, SideEffects: sideEffects, AdmissionReviewVersions: []string{"v1"}, @@ -792,7 +911,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf admissionregistrationv1.ValidatingWebhook{ Name: config.ValidatingWebhookName + "-fail", ClientConfig: c.clientConfig(caBundle, config.ValidatingWebhookServicePath+"/fail"), - Rules: fail.buildRulesWithOperations(admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, admissionregistrationv1.Connect), + Rules: fail.buildRulesWithOperations(operationsNeeded...), FailurePolicy: &fail.failurePolicy, SideEffects: sideEffects, AdmissionReviewVersions: []string{"v1"}, diff --git a/pkg/controllers/webhook/controller_test.go b/pkg/controllers/webhook/controller_test.go new file mode 100644 index 000000000000..50a0723a1849 --- /dev/null +++ b/pkg/controllers/webhook/controller_test.go @@ -0,0 +1,189 @@ +package webhook + +import ( + "reflect" + "sort" + "testing" + + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" + v1 "github.com/kyverno/kyverno/api/kyverno/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" +) + +func TestGetMinimumOperations(t *testing.T) { + testCases := []struct { + name string + inputMap map[string]bool + expectedResult []admissionregistrationv1.OperationType + }{ + { + name: "Test Case 1", + inputMap: map[string]bool{ + "CREATE": true, + "UPDATE": false, + "DELETE": true, + }, + expectedResult: []admissionregistrationv1.OperationType{"CREATE", "DELETE"}, + }, + { + name: "Test Case 2", + inputMap: map[string]bool{ + "CREATE": false, + "UPDATE": false, + "DELETE": false, + "CONNECT": true, + }, + expectedResult: []admissionregistrationv1.OperationType{"CONNECT"}, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + result := getMinimumOperations(testCase.inputMap) + sort.Slice(result, func(i, j int) bool { + return result[i] < result[j] + }) + sort.Slice(testCase.expectedResult, func(i, j int) bool { + return testCase.expectedResult[i] < testCase.expectedResult[j] + }) + + 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, + "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, + webhookDelete: 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, + webhookConnect: true, + webhookDelete: 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/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: '*'