From dd7ecff386c0ffd5535bc121b6460d86e60662bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Thu, 23 Mar 2023 13:58:52 +0100 Subject: [PATCH] refactor: remove more pointers from engine api (#6651) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: remove more pointers from engine api Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * debug Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché --- .../kubectl-kyverno/utils/common/common.go | 6 ++--- .../report/background/controller.go | 4 ++-- pkg/controllers/report/utils/events.go | 10 ++++----- pkg/controllers/report/utils/scanner.go | 2 +- pkg/engine/api/engine.go | 2 +- pkg/engine/api/engineresponse.go | 6 ++--- pkg/engine/api/imageverifymetadata.go | 8 +++---- pkg/engine/api/imageverifymetadata_test.go | 6 ++--- pkg/engine/background.go | 4 ++-- pkg/engine/engine.go | 2 +- pkg/engine/generation.go | 4 ++-- pkg/engine/imageVerify.go | 17 ++++++++------ pkg/engine/imageVerify_test.go | 6 +---- pkg/engine/mutation.go | 6 ++--- pkg/engine/validation.go | 2 +- pkg/event/events.go | 8 +++---- pkg/utils/annotations.go | 4 ++-- pkg/utils/annotations_test.go | 16 +++++++------- pkg/utils/engine/response.go | 4 ++-- pkg/utils/report/new.go | 2 +- pkg/utils/report/results.go | 4 ++-- .../resource/imageverification/handler.go | 6 ++--- pkg/webhooks/resource/mutation/mutation.go | 12 +++++----- .../resource/validation/validation.go | 12 +++++----- pkg/webhooks/resource/validation_test.go | 6 ++--- pkg/webhooks/utils/block.go | 4 ++-- pkg/webhooks/utils/block_test.go | 22 +++++++++---------- pkg/webhooks/utils/error.go | 2 +- pkg/webhooks/utils/event.go | 2 +- pkg/webhooks/utils/warning.go | 2 +- pkg/webhooks/utils/warning_test.go | 8 +++---- 31 files changed, 99 insertions(+), 100 deletions(-) diff --git a/cmd/cli/kubectl-kyverno/utils/common/common.go b/cmd/cli/kubectl-kyverno/utils/common/common.go index fbf6196806..5dd6f171f0 100644 --- a/cmd/cli/kubectl-kyverno/utils/common/common.go +++ b/cmd/cli/kubectl-kyverno/utils/common/common.go @@ -523,9 +523,9 @@ OuterLoop: } verifyImageResponse, _ := eng.VerifyAndPatchImages(context.TODO(), policyContext) - if verifyImageResponse != nil && !verifyImageResponse.IsEmpty() { - engineResponses = append(engineResponses, verifyImageResponse) - info = ProcessValidateEngineResponse(c.Policy, verifyImageResponse, resPath, c.Rc, c.PolicyReport, c.AuditWarn) + if !verifyImageResponse.IsEmpty() { + engineResponses = append(engineResponses, &verifyImageResponse) + info = ProcessValidateEngineResponse(c.Policy, &verifyImageResponse, resPath, c.Rc, c.PolicyReport, c.AuditWarn) } var policyHasGenerate bool diff --git a/pkg/controllers/report/background/controller.go b/pkg/controllers/report/background/controller.go index a3e859a94f..99c23d58fc 100644 --- a/pkg/controllers/report/background/controller.go +++ b/pkg/controllers/report/background/controller.go @@ -309,8 +309,8 @@ func (c *controller) reconcileReport( if result.Error != nil { return result.Error } else { - ruleResults = append(ruleResults, reportutils.EngineResponseToReportResults(result.EngineResponse)...) - utils.GenerateEvents(logger, c.eventGen, c.config, result.EngineResponse) + ruleResults = append(ruleResults, reportutils.EngineResponseToReportResults(*result.EngineResponse)...) + utils.GenerateEvents(logger, c.eventGen, c.config, *result.EngineResponse) } } } diff --git a/pkg/controllers/report/utils/events.go b/pkg/controllers/report/utils/events.go index a1e3aaf9ef..c454f2aa1e 100644 --- a/pkg/controllers/report/utils/events.go +++ b/pkg/controllers/report/utils/events.go @@ -7,7 +7,7 @@ import ( "github.com/kyverno/kyverno/pkg/event" ) -func GenerateEvents(logger logr.Logger, eventGen event.Interface, config config.Configuration, results ...*engineapi.EngineResponse) { +func GenerateEvents(logger logr.Logger, eventGen event.Interface, config config.Configuration, results ...engineapi.EngineResponse) { for _, result := range results { var eventInfos []event.Info eventInfos = append(eventInfos, generateFailEvents(logger, result)...) @@ -19,7 +19,7 @@ func GenerateEvents(logger logr.Logger, eventGen event.Interface, config config. } } -func generateSuccessEvents(log logr.Logger, ers ...*engineapi.EngineResponse) (eventInfos []event.Info) { +func generateSuccessEvents(log logr.Logger, ers ...engineapi.EngineResponse) (eventInfos []event.Info) { for _, er := range ers { logger := log.WithValues("policy", er.Policy.GetName(), "kind", er.Resource.GetKind(), "namespace", er.Resource.GetNamespace(), "name", er.Resource.GetName()) if !er.IsFailed() { @@ -31,7 +31,7 @@ func generateSuccessEvents(log logr.Logger, ers ...*engineapi.EngineResponse) (e return eventInfos } -func generateExceptionEvents(log logr.Logger, ers ...*engineapi.EngineResponse) (eventInfos []event.Info) { +func generateExceptionEvents(log logr.Logger, ers ...engineapi.EngineResponse) (eventInfos []event.Info) { for _, er := range ers { for i, ruleResp := range er.PolicyResponse.Rules { isException := ruleResp.Exception != nil @@ -43,14 +43,14 @@ func generateExceptionEvents(log logr.Logger, ers ...*engineapi.EngineResponse) return eventInfos } -func generateFailEvents(log logr.Logger, ers ...*engineapi.EngineResponse) (eventInfos []event.Info) { +func generateFailEvents(log logr.Logger, ers ...engineapi.EngineResponse) (eventInfos []event.Info) { for _, er := range ers { eventInfos = append(eventInfos, generateFailEventsPerEr(log, er)...) } return eventInfos } -func generateFailEventsPerEr(log logr.Logger, er *engineapi.EngineResponse) []event.Info { +func generateFailEventsPerEr(log logr.Logger, er engineapi.EngineResponse) []event.Info { var eventInfos []event.Info logger := log.WithValues( "policy", er.Policy.GetName(), diff --git a/pkg/controllers/report/utils/scanner.go b/pkg/controllers/report/utils/scanner.go index 312b12c9ef..10b0af8491 100644 --- a/pkg/controllers/report/utils/scanner.go +++ b/pkg/controllers/report/utils/scanner.go @@ -111,5 +111,5 @@ func (s *scanner) validateImages(ctx context.Context, resource unstructured.Unst if len(response.PolicyResponse.Rules) > 0 { s.logger.Info("validateImages", "policy", policy, "response", response) } - return response, nil + return &response, nil } diff --git a/pkg/engine/api/engine.go b/pkg/engine/api/engine.go index e6dd4eb9ee..18bb460117 100644 --- a/pkg/engine/api/engine.go +++ b/pkg/engine/api/engine.go @@ -28,7 +28,7 @@ type Engine interface { VerifyAndPatchImages( ctx context.Context, policyContext PolicyContext, - ) (*EngineResponse, *ImageVerificationMetadata) + ) (EngineResponse, ImageVerificationMetadata) // ApplyBackgroundChecks checks for validity of generate and mutateExisting rules on the resource // 1. validate variables to be substitute in the general ruleInfo (match,exclude,condition) diff --git a/pkg/engine/api/engineresponse.go b/pkg/engine/api/engineresponse.go index 4a9f2106ec..c8bf26d400 100644 --- a/pkg/engine/api/engineresponse.go +++ b/pkg/engine/api/engineresponse.go @@ -35,7 +35,7 @@ func Resource(policyContext PolicyContext) unstructured.Unstructured { func NewEngineResponseFromPolicyContext( policyContext PolicyContext, policyResponse *PolicyResponse, -) *EngineResponse { +) EngineResponse { return NewEngineResponse( Resource(policyContext), policyContext.Policy(), @@ -49,8 +49,8 @@ func NewEngineResponse( policy kyvernov1.PolicyInterface, namespaceLabels map[string]string, policyResponse *PolicyResponse, -) *EngineResponse { - response := &EngineResponse{ +) EngineResponse { + response := EngineResponse{ Resource: resource, Policy: policy, NamespaceLabels: namespaceLabels, diff --git a/pkg/engine/api/imageverifymetadata.go b/pkg/engine/api/imageverifymetadata.go index 8e9f5ecf3f..6154ecef51 100644 --- a/pkg/engine/api/imageverifymetadata.go +++ b/pkg/engine/api/imageverifymetadata.go @@ -68,11 +68,9 @@ func (ivm *ImageVerificationMetadata) Patches(hasAnnotations bool, log logr.Logg } } -func (ivm *ImageVerificationMetadata) Merge(other *ImageVerificationMetadata) { - if other != nil { - for k, v := range other.Data { - ivm.Add(k, v) - } +func (ivm *ImageVerificationMetadata) Merge(other ImageVerificationMetadata) { + for k, v := range other.Data { + ivm.Add(k, v) } } diff --git a/pkg/engine/api/imageverifymetadata_test.go b/pkg/engine/api/imageverifymetadata_test.go index bcd47a50a3..e9575bdbcf 100644 --- a/pkg/engine/api/imageverifymetadata_test.go +++ b/pkg/engine/api/imageverifymetadata_test.go @@ -218,7 +218,7 @@ func TestImageVerificationMetadata_Merge(t *testing.T) { Data map[string]bool } type args struct { - other *ImageVerificationMetadata + other ImageVerificationMetadata } tests := []struct { name string @@ -234,7 +234,7 @@ func TestImageVerificationMetadata_Merge(t *testing.T) { }, }, args: args{ - other: &ImageVerificationMetadata{ + other: ImageVerificationMetadata{ Data: map[string]bool{ "test": false, }, @@ -252,7 +252,7 @@ func TestImageVerificationMetadata_Merge(t *testing.T) { }, }, args: args{ - other: &ImageVerificationMetadata{ + other: ImageVerificationMetadata{ Data: map[string]bool{ "test2": false, }, diff --git a/pkg/engine/background.go b/pkg/engine/background.go index 68614c3c57..7795d77415 100644 --- a/pkg/engine/background.go +++ b/pkg/engine/background.go @@ -47,7 +47,7 @@ func (e *engine) filterRules( if e.configuration.ToFilter(kind, namespace, name) { logger.Info("resource excluded") - return *resp + return resp } applyRules := policy.GetSpec().GetApplyRules() @@ -61,7 +61,7 @@ func (e *engine) filterRules( } } - return *resp + return resp } func (e *engine) filterRule( diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index bc408c2b63..5a30185d09 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -57,7 +57,7 @@ func (e *engine) Mutate( func (e *engine) VerifyAndPatchImages( ctx context.Context, policyContext engineapi.PolicyContext, -) (*engineapi.EngineResponse, *engineapi.ImageVerificationMetadata) { +) (engineapi.EngineResponse, engineapi.ImageVerificationMetadata) { logger := internal.LoggerWithPolicyContext(logging.WithName("engine.verify"), policyContext) return e.verifyAndPatchImages(ctx, logger, policyContext) } diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index ce997988d1..d510885d96 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -41,7 +41,7 @@ func (e *engine) filterGenerateRules( } if e.configuration.ToFilter(kind, namespace, name) { logger.Info("resource excluded") - return *resp + return resp } for _, rule := range autogen.ComputeRules(policyContext.Policy()) { logger := internal.LoggerWithRule(logger, rule) @@ -49,5 +49,5 @@ func (e *engine) filterGenerateRules( resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *ruleResp) } } - return *resp + return resp } diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index c316065a4a..c199cf4799 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -23,21 +23,24 @@ func (e *engine) verifyAndPatchImages( ctx context.Context, logger logr.Logger, policyContext engineapi.PolicyContext, -) (*engineapi.EngineResponse, *engineapi.ImageVerificationMetadata) { +) (engineapi.EngineResponse, engineapi.ImageVerificationMetadata) { policy := policyContext.Policy() resp := engineapi.NewEngineResponseFromPolicyContext(policyContext, nil) startTime := time.Now() + defer func() { - internal.BuildResponse(policyContext, resp, startTime) - logger.V(4).Info("processed image verification rules", + logger.V(4).Info( + "processed image verification rules", "time", resp.PolicyResponse.Stats.ProcessingTime.String(), - "applied", resp.PolicyResponse.Stats.RulesAppliedCount, "successful", resp.IsSuccessful()) + "applied", resp.PolicyResponse.Stats.RulesAppliedCount, + "successful", resp.IsSuccessful(), + ) }() policyContext.JSONContext().Checkpoint() defer policyContext.JSONContext().Restore() - ivm := &engineapi.ImageVerificationMetadata{} + ivm := engineapi.ImageVerificationMetadata{} rules := autogen.ComputeRules(policyContext.Policy()) applyRules := policy.GetSpec().GetApplyRules() @@ -49,7 +52,7 @@ func (e *engine) verifyAndPatchImages( "pkg/engine", fmt.Sprintf("RULE %s", rule.Name), func(ctx context.Context, span trace.Span) { - e.doVerifyAndPatch(ctx, logger, policyContext, rule, resp, ivm) + e.doVerifyAndPatch(ctx, logger, policyContext, rule, &resp, &ivm) }, ) @@ -57,7 +60,7 @@ func (e *engine) verifyAndPatchImages( break } } - + internal.BuildResponse(policyContext, &resp, startTime) return resp, ivm } diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index cb2dd00d8f..f115a4a4ea 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -167,7 +167,7 @@ func testVerifyAndPatchImages( cmResolver engineapi.ConfigmapResolver, pContext engineapi.PolicyContext, cfg config.Configuration, -) (*engineapi.EngineResponse, *engineapi.ImageVerificationMetadata) { +) (engineapi.EngineResponse, engineapi.ImageVerificationMetadata) { e := NewEngine( cfg, nil, @@ -769,11 +769,9 @@ func Test_MarkImageVerified(t *testing.T) { assert.NilError(t, err) engineResponse, verifiedImages := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) - assert.Assert(t, engineResponse != nil) assert.Equal(t, len(engineResponse.PolicyResponse.Rules), 1) assert.Equal(t, engineResponse.PolicyResponse.Rules[0].Status, engineapi.RuleStatusPass) - assert.Assert(t, verifiedImages != nil) assert.Assert(t, verifiedImages.Data != nil) assert.Equal(t, len(verifiedImages.Data), 1) assert.Equal(t, verifiedImages.IsVerified(image), true) @@ -862,11 +860,9 @@ func Test_ParsePEMDelimited(t *testing.T) { assert.NilError(t, err) engineResponse, verifiedImages := testVerifyAndPatchImages(context.TODO(), registryclient.NewOrDie(), nil, policyContext, cfg) - assert.Assert(t, engineResponse != nil) assert.Equal(t, len(engineResponse.PolicyResponse.Rules), 1) assert.Equal(t, engineResponse.PolicyResponse.Rules[0].Status, engineapi.RuleStatusPass) - assert.Assert(t, verifiedImages != nil) assert.Assert(t, verifiedImages.Data != nil) assert.Equal(t, len(verifiedImages.Data), 1) assert.Equal(t, verifiedImages.IsVerified(image), true) diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index 6002da31a1..0982136c86 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -33,8 +33,7 @@ func (e *engine) mutate( logger.V(4).Info("start mutate policy processing", "startTime", startTime) - startMutateResultResponse(resp, policy, matchedResource) - defer endMutateResultResponse(logger, resp, startTime) + startMutateResultResponse(&resp, policy, matchedResource) policyContext.JSONContext().Checkpoint() defer policyContext.JSONContext().Restore() @@ -167,7 +166,8 @@ func (e *engine) mutate( } resp.PatchedResource = matchedResource - return *resp + endMutateResultResponse(logger, &resp, startTime) + return resp } func mutateResource(rule *kyvernov1.Rule, ctx engineapi.PolicyContext, resource unstructured.Unstructured, logger logr.Logger) *mutate.Response { diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index fcee74ee2e..47d9aaaf79 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -40,7 +40,7 @@ func (e *engine) validate( defer logger.V(4).Info("finished policy processing", "processingTime", policyResponse.Stats.ProcessingTime.String(), "validationRulesApplied", policyResponse.Stats.RulesAppliedCount) engineResponse := engineapi.NewEngineResponseFromPolicyContext(policyContext, nil) engineResponse.PolicyResponse = *policyResponse - return *internal.BuildResponse(policyContext, engineResponse, startTime) + return *internal.BuildResponse(policyContext, &engineResponse, startTime) } func (e *engine) validateResource( diff --git a/pkg/event/events.go b/pkg/event/events.go index e6ba59f720..5b7ebc37d1 100644 --- a/pkg/event/events.go +++ b/pkg/event/events.go @@ -9,7 +9,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func NewPolicyFailEvent(source Source, reason Reason, engineResponse *engineapi.EngineResponse, ruleResp *engineapi.RuleResponse, blocked bool) Info { +func NewPolicyFailEvent(source Source, reason Reason, engineResponse engineapi.EngineResponse, ruleResp *engineapi.RuleResponse, blocked bool) Info { return Info{ Kind: getPolicyKind(engineResponse.Policy), Name: engineResponse.Policy.GetName(), @@ -47,7 +47,7 @@ func getPolicyKind(policy kyvernov1.PolicyInterface) string { return "ClusterPolicy" } -func NewPolicyAppliedEvent(source Source, engineResponse *engineapi.EngineResponse) Info { +func NewPolicyAppliedEvent(source Source, engineResponse engineapi.EngineResponse) Info { resource := engineResponse.Resource var bldr strings.Builder defer bldr.Reset() @@ -68,7 +68,7 @@ func NewPolicyAppliedEvent(source Source, engineResponse *engineapi.EngineRespon } } -func NewResourceViolationEvent(source Source, reason Reason, engineResponse *engineapi.EngineResponse, ruleResp *engineapi.RuleResponse) Info { +func NewResourceViolationEvent(source Source, reason Reason, engineResponse engineapi.EngineResponse, ruleResp *engineapi.RuleResponse) Info { var bldr strings.Builder defer bldr.Reset() @@ -123,7 +123,7 @@ func NewBackgroundSuccessEvent(policy, rule string, source Source, r *unstructur return events } -func NewPolicyExceptionEvents(engineResponse *engineapi.EngineResponse, ruleResp *engineapi.RuleResponse) []Info { +func NewPolicyExceptionEvents(engineResponse engineapi.EngineResponse, ruleResp *engineapi.RuleResponse) []Info { exceptionName, exceptionNamespace := ruleResp.Exception.GetName(), ruleResp.Exception.GetNamespace() policyMessage := fmt.Sprintf("resource %s was skipped from rule %s due to policy exception %s/%s", resourceKey(engineResponse.PatchedResource), ruleResp.Name, exceptionNamespace, exceptionName) var exceptionMessage string diff --git a/pkg/utils/annotations.go b/pkg/utils/annotations.go index 874b3e77af..6c9d0fb64b 100644 --- a/pkg/utils/annotations.go +++ b/pkg/utils/annotations.go @@ -33,7 +33,7 @@ var OperationToPastTense = map[string]string{ "test": "tested", } -func GenerateAnnotationPatches(engineResponses []*engineapi.EngineResponse, log logr.Logger) [][]byte { +func GenerateAnnotationPatches(engineResponses []engineapi.EngineResponse, log logr.Logger) [][]byte { var annotations map[string]string var patchBytes [][]byte for _, er := range engineResponses { @@ -91,7 +91,7 @@ func GenerateAnnotationPatches(engineResponses []*engineapi.EngineResponse, log return patchBytes } -func annotationFromEngineResponses(engineResponses []*engineapi.EngineResponse, log logr.Logger) []byte { +func annotationFromEngineResponses(engineResponses []engineapi.EngineResponse, log logr.Logger) []byte { annotationContent := make(map[string]string) for _, engineResponse := range engineResponses { if !engineResponse.IsSuccessful() { diff --git a/pkg/utils/annotations_test.go b/pkg/utils/annotations_test.go index 95518afb76..6e3828f1ed 100644 --- a/pkg/utils/annotations_test.go +++ b/pkg/utils/annotations_test.go @@ -28,7 +28,7 @@ func newPolicyResponse(rule string, patchesStr []string, status engineapi.RuleSt } } -func newEngineResponse(policy, rule string, patchesStr []string, status engineapi.RuleStatus, annotation map[string]interface{}) *engineapi.EngineResponse { +func newEngineResponse(policy, rule string, patchesStr []string, status engineapi.RuleStatus, annotation map[string]interface{}) engineapi.EngineResponse { p := &kyvernov1.ClusterPolicy{ ObjectMeta: metav1.ObjectMeta{ Name: policy, @@ -49,7 +49,7 @@ func newEngineResponse(policy, rule string, patchesStr []string, status engineap func Test_empty_annotation(t *testing.T) { patchStr := `{ "op": "replace", "path": "/spec/containers/0/imagePullPolicy", "value": "IfNotPresent" }` engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []string{patchStr}, engineapi.RuleStatusPass, nil) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) expectedPatches := `{"path":"/metadata/annotations","op":"add","value":{"policies.kyverno.io/last-applied-patches":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}}` assert.Equal(t, string(annPatches[0]), expectedPatches) } @@ -60,7 +60,7 @@ func Test_exist_annotation(t *testing.T) { } patchStr := `{ "op": "replace", "path": "/spec/containers/0/imagePullPolicy", "value": "IfNotPresent" }` engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []string{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) expectedPatches := `{"path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","op":"add","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` assert.Equal(t, string(annPatches[0]), expectedPatches) } @@ -71,7 +71,7 @@ func Test_exist_kyverno_annotation(t *testing.T) { } patchStr := `{ "op": "replace", "path": "/spec/containers/0/imagePullPolicy", "value": "IfNotPresent" }` engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []string{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) expectedPatches := `{"path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","op":"add","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` assert.Equal(t, string(annPatches[0]), expectedPatches) } @@ -81,10 +81,10 @@ func Test_annotation_nil_patch(t *testing.T) { "policies.kyverno.patches": "old-annotation", } engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", nil, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) assert.Assert(t, annPatches == nil) engineResponseNew := newEngineResponse("mutate-container", "default-imagepullpolicy", []string{""}, engineapi.RuleStatusPass, annotation) - annPatchesNew := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponseNew}, logging.GlobalLogger()) + annPatchesNew := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponseNew}, logging.GlobalLogger()) assert.Assert(t, annPatchesNew == nil) } @@ -93,7 +93,7 @@ func Test_annotation_failed_Patch(t *testing.T) { "policies.kyverno.patches": "old-annotation", } engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", nil, engineapi.RuleStatusFail, annotation) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) assert.Assert(t, annPatches == nil) } @@ -103,7 +103,7 @@ func Test_exist_patches(t *testing.T) { } patchStr := `{ "op": "replace", "path": "/spec/containers/0/imagePullPolicy", "value": "IfNotPresent" }` engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []string{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]*engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) + annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) expectedPatches1 := `{"path":"/metadata/annotations/policies.kyverno.io~1patches","op":"remove"}` expectedPatches2 := `{"path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","op":"add","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` assert.Equal(t, string(annPatches[0]), expectedPatches1) diff --git a/pkg/utils/engine/response.go b/pkg/utils/engine/response.go index f83c909650..fddf526ceb 100644 --- a/pkg/utils/engine/response.go +++ b/pkg/utils/engine/response.go @@ -6,7 +6,7 @@ import ( ) // IsResponseSuccessful return true if all responses are successful -func IsResponseSuccessful(engineReponses []*engineapi.EngineResponse) bool { +func IsResponseSuccessful(engineReponses []engineapi.EngineResponse) bool { for _, er := range engineReponses { if !er.IsSuccessful() { return false @@ -18,7 +18,7 @@ func IsResponseSuccessful(engineReponses []*engineapi.EngineResponse) bool { // BlockRequest returns true when: // 1. a policy fails (i.e. creates a violation) and validationFailureAction is set to 'enforce' // 2. a policy has a processing error and failurePolicy is set to 'Fail` -func BlockRequest(er *engineapi.EngineResponse, failurePolicy kyvernov1.FailurePolicyType) bool { +func BlockRequest(er engineapi.EngineResponse, failurePolicy kyvernov1.FailurePolicyType) bool { if er.IsFailed() && er.GetValidationFailureAction().Enforce() { return true } diff --git a/pkg/utils/report/new.go b/pkg/utils/report/new.go index af4bd69edb..b5bd2859e6 100644 --- a/pkg/utils/report/new.go +++ b/pkg/utils/report/new.go @@ -27,7 +27,7 @@ func NewAdmissionReport(namespace, name string, gvr schema.GroupVersionResource, return report } -func BuildAdmissionReport(resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, responses ...*engineapi.EngineResponse) kyvernov1alpha2.ReportInterface { +func BuildAdmissionReport(resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, responses ...engineapi.EngineResponse) kyvernov1alpha2.ReportInterface { report := NewAdmissionReport(resource.GetNamespace(), string(request.UID), schema.GroupVersionResource(request.Resource), resource) SetResponses(report, responses...) return report diff --git a/pkg/utils/report/results.go b/pkg/utils/report/results.go index dd5e586387..83cec00177 100644 --- a/pkg/utils/report/results.go +++ b/pkg/utils/report/results.go @@ -81,7 +81,7 @@ func severityFromString(severity string) policyreportv1alpha2.PolicySeverity { return "" } -func EngineResponseToReportResults(response *engineapi.EngineResponse) []policyreportv1alpha2.PolicyReportResult { +func EngineResponseToReportResults(response engineapi.EngineResponse) []policyreportv1alpha2.PolicyReportResult { key, _ := cache.MetaNamespaceKeyFunc(response.Policy) var results []policyreportv1alpha2.PolicyReportResult for _, ruleResult := range response.PolicyResponse.Rules { @@ -153,7 +153,7 @@ func SetResults(report kyvernov1alpha2.ReportInterface, results ...policyreportv report.SetSummary(CalculateSummary(results)) } -func SetResponses(report kyvernov1alpha2.ReportInterface, engineResponses ...*engineapi.EngineResponse) { +func SetResponses(report kyvernov1alpha2.ReportInterface, engineResponses ...engineapi.EngineResponse) { var ruleResults []policyreportv1alpha2.PolicyReportResult for _, result := range engineResponses { SetPolicyLabel(report, result.Policy) diff --git a/pkg/webhooks/resource/imageverification/handler.go b/pkg/webhooks/resource/imageverification/handler.go index dc477f5ee3..b9d3165f3e 100644 --- a/pkg/webhooks/resource/imageverification/handler.go +++ b/pkg/webhooks/resource/imageverification/handler.go @@ -79,9 +79,9 @@ func (h *imageVerificationHandler) handleVerifyImages( if len(policies) == 0 { return true, "", nil, nil } - var engineResponses []*engineapi.EngineResponse + var engineResponses []engineapi.EngineResponse var patches [][]byte - verifiedImageData := &engineapi.ImageVerificationMetadata{} + verifiedImageData := engineapi.ImageVerificationMetadata{} for _, policy := range policies { tracing.ChildSpan( ctx, @@ -148,7 +148,7 @@ func (v *imageVerificationHandler) handleAudit( resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, namespaceLabels map[string]string, - engineResponses ...*engineapi.EngineResponse, + engineResponses ...engineapi.EngineResponse, ) { createReport := v.admissionReports if admissionutils.IsDryRun(request) { diff --git a/pkg/webhooks/resource/mutation/mutation.go b/pkg/webhooks/resource/mutation/mutation.go index 6dd8bd3aa4..f7acba1b1d 100644 --- a/pkg/webhooks/resource/mutation/mutation.go +++ b/pkg/webhooks/resource/mutation/mutation.go @@ -79,7 +79,7 @@ func (v *mutationHandler) applyMutations( request *admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, -) ([]byte, []*engineapi.EngineResponse, error) { +) ([]byte, []engineapi.EngineResponse, error) { if len(policies) == 0 { return nil, nil, nil } @@ -89,7 +89,7 @@ func (v *mutationHandler) applyMutations( } var patches [][]byte - var engineResponses []*engineapi.EngineResponse + var engineResponses []engineapi.EngineResponse for _, policy := range policies { spec := policy.GetSpec() @@ -117,8 +117,10 @@ func (v *mutationHandler) applyMutations( } } - policyContext = currentContext.WithNewResource(engineResponse.PatchedResource) - engineResponses = append(engineResponses, engineResponse) + if engineResponse != nil { + policyContext = currentContext.WithNewResource(engineResponse.PatchedResource) + engineResponses = append(engineResponses, *engineResponse) + } // registering the kyverno_policy_results_total metric concurrently go webhookutils.RegisterPolicyResultsMetricMutation(context.TODO(), v.log, v.metrics, string(request.Operation), policy, *engineResponse) @@ -171,7 +173,7 @@ func (h *mutationHandler) applyMutation(ctx context.Context, request *admissionv return &engineResponse, policyPatches, nil } -func logMutationResponse(patches [][]byte, engineResponses []*engineapi.EngineResponse, logger logr.Logger) { +func logMutationResponse(patches [][]byte, engineResponses []engineapi.EngineResponse, logger logr.Logger) { if len(patches) != 0 { logger.V(4).Info("created patches", "count", len(patches)) } diff --git a/pkg/webhooks/resource/validation/validation.go b/pkg/webhooks/resource/validation/validation.go index 1bf55b2318..aa758f0c12 100644 --- a/pkg/webhooks/resource/validation/validation.go +++ b/pkg/webhooks/resource/validation/validation.go @@ -89,7 +89,7 @@ func (v *validationHandler) HandleValidation( return true, "", nil } - var engineResponses []*engineapi.EngineResponse + var engineResponses []engineapi.EngineResponse failurePolicy := kyvernov1.Ignore for _, policy := range policies { tracing.ChildSpan( @@ -112,7 +112,7 @@ func (v *validationHandler) HandleValidation( go webhookutils.RegisterPolicyResultsMetricValidation(ctx, logger, v.metrics, string(request.Operation), policyContext.Policy(), engineResponse) go webhookutils.RegisterPolicyExecutionDurationMetricValidate(ctx, logger, v.metrics, string(request.Operation), policyContext.Policy(), engineResponse) - engineResponses = append(engineResponses, &engineResponse) + engineResponses = append(engineResponses, engineResponse) if !engineResponse.IsSuccessful() { logger.V(2).Info("validation failed", "action", policy.GetSpec().ValidationFailureAction, "policy", policy.GetName(), "failed rules", engineResponse.GetFailedRules()) return @@ -147,14 +147,14 @@ func (v *validationHandler) buildAuditResponses( resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, namespaceLabels map[string]string, -) ([]*engineapi.EngineResponse, error) { +) ([]engineapi.EngineResponse, error) { gvr := schema.GroupVersionResource(request.Resource) policies := v.pCache.GetPolicies(policycache.ValidateAudit, gvr, request.SubResource, request.Namespace) policyContext, err := v.pcBuilder.Build(request) if err != nil { return nil, err } - var responses []*engineapi.EngineResponse + var responses []engineapi.EngineResponse for _, policy := range policies { tracing.ChildSpan( ctx, @@ -163,7 +163,7 @@ func (v *validationHandler) buildAuditResponses( func(ctx context.Context, span trace.Span) { policyContext := policyContext.WithPolicy(policy).WithNamespaceLabels(namespaceLabels) response := v.engine.Validate(ctx, policyContext) - responses = append(responses, &response) + responses = append(responses, response) go webhookutils.RegisterPolicyResultsMetricValidation(ctx, v.log, v.metrics, string(request.Operation), policyContext.Policy(), response) go webhookutils.RegisterPolicyExecutionDurationMetricValidate(ctx, v.log, v.metrics, string(request.Operation), policyContext.Policy(), response) }, @@ -177,7 +177,7 @@ func (v *validationHandler) handleAudit( resource unstructured.Unstructured, request *admissionv1.AdmissionRequest, namespaceLabels map[string]string, - engineResponses ...*engineapi.EngineResponse, + engineResponses ...engineapi.EngineResponse, ) { createReport := v.admissionReports if admissionutils.IsDryRun(request) { diff --git a/pkg/webhooks/resource/validation_test.go b/pkg/webhooks/resource/validation_test.go index b8d4da0f3c..5e735633f3 100644 --- a/pkg/webhooks/resource/validation_test.go +++ b/pkg/webhooks/resource/validation_test.go @@ -1077,7 +1077,7 @@ func TestValidate_failure_action_overrides(t *testing.T) { } failurePolicy := kyvernov1.Fail - blocked := webhookutils.BlockRequest([]*engineapi.EngineResponse{&er}, failurePolicy, log.WithName("WebhookServer")) + blocked := webhookutils.BlockRequest([]engineapi.EngineResponse{er}, failurePolicy, log.WithName("WebhookServer")) assert.Assert(t, tc.blocked == blocked) }) } @@ -1143,7 +1143,7 @@ func Test_RuleSelector(t *testing.T) { assert.Assert(t, resp.PolicyResponse.Stats.RulesErrorCount == 0) log := log.WithName("Test_RuleSelector") - blocked := webhookutils.BlockRequest([]*engineapi.EngineResponse{&resp}, kyvernov1.Fail, log) + blocked := webhookutils.BlockRequest([]engineapi.EngineResponse{resp}, kyvernov1.Fail, log) assert.Assert(t, blocked == true) applyOne := kyvernov1.ApplyOne @@ -1155,6 +1155,6 @@ func Test_RuleSelector(t *testing.T) { assert.Assert(t, resp.PolicyResponse.Stats.RulesAppliedCount == 1) assert.Assert(t, resp.PolicyResponse.Stats.RulesErrorCount == 0) - blocked = webhookutils.BlockRequest([]*engineapi.EngineResponse{&resp}, kyvernov1.Fail, log) + blocked = webhookutils.BlockRequest([]engineapi.EngineResponse{resp}, kyvernov1.Fail, log) assert.Assert(t, blocked == false) } diff --git a/pkg/webhooks/utils/block.go b/pkg/webhooks/utils/block.go index 672ef69288..ed0c724712 100644 --- a/pkg/webhooks/utils/block.go +++ b/pkg/webhooks/utils/block.go @@ -23,7 +23,7 @@ func getAction(hasViolations bool, i int) string { // returns true -> if there is even one policy that blocks resource request // returns false -> if all the policies are meant to report only, we dont block resource request -func BlockRequest(engineResponses []*engineapi.EngineResponse, failurePolicy kyvernov1.FailurePolicyType, log logr.Logger) bool { +func BlockRequest(engineResponses []engineapi.EngineResponse, failurePolicy kyvernov1.FailurePolicyType, log logr.Logger) bool { for _, er := range engineResponses { if engineutils.BlockRequest(er, failurePolicy) { log.V(2).Info("blocking admission request", "policy", er.Policy.GetName()) @@ -35,7 +35,7 @@ func BlockRequest(engineResponses []*engineapi.EngineResponse, failurePolicy kyv } // GetBlockedMessages gets the error messages for rules with error or fail status -func GetBlockedMessages(engineResponses []*engineapi.EngineResponse) string { +func GetBlockedMessages(engineResponses []engineapi.EngineResponse) string { if len(engineResponses) == 0 { return "" } diff --git a/pkg/webhooks/utils/block_test.go b/pkg/webhooks/utils/block_test.go index 3e68131719..0f33f5318c 100644 --- a/pkg/webhooks/utils/block_test.go +++ b/pkg/webhooks/utils/block_test.go @@ -72,7 +72,7 @@ func TestBlockRequest(t *testing.T) { }, } type args struct { - engineResponses []*engineapi.EngineResponse + engineResponses []engineapi.EngineResponse failurePolicy kyvernov1.FailurePolicyType log logr.Logger } @@ -83,7 +83,7 @@ func TestBlockRequest(t *testing.T) { }{{ name: "failure - enforce", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -101,7 +101,7 @@ func TestBlockRequest(t *testing.T) { }, { name: "failure - audit", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -119,7 +119,7 @@ func TestBlockRequest(t *testing.T) { }, { name: "error - fail", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -137,7 +137,7 @@ func TestBlockRequest(t *testing.T) { }, { name: "error - ignore", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -155,7 +155,7 @@ func TestBlockRequest(t *testing.T) { }, { name: "warning - ignore", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -173,7 +173,7 @@ func TestBlockRequest(t *testing.T) { }, { name: "warning - fail", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, auditPolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -216,7 +216,7 @@ func TestGetBlockedMessages(t *testing.T) { }, } type args struct { - engineResponses []*engineapi.EngineResponse + engineResponses []engineapi.EngineResponse } tests := []struct { name string @@ -225,7 +225,7 @@ func TestGetBlockedMessages(t *testing.T) { }{{ name: "failure - enforce", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -241,7 +241,7 @@ func TestGetBlockedMessages(t *testing.T) { }, { name: "error - enforce", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { @@ -257,7 +257,7 @@ func TestGetBlockedMessages(t *testing.T) { }, { name: "error and failure - enforce", args: args{ - engineResponses: []*engineapi.EngineResponse{ + engineResponses: []engineapi.EngineResponse{ engineapi.NewEngineResponse(resource, enforcePolicy, nil, &engineapi.PolicyResponse{ Rules: []engineapi.RuleResponse{ { diff --git a/pkg/webhooks/utils/error.go b/pkg/webhooks/utils/error.go index 0c60229856..4c4f6b6cb4 100644 --- a/pkg/webhooks/utils/error.go +++ b/pkg/webhooks/utils/error.go @@ -7,7 +7,7 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" ) -func GetErrorMsg(engineReponses []*engineapi.EngineResponse) string { +func GetErrorMsg(engineReponses []engineapi.EngineResponse) string { var str []string var resourceInfo string for _, er := range engineReponses { diff --git a/pkg/webhooks/utils/event.go b/pkg/webhooks/utils/event.go index 62a8590b4b..f37c52005f 100644 --- a/pkg/webhooks/utils/event.go +++ b/pkg/webhooks/utils/event.go @@ -6,7 +6,7 @@ import ( ) // GenerateEvents generates event info for the engine responses -func GenerateEvents(engineResponses []*engineapi.EngineResponse, blocked bool) []event.Info { +func GenerateEvents(engineResponses []engineapi.EngineResponse, blocked bool) []event.Info { var events []event.Info // - Some/All policies fail or error // - report failure events on policy diff --git a/pkg/webhooks/utils/warning.go b/pkg/webhooks/utils/warning.go index 44e527358d..5390b871ce 100644 --- a/pkg/webhooks/utils/warning.go +++ b/pkg/webhooks/utils/warning.go @@ -6,7 +6,7 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" ) -func GetWarningMessages(engineResponses []*engineapi.EngineResponse) []string { +func GetWarningMessages(engineResponses []engineapi.EngineResponse) []string { var warnings []string for _, er := range engineResponses { for _, rule := range er.PolicyResponse.Rules { diff --git a/pkg/webhooks/utils/warning_test.go b/pkg/webhooks/utils/warning_test.go index 5c63e027a1..d5c6e570b1 100644 --- a/pkg/webhooks/utils/warning_test.go +++ b/pkg/webhooks/utils/warning_test.go @@ -11,7 +11,7 @@ import ( func TestGetWarningMessages(t *testing.T) { type args struct { - engineResponses []*engineapi.EngineResponse + engineResponses []engineapi.EngineResponse } tests := []struct { name string @@ -23,11 +23,11 @@ func TestGetWarningMessages(t *testing.T) { want: nil, }, { name: "enmpty response", - args: args{[]*engineapi.EngineResponse{}}, + args: args{[]engineapi.EngineResponse{}}, want: nil, }, { name: "warning", - args: args{[]*engineapi.EngineResponse{ + args: args{[]engineapi.EngineResponse{ { Policy: &v1.ClusterPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -50,7 +50,7 @@ func TestGetWarningMessages(t *testing.T) { }, }, { name: "multiple rules", - args: args{[]*engineapi.EngineResponse{ + args: args{[]engineapi.EngineResponse{ { Policy: &v1.ClusterPolicy{ ObjectMeta: metav1.ObjectMeta{