refactor: introduce pss validation handler (#6724)

* refactor: remove rules pointer

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* refactor: introduce pss validation handler

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

* handler

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

---------

Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>

api/kyverno/v1/rule_types.go

@@ -109,31 +109,24 @@ func (r *Rule) HasVerifyImages() bool {
 	return false
 }
 
-// HasYAMLSignatureVerify checks for validate.manifests rule
+// HasVerifyImageChecks checks whether the verifyImages rule has validation checks
-func (r Rule) HasYAMLSignatureVerify() bool {
+func (r *Rule) HasVerifyImageChecks() bool {
+	for _, verifyImage := range r.VerifyImages {
+		if verifyImage.VerifyDigest || verifyImage.Required {
+			return true
+		}
+	}
+	return false
+}
+
+// HasVerifyManifests checks for validate.manifests rule
+func (r Rule) HasVerifyManifests() bool {
 	return r.Validation.Manifests != nil && len(r.Validation.Manifests.Attestors) != 0
 }
 
-// HasImagesValidationChecks checks whether the verifyImages rule has validation checks
+// HasValidatePodSecurity checks for validate.podSecurity rule
-func (r *Rule) HasImagesValidationChecks() bool {
+func (r Rule) HasValidatePodSecurity() bool {
-	for _, v := range r.VerifyImages {
+	return r.Validation.PodSecurity != nil && !datautils.DeepEqual(r.Validation.PodSecurity, &PodSecurity{})
-		if v.VerifyDigest || v.Required {
-			return true
-		}
-	}
-
-	return false
-}
-
-// HasYAMLSignatureVerify checks for validate rule
-func (p *ClusterPolicy) HasYAMLSignatureVerify() bool {
-	for _, rule := range p.Spec.Rules {
-		if rule.HasYAMLSignatureVerify() {
-			return true
-		}
-	}
-
-	return false
 }
 
 // HasValidate checks for validate rule

api/kyverno/v1/spec_types.go

@@ -133,7 +133,6 @@ func (s *Spec) HasMutate() bool {
 			return true
 		}
 	}
-
 	return false
 }
 
@@ -144,7 +143,6 @@ func (s *Spec) HasValidate() bool {
 			return true
 		}
 	}
-
 	return false
 }
 
@@ -155,18 +153,16 @@ func (s *Spec) HasGenerate() bool {
 			return true
 		}
 	}
-
 	return false
 }
 
-// HasImagesValidationChecks checks for image verification rules invoked during resource validation
+// HasVerifyImageChecks checks for image verification rules invoked during resource validation
-func (s *Spec) HasImagesValidationChecks() bool {
+func (s *Spec) HasVerifyImageChecks() bool {
 	for _, rule := range s.Rules {
-		if rule.HasImagesValidationChecks() {
+		if rule.HasVerifyImageChecks() {
 			return true
 		}
 	}
-
 	return false
 }
 
@@ -177,18 +173,16 @@ func (s *Spec) HasVerifyImages() bool {
 			return true
 		}
 	}
-
 	return false
 }
 
-// HasYAMLSignatureVerify checks for image verification rules invoked during resource mutation
+// HasVerifyManifests checks for image verification rules invoked during resource mutation
-func (s *Spec) HasYAMLSignatureVerify() bool {
+func (s *Spec) HasVerifyManifests() bool {
 	for _, rule := range s.Rules {
-		if rule.HasYAMLSignatureVerify() {
+		if rule.HasVerifyManifests() {
 			return true
 		}
 	}
-
 	return false
 }
 
@@ -197,7 +191,6 @@ func (s *Spec) BackgroundProcessingEnabled() bool {
 	if s.Background == nil {
 		return true
 	}
-
 	return *s.Background
 }
 

api/kyverno/v2beta1/rule_types.go

@@ -77,31 +77,24 @@ func (r *Rule) HasVerifyImages() bool {
 	return false
 }
 
-// HasYAMLSignatureVerify checks for validate.manifests rule
+// HasVerifyImageChecks checks whether the verifyImages rule has validation checks
-func (r Rule) HasYAMLSignatureVerify() bool {
+func (r *Rule) HasVerifyImageChecks() bool {
-	return r.Validation.Manifests != nil && len(r.Validation.Manifests.Attestors) != 0
-}
-
-// HasImagesValidationChecks checks whether the verifyImages rule has validation checks
-func (r *Rule) HasImagesValidationChecks() bool {
 	for _, v := range r.VerifyImages {
 		if v.VerifyDigest || v.Required {
 			return true
 		}
 	}
-
 	return false
 }
 
-// HasYAMLSignatureVerify checks for validate rule
+// HasVerifyManifests checks for validate.manifests rule
-func (p *ClusterPolicy) HasYAMLSignatureVerify() bool {
+func (r Rule) HasVerifyManifests() bool {
-	for _, rule := range p.Spec.Rules {
+	return r.Validation.Manifests != nil && len(r.Validation.Manifests.Attestors) != 0
-		if rule.HasYAMLSignatureVerify() {
+}
-			return true
-		}
-	}
 
-	return false
+// HasValidatePodSecurity checks for validate.podSecurity rule
+func (r Rule) HasValidatePodSecurity() bool {
+	return r.Validation.PodSecurity != nil && !datautils.DeepEqual(r.Validation.PodSecurity, &kyvernov1.PodSecurity{})
 }
 
 // HasValidate checks for validate rule

api/kyverno/v2beta1/spec_types.go

@@ -121,10 +121,10 @@ func (s *Spec) HasGenerate() bool {
 	return false
 }
 
-// HasImagesValidationChecks checks for image verification rules invoked during resource validation
+// HasVerifyImageChecks checks for image verification rules invoked during resource validation
-func (s *Spec) HasImagesValidationChecks() bool {
+func (s *Spec) HasVerifyImageChecks() bool {
 	for _, rule := range s.Rules {
-		if rule.HasImagesValidationChecks() {
+		if rule.HasVerifyImageChecks() {
 			return true
 		}
 	}
@@ -143,10 +143,10 @@ func (s *Spec) HasVerifyImages() bool {
 	return false
 }
 
-// HasYAMLSignatureVerify checks for image verification rules invoked during resource mutation
+// HasVerifyManifests checks for image verification rules invoked during resource mutation
-func (s *Spec) HasYAMLSignatureVerify() bool {
+func (s *Spec) HasVerifyManifests() bool {
 	for _, rule := range s.Rules {
-		if rule.HasYAMLSignatureVerify() {
+		if rule.HasVerifyManifests() {
 			return true
 		}
 	}

cmd/cli/kubectl-kyverno/utils/common/common.go

@@ -502,7 +502,7 @@ OuterLoop:
 
 	var policyHasValidate bool
 	for _, rule := range autogen.ComputeRules(c.Policy) {
-		if rule.HasValidate() || rule.HasImagesValidationChecks() {
+		if rule.HasValidate() || rule.HasVerifyImageChecks() {
 			policyHasValidate = true
 		}
 	}
@@ -702,7 +702,7 @@ func ProcessValidateEngineResponse(policy kyvernov1.PolicyInterface, validateRes
 	printCount := 0
 	for _, policyRule := range autogen.ComputeRules(policy) {
 		ruleFoundInEngineResponse := false
-		if !policyRule.HasValidate() && !policyRule.HasImagesValidationChecks() && !policyRule.HasVerifyImages() {
+		if !policyRule.HasValidate() && !policyRule.HasVerifyImageChecks() && !policyRule.HasVerifyImages() {
 			continue
 		}
 

pkg/controllers/report/utils/utils.go

@@ -47,7 +47,7 @@ func RemoveNonValidationPolicies(policies ...kyvernov1.PolicyInterface) []kyvern
 	var validationPolicies []kyvernov1.PolicyInterface
 	for _, pol := range policies {
 		spec := pol.GetSpec()
-		if spec.HasVerifyImages() || spec.HasValidate() || spec.HasYAMLSignatureVerify() {
+		if spec.HasVerifyImages() || spec.HasValidate() || spec.HasVerifyManifests() {
 			validationPolicies = append(validationPolicies, pol)
 		}
 	}

pkg/controllers/webhook/controller.go

@@ -735,7 +735,7 @@ func (c *controller) buildResourceValidatingWebhookConfiguration(cfg config.Conf
 		c.recordPolicyState(config.ValidatingWebhookConfigurationName, policies...)
 		for _, p := range policies {
 			spec := p.GetSpec()
-			if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasImagesValidationChecks() || spec.HasYAMLSignatureVerify() {
+			if spec.HasValidate() || spec.HasGenerate() || spec.HasMutate() || spec.HasVerifyImageChecks() || spec.HasVerifyManifests() {
 				if spec.GetFailurePolicy() == kyvernov1.Ignore {
 					c.mergeWebhook(ignore, p, true)
 				} else {
@@ -826,10 +826,10 @@ func (c *controller) mergeWebhook(dst *webhook, policy kyvernov1.PolicyInterface
 			matchedGVK = append(matchedGVK, rule.Generation.CloneList.Kinds...)
 			continue
 		}
-		if (updateValidate && rule.HasValidate() || rule.HasImagesValidationChecks()) ||
+		if (updateValidate && rule.HasValidate() || rule.HasVerifyImageChecks()) ||
 			(updateValidate && rule.HasMutate() && rule.IsMutateExisting()) ||
 			(!updateValidate && rule.HasMutate()) && !rule.IsMutateExisting() ||
-			(!updateValidate && rule.HasVerifyImages()) || (!updateValidate && rule.HasYAMLSignatureVerify()) {
+			(!updateValidate && rule.HasVerifyImages()) || (!updateValidate && rule.HasVerifyManifests()) {
 			matchedGVK = append(matchedGVK, rule.MatchResources.GetKinds()...)
 		}
 	}

pkg/engine/engine.go

@@ -30,11 +30,12 @@ type engine struct {
 	rclient                    registryclient.Client
 	engineContextLoaderFactory engineapi.EngineContextLoaderFactory
 	exceptionSelector          engineapi.PolicyExceptionSelector
-	validateManifestHandler    handlers.Handler
-	mutateResourceHandler      handlers.Handler
-	mutateExistingHandler      handlers.Handler
 	validateResourceHandler    handlers.Handler
 	validateImageHandler       handlers.Handler
+	validateManifestHandler    handlers.Handler
+	validatePssHandler         handlers.Handler
+	mutateResourceHandler      handlers.Handler
+	mutateExistingHandler      handlers.Handler
 }
 
 func NewEngine(
@@ -62,9 +63,10 @@ func NewEngine(
 		rclient:                    rclient,
 		engineContextLoaderFactory: engineContextLoaderFactory,
 		exceptionSelector:          exceptionSelector,
-		validateManifestHandler:    validation.NewValidateManifestHandler(client),
-		validateImageHandler:       validation.NewValidateImageHandler(configuration),
 		validateResourceHandler:    validation.NewValidateResourceHandler(engineContextLoaderFactory),
+		validateImageHandler:       validation.NewValidateImageHandler(configuration),
+		validateManifestHandler:    validation.NewValidateManifestHandler(client),
+		validatePssHandler:         validation.NewValidatePssHandler(),
 		mutateResourceHandler:      mutation.NewMutateResourceHandler(engineContextLoaderFactory),
 		mutateExistingHandler:      mutation.NewMutateExistingHandler(client, engineContextLoaderFactory),
 	}

pkg/engine/handlers/validation/validate_pss.go

@@ -0,0 +1,115 @@
+package validation
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
+	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/pss"
+	appsv1 "k8s.io/api/apps/v1"
+	batchv1 "k8s.io/api/batch/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+type validatePssHandler struct{}
+
+func NewValidatePssHandler() handlers.Handler {
+	return validatePssHandler{}
+}
+
+func (h validatePssHandler) Process(
+	ctx context.Context,
+	logger logr.Logger,
+	policyContext engineapi.PolicyContext,
+	resource unstructured.Unstructured,
+	rule kyvernov1.Rule,
+) (unstructured.Unstructured, []engineapi.RuleResponse) {
+	podSecurity := rule.Validation.PodSecurity
+	// Marshal pod metadata and spec
+	podSpec, metadata, err := getSpec(resource)
+	if err != nil {
+		return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "Error while getting new resource", err))
+	}
+	pod := &corev1.Pod{
+		Spec:       *podSpec,
+		ObjectMeta: *metadata,
+	}
+	allowed, pssChecks, err := pss.EvaluatePod(podSecurity, pod)
+	if err != nil {
+		return resource, handlers.RuleResponses(internal.RuleError(rule, engineapi.Validation, "failed to parse pod security api version", err))
+	}
+	podSecurityChecks := &engineapi.PodSecurityChecks{
+		Level:   podSecurity.Level,
+		Version: podSecurity.Version,
+		Checks:  pssChecks,
+	}
+	if allowed {
+		msg := fmt.Sprintf("Validation rule '%s' passed.", rule.Name)
+		rspn := internal.RulePass(rule, engineapi.Validation, msg)
+		rspn.PodSecurityChecks = podSecurityChecks
+		return resource, handlers.RuleResponses(rspn)
+	} else {
+		msg := fmt.Sprintf(`Validation rule '%s' failed. It violates PodSecurity "%s:%s": %s`, rule.Name, podSecurity.Level, podSecurity.Version, pss.FormatChecksPrint(pssChecks))
+		rspn := internal.RuleResponse(rule, engineapi.Validation, msg, engineapi.RuleStatusFail)
+		rspn.PodSecurityChecks = podSecurityChecks
+		return resource, handlers.RuleResponses(rspn)
+	}
+}
+
+func getSpec(resource unstructured.Unstructured) (podSpec *corev1.PodSpec, metadata *metav1.ObjectMeta, err error) {
+	kind := resource.GetKind()
+
+	if kind == "DaemonSet" || kind == "Deployment" || kind == "Job" || kind == "StatefulSet" || kind == "ReplicaSet" || kind == "ReplicationController" {
+		var deployment appsv1.Deployment
+
+		resourceBytes, err := resource.MarshalJSON()
+		if err != nil {
+			return nil, nil, err
+		}
+		err = json.Unmarshal(resourceBytes, &deployment)
+		if err != nil {
+			return nil, nil, err
+		}
+		podSpec = &deployment.Spec.Template.Spec
+		metadata = &deployment.Spec.Template.ObjectMeta
+		return podSpec, metadata, nil
+	} else if kind == "CronJob" {
+		var cronJob batchv1.CronJob
+
+		resourceBytes, err := resource.MarshalJSON()
+		if err != nil {
+			return nil, nil, err
+		}
+		err = json.Unmarshal(resourceBytes, &cronJob)
+		if err != nil {
+			return nil, nil, err
+		}
+		podSpec = &cronJob.Spec.JobTemplate.Spec.Template.Spec
+		metadata = &cronJob.Spec.JobTemplate.ObjectMeta
+	} else if kind == "Pod" {
+		var pod corev1.Pod
+
+		resourceBytes, err := resource.MarshalJSON()
+		if err != nil {
+			return nil, nil, err
+		}
+		err = json.Unmarshal(resourceBytes, &pod)
+		if err != nil {
+			return nil, nil, err
+		}
+		podSpec = &pod.Spec
+		metadata = &pod.ObjectMeta
+		return podSpec, metadata, nil
+	}
+	if err != nil {
+		return nil, nil, err
+	}
+	return podSpec, metadata, err
+}

pkg/engine/handlers/validation/validate_resource.go

@@ -15,14 +15,9 @@ import (
 	engineutils "github.com/kyverno/kyverno/pkg/engine/utils"
 	"github.com/kyverno/kyverno/pkg/engine/validate"
 	"github.com/kyverno/kyverno/pkg/engine/variables"
-	"github.com/kyverno/kyverno/pkg/pss"
 	"github.com/kyverno/kyverno/pkg/utils/api"
 	datautils "github.com/kyverno/kyverno/pkg/utils/data"
-	appsv1 "k8s.io/api/apps/v1"
-	batchv1 "k8s.io/api/batch/v1"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
 
@@ -60,7 +55,6 @@ type validator struct {
 	pattern          apiextensions.JSON
 	anyPattern       apiextensions.JSON
 	deny             *kyvernov1.Deny
-	podSecurity      *kyvernov1.PodSecurity
 	forEach          []kyvernov1.ForEachValidation
 	contextLoader    engineapi.EngineContextLoader
 	nesting          int
@@ -77,7 +71,6 @@ func newValidator(log logr.Logger, contextLoader engineapi.EngineContextLoader,
 		pattern:          rule.Validation.GetPattern(),
 		anyPattern:       rule.Validation.GetAnyPattern(),
 		deny:             rule.Validation.Deny,
-		podSecurity:      rule.Validation.PodSecurity,
 		forEach:          rule.Validation.ForEachValidation,
 	}
 }
@@ -142,13 +135,6 @@ func (v *validator) validate(ctx context.Context) *engineapi.RuleResponse {
 		return ruleResponse
 	}
 
-	if v.podSecurity != nil {
-		if !engineutils.IsDeleteRequest(v.policyContext) {
-			ruleResponse := v.validatePodSecurity()
-			return ruleResponse
-		}
-	}
-
 	if v.forEach != nil {
 		ruleResponse := v.validateForEach(ctx)
 		return ruleResponse
@@ -274,93 +260,6 @@ func (v *validator) getDenyMessage(deny bool) string {
 	}
 }
 
-func getSpec(v *validator) (podSpec *corev1.PodSpec, metadata *metav1.ObjectMeta, err error) {
-	newResource := v.policyContext.NewResource()
-	kind := newResource.GetKind()
-
-	if kind == "DaemonSet" || kind == "Deployment" || kind == "Job" || kind == "StatefulSet" || kind == "ReplicaSet" || kind == "ReplicationController" {
-		var deployment appsv1.Deployment
-
-		resourceBytes, err := newResource.MarshalJSON()
-		if err != nil {
-			return nil, nil, err
-		}
-		err = json.Unmarshal(resourceBytes, &deployment)
-		if err != nil {
-			return nil, nil, err
-		}
-		podSpec = &deployment.Spec.Template.Spec
-		metadata = &deployment.Spec.Template.ObjectMeta
-		return podSpec, metadata, nil
-	} else if kind == "CronJob" {
-		var cronJob batchv1.CronJob
-
-		resourceBytes, err := newResource.MarshalJSON()
-		if err != nil {
-			return nil, nil, err
-		}
-		err = json.Unmarshal(resourceBytes, &cronJob)
-		if err != nil {
-			return nil, nil, err
-		}
-		podSpec = &cronJob.Spec.JobTemplate.Spec.Template.Spec
-		metadata = &cronJob.Spec.JobTemplate.ObjectMeta
-	} else if kind == "Pod" {
-		var pod corev1.Pod
-
-		resourceBytes, err := newResource.MarshalJSON()
-		if err != nil {
-			return nil, nil, err
-		}
-		err = json.Unmarshal(resourceBytes, &pod)
-		if err != nil {
-			return nil, nil, err
-		}
-		podSpec = &pod.Spec
-		metadata = &pod.ObjectMeta
-		return podSpec, metadata, nil
-	}
-
-	if err != nil {
-		return nil, nil, err
-	}
-	return podSpec, metadata, err
-}
-
-// Unstructured
-func (v *validator) validatePodSecurity() *engineapi.RuleResponse {
-	// Marshal pod metadata and spec
-	podSpec, metadata, err := getSpec(v)
-	if err != nil {
-		return internal.RuleError(v.rule, engineapi.Validation, "Error while getting new resource", err)
-	}
-
-	pod := &corev1.Pod{
-		Spec:       *podSpec,
-		ObjectMeta: *metadata,
-	}
-	allowed, pssChecks, err := pss.EvaluatePod(v.podSecurity, pod)
-	if err != nil {
-		return internal.RuleError(v.rule, engineapi.Validation, "failed to parse pod security api version", err)
-	}
-	podSecurityChecks := &engineapi.PodSecurityChecks{
-		Level:   v.podSecurity.Level,
-		Version: v.podSecurity.Version,
-		Checks:  pssChecks,
-	}
-	if allowed {
-		msg := fmt.Sprintf("Validation rule '%s' passed.", v.rule.Name)
-		rspn := internal.RulePass(v.rule, engineapi.Validation, msg)
-		rspn.PodSecurityChecks = podSecurityChecks
-		return rspn
-	} else {
-		msg := fmt.Sprintf(`Validation rule '%s' failed. It violates PodSecurity "%s:%s": %s`, v.rule.Name, v.podSecurity.Level, v.podSecurity.Version, pss.FormatChecksPrint(pssChecks))
-		rspn := internal.RuleResponse(v.rule, engineapi.Validation, msg, engineapi.RuleStatusFail)
-		rspn.PodSecurityChecks = podSecurityChecks
-		return rspn
-	}
-}
-
 func (v *validator) validateResourceWithRule() *engineapi.RuleResponse {
 	element := v.policyContext.Element()
 	if !engineutils.IsEmptyUnstructured(&element) {

pkg/engine/validation.go

@@ -42,18 +42,23 @@ func (e *engine) validateResource(
 		logger := internal.LoggerWithRule(logger, rule)
 		startTime := time.Now()
 		hasValidate := rule.HasValidate()
-		hasValidateImage := rule.HasImagesValidationChecks()
+		hasVerifyImageChecks := rule.HasVerifyImageChecks()
-		hasYAMLSignatureVerify := rule.HasYAMLSignatureVerify()
+		if !hasValidate && !hasVerifyImageChecks {
-		if !hasValidate && !hasValidateImage {
 			continue
 		}
 		var handler handlers.Handler
-		if hasValidate && !hasYAMLSignatureVerify {
+		if hasValidate {
-			handler = e.validateResourceHandler
+			hasVerifyManifest := rule.HasVerifyManifests()
-		} else if hasValidateImage {
+			hasValidatePss := rule.HasValidatePodSecurity()
+			if hasVerifyManifest {
+				handler = e.validateManifestHandler
+			} else if hasValidatePss {
+				handler = e.validatePssHandler
+			} else {
+				handler = e.validateResourceHandler
+			}
+		} else if hasVerifyImageChecks {
 			handler = e.validateImageHandler
-		} else if hasYAMLSignatureVerify {
-			handler = e.validateManifestHandler
 		}
 		if handler != nil {
 			_, ruleResp := e.invokeRuleHandler(ctx, logger, handler, policyContext, policyContext.NewResource(), rule, engineapi.Validation)

pkg/policycache/store.go

@@ -140,7 +140,7 @@ func (m *policyMap) set(key string, policy kyvernov1.PolicyInterface, client Res
 			hasValidate := rule.HasValidate()
 			hasGenerate := rule.HasGenerate()
 			hasVerifyImages := rule.HasVerifyImages()
-			hasImagesValidationChecks := rule.HasImagesValidationChecks()
+			hasImagesValidationChecks := rule.HasVerifyImageChecks()
 			for gvrs := range entries {
 				entry := kindStates[gvrs]
 				entry.hasMutate = entry.hasMutate || hasMutate