From 29a89d20ada0c176171b552ee7a345d0bed0d740 Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Mon, 15 Jul 2019 11:29:58 -0700 Subject: [PATCH] violation cleanup for existing resources --- pkg/apis/policy/v1alpha1/types.go | 13 +- pkg/apis/policy/v1alpha1/utils.go | 21 +++ .../policy/v1alpha1/zz_generated.deepcopy.go | 16 ++- pkg/controller/controller.go | 4 +- pkg/engine/engine.go | 2 +- pkg/violation/builder.go | 129 ++++++++++++++---- pkg/violation/util.go | 10 ++ pkg/webhooks/server.go | 21 ++- 8 files changed, 172 insertions(+), 44 deletions(-) diff --git a/pkg/apis/policy/v1alpha1/types.go b/pkg/apis/policy/v1alpha1/types.go index 889d1bf775..91f1f92a4e 100644 --- a/pkg/apis/policy/v1alpha1/types.go +++ b/pkg/apis/policy/v1alpha1/types.go @@ -77,16 +77,17 @@ type CloneFrom struct { // Status contains violations for existing resources type Status struct { - Violations []Violation `json:"violations,omitempty"` + // Violations map[kind/namespace/resource]Violation + Violations map[string]Violation `json:"violations,omitempty"` } // Violation for the policy type Violation struct { - Kind string `json:"kind,omitempty"` - Name string `json:"name,omitempty"` - Namespace string `json:"namespace,omitempty"` - Reason string `json:"reason,omitempty"` - Message string `json:"message,omitempty"` + Kind string `json:"kind,omitempty"` + Name string `json:"name,omitempty"` + Namespace string `json:"namespace,omitempty"` + Rules []string `json:"rules"` + Reason string `json:"reason,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/policy/v1alpha1/utils.go b/pkg/apis/policy/v1alpha1/utils.go index 59adc88a5d..17c067c5fb 100644 --- a/pkg/apis/policy/v1alpha1/utils.go +++ b/pkg/apis/policy/v1alpha1/utils.go @@ -104,3 +104,24 @@ func (in *Generation) DeepCopyInto(out *Generation) { *out = *in } } + +//IsEqual Check if violatiosn are equal +func (v *Violation) IsEqual(nv Violation) bool { + // We do not need to compare resource info as it will be same + // Reason + if v.Reason != nv.Reason { + return false + } + // Rule + if len(v.Rules) != len(nv.Rules) { + return false + } + // assumes the rules will be in order, as the rule are proceeed in order + // if the rule order changes, it means the policy has changed.. as it will afffect the order in which mutation rules are applied + for i, r := range v.Rules { + if r != nv.Rules[i] { + return false + } + } + return true +} diff --git a/pkg/apis/policy/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/policy/v1alpha1/zz_generated.deepcopy.go index 903ae1f40c..24be64c1b6 100644 --- a/pkg/apis/policy/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/policy/v1alpha1/zz_generated.deepcopy.go @@ -135,6 +135,11 @@ func (in *ResourceDescription) DeepCopyInto(out *ResourceDescription) { *out = new(string) **out = **in } + if in.Namespace != nil { + in, out := &in.Namespace, &out.Namespace + *out = new(string) + **out = **in + } if in.Selector != nil { in, out := &in.Selector, &out.Selector *out = new(v1.LabelSelector) @@ -210,8 +215,10 @@ func (in *Status) DeepCopyInto(out *Status) { *out = *in if in.Violations != nil { in, out := &in.Violations, &out.Violations - *out = make([]Violation, len(*in)) - copy(*out, *in) + *out = make(map[string]Violation, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } } return } @@ -239,6 +246,11 @@ func (in *Validation) DeepCopy() *Validation { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Violation) DeepCopyInto(out *Violation) { *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]string, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 01347ee09a..d8730adaf8 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -225,7 +225,9 @@ func createEventsAndViolations(eventController event.Generator, policyInfos []*i e := event.NewEvent("Policy", "", policyInfo.Name, event.PolicyViolation, event.FResourcePolcy, policyInfo.RNamespace+"/"+policyInfo.RName, strings.Join(fruleNames, ";")) events = append(events, e) // Violation - v := violation.NewViolationFromEvent(e, policyInfo.Name, policyInfo.RKind, policyInfo.RName, policyInfo.RNamespace) + // TODO: Violation is currently create at policy, level not resource level + // As we create violation, we check if the + v := violation.BuldNewViolation(policyInfo.Name, policyInfo.RKind, policyInfo.RNamespace, policyInfo.RName, event.PolicyViolation.String(), fruleNames) violations = append(violations, v) } // else { diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index d61545fb8f..4cc94d80e5 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -34,7 +34,7 @@ func ProcessExisting(client *client.Client, policy *types.Policy) []*info.Policy if rule.ResourceDescription.Namespace != nil { namespace = *rule.ResourceDescription.Namespace } - list, err := client.ListResource(gvr.Resource, namespace, rule.ResourceDescription.Selector) + list, err := client.ListResource(k, namespace, rule.ResourceDescription.Selector) if err != nil { glog.Errorf("unable to list resource for %s with label selector %s", gvr.Resource, rule.Selector.String()) glog.Errorf("unable to apply policy %s rule %s. err: %s", policy.Name, rule.Name, err) diff --git a/pkg/violation/builder.go b/pkg/violation/builder.go index 7fe9645274..ff48c55e8f 100644 --- a/pkg/violation/builder.go +++ b/pkg/violation/builder.go @@ -51,7 +51,7 @@ func (b *builder) Add(infos ...*Info) error { } func (b *builder) processViolation(info *Info) error { - currentViolations := []interface{}{} + currVs := map[string]interface{}{} statusMap := map[string]interface{}{} var ok bool //TODO: hack get from client @@ -71,37 +71,71 @@ func (b *builder) processViolation(info *Info) error { if !ok { glog.Info("violation not present") } + // Violations map[string][]Violations glog.Info(reflect.TypeOf(violations)) - if currentViolations, ok = violations.([]interface{}); !ok { + if currVs, ok = violations.(map[string]interface{}); !ok { return errors.New("Unable to parse violations") } } - newViolation := info.Violation - for _, violation := range currentViolations { - glog.Info(reflect.TypeOf(violation)) - if v, ok := violation.(map[string]interface{}); ok { - if name, ok := v["name"].(string); ok { - if namespace, ok := v["namespace"].(string); ok { - ok, err := b.isActive(info.Kind, name, namespace) - if err != nil { - glog.Error(err) - continue - } - if !ok { - //TODO remove the violation as it corresponds to resource that does not exist - glog.Info("removed violation") - } - } - } + // Info: + // Resource - Kind, Namespace, Name + // policy - Name + // violation, ok := currVs[info.getKey()] + // Key -> resource + // 1> Check if there were any previous violations for the given key + // 2> If No, create a new one + if !ok { + currVs[info.getKey()] = info.Violation + } else { + currV := currVs[info.getKey()] + glog.Info(reflect.TypeOf(currV)) + v, ok := currV.(map[string]interface{}) + if !ok { + glog.Info("type not matching") } + // get rules + rules, ok := v["rules"] + if !ok { + glog.Info("rules not found") + } + glog.Info(reflect.TypeOf(rules)) + rs, ok := rules.([]interface{}) + if !ok { + glog.Info("type not matching") + } + // check if rules are samre + if isRuleNamesEqual(rs, info.Violation.Rules) { + return nil + } + // else update the errors + currVs[info.getKey()] = info.Violation } - currentViolations = append(currentViolations, newViolation) - // update violations + // newViolation := info.Violation + // for _, violation := range currentViolations { + // glog.Info(reflect.TypeOf(violation)) + // if v, ok := violation.(map[string]interface{}); ok { + // if name, ok := v["name"].(string); ok { + // if namespace, ok := v["namespace"].(string); ok { + // ok, err := b.isActive(info.Kind, name, namespace) + // if err != nil { + // glog.Error(err) + // continue + // } + // if !ok { + // //TODO remove the violation as it corresponds to resource that does not exist + // glog.Info("removed violation") + // } + // } + // } + // } + // } + // currentViolations = append(currentViolations, newViolation) + // // update violations // set the updated status - statusMap["violations"] = currentViolations + statusMap["violations"] = currVs unstr["status"] = statusMap p1.SetUnstructuredContent(unstr) - _, err = b.client.UpdateStatusResource("policies", "", p1, false) + _, err = b.client.UpdateStatusResource("Policy", "", p1, false) if err != nil { return err } @@ -126,21 +160,58 @@ func NewViolation(reason event.Reason, policyName, kind, rname, rnamespace, msg Name: rname, Namespace: rnamespace, Reason: reason.String(), - Message: msg, }, } } -//NewViolationFromEvent returns violation info from event -func NewViolationFromEvent(e *event.Info, pName, rKind, rName, rnamespace string) *Info { +// //NewViolationFromEvent returns violation info from event +// func NewViolationFromEvent(e *event.Info, pName, rKind, rName, rnamespace string) *Info { +// return &Info{ +// Policy: pName, +// Violation: types.Violation{ +// Kind: rKind, +// Name: rName, +// Namespace: rnamespace, +// Reason: e.Reason, +// Message: e.Message, +// }, +// } +// } +// Build a new Violation +func BuldNewViolation(pName string, rKind string, rNs string, rName string, reason string, rules []string) *Info { return &Info{ Policy: pName, Violation: types.Violation{ Kind: rKind, + Namespace: rNs, Name: rName, - Namespace: rnamespace, - Reason: e.Reason, - Message: e.Message, + Reason: reason, + Rules: rules, }, } } + +func isRuleNamesEqual(currRules []interface{}, newRules []string) bool { + if len(currRules) != len(newRules) { + return false + } + for i, r := range currRules { + name, ok := r.(string) + if !ok { + return false + } + if name != newRules[i] { + return false + } + } + return true +} + +//RemoveViolation will remove the violation for the resource if there was one +func RemoveViolation(policy *types.Policy, rKind string, rNs string, rName string) { + // Remove the pair from map + if policy.Status.Violations != nil { + glog.Infof("Cleaning up violalation for policy %s, resource %s/%s/%s", policy.Name, rKind, rNs, rName) + delete(policy.Status.Violations, BuildKey(rKind, rNs, rName)) + } +} diff --git a/pkg/violation/util.go b/pkg/violation/util.go index e83b9916ab..bd0b9a1c3a 100644 --- a/pkg/violation/util.go +++ b/pkg/violation/util.go @@ -16,3 +16,13 @@ type Info struct { Policy string policytype.Violation } + +func (i Info) getKey() string { + return i.Kind + "/" + i.Namespace + "/" + i.Name +} + +//BuildKey returns the key format +func BuildKey(rKind, rNs, rName string) string { + return rKind + "/" + rNs + "/" + rName +} + diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index b38e3f9796..29b9830629 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -22,6 +22,7 @@ import ( "github.com/nirmata/kyverno/pkg/sharedinformer" tlsutils "github.com/nirmata/kyverno/pkg/tls" "github.com/nirmata/kyverno/pkg/utils" + "github.com/nirmata/kyverno/pkg/violation" v1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -182,9 +183,14 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest) *v1be for _, r := range ruleInfos { glog.Warning(r.Msgs) } - } else if len(policyPatches) > 0 { - allPatches = append(allPatches, policyPatches...) - glog.Infof("Mutation from policy %s has applied succesfully to %s %s/%s", policy.Name, request.Kind.Kind, rname, rns) + } else { + fmt.Println("cleanup") + // CleanUp Violations if exists + violation.RemoveViolation(policy, request.Kind.Kind, rns, rname) + if len(policyPatches) > 0 { + allPatches = append(allPatches, policyPatches...) + glog.Infof("Mutation from policy %s has applied succesfully to %s %s/%s", policy.Name, request.Kind.Kind, rname, rns) + } } policyInfos = append(policyInfos, policyInfo) } @@ -274,8 +280,13 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest) *v1 for _, r := range ruleInfos { glog.Warning(r.Msgs) } - } else if len(ruleInfos) > 0 { - glog.Infof("Validation from policy %s has applied succesfully to %s %s/%s", policy.Name, request.Kind.Kind, rname, rns) + } else { + // CleanUp Violations if exists + violation.RemoveViolation(policy, request.Kind.Kind, rns, rname) + + if len(ruleInfos) > 0 { + glog.Infof("Validation from policy %s has applied succesfully to %s %s/%s", policy.Name, request.Kind.Kind, rname, rns) + } } policyInfos = append(policyInfos, policyInfo) }