refactor: use policy interface and introduce admission utils package (#3512)

* refactor: use more policy interface

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

* refactor: migrate to policy interface

Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com>

pkg/engine/mutation.go

@@ -18,9 +18,11 @@ import (
 
 // Mutate performs mutation. Overlay first and then mutation patches
 func Mutate(policyContext *PolicyContext) (resp *response.EngineResponse) {
-	resp = &response.EngineResponse{}
 	startTime := time.Now()
 	policy := policyContext.Policy
+	resp = &response.EngineResponse{
+		Policy: policy,
+	}
 	patchedResource := policyContext.NewResource
 	ctx := policyContext.JSONContext
 	var skippedRules []string

pkg/policy/metrics.go

@@ -9,61 +9,63 @@ import (
 	policyRuleInfoMetric "github.com/kyverno/kyverno/pkg/metrics/policyruleinfo"
 )
 
-func (pc *PolicyController) registerPolicyRuleInfoMetricAddPolicy(logger logr.Logger, p *kyverno.ClusterPolicy) {
+func (pc *PolicyController) registerPolicyRuleInfoMetricAddPolicy(logger logr.Logger, p kyverno.PolicyInterface) {
 	err := policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).AddPolicy(p)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's creation", "name", p.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's creation", "name", p.GetName())
 	}
 }
 
-func (pc *PolicyController) registerPolicyRuleInfoMetricUpdatePolicy(logger logr.Logger, oldP, curP *kyverno.ClusterPolicy) {
+func (pc *PolicyController) registerPolicyRuleInfoMetricUpdatePolicy(logger logr.Logger, oldP, curP kyverno.PolicyInterface) {
 	// removing the old rules associated metrics
 	err := policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).RemovePolicy(oldP)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.GetName())
 	}
 	// adding the new rules associated metrics
 	err = policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).AddPolicy(curP)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.GetName())
 	}
 }
 
-func (pc *PolicyController) registerPolicyRuleInfoMetricDeletePolicy(logger logr.Logger, p *kyverno.ClusterPolicy) {
+func (pc *PolicyController) registerPolicyRuleInfoMetricDeletePolicy(logger logr.Logger, p kyverno.PolicyInterface) {
 	err := policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).RemovePolicy(p)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's deletion", "name", p.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's deletion", "name", p.GetName())
 	}
 }
 
-func (pc *PolicyController) registerPolicyChangesMetricAddPolicy(logger logr.Logger, p *kyverno.ClusterPolicy) {
+func (pc *PolicyController) registerPolicyChangesMetricAddPolicy(logger logr.Logger, p kyverno.PolicyInterface) {
 	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(p, policyChangesMetric.PolicyCreated)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's creation", "name", p.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's creation", "name", p.GetName())
 	}
 }
 
-func (pc *PolicyController) registerPolicyChangesMetricUpdatePolicy(logger logr.Logger, oldP, curP *kyverno.ClusterPolicy) {
-	if reflect.DeepEqual((*oldP).Spec, (*curP).Spec) {
+func (pc *PolicyController) registerPolicyChangesMetricUpdatePolicy(logger logr.Logger, oldP, curP kyverno.PolicyInterface) {
+	oldSpec := oldP.GetSpec()
+	curSpec := curP.GetSpec()
+	if reflect.DeepEqual(oldSpec, curSpec) {
 		return
 	}
 	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(oldP, policyChangesMetric.PolicyUpdated)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", oldP.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", oldP.GetName())
 	}
 	// curP will require a new kyverno_policy_changes_total metric if the above update involved change in the following fields:
-	if curP.Spec.Background != oldP.Spec.Background || curP.Spec.ValidationFailureAction != oldP.Spec.ValidationFailureAction {
+	if curSpec.Background != oldSpec.Background || curSpec.ValidationFailureAction != oldSpec.ValidationFailureAction {
 		err = policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(curP, policyChangesMetric.PolicyUpdated)
 		if err != nil {
-			logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", curP.Name)
+			logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", curP.GetName())
 		}
 	}
 }
 
-func (pc *PolicyController) registerPolicyChangesMetricDeletePolicy(logger logr.Logger, p *kyverno.ClusterPolicy) {
+func (pc *PolicyController) registerPolicyChangesMetricDeletePolicy(logger logr.Logger, p kyverno.PolicyInterface) {
 	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(p, policyChangesMetric.PolicyDeleted)
 	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's deletion", "name", p.Name)
+		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's deletion", "name", p.GetName())
 	}
 }
 
@@ -73,54 +75,3 @@ func (pc *PolicyController) registerPolicyRuleInfoMetricDeleteNsPolicy(logger lo
 		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's deletion", "name", p.Name)
 	}
 }
-
-func (pc *PolicyController) registerPolicyChangesMetricAddNsPolicy(logger logr.Logger, p *kyverno.Policy) {
-	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(p, policyChangesMetric.PolicyCreated)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's creation", "name", p.Name)
-	}
-}
-
-func (pc *PolicyController) registerPolicyChangesMetricUpdateNsPolicy(logger logr.Logger, oldP, curP *kyverno.Policy) {
-	if reflect.DeepEqual((*oldP).Spec, (*curP).Spec) {
-		return
-	}
-	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(oldP, policyChangesMetric.PolicyUpdated)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", oldP.Name)
-	}
-	// curP will require a new kyverno_policy_changes_total metric if the above update involved change in the following fields:
-	if curP.Spec.Background != oldP.Spec.Background || curP.Spec.ValidationFailureAction != oldP.Spec.ValidationFailureAction {
-		err = policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(curP, policyChangesMetric.PolicyUpdated)
-		if err != nil {
-			logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's updation", "name", curP.Name)
-		}
-	}
-}
-
-func (pc *PolicyController) registerPolicyChangesMetricDeleteNsPolicy(logger logr.Logger, p *kyverno.Policy) {
-	err := policyChangesMetric.ParsePromConfig(*pc.promConfig).RegisterPolicy(p, policyChangesMetric.PolicyDeleted)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_changes_total metrics for the above policy's deletion", "name", p.Name)
-	}
-}
-
-func (pc *PolicyController) registerPolicyRuleInfoMetricAddNsPolicy(logger logr.Logger, p *kyverno.Policy) {
-	err := policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).AddPolicy(p)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's creation", "name", p.Name)
-	}
-}
-
-func (pc *PolicyController) registerPolicyRuleInfoMetricUpdateNsPolicy(logger logr.Logger, oldP, curP *kyverno.Policy) {
-	// removing the old rules associated metrics
-	err := policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).RemovePolicy(oldP)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.Name)
-	}
-	// adding the new rules associated metrics
-	err = policyRuleInfoMetric.ParsePromConfig(*pc.promConfig).AddPolicy(curP)
-	if err != nil {
-		logger.Error(err, "error occurred while registering kyverno_policy_rule_info_total metrics for the above policy's updation", "name", oldP.Name)
-	}
-}

pkg/policy/policy_controller.go

@@ -282,9 +282,9 @@ func (pc *PolicyController) addNsPolicy(obj interface{}) {
 	p := obj.(*kyverno.Policy)
 
 	// register kyverno_policy_rule_info_total metric concurrently
-	go pc.registerPolicyRuleInfoMetricAddNsPolicy(logger, p)
+	go pc.registerPolicyRuleInfoMetricAddPolicy(logger, p)
 	// register kyverno_policy_changes_total metric concurrently
-	go pc.registerPolicyChangesMetricAddNsPolicy(logger, p)
+	go pc.registerPolicyChangesMetricAddPolicy(logger, p)
 
 	logger.Info("policy created", "uid", p.UID, "kind", "Policy", "name", p.Name, "namespaces", p.Namespace)
 
@@ -309,9 +309,9 @@ func (pc *PolicyController) updateNsPolicy(old, cur interface{}) {
 	curP := cur.(*kyverno.Policy)
 
 	// register kyverno_policy_rule_info_total metric concurrently
-	go pc.registerPolicyRuleInfoMetricUpdateNsPolicy(logger, oldP, curP)
+	go pc.registerPolicyRuleInfoMetricUpdatePolicy(logger, oldP, curP)
 	// register kyverno_policy_changes_total metric concurrently
-	go pc.registerPolicyChangesMetricUpdateNsPolicy(logger, oldP, curP)
+	go pc.registerPolicyChangesMetricUpdatePolicy(logger, oldP, curP)
 
 	if curP.Spec.Background == nil || curP.Spec.ValidationFailureAction == "" || missingAutoGenRules(curP, logger) {
 		nsPol, _ := common.MutatePolicy(curP, logger)
@@ -353,9 +353,9 @@ func (pc *PolicyController) deleteNsPolicy(obj interface{}) {
 	}
 
 	// register kyverno_policy_rule_info_total metric concurrently
-	go pc.registerPolicyRuleInfoMetricDeleteNsPolicy(logger, p)
+	go pc.registerPolicyRuleInfoMetricDeletePolicy(logger, p)
 	// register kyverno_policy_changes_total metric concurrently
-	go pc.registerPolicyChangesMetricDeleteNsPolicy(logger, p)
+	go pc.registerPolicyChangesMetricDeletePolicy(logger, p)
 
 	logger.Info("policy deleted event", "uid", p.UID, "kind", "Policy", "policy_name", p.Name, "namespaces", p.Namespace)
 

pkg/utils/admission/utils.go

@@ -0,0 +1,64 @@
+package admission
+
+import (
+	"encoding/json"
+	"fmt"
+
+	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
+	v1beta1 "k8s.io/api/admission/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func UnmarshalPolicy(kind string, raw []byte) (kyverno.PolicyInterface, error) {
+	if kind == "ClusterPolicy" {
+		var policy *kyverno.ClusterPolicy
+		if err := json.Unmarshal(raw, &policy); err != nil {
+			return nil, err
+		}
+		return policy, nil
+	} else if kind == "Policy" {
+		var policy *kyverno.Policy
+		if err := json.Unmarshal(raw, &policy); err != nil {
+			return nil, err
+		}
+		return policy, nil
+	}
+	return nil, fmt.Errorf("admission request does not contain a policy")
+}
+
+func GetPolicy(request *v1beta1.AdmissionRequest) (kyverno.PolicyInterface, error) {
+	return UnmarshalPolicy(request.Kind.Kind, request.Object.Raw)
+}
+
+func GetPolicies(request *v1beta1.AdmissionRequest) (kyverno.PolicyInterface, kyverno.PolicyInterface, error) {
+	policy, err := UnmarshalPolicy(request.Kind.Kind, request.Object.Raw)
+	if err != nil {
+		return policy, nil, err
+	}
+	if request.Operation == v1beta1.Update {
+		oldPolicy, err := UnmarshalPolicy(request.Kind.Kind, request.OldObject.Raw)
+		return policy, oldPolicy, err
+	}
+	return policy, nil, nil
+}
+
+func Response(allowed bool) *v1beta1.AdmissionResponse {
+	r := &v1beta1.AdmissionResponse{
+		Allowed: allowed,
+	}
+	return r
+}
+
+func ResponseWithMessage(allowed bool, msg string) *v1beta1.AdmissionResponse {
+	r := Response(allowed)
+	r.Result = &metav1.Status{
+		Message: msg,
+	}
+	return r
+}
+
+func ResponseWithMessageAndPatch(allowed bool, msg string, patch []byte) *v1beta1.AdmissionResponse {
+	r := ResponseWithMessage(allowed, msg)
+	r.Patch = patch
+	return r
+}

pkg/webhooks/policymutation.go

@@ -1,97 +1,47 @@
 package webhooks
 
 import (
-	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
 	"time"
 
-	logr "github.com/go-logr/logr"
 	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/policymutation"
+	admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
 	v1beta1 "k8s.io/api/admission/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func (ws *WebhookServer) policyMutation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
 	logger := ws.log.WithValues("action", "policy mutation", "uid", request.UID, "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation, "gvk", request.Kind.String())
-	var policy *kyverno.ClusterPolicy
-	raw := request.Object.Raw
-
-	if err := json.Unmarshal(raw, &policy); err != nil {
-		logger.Error(err, "failed to unmarshal policy admission request")
-		return &v1beta1.AdmissionResponse{
-			Allowed: true,
-			Result: &metav1.Status{
-				Message: fmt.Sprintf("failed to default value, check kyverno controller logs for details: %v", err),
-			},
-		}
+	policy, oldPolicy, err := admissionutils.GetPolicies(request)
+	if err != nil {
+		logger.Error(err, "failed to unmarshal policies from admission request")
+		return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to default value, check kyverno controller logs for details: %v", err))
 	}
-
-	if request.Operation == v1beta1.Update {
-		admissionResponse := hasPolicyChanged(policy, request.OldObject.Raw, logger)
-		if admissionResponse != nil {
-			logger.V(4).Info("skip policy mutation on status update")
-			return admissionResponse
-		}
+	if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
+		logger.V(4).Info("skip policy mutation on status update")
+		return admissionutils.Response(true)
 	}
-
 	startTime := time.Now()
 	logger.V(3).Info("start policy change mutation")
 	defer logger.V(3).Info("finished policy change mutation", "time", time.Since(startTime).String())
-
 	// Generate JSON Patches for defaults
-	patches, updateMsgs := policymutation.GenerateJSONPatchesForDefaults(policy, logger)
-	if len(patches) != 0 {
-		patchType := v1beta1.PatchTypeJSONPatch
-		return &v1beta1.AdmissionResponse{
-			Allowed: true,
-			Result: &metav1.Status{
-				Message: strings.Join(updateMsgs, "'"),
-			},
-			Patch:     patches,
-			PatchType: &patchType,
-		}
-	}
-
-	return &v1beta1.AdmissionResponse{
-		Allowed: true,
+	if patches, updateMsgs := policymutation.GenerateJSONPatchesForDefaults(policy, logger); len(patches) != 0 {
+		return admissionutils.ResponseWithMessageAndPatch(true, strings.Join(updateMsgs, "'"), patches)
 	}
+	return admissionutils.Response(true)
 }
 
-func hasPolicyChanged(policy *kyverno.ClusterPolicy, oldRaw []byte, logger logr.Logger) *v1beta1.AdmissionResponse {
-	var oldPolicy *kyverno.ClusterPolicy
-	if err := json.Unmarshal(oldRaw, &oldPolicy); err != nil {
-		logger.Error(err, "failed to unmarshal old policy admission request")
-		return &v1beta1.AdmissionResponse{
-			Allowed: true,
-			Result: &metav1.Status{
-				Message: fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err),
-			},
-		}
-
-	}
-
-	if isStatusUpdate(oldPolicy, policy) {
-		return &v1beta1.AdmissionResponse{Allowed: true}
-	}
-
-	return nil
-}
-
-func isStatusUpdate(old, new *kyverno.ClusterPolicy) bool {
+func isStatusUpdate(old, new kyverno.PolicyInterface) bool {
 	if !reflect.DeepEqual(old.GetAnnotations(), new.GetAnnotations()) {
 		return false
 	}
-
 	if !reflect.DeepEqual(old.GetLabels(), new.GetLabels()) {
 		return false
 	}
-
-	if !reflect.DeepEqual(old.Spec, new.Spec) {
+	if !reflect.DeepEqual(old.GetSpec(), new.GetSpec()) {
 		return false
 	}
-
 	return true
 }

pkg/webhooks/policyvalidation.go

@@ -1,57 +1,36 @@
 package webhooks
 
 import (
-	"encoding/json"
 	"fmt"
 	"time"
 
-	kyverno "github.com/kyverno/kyverno/api/kyverno/v1"
 	policyvalidate "github.com/kyverno/kyverno/pkg/policy"
+	admissionutils "github.com/kyverno/kyverno/pkg/utils/admission"
 	v1beta1 "k8s.io/api/admission/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-//HandlePolicyValidation performs the validation check on policy resource
+//policyValidation performs the validation check on policy resource
 func (ws *WebhookServer) policyValidation(request *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse {
 	logger := ws.log.WithValues("action", "policy validation", "uid", request.UID, "kind", request.Kind, "namespace", request.Namespace, "name", request.Name, "operation", request.Operation, "gvk", request.Kind.String())
-	var policy *kyverno.ClusterPolicy
-
-	if err := json.Unmarshal(request.Object.Raw, &policy); err != nil {
-		logger.Error(err, "failed to unmarshal policy admission request")
-		return &v1beta1.AdmissionResponse{
-			Allowed: true,
-			Result: &metav1.Status{
-				Message: fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err),
-			},
-		}
+	policy, oldPolicy, err := admissionutils.GetPolicies(request)
+	if err != nil {
+		logger.Error(err, "failed to unmarshal policies from admission request")
+		return admissionutils.ResponseWithMessage(true, fmt.Sprintf("failed to validate policy, check kyverno controller logs for details: %v", err))
 	}
-
-	if request.Operation == v1beta1.Update {
-		admissionResponse := hasPolicyChanged(policy, request.OldObject.Raw, logger)
-		if admissionResponse != nil {
-			logger.V(4).Info("skip policy validation on status update")
-			return admissionResponse
-		}
+	if oldPolicy != nil && isStatusUpdate(oldPolicy, policy) {
+		logger.V(4).Info("skip policy validation on status update")
+		return admissionutils.Response(true)
 	}
-
 	startTime := time.Now()
 	logger.V(3).Info("start policy change validation")
 	defer logger.V(3).Info("finished policy change validation", "time", time.Since(startTime).String())
-
 	response, err := policyvalidate.Validate(policy, ws.client, false, ws.openAPIController)
 	if err != nil {
 		logger.Error(err, "policy validation errors")
-		return &v1beta1.AdmissionResponse{
-			Allowed: false,
-			Result: &metav1.Status{
-				Message: err.Error(),
-			},
-		}
+		return admissionutils.ResponseWithMessage(true, err.Error())
 	}
 	if response != nil && len(response.Warnings) != 0 {
 		return response
 	}
-	return &v1beta1.AdmissionResponse{
-		Allowed: true,
-	}
+	return admissionutils.Response(true)
 }

pkg/webhooks/report.go

@@ -29,13 +29,9 @@ func generateEvents(engineResponses []*response.EngineResponse, blocked, onUpdat
 			failedRulesStr := strings.Join(failedRules, ";")
 
 			// Event on the policy
-			kind := "ClusterPolicy"
-			if er.PolicyResponse.Policy.Namespace != "" {
-				kind = "Policy"
-			}
 			pe := event.NewEvent(
 				log,
-				kind,
+				er.Policy.GetKind(),
 				kyvernov1alpha2.SchemeGroupVersion.String(),
 				er.PolicyResponse.Policy.Namespace,
 				er.PolicyResponse.Policy.Name,
@@ -67,13 +63,9 @@ func generateEvents(engineResponses []*response.EngineResponse, blocked, onUpdat
 			successRulesStr := strings.Join(successRules, ";")
 
 			// Event on the policy
-			kind := "ClusterPolicy"
-			if er.PolicyResponse.Policy.Namespace != "" {
-				kind = "Policy"
-			}
 			e := event.NewEvent(
 				log,
-				kind,
+				er.Policy.GetKind(),
 				kyvernov1alpha2.SchemeGroupVersion.String(),
 				er.PolicyResponse.Policy.Namespace,
 				er.PolicyResponse.Policy.Name,