refactor: remove validate failure actions from response (#6304)

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

pkg/engine/api/engineresponse.go

@@ -143,7 +143,8 @@ func (er EngineResponse) getRules(predicate func(RuleResponse) bool) []string {
 }
 
 func (er *EngineResponse) GetValidationFailureAction() kyvernov1.ValidationFailureAction {
-	for _, v := range er.PolicyResponse.ValidationFailureActionOverrides {
+	spec := er.Policy.GetSpec()
+	for _, v := range spec.ValidationFailureActionOverrides {
 		if !v.Action.IsValid() {
 			continue
 		}
@@ -165,5 +166,5 @@ func (er *EngineResponse) GetValidationFailureAction() kyvernov1.ValidationFailu
 			}
 		}
 	}
-	return er.PolicyResponse.ValidationFailureAction
+	return spec.ValidationFailureAction
 }

pkg/engine/api/engineresponse_test.go

@@ -736,61 +736,73 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 		want   kyvernov1.ValidationFailureAction
 	}{{
 		fields: fields{
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Audit,
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Audit,
+				},
 			},
 		},
 		want: kyvernov1.Audit,
 	}, {
 		fields: fields{
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
 	}, {
 		fields: fields{
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"*"},
-				}},
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"*"},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Audit,
 	}, {
 		fields: fields{
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     "invalid",
-					Namespaces: []string{"*"},
-				}},
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     "invalid",
+						Namespaces: []string{"*"},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
 	}, {
 		fields: fields{
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"foo"},
-				}},
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"foo"},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Audit,
 	}, {
 		fields: fields{
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"bar"},
-				}},
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"bar"},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
@@ -800,16 +812,18 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action: kyvernov1.Audit,
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"bar": "foo",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action: kyvernov1.Audit,
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"bar": "foo",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
@@ -819,16 +833,18 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action: kyvernov1.Audit,
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"foo": "bar",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action: kyvernov1.Audit,
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Audit,
@@ -838,17 +854,19 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"foo"},
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"bar": "foo",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"foo"},
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"bar": "foo",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
@@ -858,17 +876,19 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"bar"},
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"foo": "bar",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"bar"},
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Enforce,
@@ -878,17 +898,19 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"foo"},
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"foo": "bar",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"foo"},
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Audit,
@@ -898,17 +920,19 @@ func TestEngineResponse_GetValidationFailureAction(t *testing.T) {
 				"foo": "bar",
 			},
 			PatchedResource: resource,
-			PolicyResponse: PolicyResponse{
-				ValidationFailureAction: kyvernov1.Enforce,
-				ValidationFailureActionOverrides: []ValidationFailureActionOverride{{
-					Action:     kyvernov1.Audit,
-					Namespaces: []string{"*"},
-					NamespaceSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"foo": "bar",
+			Policy: &kyvernov1.ClusterPolicy{
+				Spec: kyvernov1.Spec{
+					ValidationFailureAction: kyvernov1.Enforce,
+					ValidationFailureActionOverrides: []kyvernov1.ValidationFailureActionOverride{{
+						Action:     kyvernov1.Audit,
+						Namespaces: []string{"*"},
+						NamespaceSelector: &metav1.LabelSelector{
+							MatchLabels: map[string]string{
+								"foo": "bar",
+							},
 						},
-					},
-				}},
+					}},
+				},
 			},
 		},
 		want: kyvernov1.Audit,

pkg/engine/api/policyresponse.go

@@ -1,24 +1,9 @@
 package api
 
-import (
-	kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-)
-
-type ValidationFailureActionOverride struct {
-	Action            kyvernov1.ValidationFailureAction
-	Namespaces        []string
-	NamespaceSelector *metav1.LabelSelector
-}
-
 // PolicyResponse policy application response
 type PolicyResponse struct {
 	// Stats contains policy statistics
 	Stats PolicyStats
 	// Rules contains policy rules responses
 	Rules []RuleResponse
-	// ValidationFailureAction audit (default) or enforce
-	ValidationFailureAction kyvernov1.ValidationFailureAction
-	// ValidationFailureActionOverrides overrides
-	ValidationFailureActionOverrides []ValidationFailureActionOverride
 }

pkg/engine/internal/response.go

@@ -52,12 +52,6 @@ func BuildResponse(ctx engineapi.PolicyContext, resp *engineapi.EngineResponse,
 		}
 		resp.PatchedResource = resource
 	}
-	policy := ctx.Policy()
-	resp.PolicyResponse.ValidationFailureAction = policy.GetSpec().ValidationFailureAction
-	for _, v := range policy.GetSpec().ValidationFailureActionOverrides {
-		newOverrides := engineapi.ValidationFailureActionOverride{Action: v.Action, Namespaces: v.Namespaces, NamespaceSelector: v.NamespaceSelector}
-		resp.PolicyResponse.ValidationFailureActionOverrides = append(resp.PolicyResponse.ValidationFailureActionOverrides, newOverrides)
-	}
 	resp.PolicyResponse.Stats.ProcessingTime = time.Since(startTime)
 	resp.PolicyResponse.Stats.Timestamp = startTime.Unix()
 	return resp

pkg/webhooks/utils/block_test.go

@@ -46,10 +46,21 @@ func Test_getAction(t *testing.T) {
 }
 
 func TestBlockRequest(t *testing.T) {
-	policy := &kyvernov1.ClusterPolicy{
+	auditPolicy := &kyvernov1.ClusterPolicy{
 		ObjectMeta: v1.ObjectMeta{
 			Name: "test",
 		},
+		Spec: kyvernov1.Spec{
+			ValidationFailureAction: kyvernov1.Audit,
+		},
+	}
+	enforcePolicy := &kyvernov1.ClusterPolicy{
+		ObjectMeta: v1.ObjectMeta{
+			Name: "test",
+		},
+		Spec: kyvernov1.Spec{
+			ValidationFailureAction: kyvernov1.Enforce,
+		},
 	}
 	resource := unstructured.Unstructured{
 		Object: map[string]interface{}{
@@ -73,8 +84,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "failure - enforce",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Enforce",
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-fail",
@@ -92,8 +102,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "failure - audit",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Audit",
+				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-fail",
@@ -111,8 +120,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "error - fail",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Audit",
+				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-error",
@@ -130,8 +138,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "error - ignore",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Audit",
+				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-error",
@@ -149,8 +156,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "warning - ignore",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Audit",
+				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-warning",
@@ -168,8 +174,7 @@ func TestBlockRequest(t *testing.T) {
 		name: "warning - fail",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Audit",
+				engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-warning",
@@ -193,10 +198,13 @@ func TestBlockRequest(t *testing.T) {
 }
 
 func TestGetBlockedMessages(t *testing.T) {
-	policy := &kyvernov1.ClusterPolicy{
+	enforcePolicy := &kyvernov1.ClusterPolicy{
 		ObjectMeta: v1.ObjectMeta{
 			Name: "test",
 		},
+		Spec: kyvernov1.Spec{
+			ValidationFailureAction: kyvernov1.Enforce,
+		},
 	}
 	resource := unstructured.Unstructured{
 		Object: map[string]interface{}{
@@ -218,8 +226,7 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "failure - enforce",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Enforce",
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-fail",
@@ -235,8 +242,7 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "error - enforce",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Enforce",
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-error",
@@ -252,8 +258,7 @@ func TestGetBlockedMessages(t *testing.T) {
 		name: "error and failure - enforce",
 		args: args{
 			engineResponses: []*engineapi.EngineResponse{
-				engineapi.NewEngineResponse(resource, policy, nil, &engineapi.PolicyResponse{
-					ValidationFailureAction: "Enforce",
+				engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{
 					Rules: []engineapi.RuleResponse{
 						{
 							Name:    "rule-fail",