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