Merge pull request #895 from realshuting/892_violation_reconciliation

bug fixes - #894, #892, #891

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

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=

pkg/checker/status.go

@@ -110,7 +110,7 @@ func (vc StatusControl) IncrementAnnotation() error {
 	var err error
 	deploy, err := vc.client.GetResource("Deployment", deployNamespace, deployName)
 	if err != nil {
-		logger.Error(err, "failed to find deployment %s in namespace %s", deployName, deployNamespace)
+		logger.Error(err, "failed to find Kyverno", "deploymeny", deployName, "namespace", deployNamespace)
 		return err
 	}
 

pkg/engine/mutation.go

@@ -141,7 +141,7 @@ func patchedResourceHasPodControllerAnnotation(resource unstructured.Unstructure
 
 	val, ok := podController.Spec.Template.Metadata.Annotations[PodTemplateAnnotation]
 
-	log.Log.Info("patchedResourceHasPodControllerAnnotation", "resourceRaw", string(resourceRaw), "val", val, "ok", ok)
+	log.Log.V(4).Info("patchedResourceHasPodControllerAnnotation", "resourceRaw", string(resourceRaw), "val", val, "ok", ok)
 
 	return ok
 }

pkg/openapi/crdSync.go

@@ -107,17 +107,12 @@ func (c *crdSync) sync() {
 }
 
 func (o *Controller) deleteCRDFromPreviousSync() {
-	for k := range o.kindToDefinitionName {
-		delete(o.kindToDefinitionName, k)
+	for _, crd := range o.crdList {
+		delete(o.kindToDefinitionName, crd)
+		delete(o.definitions, crd)
 	}
 
-	o.kindToDefinitionName = make(map[string]string, 0)
-
-	for k := range o.definitions {
-		delete(o.definitions, k)
-	}
-
-	o.definitions = make(map[string]*openapi_v2.Schema, 0)
+	o.crdList = make([]string, 0)
 }
 
 func (o *Controller) parseCRD(crd unstructured.Unstructured) {
@@ -166,6 +161,7 @@ func (o *Controller) parseCRD(crd unstructured.Unstructured) {
 		return
 	}
 
+	o.crdList = append(o.crdList, crdName)
 	o.kindToDefinitionName[crdName] = crdName
 	o.definitions[crdName] = parsedSchema
 }

pkg/openapi/validation.go

@@ -26,14 +26,20 @@ import (
 )
 
 type Controller struct {
-	mutex                sync.RWMutex
-	definitions          map[string]*openapi_v2.Schema
+	mutex       sync.RWMutex
+	definitions map[string]*openapi_v2.Schema
+	// kindToDefinitionName holds the kind - definition map
+	// i.e. - Namespace: io.k8s.api.core.v1.Namespace
 	kindToDefinitionName map[string]string
+	crdList              []string
 	models               proto.Models
 }
 
 func NewOpenAPIController() (*Controller, error) {
-	controller := &Controller{}
+	controller := &Controller{
+		definitions:          make(map[string]*openapi_v2.Schema),
+		kindToDefinitionName: make(map[string]string),
+	}
 
 	defaultDoc, err := getSchemaDocument()
 	if err != nil {
@@ -82,7 +88,7 @@ func (o *Controller) ValidateResource(patchedResource unstructured.Unstructured,
 		// Check if kind is a CRD
 		schema, err = o.getCRDSchema(kind)
 		if err != nil || schema == nil {
-			return fmt.Errorf("pre-validation: couldn't find model %s", kind)
+			return fmt.Errorf("pre-validation: couldn't find model %s, err: %v", kind, err)
 		}
 		delete(patchedResource.Object, "kind")
 	}
@@ -146,8 +152,6 @@ func (o *Controller) useOpenApiDocument(doc *openapi_v2.Document) error {
 	o.mutex.Lock()
 	defer o.mutex.Unlock()
 
-	o.definitions = make(map[string]*openapi_v2.Schema)
-	o.kindToDefinitionName = make(map[string]string)
 	for _, definition := range doc.GetDefinitions().AdditionalProperties {
 		o.definitions[definition.GetName()] = definition.GetValue()
 		path := strings.Split(definition.GetName(), ".")

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
 	}

pkg/policyviolation/common.go

@@ -2,6 +2,7 @@ package policyviolation
 
 import (
 	"fmt"
+	"reflect"
 	"time"
 
 	backoff "github.com/cenkalti/backoff"
@@ -105,3 +106,25 @@ 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 = ""
+	}
+
+	return !reflect.DeepEqual(*new, *old)
+}

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
 	}

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

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)