From 8d8d72bf3545fd93c24d84ea678b7591a9ecfb9c Mon Sep 17 00:00:00 2001 From: Bhargav Ravuri Date: Thu, 1 Feb 2024 13:41:55 +0530 Subject: [PATCH] fix(reports-controller): Allow UPDATE operation in validation Pass operations list instead of one to check for resources that match policy's condition block. Also, decouple operation list for resource validation from the policy context's operation. Signed-off-by: Bhargav Ravuri --- pkg/controllers/report/utils/scanner.go | 4 +- pkg/engine/api/engine.go | 1 + pkg/engine/background.go | 4 +- pkg/engine/engine.go | 11 +- pkg/engine/utils/match.go | 37 +- pkg/engine/utils/utils_test.go | 885 ++++++++++++++++++-- pkg/engine/validation.go | 2 + pkg/webhooks/resource/generation/handler.go | 2 +- 8 files changed, 843 insertions(+), 103 deletions(-) diff --git a/pkg/controllers/report/utils/scanner.go b/pkg/controllers/report/utils/scanner.go index 9b4da7b96bea..564c11433921 100644 --- a/pkg/controllers/report/utils/scanner.go +++ b/pkg/controllers/report/utils/scanner.go @@ -105,7 +105,9 @@ func (s *scanner) validateResource(ctx context.Context, resource unstructured.Un WithNewResource(resource). WithPolicy(policy). WithNamespaceLabels(nsLabels) - response := s.engine.Validate(ctx, policyCtx) + response := s.engine.Validate(ctx, policyCtx, + []kyvernov1.AdmissionOperation{kyvernov1.Create, kyvernov1.Update}..., // Pass operations that are allowed + ) return &response, nil } diff --git a/pkg/engine/api/engine.go b/pkg/engine/api/engine.go index 219098154224..21b6ac33249d 100644 --- a/pkg/engine/api/engine.go +++ b/pkg/engine/api/engine.go @@ -19,6 +19,7 @@ type Engine interface { Validate( ctx context.Context, policyContext PolicyContext, + allowedOperations ...kyvernov1.AdmissionOperation, ) EngineResponse // Mutate performs mutation. Overlay first and then mutation patches diff --git a/pkg/engine/background.go b/pkg/engine/background.go index 4c30de16d5a0..45328e7e3c65 100644 --- a/pkg/engine/background.go +++ b/pkg/engine/background.go @@ -87,10 +87,10 @@ func (e *engine) filterRule( policy := policyContext.Policy() gvk, subresource := policyContext.ResourceKind() - if err := engineutils.MatchesResourceDescription(newResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, policyContext.Operation()); err != nil { + if err := engineutils.MatchesResourceDescription(newResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, []kyvernov1.AdmissionOperation{policyContext.Operation()}); err != nil { if ruleType == engineapi.Generation { // if the oldResource matched, return "false" to delete GR for it - if err = engineutils.MatchesResourceDescription(oldResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, policyContext.Operation()); err == nil { + if err = engineutils.MatchesResourceDescription(oldResource, rule, admissionInfo, namespaceLabels, policy.GetNamespace(), gvk, subresource, []kyvernov1.AdmissionOperation{policyContext.Operation()}); err == nil { return engineapi.RuleFail(rule.Name, ruleType, "") } } diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 8b16028f14d1..710c6733c59b 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -87,12 +87,13 @@ func NewEngine( func (e *engine) Validate( ctx context.Context, policyContext engineapi.PolicyContext, + allowedOperations ...kyvernov1.AdmissionOperation, ) engineapi.EngineResponse { startTime := time.Now() response := engineapi.NewEngineResponseFromPolicyContext(policyContext) logger := internal.LoggerWithPolicyContext(logging.WithName("engine.validate"), policyContext) if internal.MatchPolicyContext(logger, e.client, policyContext, e.configuration) { - policyResponse := e.validate(ctx, logger, policyContext) + policyResponse := e.validate(ctx, logger, policyContext, allowedOperations) response = response.WithPolicyResponse(policyResponse) } response = response.WithStats(engineapi.NewExecutionStats(startTime, time.Now())) @@ -191,6 +192,7 @@ func (e *engine) matches( rule kyvernov1.Rule, policyContext engineapi.PolicyContext, resource unstructured.Unstructured, + allowedOperations []kyvernov1.AdmissionOperation, ) error { if policyContext.AdmissionOperation() { request := policyContext.AdmissionInfo() @@ -207,7 +209,7 @@ func (e *engine) matches( policyContext.Policy().GetNamespace(), gvk, subresource, - policyContext.Operation(), + allowedOperations, ) if err == nil { return nil @@ -222,7 +224,7 @@ func (e *engine) matches( policyContext.Policy().GetNamespace(), gvk, subresource, - policyContext.Operation(), + allowedOperations, ) if err == nil { return nil @@ -239,6 +241,7 @@ func (e *engine) invokeRuleHandler( resource unstructured.Unstructured, rule kyvernov1.Rule, ruleType engineapi.RuleType, + allowedOperations ...kyvernov1.AdmissionOperation, ) (unstructured.Unstructured, []engineapi.RuleResponse) { return tracing.ChildSpan2( ctx, @@ -246,7 +249,7 @@ func (e *engine) invokeRuleHandler( fmt.Sprintf("RULE %s", rule.Name), func(ctx context.Context, span trace.Span) (patchedResource unstructured.Unstructured, results []engineapi.RuleResponse) { // check if resource and rule match - if err := e.matches(rule, policyContext, resource); err != nil { + if err := e.matches(rule, policyContext, resource, allowedOperations); err != nil { logger.V(4).Info("rule not matched", "reason", err.Error()) return resource, nil } diff --git a/pkg/engine/utils/match.go b/pkg/engine/utils/match.go index ce071a1f9c52..39b8af731f14 100644 --- a/pkg/engine/utils/match.go +++ b/pkg/engine/utils/match.go @@ -57,13 +57,18 @@ func doesResourceMatchConditionBlock( namespaceLabels map[string]string, gvk schema.GroupVersionKind, subresource string, - operation kyvernov1.AdmissionOperation, + allowedOperations []kyvernov1.AdmissionOperation, ) []error { if len(conditionBlock.Operations) > 0 { - if !slices.Contains(conditionBlock.Operations, operation) { - // if operation does not match, return immediately - err := fmt.Errorf("operation does not match") - return []error{err} + // Check if all operations in condition block are in the allowed operations list + if !slices.ContainsFunc(allowedOperations, func(op kyvernov1.AdmissionOperation) bool { + return slices.Contains(conditionBlock.Operations, op) + }) { + // If none of the input operations match the operations from condition block, return immediately + return []error{ + fmt.Errorf("one or more operations from the %v don't match any from the allowed operations list %v", + allowedOperations, conditionBlock.Operations), + } } } @@ -173,7 +178,7 @@ func MatchesResourceDescription( policyNamespace string, gvk schema.GroupVersionKind, subresource string, - operation kyvernov1.AdmissionOperation, + allowedOperations []kyvernov1.AdmissionOperation, ) error { if resource.Object == nil { return fmt.Errorf("resource is empty") @@ -190,7 +195,7 @@ func MatchesResourceDescription( oneMatched := false for _, rmr := range rule.MatchResources.Any { // if there are no errors it means it was a match - if len(matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 { + if len(matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)) == 0 { oneMatched = true break } @@ -201,11 +206,11 @@ func MatchesResourceDescription( } else if len(rule.MatchResources.All) > 0 { // include object if ALL of the criteria match for _, rmr := range rule.MatchResources.All { - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)...) } } else { rmr := kyvernov1.ResourceFilter{UserInfo: rule.MatchResources.UserInfo, ResourceDescription: rule.MatchResources.ResourceDescription} - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionMatchHelper(rmr, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)...) } // check exlude conditions only if match succeeds @@ -213,7 +218,7 @@ func MatchesResourceDescription( if len(rule.ExcludeResources.Any) > 0 { // exclude the object if ANY of the criteria match for _, rer := range rule.ExcludeResources.Any { - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)...) } } else if len(rule.ExcludeResources.All) > 0 { // exclude the object if ALL the criteria match @@ -221,7 +226,7 @@ func MatchesResourceDescription( for _, rer := range rule.ExcludeResources.All { // we got no errors inplying a resource did NOT exclude it // "matchesResourceDescriptionExcludeHelper" returns errors if resource is excluded by a filter - if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)) == 0 { + if len(matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)) == 0 { excludedByAll = false break } @@ -231,7 +236,7 @@ func MatchesResourceDescription( } } else { rer := kyvernov1.ResourceFilter{UserInfo: rule.ExcludeResources.UserInfo, ResourceDescription: rule.ExcludeResources.ResourceDescription} - reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, operation)...) + reasonsForFailure = append(reasonsForFailure, matchesResourceDescriptionExcludeHelper(rer, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations)...) } } @@ -257,7 +262,7 @@ func matchesResourceDescriptionMatchHelper( namespaceLabels map[string]string, gvk schema.GroupVersionKind, subresource string, - operation kyvernov1.AdmissionOperation, + allowedOperations []kyvernov1.AdmissionOperation, ) []error { var errs []error if datautils.DeepEqual(admissionInfo, kyvernov1beta1.RequestInfo{}) { @@ -267,7 +272,7 @@ func matchesResourceDescriptionMatchHelper( // checking if resource matches the rule if !datautils.DeepEqual(rmr.ResourceDescription, kyvernov1.ResourceDescription{}) || !datautils.DeepEqual(rmr.UserInfo, kyvernov1.UserInfo{}) { - matchErrs := doesResourceMatchConditionBlock(rmr.ResourceDescription, rmr.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operation) + matchErrs := doesResourceMatchConditionBlock(rmr.ResourceDescription, rmr.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations) errs = append(errs, matchErrs...) } else { errs = append(errs, fmt.Errorf("match cannot be empty")) @@ -282,13 +287,13 @@ func matchesResourceDescriptionExcludeHelper( namespaceLabels map[string]string, gvk schema.GroupVersionKind, subresource string, - operation kyvernov1.AdmissionOperation, + allowedOperations []kyvernov1.AdmissionOperation, ) []error { var errs []error // checking if resource matches the rule if !datautils.DeepEqual(rer.ResourceDescription, kyvernov1.ResourceDescription{}) || !datautils.DeepEqual(rer.UserInfo, kyvernov1.UserInfo{}) { - excludeErrs := doesResourceMatchConditionBlock(rer.ResourceDescription, rer.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, operation) + excludeErrs := doesResourceMatchConditionBlock(rer.ResourceDescription, rer.UserInfo, admissionInfo, resource, namespaceLabels, gvk, subresource, allowedOperations) // it was a match so we want to exclude it if len(excludeErrs) == 0 { errs = append(errs, fmt.Errorf("resource excluded since one of the criteria excluded it")) diff --git a/pkg/engine/utils/utils_test.go b/pkg/engine/utils/utils_test.go index 2522bc79998f..6b120de1fcb2 100644 --- a/pkg/engine/utils/utils_test.go +++ b/pkg/engine/utils/utils_test.go @@ -21,6 +21,7 @@ func TestMatchesResourceDescription(t *testing.T) { Resource []byte Policy []byte areErrorsExpected bool + allowedOperations []v1.AdmissionOperation }{ { Description: "Match Any matches the Pod", @@ -192,6 +193,182 @@ func TestMatchesResourceDescription(t *testing.T) { }`), areErrorsExpected: true, }, + { + Description: "Match Any matches the Pod with condition block's operations list", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "match-any-create-or-update-rule", + "match": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names" : ["dev"], + "operations": ["CREATE","UPDATE"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"], + "operations": ["CREATE","UPDATE"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: false, + allowedOperations: []v1.AdmissionOperation{"CREATE"}, + }, + { + Description: "Match Any does not match the Pod as the input operation doesn't match any operation in the condition block", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "match-any-create-or-update-rule", + "match": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names" : ["dev"], + "operations": ["CREATE","UPDATE"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"], + "operations": ["CREATE","UPDATE"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: true, + allowedOperations: []v1.AdmissionOperation{"RANDOM"}, // Some operation that doesn't exist in match block above. + }, { Description: "Match All matches the Pod", AdmissionInfo: v1beta1.RequestInfo{ @@ -246,7 +423,466 @@ func TestMatchesResourceDescription(t *testing.T) { "kinds": [ "Pod" ], - "names" : ["abc"] + "names" : ["abc"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: false, + }, + { + Description: "Match All does not match the Pod", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names" : ["xyz"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: true, + }, + { + Description: "Match All matches the Pod with condition block's operations list", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names" : ["abc"], + "operations": ["CREATE","UPDATE"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"], + "operations": ["CREATE","UPDATE"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: false, + allowedOperations: []v1.AdmissionOperation{"CREATE"}, + }, + { + Description: "Match All does not match the Pod as the input operation doesn't match any operation in the condition block", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names" : ["abc"], + "operations": ["CREATE","UPDATE"] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces" : ["prod"], + "operations": ["CREATE","UPDATE"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: true, + allowedOperations: []v1.AdmissionOperation{"RANDOM"}, // Some operation that doesn't exist in match block above. + }, + { + Description: "Exclude Any excludes the Pod", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "dev", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": [ + "Pod" + ] + } + } + ] + }, + "exclude": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names": [ + "dev" + ] + } + }, + { + "resources": { + "kinds": [ + "Pod" + ], + "namespaces": [ + "default" + ] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: true, + }, + { + Description: "Exclude Any does not exclude the Pod", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "abc", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": [ + "Pod" + ] + } + } + ] + }, + "exclude": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "names": [ + "dev" + ] } }, { @@ -254,7 +890,9 @@ func TestMatchesResourceDescription(t *testing.T) { "kinds": [ "Pod" ], - "namespaces" : ["prod"] + "namespaces": [ + "default" + ] } } ] @@ -278,7 +916,7 @@ func TestMatchesResourceDescription(t *testing.T) { areErrorsExpected: false, }, { - Description: "Match All does not match the Pod", + Description: "Exclude Any excludes the Pod with condition block's operations list", AdmissionInfo: v1beta1.RequestInfo{ ClusterRoles: []string{"admin"}, }, @@ -286,7 +924,7 @@ func TestMatchesResourceDescription(t *testing.T) { "apiVersion": "v1", "kind": "Pod", "metadata": { - "name": "abc", + "name": "dev", "namespace" : "prod" }, "spec": { @@ -328,18 +966,25 @@ func TestMatchesResourceDescription(t *testing.T) { "all": [ { "resources": { - "kinds": [ - "Pod" - ], - "names" : ["xyz"] + "kinds": ["Pod"] + } + } + ] + }, + "exclude": { + "any": [ + { + "resources": { + "kinds": ["Pod"], + "names": ["dev"], + "operations": ["CREATE","UPDATE"] } }, { "resources": { - "kinds": [ - "Pod" - ], - "namespaces" : ["prod"] + "kinds": ["Pod"], + "namespaces": ["default"], + "operations": ["CREATE","UPDATE"] } } ] @@ -361,9 +1006,103 @@ func TestMatchesResourceDescription(t *testing.T) { } }`), areErrorsExpected: true, + allowedOperations: []v1.AdmissionOperation{"CREATE"}, }, { - Description: "Exclude Any excludes the Pod", + Description: "Exclude Any does not exclude the Pod as the input operation doesn't match any operation in the condition block", + AdmissionInfo: v1beta1.RequestInfo{ + ClusterRoles: []string{"admin"}, + }, + Resource: []byte(`{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "dev", + "namespace" : "prod" + }, + "spec": { + "containers": [ + { + "name": "cont-name", + "image": "cont-img", + "ports": [ + { + "containerPort": 81 + } + ], + "resources": { + "limits": { + "memory": "30Mi", + "cpu": "0.2" + }, + "requests": { + "memory": "20Mi", + "cpu": "0.1" + } + } + } + ] + } + }`), + Policy: []byte(`{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-policy" + }, + "spec": { + "background": false, + "rules": [ + { + "name": "test-rule", + "match": { + "all": [ + { + "resources": { + "kinds": ["Pod"] + } + } + ] + }, + "exclude": { + "any": [ + { + "resources": { + "kinds": ["Pod"], + "names": ["dev"], + "operations": ["CREATE","UPDATE"] + } + }, + { + "resources": { + "kinds": ["Pod"], + "namespaces": ["default"], + "operations": ["CREATE","UPDATE"] + } + } + ] + }, + "mutate": { + "overlay": { + "spec": { + "containers": [ + { + "(image)": "*", + "imagePullPolicy": "IfNotPresent" + } + ] + } + } + } + } + ] + } + }`), + areErrorsExpected: false, + allowedOperations: []v1.AdmissionOperation{"RANDOM"}, // Some operation that doesn't exist in match block above. + }, + { + Description: "Exclude All excludes the Pod", AdmissionInfo: v1beta1.RequestInfo{ ClusterRoles: []string{"admin"}, }, @@ -421,7 +1160,7 @@ func TestMatchesResourceDescription(t *testing.T) { ] }, "exclude": { - "any": [ + "all": [ { "resources": { "kinds": [ @@ -438,7 +1177,7 @@ func TestMatchesResourceDescription(t *testing.T) { "Pod" ], "namespaces": [ - "default" + "prod" ] } } @@ -463,7 +1202,7 @@ func TestMatchesResourceDescription(t *testing.T) { areErrorsExpected: true, }, { - Description: "Exclude Any does not exclude the Pod", + Description: "Exclude All does not exclude the Pod", AdmissionInfo: v1beta1.RequestInfo{ ClusterRoles: []string{"admin"}, }, @@ -521,14 +1260,14 @@ func TestMatchesResourceDescription(t *testing.T) { ] }, "exclude": { - "any": [ + "all": [ { "resources": { "kinds": [ "Pod" ], "names": [ - "dev" + "abc" ] } }, @@ -563,7 +1302,7 @@ func TestMatchesResourceDescription(t *testing.T) { areErrorsExpected: false, }, { - Description: "Exclude All excludes the Pod", + Description: "Exclude All excludes the Pod with condition block's operations list", AdmissionInfo: v1beta1.RequestInfo{ ClusterRoles: []string{"admin"}, }, @@ -613,9 +1352,7 @@ func TestMatchesResourceDescription(t *testing.T) { "all": [ { "resources": { - "kinds": [ - "Pod" - ] + "kinds": ["Pod"] } } ] @@ -624,22 +1361,16 @@ func TestMatchesResourceDescription(t *testing.T) { "all": [ { "resources": { - "kinds": [ - "Pod" - ], - "names": [ - "dev" - ] + "kinds": ["Pod"], + "names": ["dev"], + "operations": ["CREATE","UPDATE"] } }, { "resources": { - "kinds": [ - "Pod" - ], - "namespaces": [ - "prod" - ] + "kinds": ["Pod"], + "namespaces": ["prod"], + "operations": ["CREATE","UPDATE"] } } ] @@ -661,9 +1392,10 @@ func TestMatchesResourceDescription(t *testing.T) { } }`), areErrorsExpected: true, + allowedOperations: []v1.AdmissionOperation{"CREATE"}, }, { - Description: "Exclude All does not exclude the Pod", + Description: "Exclude All does not exclude the Pod as the input operation doesn't match any operation in the condition block", AdmissionInfo: v1beta1.RequestInfo{ ClusterRoles: []string{"admin"}, }, @@ -671,7 +1403,7 @@ func TestMatchesResourceDescription(t *testing.T) { "apiVersion": "v1", "kind": "Pod", "metadata": { - "name": "abc", + "name": "dev", "namespace" : "prod" }, "spec": { @@ -713,9 +1445,7 @@ func TestMatchesResourceDescription(t *testing.T) { "all": [ { "resources": { - "kinds": [ - "Pod" - ] + "kinds": ["Pod"] } } ] @@ -724,22 +1454,16 @@ func TestMatchesResourceDescription(t *testing.T) { "all": [ { "resources": { - "kinds": [ - "Pod" - ], - "names": [ - "abc" - ] + "kinds": ["Pod"], + "names": ["dev"], + "operations": ["CREATE","UPDATE"] } }, { "resources": { - "kinds": [ - "Pod" - ], - "namespaces": [ - "default" - ] + "kinds": ["Pod"], + "namespaces": ["prod"], + "operations": ["CREATE","UPDATE"] } } ] @@ -761,6 +1485,7 @@ func TestMatchesResourceDescription(t *testing.T) { } }`), areErrorsExpected: false, + allowedOperations: []v1.AdmissionOperation{"RANDOM"}, // Some operation that doesn't exist in match block above. }, { Description: "Should match pod and not exclude it", @@ -897,25 +1622,27 @@ func TestMatchesResourceDescription(t *testing.T) { } for i, tc := range tcs { - var policy v1.Policy - err := json.Unmarshal(tc.Policy, &policy) - if err != nil { - t.Errorf("Testcase %d invalid policy raw", i+1) - } - resource, _ := kubeutils.BytesToUnstructured(tc.Resource) - - for _, rule := range autogen.ComputeRules(&policy) { - err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo, nil, "", resource.GroupVersionKind(), "", "CREATE") + t.Run(tc.Description, func(t *testing.T) { + var policy v1.Policy + err := json.Unmarshal(tc.Policy, &policy) if err != nil { - if !tc.areErrorsExpected { - t.Errorf("Testcase %d Unexpected error: %v\nmsg: %s", i+1, err, tc.Description) - } - } else { - if tc.areErrorsExpected { - t.Errorf("Testcase %d Expected Error but received no error", i+1) + t.Errorf("Testcase %d invalid policy raw", i+1) + } + resource, _ := kubeutils.BytesToUnstructured(tc.Resource) + + for _, rule := range autogen.ComputeRules(&policy) { + err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo, nil, "", resource.GroupVersionKind(), "", tc.allowedOperations) + if err != nil { + if !tc.areErrorsExpected { + t.Errorf("Testcase %d Unexpected error: %v\nmsg: %s", i+1, err, tc.Description) + } + } else { + if tc.areErrorsExpected { + t.Errorf("Testcase %d Expected Error but received no error", i+1) + } } } - } + }) } } @@ -1810,7 +2537,7 @@ func TestMatchesResourceDescription_GenerateName(t *testing.T) { resource, _ := kubeutils.BytesToUnstructured(tc.Resource) for _, rule := range autogen.ComputeRules(&policy) { - err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo, nil, "", resource.GroupVersionKind(), "", "CREATE") + err := MatchesResourceDescription(*resource, rule, tc.AdmissionInfo, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}) if err != nil { if !tc.areErrorsExpected { t.Errorf("Testcase %d Unexpected error: %v\nmsg: %s", i+1, err, tc.Description) @@ -1877,7 +2604,7 @@ func TestResourceDescriptionMatch_MultipleKind(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -1980,7 +2707,7 @@ func TestResourceDescriptionMatch_ExcludeDefaultGroups(t *testing.T) { } // First test: confirm that this above rule produces errors (and raise an error if err == nil) - if err := MatchesResourceDescription(*resource, rule, requestInfo, nil, "", resource.GroupVersionKind(), "", "CREATE"); err == nil { + if err := MatchesResourceDescription(*resource, rule, requestInfo, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err == nil { t.Error("Testcase was expected to fail, but err was nil") } @@ -2000,7 +2727,7 @@ func TestResourceDescriptionMatch_ExcludeDefaultGroups(t *testing.T) { } // Second test: confirm that matching this rule does not create any errors (and raise if err != nil) - if err := MatchesResourceDescription(*resource, rule2, requestInfo, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule2, requestInfo, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase was expected to not fail, but err was %s", err) } @@ -2014,7 +2741,7 @@ func TestResourceDescriptionMatch_ExcludeDefaultGroups(t *testing.T) { }} // Third test: confirm that now the custom exclude-snippet should run in CheckSubjects() and that should result in this rule failing (raise if err == nil for that reason) - if err := MatchesResourceDescription(*resource, rule2, requestInfo, nil, "", resource.GroupVersionKind(), "", "CREATE"); err == nil { + if err := MatchesResourceDescription(*resource, rule2, requestInfo, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err == nil { t.Error("Testcase was expected to fail, but err was nil #1!") } } @@ -2073,7 +2800,7 @@ func TestResourceDescriptionMatch_Name(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2131,7 +2858,7 @@ func TestResourceDescriptionMatch_GenerateName(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2190,7 +2917,7 @@ func TestResourceDescriptionMatch_Name_Regex(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2248,7 +2975,7 @@ func TestResourceDescriptionMatch_GenerateName_Regex(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2315,7 +3042,7 @@ func TestResourceDescriptionMatch_Label_Expression_NotMatch(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2383,7 +3110,7 @@ func TestResourceDescriptionMatch_Label_Expression_Match(t *testing.T) { } rule := v1.Rule{MatchResources: v1.MatchResources{ResourceDescription: resourceDescription}} - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err != nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err != nil { t.Errorf("Testcase has failed due to the following:%v", err) } } @@ -2464,7 +3191,7 @@ func TestResourceDescriptionExclude_Label_Expression_Match(t *testing.T) { ExcludeResources: v1.MatchResources{ResourceDescription: resourceDescriptionExclude}, } - if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", "CREATE"); err == nil { + if err := MatchesResourceDescription(*resource, rule, v1beta1.RequestInfo{}, nil, "", resource.GroupVersionKind(), "", []v1.AdmissionOperation{"CREATE"}); err == nil { t.Errorf("Testcase has failed due to the following:\n Function has returned no error, even though it was supposed to fail") } } diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index bd6ab4579bbc..8350d19ae872 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -17,6 +17,7 @@ func (e *engine) validate( ctx context.Context, logger logr.Logger, policyContext engineapi.PolicyContext, + allowedOperations []kyvernov1.AdmissionOperation, ) engineapi.PolicyResponse { resp := engineapi.NewPolicyResponse() policy := policyContext.Policy() @@ -69,6 +70,7 @@ func (e *engine) validate( matchedResource, rule, engineapi.Validation, + allowedOperations..., ) matchedResource = resource resp.Add(engineapi.NewExecutionStats(startTime, time.Now()), ruleResp...) diff --git a/pkg/webhooks/resource/generation/handler.go b/pkg/webhooks/resource/generation/handler.go index f1d1366f85db..595bf2d5b0e5 100644 --- a/pkg/webhooks/resource/generation/handler.go +++ b/pkg/webhooks/resource/generation/handler.go @@ -321,7 +321,7 @@ func (h *generationHandler) processRequest(ctx context.Context, policyContext *e policy.GetNamespace(), gvk, subresource, - policyContext.Operation(), + []kyvernov1.AdmissionOperation{policyContext.Operation()}, ); err == nil { h.log.V(4).Info("skip creating UR as the admission resource is both the source and the trigger") continue