From a1d7816c1002ec119c2d16bfdc12291d2059600b Mon Sep 17 00:00:00 2001 From: Shuting Zhao <shutting06@gmail.com> Date: Mon, 1 Jun 2020 19:37:48 -0700 Subject: [PATCH] fix violation updates when there's no change --- go.mod | 1 + go.sum | 3 +++ pkg/policyviolation/clusterpv.go | 3 +-- pkg/policyviolation/common.go | 26 ++++++++++++++++++++++++++ pkg/policyviolation/namespacedpv.go | 3 +-- pkg/webhooks/mutation.go | 2 +- pkg/webhooks/server.go | 9 +++++---- 7 files changed, 38 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index fb142d89bd..22e79a808e 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/julienschmidt/httprouter v1.3.0 github.com/minio/minio v0.0.0-20200114012931-30922148fbb5 github.com/ory/go-acc v0.2.1 // indirect + github.com/prometheus/common v0.4.1 github.com/rogpeppe/godef v1.1.2 // indirect github.com/spf13/cobra v0.0.5 github.com/tevino/abool v0.0.0-20170917061928-9b9efcf221b5 diff --git a/go.sum b/go.sum index a1dcba8344..962574f41c 100644 --- a/go.sum +++ b/go.sum @@ -48,7 +48,9 @@ github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d/go.mod h1:3eOhrU github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM= github.com/ajg/form v0.0.0-20160822230020-523a5da1a92f/go.mod h1:uL1WgH+h2mgNtvBq0339dVnzXdBETtL2LeUXaIv25UY= github.com/alecthomas/participle v0.2.1/go.mod h1:SW6HZGeZgSIpcUWX3fXpfZhuaWHnmoD5KCVaqSaNTkk= +github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc h1:cAKDfWh5VpdgMhJosfJnn5/FoN2SRZ4p7fJNX58YPaU= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= +github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf h1:qet1QNfXsQxTZqLG4oE62mJzwPIB8+Tee4RNCL9ulrY= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/aliyun/aliyun-oss-go-sdk v0.0.0-20190307165228-86c17b95fcd5/go.mod h1:T/Aws4fEfogEE9v+HPhhw+CntffsBHJ8nXQCwKr0/g8= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= @@ -1083,6 +1085,7 @@ google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ij google.golang.org/grpc v1.22.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= google.golang.org/grpc v1.23.1/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= gopkg.in/airbrake/gobrake.v2 v2.0.9/go.mod h1:/h5ZAUhDkGaJfjzjKLSjv6zCL6O0LLBxU4K+aSYdM/U= +gopkg.in/alecthomas/kingpin.v2 v2.2.6 h1:jMFz6MfLP0/4fUyZle81rXUoxOBFi19VUFKVDOQfozc= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc/go.mod h1:m7x9LTH6d71AHyAX77c9yqWCCa3UKHcVEj9y7hAtKDk= gopkg.in/asn1-ber.v1 v1.0.0-20181015200546-f715ec2f112d/go.mod h1:cuepJuh7vyXfUyUwEgHQXw849cJrilpS5NeIjOWESAw= diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index fedf568863..6ddbe7e05b 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -2,7 +2,6 @@ package policyviolation import ( "fmt" - "reflect" "github.com/go-logr/logr" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -125,7 +124,7 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err logger := cpv.log.WithValues("policy", newPv.Spec.Policy, "kind", newPv.Spec.ResourceSpec.Kind, "namespace", newPv.Spec.ResourceSpec.Namespace, "name", newPv.Spec.ResourceSpec.Name) var err error // check if there is any update - if reflect.DeepEqual(newPv.Spec, oldPv.Spec) { + if !hasViolationSpecChanged(newPv.Spec.DeepCopy(), oldPv.Spec.DeepCopy()) { logger.V(4).Info("policy violation spec did not change, not upadating the resource") return nil } diff --git a/pkg/policyviolation/common.go b/pkg/policyviolation/common.go index e8e77308a7..2ec445c087 100644 --- a/pkg/policyviolation/common.go +++ b/pkg/policyviolation/common.go @@ -2,6 +2,7 @@ package policyviolation import ( "fmt" + "reflect" "time" backoff "github.com/cenkalti/backoff" @@ -105,3 +106,28 @@ func (vc violationCount) UpdateStatus(status kyverno.PolicyStatus) kyverno.Polic return status } + +// hasViolationSpecChanged returns true if oldSpec & newSpec +// are identical, exclude message in violated rules +func hasViolationSpecChanged(new, old *kyverno.PolicyViolationSpec) bool { + if new.Policy != old.Policy { + return true + } + + if new.ResourceSpec.ToKey() != old.ResourceSpec.ToKey() { + return true + } + + for i := range new.ViolatedRules { + new.ViolatedRules[i].Message = "" + } + + for i := range old.ViolatedRules { + old.ViolatedRules[i].Message = "" + } + + fmt.Println("====new", new) + fmt.Println("====old", old) + + return !reflect.DeepEqual(*new, *old) +} diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index db13e33ff9..0fb28f9bd1 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -2,7 +2,6 @@ package policyviolation import ( "fmt" - "reflect" "github.com/go-logr/logr" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -130,7 +129,7 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error logger := nspv.log.WithValues("policy", newPv.Spec.Policy, "kind", newPv.Spec.ResourceSpec.Kind, "namespace", newPv.Spec.ResourceSpec.Namespace, "name", newPv.Spec.ResourceSpec.Name) var err error // check if there is any update - if reflect.DeepEqual(newPv.Spec, oldPv.Spec) { + if !hasViolationSpecChanged(newPv.Spec.DeepCopy(), oldPv.Spec.DeepCopy()) { logger.V(4).Info("policy violation spec did not change, not upadating the resource") return nil } diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index ace0c41e5f..041046cd65 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -46,7 +46,6 @@ func (ws *WebhookServer) HandleMutation( policyContext.Policy = *policy engineResponse := engine.Mutate(policyContext) - engineResponses = append(engineResponses, engineResponse) ws.statusListener.Send(mutateStats{resp: engineResponse}) if !engineResponse.IsSuccesful() { logger.Info("failed to apply policy", "policy", policy.Name, "failed rules", engineResponse.GetFailedRules()) @@ -66,6 +65,7 @@ func (ws *WebhookServer) HandleMutation( } policyContext.NewResource = engineResponse.PatchedResource + engineResponses = append(engineResponses, engineResponse) } // generate annotations diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 80be8e94f6..d711caaa62 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -7,10 +7,11 @@ import ( "errors" "fmt" "io/ioutil" - "k8s.io/apimachinery/pkg/labels" "net/http" "time" + "k8s.io/apimachinery/pkg/labels" + "github.com/go-logr/logr" "github.com/julienschmidt/httprouter" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -83,7 +84,7 @@ type WebhookServer struct { pvGenerator policyviolation.GeneratorInterface // generate request generator - grGenerator *generate.Generator + grGenerator *generate.Generator resourceWebhookWatcher *webhookconfig.ResourceWebhookRegister log logr.Logger @@ -153,7 +154,7 @@ func NewWebhookServer( // Handle Liveness responds to a Kubernetes Liveness probe // Fail this request if Kubernetes should restart this instance - mux.HandlerFunc("GET", config.LivenessServicePath, func(w http.ResponseWriter, r *http.Request){ + mux.HandlerFunc("GET", config.LivenessServicePath, func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() w.WriteHeader(http.StatusOK) @@ -161,7 +162,7 @@ func NewWebhookServer( // Handle Readiness responds to a Kubernetes Readiness probe // Fail this request if this instance can't accept traffic, but Kubernetes shouldn't restart it - mux.HandlerFunc("GET", config.ReadinessServicePath, func(w http.ResponseWriter, r *http.Request){ + mux.HandlerFunc("GET", config.ReadinessServicePath, func(w http.ResponseWriter, r *http.Request) { defer r.Body.Close() w.WriteHeader(http.StatusOK)