From c8bbb5bead57da535dc5ecb9584d660bbc654f28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 7 Sep 2022 16:01:42 +0200 Subject: [PATCH] refactor: utils for warnings and unit tests (#4523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- pkg/webhooks/resource/handlers.go | 4 +- pkg/webhooks/resource/utils.go | 14 ---- pkg/webhooks/resource/validation.go | 3 +- pkg/webhooks/utils/warning.go | 21 ++++++ pkg/webhooks/utils/warning_test.go | 104 ++++++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 17 deletions(-) create mode 100644 pkg/webhooks/utils/warning.go create mode 100644 pkg/webhooks/utils/warning_test.go diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index 7749c22b0f..0219829e08 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -227,7 +227,7 @@ func (h *handlers) applyMutatePolicies(logger logr.Logger, request *admissionv1. go h.registerAdmissionReviewDurationMetricMutate(logger, string(request.Operation), mutateEngineResponses, admissionReviewLatencyDuration) go h.registerAdmissionRequestsMetricMutate(logger, string(request.Operation), mutateEngineResponses) - warnings := getWarningMessages(mutateEngineResponses) + warnings := webhookutils.GetWarningMessages(mutateEngineResponses) return mutatePatches, warnings, nil } @@ -376,7 +376,7 @@ func (h *handlers) handleVerifyImages(logger logr.Logger, request *admissionv1.A } } - warnings := getWarningMessages(engineResponses) + warnings := webhookutils.GetWarningMessages(engineResponses) return true, "", jsonutils.JoinPatches(patches...), warnings } diff --git a/pkg/webhooks/resource/utils.go b/pkg/webhooks/resource/utils.go index d94328ccdb..0bd0d1a23b 100644 --- a/pkg/webhooks/resource/utils.go +++ b/pkg/webhooks/resource/utils.go @@ -134,20 +134,6 @@ func getBlockedMessages(engineResponses []*response.EngineResponse) string { return msg } -func getWarningMessages(engineResponses []*response.EngineResponse) []string { - var warnings []string - for _, er := range engineResponses { - for _, rule := range er.PolicyResponse.Rules { - if rule.Status != response.RuleStatusPass { - msg := fmt.Sprintf("policy %s.%s: %s", er.Policy.GetName(), rule.Name, rule.Message) - warnings = append(warnings, msg) - } - } - } - - return warnings -} - func getAction(hasViolations bool, i int) string { action := "error" if hasViolations { diff --git a/pkg/webhooks/resource/validation.go b/pkg/webhooks/resource/validation.go index c7b703a690..7ac7f3f475 100644 --- a/pkg/webhooks/resource/validation.go +++ b/pkg/webhooks/resource/validation.go @@ -12,6 +12,7 @@ import ( "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/policyreport" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" + webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" admissionv1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -98,7 +99,7 @@ func (v *validationHandler) handleValidation( v.generateReportChangeRequests(request, engineResponses, policyContext, logger) v.generateMetrics(request, admissionRequestTimestamp, engineResponses, metricsConfig, logger) - warnings := getWarningMessages(engineResponses) + warnings := webhookutils.GetWarningMessages(engineResponses) return true, "", warnings } diff --git a/pkg/webhooks/utils/warning.go b/pkg/webhooks/utils/warning.go new file mode 100644 index 0000000000..3bc9f7e95f --- /dev/null +++ b/pkg/webhooks/utils/warning.go @@ -0,0 +1,21 @@ +package utils + +import ( + "fmt" + + "github.com/kyverno/kyverno/pkg/engine/response" +) + +func GetWarningMessages(engineResponses []*response.EngineResponse) []string { + var warnings []string + for _, er := range engineResponses { + for _, rule := range er.PolicyResponse.Rules { + if rule.Status != response.RuleStatusPass { + msg := fmt.Sprintf("policy %s.%s: %s", er.Policy.GetName(), rule.Name, rule.Message) + warnings = append(warnings, msg) + } + } + } + + return warnings +} diff --git a/pkg/webhooks/utils/warning_test.go b/pkg/webhooks/utils/warning_test.go new file mode 100644 index 0000000000..fe19951b4c --- /dev/null +++ b/pkg/webhooks/utils/warning_test.go @@ -0,0 +1,104 @@ +package utils + +import ( + "testing" + + v1 "github.com/kyverno/kyverno/api/kyverno/v1" + "github.com/kyverno/kyverno/pkg/engine/response" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestGetWarningMessages(t *testing.T) { + type args struct { + engineResponses []*response.EngineResponse + } + tests := []struct { + name string + args args + want []string + }{{ + name: "nil response", + args: args{nil}, + want: nil, + }, { + name: "enmpty response", + args: args{[]*response.EngineResponse{}}, + want: nil, + }, { + name: "warning", + args: args{[]*response.EngineResponse{ + { + Policy: &v1.ClusterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + PolicyResponse: response.PolicyResponse{ + Rules: []response.RuleResponse{ + { + Name: "rule", + Status: response.RuleStatusWarn, + Message: "message warn", + }, + }, + }, + }, + }}, + want: []string{ + "policy test.rule: message warn", + }, + }, { + name: "multiple rules", + args: args{[]*response.EngineResponse{ + { + Policy: &v1.ClusterPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + }, + PolicyResponse: response.PolicyResponse{ + Rules: []response.RuleResponse{ + { + Name: "rule-pass", + Status: response.RuleStatusPass, + Message: "message pass", + }, + { + Name: "rule-warn", + Status: response.RuleStatusWarn, + Message: "message warn", + }, + { + Name: "rule-fail", + Status: response.RuleStatusFail, + Message: "message fail", + }, + { + Name: "rule-error", + Status: response.RuleStatusError, + Message: "message error", + }, + { + Name: "rule-skip", + Status: response.RuleStatusSkip, + Message: "message skip", + }, + }, + }, + }, + }}, + want: []string{ + "policy test.rule-warn: message warn", + "policy test.rule-fail: message fail", + "policy test.rule-error: message error", + "policy test.rule-skip: message skip", + }, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := GetWarningMessages(tt.args.engineResponses) + assert.Equal(t, tt.want, got) + }) + } +}