From 945cb1a8099a93289408795887f9ec86fa708d00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 7 Jun 2023 06:51:02 +0200 Subject: [PATCH] chore: remove last-applied-patches annotation (#7438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- pkg/background/mutate/mutate.go | 52 +------- pkg/utils/annotations.go | 141 --------------------- pkg/utils/annotations_test.go | 119 ----------------- pkg/webhooks/resource/mutation/mutation.go | 6 - 4 files changed, 1 insertion(+), 317 deletions(-) delete mode 100644 pkg/utils/annotations_test.go diff --git a/pkg/background/mutate/mutate.go b/pkg/background/mutate/mutate.go index d533826edd..4cfb3b3459 100644 --- a/pkg/background/mutate/mutate.go +++ b/pkg/background/mutate/mutate.go @@ -14,11 +14,9 @@ import ( engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/jmespath" "github.com/kyverno/kyverno/pkg/event" - "github.com/kyverno/kyverno/pkg/utils" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" engineutils "github.com/kyverno/kyverno/pkg/utils/engine" "go.uber.org/multierr" - yamlv2 "gopkg.in/yaml.v2" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" @@ -165,13 +163,7 @@ func (c *mutateExistingController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) e c.report(err, ur.Spec.Policy, rule.Name, patched) case engineapi.RuleStatusPass: - - patchedNew, err := addAnnotation(policy, patched, r) - if err != nil { - logger.Error(err, "failed to apply patches") - errs = append(errs, err) - } - + patchedNew := patched if patchedNew == nil { logger.Error(ErrEmptyPatch, "", "rule", r.Name(), "message", r.Message()) errs = append(errs, err) @@ -254,45 +246,3 @@ func updateURStatus(statusControl common.StatusControlInterface, ur kyvernov1bet } return nil } - -func addAnnotation(policy kyvernov1.PolicyInterface, patched *unstructured.Unstructured, r engineapi.RuleResponse) (patchedNew *unstructured.Unstructured, err error) { - if patched == nil { - return - } - - patchedNew = patched - var rulePatches []utils.RulePatch - - for _, patch := range r.DeprecatedPatches() { - rulePatches = append(rulePatches, utils.RulePatch{ - RuleName: r.Name(), - Op: patch.Operation, - Path: patch.Path, - }) - } - - annotationContent := make(map[string]string) - policyName := policy.GetName() - if policy.GetNamespace() != "" { - policyName = policy.GetNamespace() + "/" + policy.GetName() - } - - for _, rulePatch := range rulePatches { - annotationContent[rulePatch.RuleName+"."+policyName+".kyverno.io"] = utils.OperationToPastTense[rulePatch.Op] + " " + rulePatch.Path - } - - if len(annotationContent) == 0 { - return - } - - result, _ := yamlv2.Marshal(annotationContent) - - ann := patchedNew.GetAnnotations() - if ann == nil { - ann = make(map[string]string) - } - ann[utils.PolicyAnnotation] = string(result) - patchedNew.SetAnnotations(ann) - - return -} diff --git a/pkg/utils/annotations.go b/pkg/utils/annotations.go index 4f19ff0033..be5812eb09 100644 --- a/pkg/utils/annotations.go +++ b/pkg/utils/annotations.go @@ -1,147 +1,6 @@ package utils -import ( - "strings" - - "github.com/go-logr/logr" - engineapi "github.com/kyverno/kyverno/pkg/engine/api" - "github.com/mattbaird/jsonpatch" - yamlv2 "gopkg.in/yaml.v2" -) - const ( - PolicyAnnotation = "policies.kyverno.io/last-applied-patches" - policyAnnotation = "policies.kyverno.io~1last-applied-patches" - oldAnnotation = "policies.kyverno.io~1patches" ManagedByLabel = "webhook.kyverno.io/managed-by" KyvernoComponentLabel = "app.kubernetes.io/component" ) - -type RulePatch struct { - RuleName string `json:"rulename"` - Op string `json:"op"` - Path string `json:"path"` -} - -var OperationToPastTense = map[string]string{ - "add": "added", - "remove": "removed", - "replace": "replaced", - "move": "moved", - "copy": "copied", - "test": "tested", -} - -func GenerateAnnotationPatches(engineResponses []engineapi.EngineResponse, log logr.Logger) []jsonpatch.JsonPatchOperation { - var annotations map[string]string - var patchBytes []jsonpatch.JsonPatchOperation - for _, er := range engineResponses { - if ann := er.PatchedResource.GetAnnotations(); ann != nil { - annotations = ann - break - } - } - if annotations == nil { - annotations = make(map[string]string) - } - var patchResponse jsonpatch.JsonPatchOperation - value := annotationFromEngineResponses(engineResponses, log) - if value == nil { - // no patches or error while processing patches - return nil - } - if _, ok := annotations[strings.ReplaceAll(policyAnnotation, "~1", "/")]; ok { - // create update patch string - if _, ok := annotations["policies.kyverno.io/patches"]; ok { - patchResponse = jsonpatch.JsonPatchOperation{ - Operation: "remove", - Path: "/metadata/annotations/" + oldAnnotation, - Value: nil, - } - delete(annotations, "policies.kyverno.io/patches") - patchBytes = append(patchBytes, patchResponse) - } - patchResponse = jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/metadata/annotations/" + policyAnnotation, - Value: string(value), - } - patchBytes = append(patchBytes, patchResponse) - } else { - // mutate rule has annotation patches - if len(annotations) > 0 { - if _, ok := annotations["policies.kyverno.io/patches"]; ok { - patchResponse = jsonpatch.JsonPatchOperation{ - Operation: "remove", - Path: "/metadata/annotations/" + oldAnnotation, - Value: nil, - } - delete(annotations, "policies.kyverno.io/patches") - patchBytes = append(patchBytes, patchResponse) - } - patchResponse = jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/metadata/annotations/" + policyAnnotation, - Value: string(value), - } - patchBytes = append(patchBytes, patchResponse) - } else { - // insert 'policies.kyverno.patches' entry in annotation map - annotations[strings.ReplaceAll(policyAnnotation, "~1", "/")] = string(value) - patchResponse = jsonpatch.JsonPatchOperation{ - Operation: "add", - Path: "/metadata/annotations", - Value: annotations, - } - patchBytes = append(patchBytes, patchResponse) - } - } - return patchBytes -} - -func annotationFromEngineResponses(engineResponses []engineapi.EngineResponse, log logr.Logger) []byte { - annotationContent := make(map[string]string) - for _, engineResponse := range engineResponses { - if !engineResponse.IsSuccessful() { - log.V(3).Info("skip building annotation; policy failed to apply", "policy", engineResponse.Policy().GetName()) - continue - } - rulePatches := annotationFromPolicyResponse(engineResponse.PolicyResponse, log) - if rulePatches == nil { - continue - } - policyName := engineResponse.Policy().GetName() - for _, rulePatch := range rulePatches { - annotationContent[rulePatch.RuleName+"."+policyName+".kyverno.io"] = OperationToPastTense[rulePatch.Op] + " " + rulePatch.Path - } - } - - // return nil if there's no patches - // otherwise result = null, len(result) = 4 - if len(annotationContent) == 0 { - return nil - } - - result, _ := yamlv2.Marshal(annotationContent) - - return result -} - -func annotationFromPolicyResponse(policyResponse engineapi.PolicyResponse, log logr.Logger) []RulePatch { - var RulePatches []RulePatch - for _, ruleInfo := range policyResponse.Rules { - for _, patch := range ruleInfo.DeprecatedPatches() { - rp := RulePatch{ - RuleName: ruleInfo.Name(), - Op: patch.Operation, - Path: patch.Path, - } - RulePatches = append(RulePatches, rp) - log.V(4).Info("annotation value prepared", "patches", RulePatches) - } - } - if len(RulePatches) == 0 { - return nil - } - return RulePatches -} diff --git a/pkg/utils/annotations_test.go b/pkg/utils/annotations_test.go deleted file mode 100644 index 44dac77acd..0000000000 --- a/pkg/utils/annotations_test.go +++ /dev/null @@ -1,119 +0,0 @@ -package utils - -import ( - "testing" - - kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" - engineapi "github.com/kyverno/kyverno/pkg/engine/api" - "github.com/kyverno/kyverno/pkg/logging" - "github.com/mattbaird/jsonpatch" - "gotest.tools/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" -) - -func newPolicyResponse(rule string, patches []jsonpatch.JsonPatchOperation, status engineapi.RuleStatus) engineapi.PolicyResponse { - return engineapi.PolicyResponse{ - Rules: []engineapi.RuleResponse{ - *engineapi.NewRuleResponse(rule, engineapi.Mutation, "", status).WithPatches(patches...), - }, - } -} - -func newEngineResponse(policy, rule string, patchesStr []jsonpatch.JsonPatchOperation, status engineapi.RuleStatus, annotation map[string]interface{}) engineapi.EngineResponse { - p := &kyvernov1.ClusterPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: policy, - }, - } - policyResponse := newPolicyResponse(rule, patchesStr, status) - response := engineapi.NewEngineResponse(unstructured.Unstructured{}, p, nil).WithPolicyResponse(policyResponse) - response.PatchedResource = unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "annotations": annotation, - }, - }, - } - return response -} - -func Test_empty_annotation(t *testing.T) { - patchStr := jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/containers/0/imagePullPolicy", - Value: "IfNotPresent", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []jsonpatch.JsonPatchOperation{patchStr}, engineapi.RuleStatusPass, nil) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - expectedPatches := `{"op":"add","path":"/metadata/annotations","value":{"policies.kyverno.io/last-applied-patches":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}}` - assert.Equal(t, annPatches[0].Json(), expectedPatches) -} - -func Test_exist_annotation(t *testing.T) { - annotation := map[string]interface{}{ - "test": "annotation", - } - patchStr := jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/containers/0/imagePullPolicy", - Value: "IfNotPresent", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []jsonpatch.JsonPatchOperation{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - expectedPatches := `{"op":"add","path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` - assert.Equal(t, annPatches[0].Json(), expectedPatches) -} - -func Test_exist_kyverno_annotation(t *testing.T) { - annotation := map[string]interface{}{ - "policies.kyverno.patches": "old-annotation", - } - patchStr := jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/containers/0/imagePullPolicy", - Value: "IfNotPresent", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []jsonpatch.JsonPatchOperation{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - expectedPatches := `{"op":"add","path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` - assert.Equal(t, annPatches[0].Json(), expectedPatches) -} - -func Test_annotation_nil_patch(t *testing.T) { - annotation := map[string]interface{}{ - "policies.kyverno.patches": "old-annotation", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", nil, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - assert.Assert(t, annPatches == nil) - engineResponseNew := newEngineResponse("mutate-container", "default-imagepullpolicy", []jsonpatch.JsonPatchOperation{}, engineapi.RuleStatusPass, annotation) - annPatchesNew := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponseNew}, logging.GlobalLogger()) - assert.Assert(t, annPatchesNew == nil) -} - -func Test_annotation_failed_Patch(t *testing.T) { - annotation := map[string]interface{}{ - "policies.kyverno.patches": "old-annotation", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", nil, engineapi.RuleStatusFail, annotation) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - assert.Assert(t, annPatches == nil) -} - -func Test_exist_patches(t *testing.T) { - annotation := map[string]interface{}{ - "policies.kyverno.io/patches": "present", - } - patchStr := jsonpatch.JsonPatchOperation{ - Operation: "replace", - Path: "/spec/containers/0/imagePullPolicy", - Value: "IfNotPresent", - } - engineResponse := newEngineResponse("mutate-container", "default-imagepullpolicy", []jsonpatch.JsonPatchOperation{patchStr}, engineapi.RuleStatusPass, annotation) - annPatches := GenerateAnnotationPatches([]engineapi.EngineResponse{engineResponse}, logging.GlobalLogger()) - expectedPatches1 := `{"op":"remove","path":"/metadata/annotations/policies.kyverno.io~1patches"}` - expectedPatches2 := `{"op":"add","path":"/metadata/annotations/policies.kyverno.io~1last-applied-patches","value":"default-imagepullpolicy.mutate-container.kyverno.io: replaced /spec/containers/0/imagePullPolicy\n"}` - assert.Equal(t, annPatches[0].Json(), expectedPatches1) - assert.Equal(t, annPatches[1].Json(), expectedPatches2) -} diff --git a/pkg/webhooks/resource/mutation/mutation.go b/pkg/webhooks/resource/mutation/mutation.go index 7a39958ec4..cb1f64331f 100644 --- a/pkg/webhooks/resource/mutation/mutation.go +++ b/pkg/webhooks/resource/mutation/mutation.go @@ -14,7 +14,6 @@ import ( "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/openapi" "github.com/kyverno/kyverno/pkg/tracing" - "github.com/kyverno/kyverno/pkg/utils" engineutils "github.com/kyverno/kyverno/pkg/utils/engine" jsonutils "github.com/kyverno/kyverno/pkg/utils/json" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" @@ -127,11 +126,6 @@ func (v *mutationHandler) applyMutations( } } - // generate annotations - if annPatches := utils.GenerateAnnotationPatches(engineResponses, v.log); annPatches != nil { - patches = append(patches, annPatches...) - } - events := webhookutils.GenerateEvents(engineResponses, false) v.eventGen.Add(events...)