feat: compute policy exceptions as a part of the rule execution (kyve…

…rno#8713) Signed-off-by: Mariam Fahmy <[email protected]> Co-authored-by: Jim Bugwadia <[email protected]>
MariamFahmy98 · Nov 13, 2023 · c0e0cea · c0e0cea
1 parent 31858ab
commit c0e0cea
Show file tree

Hide file tree

Showing 13 changed files with 232 additions and 97 deletions.
diff --git a/pkg/engine/background.go b/pkg/engine/background.go
@@ -11,6 +11,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/engine/internal"
 	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	"github.com/kyverno/kyverno/pkg/engine/variables"
+	"k8s.io/client-go/tools/cache"
 )
 
 // ApplyBackgroundChecks checks for validity of generate and mutateExisting rules on the resource
@@ -60,10 +61,23 @@ func (e *engine) filterRule(
 		ruleType = engineapi.Generation
 	}
 
-	// check if there is a corresponding policy exception
-	ruleResp := e.hasPolicyExceptions(logger, ruleType, policyContext, rule)
-	if ruleResp != nil {
-		return ruleResp
+	// get policy exceptions that matches both policy and rule name
+	exceptions, err := e.GetPolicyExceptions(policyContext.Policy(), rule.Name)
+	if err != nil {
+		logger.Error(err, "failed to get exceptions")
+		return nil
+	}
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return engineapi.RuleError(rule.Name, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception)
+		}
 	}
 
 	newResource := policyContext.NewResource()

diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go
@@ -283,16 +283,14 @@ func (e *engine) invokeRuleHandler(
 					s := stringutils.JoinNonEmpty([]string{"preconditions not met", msg}, "; ")
 					return resource, handlers.WithSkip(rule, ruleType, s)
 				}
-				// process handler
-				resource, ruleResponses := handler.Process(ctx, logger, policyContext, resource, rule, contextLoader)
-				// check if there's an exception if rule fails.
-				for _, ruleResp := range ruleResponses {
-					if ruleResp.Status() == engineapi.RuleStatusFail {
-						if resp := e.hasPolicyExceptions(logger, ruleType, policyContext, rule); resp != nil {
-							return resource, handlers.WithResponses(resp)
-						}
-					}
+				// get policy exceptions that matches both policy and rule name
+				exceptions, err := e.GetPolicyExceptions(policyContext.Policy(), rule.Name)
+				if err != nil {
+					logger.Error(err, "failed to get exceptions")
+					return resource, nil
 				}
+				// process handler
+				resource, ruleResponses := handler.Process(ctx, logger, policyContext, resource, rule, contextLoader, exceptions)
 				return resource, ruleResponses
 			}
 			return resource, nil

diff --git a/pkg/engine/exceptions.go b/pkg/engine/exceptions.go
@@ -3,106 +3,33 @@ package engine
 import (
 	"fmt"
 
-	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
 	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
-	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
-	"github.com/kyverno/kyverno/pkg/utils/conditions"
-	matched "github.com/kyverno/kyverno/pkg/utils/match"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/tools/cache"
 )
 
-func findExceptions(
-	selector engineapi.PolicyExceptionSelector,
+// GetPolicyExceptions get all exceptions that match both the policy and the rule.
+func (e *engine) GetPolicyExceptions(
 	policy kyvernov1.PolicyInterface,
 	rule string,
-) ([]*kyvernov2beta1.PolicyException, error) {
-	if selector == nil {
-		return nil, nil
+) ([]kyvernov2beta1.PolicyException, error) {
+	var exceptions []kyvernov2beta1.PolicyException
+	if e.exceptionSelector == nil {
+		return exceptions, nil
 	}
-	polexs, err := selector.List(labels.Everything())
+	polexs, err := e.exceptionSelector.List(labels.Everything())
 	if err != nil {
-		return nil, err
+		return exceptions, err
 	}
-	var result []*kyvernov2beta1.PolicyException
 	policyName, err := cache.MetaNamespaceKeyFunc(policy)
 	if err != nil {
-		return nil, fmt.Errorf("failed to compute policy key: %w", err)
+		return exceptions, fmt.Errorf("failed to compute policy key: %w", err)
 	}
 	for _, polex := range polexs {
 		if polex.Contains(policyName, rule) {
-			result = append(result, polex)
+			exceptions = append(exceptions, *polex)
 		}
 	}
-	return result, nil
-}
-
-// matchesException checks if an exception applies to the resource being admitted
-func matchesException(
-	selector engineapi.PolicyExceptionSelector,
-	policyContext engineapi.PolicyContext,
-	rule kyvernov1.Rule,
-	logger logr.Logger,
-) (*kyvernov2beta1.PolicyException, error) {
-	candidates, err := findExceptions(selector, policyContext.Policy(), rule.Name)
-	if err != nil {
-		return nil, err
-	}
-	gvk, subresource := policyContext.ResourceKind()
-	resource := policyContext.NewResource()
-	if resource.Object == nil {
-		resource = policyContext.OldResource()
-	}
-	for _, candidate := range candidates {
-		err := matched.CheckMatchesResources(
-			resource,
-			candidate.Spec.Match,
-			policyContext.NamespaceLabels(),
-			policyContext.AdmissionInfo(),
-			gvk,
-			subresource,
-		)
-		// if there's no error it means a match
-		if err == nil {
-			if candidate.Spec.Conditions != nil {
-				passed, err := conditions.CheckAnyAllConditions(logger, policyContext.JSONContext(), *candidate.Spec.Conditions)
-				if err != nil {
-					return nil, err
-				}
-				if !passed {
-					return nil, fmt.Errorf("conditions did not pass")
-				}
-			}
-			return candidate, nil
-		}
-	}
-	return nil, nil
-}
-
-// hasPolicyExceptions returns nil when there are no matching exceptions.
-// A rule response is returned when an exception is matched, or there is an error.
-func (e *engine) hasPolicyExceptions(
-	logger logr.Logger,
-	ruleType engineapi.RuleType,
-	ctx engineapi.PolicyContext,
-	rule kyvernov1.Rule,
-) *engineapi.RuleResponse {
-	// if matches, check if there is a corresponding policy exception
-	exception, err := matchesException(e.exceptionSelector, ctx, rule, logger)
-	if err != nil {
-		logger.Error(err, "failed to match exceptions")
-		return nil
-	}
-	if exception == nil {
-		return nil
-	}
-	key, err := cache.MetaNamespaceKeyFunc(exception)
-	if err != nil {
-		logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
-		return engineapi.RuleError(rule.Name, ruleType, "failed to compute exception key", err)
-	} else {
-		logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
-		return engineapi.RuleSkip(rule.Name, ruleType, "rule skipped due to policy exception "+key).WithException(exception)
-	}
+	return exceptions, nil
 }
diff --git a/pkg/engine/handlers/handler.go b/pkg/engine/handlers/handler.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
@@ -17,6 +18,7 @@ type Handler interface {
 		unstructured.Unstructured,
 		kyvernov1.Rule,
 		engineapi.EngineContextLoader,
+		[]kyvernov2beta1.PolicyException,
 	) (unstructured.Unstructured, []engineapi.RuleResponse)
 }
 

diff --git a/pkg/engine/handlers/mutation/mutate_existing.go b/pkg/engine/handlers/mutation/mutate_existing.go
@@ -5,12 +5,15 @@ import (
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/engine/handlers"
 	"github.com/kyverno/kyverno/pkg/engine/internal"
 	"github.com/kyverno/kyverno/pkg/engine/mutate"
+	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	stringutils "github.com/kyverno/kyverno/pkg/utils/strings"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/tools/cache"
 )
 
 type mutateExistingHandler struct {
@@ -32,7 +35,23 @@ func (h mutateExistingHandler) Process(
 	resource unstructured.Unstructured,
 	rule kyvernov1.Rule,
 	contextLoader engineapi.EngineContextLoader,
+	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return resource, handlers.WithError(rule, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return resource, handlers.WithResponses(
+				engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception),
+			)
+		}
+	}
+
 	var responses []engineapi.RuleResponse
 	logger.V(3).Info("processing mutate rule")
 	targets, err := loadTargets(ctx, h.client, rule.Mutation.Targets, policyContext, logger)

diff --git a/pkg/engine/handlers/mutation/mutate_image.go b/pkg/engine/handlers/mutation/mutate_image.go
@@ -6,6 +6,7 @@ import (
 	json_patch "github.com/evanphx/json-patch/v5"
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	"github.com/kyverno/kyverno/pkg/config"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	enginecontext "github.com/kyverno/kyverno/pkg/engine/context"
@@ -19,6 +20,7 @@ import (
 	jsonutils "github.com/kyverno/kyverno/pkg/utils/json"
 	"gomodules.xyz/jsonpatch/v2"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/tools/cache"
 )
 
 type mutateImageHandler struct {
@@ -67,7 +69,23 @@ func (h mutateImageHandler) Process(
 	resource unstructured.Unstructured,
 	rule kyvernov1.Rule,
 	contextLoader engineapi.EngineContextLoader,
+	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return resource, handlers.WithError(rule, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return resource, handlers.WithResponses(
+				engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception),
+			)
+		}
+	}
+
 	jsonContext := policyContext.JSONContext()
 	ruleCopy, err := substituteVariables(rule, jsonContext, logger)
 	if err != nil {

diff --git a/pkg/engine/handlers/mutation/mutate_resource.go b/pkg/engine/handlers/mutation/mutate_resource.go
@@ -5,11 +5,14 @@ import (
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/engine/handlers"
 	"github.com/kyverno/kyverno/pkg/engine/mutate"
+	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/tools/cache"
 )
 
 type mutateResourceHandler struct{}
@@ -25,7 +28,23 @@ func (h mutateResourceHandler) Process(
 	resource unstructured.Unstructured,
 	rule kyvernov1.Rule,
 	contextLoader engineapi.EngineContextLoader,
+	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return resource, handlers.WithError(rule, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return resource, handlers.WithResponses(
+				engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception),
+			)
+		}
+	}
+
 	_, subresource := policyContext.ResourceKind()
 	logger.V(3).Info("processing mutate rule")
 	var parentResourceGVR metav1.GroupVersionResource

diff --git a/pkg/engine/handlers/validation/validate_cel.go b/pkg/engine/handlers/validation/validate_cel.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/engine/handlers"
 	"github.com/kyverno/kyverno/pkg/engine/internal"
@@ -22,6 +23,7 @@ import (
 	"k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy"
 	"k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions"
 	celconfig "k8s.io/apiserver/pkg/apis/cel"
+	"k8s.io/client-go/tools/cache"
 )
 
 type validateCELHandler struct {
@@ -41,11 +43,28 @@ func (h validateCELHandler) Process(
 	resource unstructured.Unstructured,
 	rule kyvernov1.Rule,
 	_ engineapi.EngineContextLoader,
+	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
 	if engineutils.IsDeleteRequest(policyContext) {
 		logger.V(3).Info("skipping CEL validation on deleted resource")
 		return resource, nil
 	}
+
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return resource, handlers.WithError(rule, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return resource, handlers.WithResponses(
+				engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception),
+			)
+		}
+	}
+
 	// check if a corresponding validating admission policy is generated
 	vapStatus := policyContext.Policy().GetStatus().ValidatingAdmissionPolicy
 	if vapStatus.Generated {

diff --git a/pkg/engine/handlers/validation/validate_image.go b/pkg/engine/handlers/validation/validate_image.go
@@ -6,12 +6,14 @@ import (
 
 	"github.com/go-logr/logr"
 	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	kyvernov2beta1 "github.com/kyverno/kyverno/api/kyverno/v2beta1"
 	"github.com/kyverno/kyverno/pkg/config"
 	engineapi "github.com/kyverno/kyverno/pkg/engine/api"
 	"github.com/kyverno/kyverno/pkg/engine/handlers"
 	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	apiutils "github.com/kyverno/kyverno/pkg/utils/api"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/client-go/tools/cache"
 )
 
 type validateImageHandler struct{}
@@ -42,7 +44,23 @@ func (h validateImageHandler) Process(
 	resource unstructured.Unstructured,
 	rule kyvernov1.Rule,
 	_ engineapi.EngineContextLoader,
+	exceptions []kyvernov2beta1.PolicyException,
 ) (unstructured.Unstructured, []engineapi.RuleResponse) {
+	// check if there is a policy exception matches the incoming resource
+	exception := engineutils.MatchesException(exceptions, policyContext, logger)
+	if exception != nil {
+		key, err := cache.MetaNamespaceKeyFunc(exception)
+		if err != nil {
+			logger.Error(err, "failed to compute policy exception key", "namespace", exception.GetNamespace(), "name", exception.GetName())
+			return resource, handlers.WithError(rule, engineapi.Validation, "failed to compute exception key", err)
+		} else {
+			logger.V(3).Info("policy rule skipped due to policy exception", "exception", key)
+			return resource, handlers.WithResponses(
+				engineapi.RuleSkip(rule.Name, engineapi.Validation, "rule skipped due to policy exception "+key).WithException(exception),
+			)
+		}
+	}
+
 	for _, v := range rule.VerifyImages {
 		imageVerify := v.Convert()
 		for _, infoMap := range policyContext.JSONContext().ImageInfo() {