From 1bbe84bbc9e95183a111c055ca494c09a5e27287 Mon Sep 17 00:00:00 2001 From: shravan Date: Mon, 10 Feb 2020 20:29:40 +0530 Subject: [PATCH 01/34] 527 do not record stats during sync --- pkg/policy/apply.go | 40 +--------------------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go index ed4b72ac65..5b79cc4bec 100644 --- a/pkg/policy/apply.go +++ b/pkg/policy/apply.go @@ -21,43 +21,12 @@ import ( //TODO: generation rules func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, policyStatus PolicyStatusInterface) (responses []response.EngineResponse) { startTime := time.Now() - var policyStats []PolicyStat + glog.V(4).Infof("Started apply policy %s on resource %s/%s/%s (%v)", policy.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), startTime) defer func() { glog.V(4).Infof("Finished applying %s on resource %s/%s/%s (%v)", policy.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), time.Since(startTime)) }() - // gather stats from the engine response - gatherStat := func(policyName string, policyResponse response.PolicyResponse) { - ps := PolicyStat{} - ps.PolicyName = policyName - ps.Stats.MutationExecutionTime = policyResponse.ProcessingTime - ps.Stats.RulesAppliedCount = policyResponse.RulesAppliedCount - // capture rule level stats - for _, rule := range policyResponse.Rules { - rs := RuleStatinfo{} - rs.RuleName = rule.Name - rs.ExecutionTime = rule.RuleStats.ProcessingTime - if rule.Success { - rs.RuleAppliedCount++ - } else { - rs.RulesFailedCount++ - } - if rule.Patches != nil { - rs.MutationCount++ - } - ps.Stats.Rules = append(ps.Stats.Rules, rs) - } - policyStats = append(policyStats, ps) - } - // send stats for aggregation - sendStat := func(blocked bool) { - for _, stat := range policyStats { - stat.Stats.ResourceBlocked = utils.Btoi(blocked) - //SEND - policyStatus.SendStat(stat) - } - } var engineResponses []response.EngineResponse var engineResponse response.EngineResponse var err error @@ -71,17 +40,10 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure if err != nil { glog.Errorf("unable to process mutation rules: %v", err) } - gatherStat(policy.Name, engineResponse.PolicyResponse) - //send stats - sendStat(false) //VALIDATION engineResponse = engine.Validate(engine.PolicyContext{Policy: policy, Context: ctx, NewResource: resource}) engineResponses = append(engineResponses, engineResponse) - // gather stats - gatherStat(policy.Name, engineResponse.PolicyResponse) - //send stats - sendStat(false) //TODO: GENERATION return engineResponses From b5e5f3eeda1e764281309ba65cc95959a703d70d Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 15 Feb 2020 16:38:59 +0530 Subject: [PATCH 02/34] 527 save commit --- cmd/kyverno/main.go | 2 +- pkg/policy/status2.go | 107 +++++++++++++++++++++++++++++++++++++++ pkg/webhooks/mutation.go | 49 +++--------------- pkg/webhooks/server.go | 9 ++-- 4 files changed, 121 insertions(+), 46 deletions(-) create mode 100644 pkg/policy/status2.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 62cefceeb9..b002ce0f76 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -215,7 +215,7 @@ func main() { kubeInformer.Rbac().V1().ClusterRoleBindings(), egen, webhookRegistrationClient, - pc.GetPolicyStatusAggregator(), + policy.NewStatusSync(pclient, stopCh), configData, policyMetaStore, pvgen, diff --git a/pkg/policy/status2.go b/pkg/policy/status2.go new file mode 100644 index 0000000000..7161f2015c --- /dev/null +++ b/pkg/policy/status2.go @@ -0,0 +1,107 @@ +package policy + +import ( + "sync" + "time" + + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/nirmata/kyverno/pkg/client/clientset/versioned" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" +) + +type statusCache struct { + mu sync.RWMutex + data map[string]v1.PolicyStatus +} + +func (c *statusCache) Get(key string) v1.PolicyStatus { + c.mu.RLock() + status := c.data[key] + c.mu.RUnlock() + return status + +} + +func (c *statusCache) GetAll() map[string]v1.PolicyStatus { + c.mu.RLock() + mapCopy := make(map[string]v1.PolicyStatus, len(c.data)) + for k, v := range c.data { + mapCopy[k] = v + } + c.mu.RUnlock() + return mapCopy + +} +func (c *statusCache) Set(key string, status v1.PolicyStatus) { + c.mu.Lock() + c.data[key] = status + c.mu.Unlock() +} +func (c *statusCache) Clear() { + c.mu.Lock() + c.data = make(map[string]v1.PolicyStatus) + c.mu.Unlock() +} + +func newStatusCache() *statusCache { + return &statusCache{ + mu: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + } +} + +func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}) *StatusSync { + return &StatusSync{ + statusReceiver: make(chan map[string]v1.PolicyStatus), + cache: newStatusCache(), + stop: stopCh, + client: client, + } +} + +type StatusSync struct { + statusReceiver chan map[string]v1.PolicyStatus + cache *statusCache + stop <-chan struct{} + client *versioned.Clientset +} + +func (s *StatusSync) Cache() *statusCache { + return s.cache +} + +func (s *StatusSync) Receiver() chan<- map[string]v1.PolicyStatus { + return s.statusReceiver +} + +func (s *StatusSync) Start() { + // receive status and store it in cache + go func() { + for { + select { + case nameToStatus := <-s.statusReceiver: + for policyName, status := range nameToStatus { + s.cache.Set(policyName, status) + } + case <-s.stop: + return + } + } + }() + + // update policy status every 10 seconds - waits for previous updateStatus to complete + wait.Until(s.updateStatus, 10*time.Second, s.stop) + <-s.stop +} + +func (s *StatusSync) updateStatus() { + for policyName, status := range s.cache.GetAll() { + var policy = &v1.ClusterPolicy{} + policy.Name = policyName + policy.Status = status + _, _ = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) + } + s.cache.Clear() +} diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index c664c8a03f..612db1e40f 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -3,15 +3,15 @@ package webhooks import ( "time" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" engineutils "github.com/nirmata/kyverno/pkg/engine/utils" - policyctr "github.com/nirmata/kyverno/pkg/policy" "github.com/nirmata/kyverno/pkg/policyviolation" - "github.com/nirmata/kyverno/pkg/utils" v1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -23,40 +23,6 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou request.Kind.Kind, request.Namespace, request.Name, request.UID, request.Operation) var patches [][]byte - var policyStats []policyctr.PolicyStat - - // gather stats from the engine response - gatherStat := func(policyName string, policyResponse response.PolicyResponse) { - ps := policyctr.PolicyStat{} - ps.PolicyName = policyName - ps.Stats.MutationExecutionTime = policyResponse.ProcessingTime - ps.Stats.RulesAppliedCount = policyResponse.RulesAppliedCount - // capture rule level stats - for _, rule := range policyResponse.Rules { - rs := policyctr.RuleStatinfo{} - rs.RuleName = rule.Name - rs.ExecutionTime = rule.RuleStats.ProcessingTime - if rule.Success { - rs.RuleAppliedCount++ - } else { - rs.RulesFailedCount++ - } - if rule.Patches != nil { - rs.MutationCount++ - } - ps.Stats.Rules = append(ps.Stats.Rules, rs) - } - policyStats = append(policyStats, ps) - } - // send stats for aggregation - sendStat := func(blocked bool) { - for _, stat := range policyStats { - stat.Stats.ResourceBlocked = utils.Btoi(blocked) - //SEND - ws.policyStatus.SendStat(stat) - } - } - var engineResponses []response.EngineResponse userRequestInfo := kyverno.RequestInfo{ @@ -91,12 +57,10 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou for _, policy := range policies { glog.V(2).Infof("Handling mutation for Kind=%s, Namespace=%s Name=%s UID=%s patchOperation=%s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), request.UID, request.Operation) - policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) - // Gather policy application statistics - gatherStat(policy.Name, engineResponse.PolicyResponse) + updateStatusWithMutate(ws.status, policy, engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue @@ -125,8 +89,6 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou events := generateEvents(engineResponses, (request.Operation == v1beta1.Update)) ws.eventGen.Add(events...) - sendStat(false) - // debug info func() { if len(patches) != 0 { @@ -146,3 +108,8 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou // patches holds all the successful patches, if no patch is created, it returns nil return engineutils.JoinPatches(patches) } + +func updateStatusWithMutate(statusSync *policy.StatusSync, policy kyverno.ClusterPolicy, response response.EngineResponse) { + status := statusSync.Cache().Get(policy.Name) + status +} diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index d401cdc449..f4a962dda2 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -10,6 +10,8 @@ import ( "net/http" "time" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/checker" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -18,7 +20,6 @@ import ( "github.com/nirmata/kyverno/pkg/config" client "github.com/nirmata/kyverno/pkg/dclient" "github.com/nirmata/kyverno/pkg/event" - "github.com/nirmata/kyverno/pkg/policy" "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/policyviolation" tlsutils "github.com/nirmata/kyverno/pkg/tls" @@ -55,7 +56,7 @@ type WebhookServer struct { // webhook registration client webhookRegistrationClient *webhookconfig.WebhookRegistrationClient // API to send policy stats for aggregation - policyStatus policy.PolicyStatusInterface + status policy.StatusSync // helpers to validate against current loaded configuration configHandler config.Interface // channel for cleanup notification @@ -82,7 +83,7 @@ func NewWebhookServer( crbInformer rbacinformer.ClusterRoleBindingInformer, eventGen event.Interface, webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, - policyStatus policy.PolicyStatusInterface, + status *policy.StatusSync, configHandler config.Interface, pMetaStore policystore.LookupInterface, pvGenerator policyviolation.GeneratorInterface, @@ -112,7 +113,7 @@ func NewWebhookServer( crbSynced: crbInformer.Informer().HasSynced, eventGen: eventGen, webhookRegistrationClient: webhookRegistrationClient, - policyStatus: policyStatus, + status: status, configHandler: configHandler, cleanUp: cleanUp, lastReqTime: resourceWebhookWatcher.LastReqTime, From e9fc19142a663c5f5a5c699035a0ea330abcd6c2 Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 19 Feb 2020 09:27:39 +0530 Subject: [PATCH 03/34] 527 save commit --- pkg/policy/status2.go | 22 +++++++++++----------- pkg/webhooks/mutation.go | 25 +++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/pkg/policy/status2.go b/pkg/policy/status2.go index 7161f2015c..1ddfbaa5b7 100644 --- a/pkg/policy/status2.go +++ b/pkg/policy/status2.go @@ -54,26 +54,26 @@ func newStatusCache() *statusCache { func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}) *StatusSync { return &StatusSync{ - statusReceiver: make(chan map[string]v1.PolicyStatus), - cache: newStatusCache(), - stop: stopCh, - client: client, + policyStatsReciever: make(chan map[string]v1.PolicyStatus), + cache: newStatusCache(), + stop: stopCh, + client: client, } } type StatusSync struct { - statusReceiver chan map[string]v1.PolicyStatus - cache *statusCache - stop <-chan struct{} - client *versioned.Clientset + policyStatsReciever chan map[string]v1.PolicyStatus + cache *statusCache + stop <-chan struct{} + client *versioned.Clientset } func (s *StatusSync) Cache() *statusCache { return s.cache } -func (s *StatusSync) Receiver() chan<- map[string]v1.PolicyStatus { - return s.statusReceiver +func (s *StatusSync) StatReceiver() chan<- map[string]v1.PolicyStatus { + return s.policyStatsReciever } func (s *StatusSync) Start() { @@ -81,7 +81,7 @@ func (s *StatusSync) Start() { go func() { for { select { - case nameToStatus := <-s.statusReceiver: + case nameToStatus := <-s.policyStatsReciever: for policyName, status := range nameToStatus { s.cache.Set(policyName, status) } diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 612db1e40f..5cd9c2689f 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -110,6 +110,27 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou } func updateStatusWithMutate(statusSync *policy.StatusSync, policy kyverno.ClusterPolicy, response response.EngineResponse) { - status := statusSync.Cache().Get(policy.Name) - status + stats := kyverno.PolicyStatus{ + ViolationCount: 0, + RulesAppliedCount: response.PolicyResponse.RulesAppliedCount, + ResourcesBlockedCount: 0, + AvgExecutionTimeMutation: response.PolicyResponse.ProcessingTime.String(), + Rules: nil, + } + + for _, rule := range response.PolicyResponse.Rules { + ruleStats := kyverno.RuleStats{ + Name: rule.Name, + ExecutionTime: rule.ProcessingTime.String(), + AppliedCount: 0, + ViolationCount: 0, + MutationCount: 0, + } + + if rule.Success { + ruleStats.AppliedCount++ + ruleStats.MutationCount++ + } + } + } From a15a741cb410678bd4ec412dd5ff2c27be86b8b6 Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 22 Feb 2020 16:57:00 +0530 Subject: [PATCH 04/34] 527 save commit --- cmd/kyverno/main.go | 2 +- pkg/api/kyverno/v1/types.go | 27 +++--- pkg/policy/status2.go | 164 +++++++++++++++++++----------------- pkg/webhooks/mutation.go | 30 +------ pkg/webhooks/server.go | 4 +- 5 files changed, 105 insertions(+), 122 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index b002ce0f76..ae9b53ce57 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -215,7 +215,7 @@ func main() { kubeInformer.Rbac().V1().ClusterRoleBindings(), egen, webhookRegistrationClient, - policy.NewStatusSync(pclient, stopCh), + policy.NewStatusSync(pclient, stopCh, policyMetaStore), configData, policyMetaStore, pvgen, diff --git a/pkg/api/kyverno/v1/types.go b/pkg/api/kyverno/v1/types.go index b3ba284b7d..1f020492f5 100644 --- a/pkg/api/kyverno/v1/types.go +++ b/pkg/api/kyverno/v1/types.go @@ -227,20 +227,19 @@ type CloneFrom struct { Name string `json:"name,omitempty"` } -//PolicyStatus provides status for violations +// PolicyStatus mostly contains statistics related to policy type PolicyStatus struct { + // average time required to process the policy rules on a resource + AvgExecutionTime string `json:"averageExecutionTime"` + // Count of rules that failed ViolationCount int `json:"violationCount"` // Count of rules that were applied RulesAppliedCount int `json:"rulesAppliedCount"` - // Count of resources for whom update/create api requests were blocked as the resoruce did not satisfy the policy rules + // Count of resources that were blocked for failing a validate, across all rules ResourcesBlockedCount int `json:"resourcesBlockedCount"` - // average time required to process the policy Mutation rules on a resource - AvgExecutionTimeMutation string `json:"averageMutationRulesExecutionTime"` - // average time required to process the policy Validation rules on a resource - AvgExecutionTimeValidation string `json:"averageValidationRulesExecutionTime"` - // average time required to process the policy Validation rules on a resource - AvgExecutionTimeGeneration string `json:"averageGenerationRulesExecutionTime"` - // statistics per rule + // Count of resources that were successfully mutated, across all rules + ResourcesMutatedCount int `json:"resourcesMutatedCount"` + Rules []RuleStats `json:"ruleStatus"` } @@ -250,12 +249,14 @@ type RuleStats struct { Name string `json:"ruleName"` // average time require to process the rule ExecutionTime string `json:"averageExecutionTime"` - // Count of rules that were applied - AppliedCount int `json:"appliedCount"` // Count of rules that failed ViolationCount int `json:"violationCount"` - // Count of mutations - MutationCount int `json:"mutationsCount"` + // Count of rules that were applied + AppliedCount int `json:"appliedCount"` + // Count of resources for whom update/create api requests were blocked as the resource did not satisfy the policy rules + ResourcesBlockedCount int `json:"resourcesBlockedCount"` + // Count of resources that were successfully mutated + ResourcesMutatedCount int `json:"resourcesMutatedCount"` } // PolicyList is a list of Policy resources diff --git a/pkg/policy/status2.go b/pkg/policy/status2.go index 1ddfbaa5b7..6be44fcf58 100644 --- a/pkg/policy/status2.go +++ b/pkg/policy/status2.go @@ -4,6 +4,10 @@ import ( "sync" "time" + "github.com/nirmata/kyverno/pkg/policystore" + + "github.com/nirmata/kyverno/pkg/engine/response" + "k8s.io/apimachinery/pkg/util/wait" "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -11,97 +15,103 @@ import ( v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) +func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { + return &StatSync{ + cache: &statusCache{ + mu: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + }, + stop: stopCh, + client: client, + } +} + type statusCache struct { mu sync.RWMutex data map[string]v1.PolicyStatus } -func (c *statusCache) Get(key string) v1.PolicyStatus { - c.mu.RLock() - status := c.data[key] - c.mu.RUnlock() - return status - +type StatSync struct { + cache *statusCache + stop <-chan struct{} + client *versioned.Clientset + policyStore *policystore.PolicyStore } -func (c *statusCache) GetAll() map[string]v1.PolicyStatus { - c.mu.RLock() - mapCopy := make(map[string]v1.PolicyStatus, len(c.data)) - for k, v := range c.data { - mapCopy[k] = v - } - c.mu.RUnlock() - return mapCopy - -} -func (c *statusCache) Set(key string, status v1.PolicyStatus) { - c.mu.Lock() - c.data[key] = status - c.mu.Unlock() -} -func (c *statusCache) Clear() { - c.mu.Lock() - c.data = make(map[string]v1.PolicyStatus) - c.mu.Unlock() -} - -func newStatusCache() *statusCache { - return &statusCache{ - mu: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), - } -} - -func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}) *StatusSync { - return &StatusSync{ - policyStatsReciever: make(chan map[string]v1.PolicyStatus), - cache: newStatusCache(), - stop: stopCh, - client: client, - } -} - -type StatusSync struct { - policyStatsReciever chan map[string]v1.PolicyStatus - cache *statusCache - stop <-chan struct{} - client *versioned.Clientset -} - -func (s *StatusSync) Cache() *statusCache { - return s.cache -} - -func (s *StatusSync) StatReceiver() chan<- map[string]v1.PolicyStatus { - return s.policyStatsReciever -} - -func (s *StatusSync) Start() { - // receive status and store it in cache - go func() { - for { - select { - case nameToStatus := <-s.policyStatsReciever: - for policyName, status := range nameToStatus { - s.cache.Set(policyName, status) - } - case <-s.stop: - return - } - } - }() - +func (s *StatSync) Start() { // update policy status every 10 seconds - waits for previous updateStatus to complete - wait.Until(s.updateStatus, 10*time.Second, s.stop) + wait.Until(s.updateStats, 1*time.Second, s.stop) <-s.stop + s.updateStats() } -func (s *StatusSync) updateStatus() { - for policyName, status := range s.cache.GetAll() { +func (s *StatSync) updateStats() { + s.cache.mu.Lock() + for policyName, status := range s.cache.data { var policy = &v1.ClusterPolicy{} policy.Name = policyName policy.Status = status _, _ = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) } - s.cache.Clear() + s.cache.data = make(map[string]v1.PolicyStatus) + s.cache.mu.Unlock() +} + +func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) { + s.cache.mu.Lock() + policyStatus := s.cache.data[response.PolicyResponse.Policy] + s.policyStore.ListAll() + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + var policyAverageExecutionTime time.Duration + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + newAverageExecutionTime := updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver) + policyAverageExecutionTime += newAverageExecutionTime + ruleStat.ExecutionTime = newAverageExecutionTime.String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + policyStatus.ResourcesMutatedCount++ + ruleStat.AppliedCount++ + ruleStat.ResourcesMutatedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() +} + +func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { + if averageOver == 0 { + return newTime + } + oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) + numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() + denominator := averageOver + 1 + newAverageTimeInNanoSeconds := numerator / denominator + return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond } diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 5cd9c2689f..dbdb8552c7 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -3,8 +3,6 @@ package webhooks import ( "time" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine" @@ -59,8 +57,8 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou resource.GetKind(), resource.GetNamespace(), resource.GetName(), request.UID, request.Operation) policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) + go ws.status.UpdateStatusWithMutateStats(engineResponse) engineResponses = append(engineResponses, engineResponse) - updateStatusWithMutate(ws.status, policy, engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue @@ -108,29 +106,3 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou // patches holds all the successful patches, if no patch is created, it returns nil return engineutils.JoinPatches(patches) } - -func updateStatusWithMutate(statusSync *policy.StatusSync, policy kyverno.ClusterPolicy, response response.EngineResponse) { - stats := kyverno.PolicyStatus{ - ViolationCount: 0, - RulesAppliedCount: response.PolicyResponse.RulesAppliedCount, - ResourcesBlockedCount: 0, - AvgExecutionTimeMutation: response.PolicyResponse.ProcessingTime.String(), - Rules: nil, - } - - for _, rule := range response.PolicyResponse.Rules { - ruleStats := kyverno.RuleStats{ - Name: rule.Name, - ExecutionTime: rule.ProcessingTime.String(), - AppliedCount: 0, - ViolationCount: 0, - MutationCount: 0, - } - - if rule.Success { - ruleStats.AppliedCount++ - ruleStats.MutationCount++ - } - } - -} diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index f4a962dda2..27dbff78ed 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -56,7 +56,7 @@ type WebhookServer struct { // webhook registration client webhookRegistrationClient *webhookconfig.WebhookRegistrationClient // API to send policy stats for aggregation - status policy.StatusSync + status policy.StatSync // helpers to validate against current loaded configuration configHandler config.Interface // channel for cleanup notification @@ -83,7 +83,7 @@ func NewWebhookServer( crbInformer rbacinformer.ClusterRoleBindingInformer, eventGen event.Interface, webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, - status *policy.StatusSync, + status *policy.StatSync, configHandler config.Interface, pMetaStore policystore.LookupInterface, pvGenerator policyviolation.GeneratorInterface, From 3cfa70bbba8910518ba10f0ebb67913cf6b511f9 Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 22 Feb 2020 19:05:54 +0530 Subject: [PATCH 05/34] 527 untested prototype --- pkg/policy/status2.go | 139 ++++++++++++++++++++++++++++++--- pkg/policystore/policystore.go | 4 + 2 files changed, 130 insertions(+), 13 deletions(-) diff --git a/pkg/policy/status2.go b/pkg/policy/status2.go index 6be44fcf58..41b4c0a90a 100644 --- a/pkg/policy/status2.go +++ b/pkg/policy/status2.go @@ -15,17 +15,6 @@ import ( v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) -func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { - return &StatSync{ - cache: &statusCache{ - mu: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), - }, - stop: stopCh, - client: client, - } -} - type statusCache struct { mu sync.RWMutex data map[string]v1.PolicyStatus @@ -38,6 +27,18 @@ type StatSync struct { policyStore *policystore.PolicyStore } +func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { + return &StatSync{ + cache: &statusCache{ + mu: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + }, + stop: stopCh, + client: client, + policyStore: pMetaStore, + } +} + func (s *StatSync) Start() { // update policy status every 10 seconds - waits for previous updateStatus to complete wait.Until(s.updateStats, 1*time.Second, s.stop) @@ -59,8 +60,14 @@ func (s *StatSync) updateStats() { func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) { s.cache.mu.Lock() - policyStatus := s.cache.data[response.PolicyResponse.Policy] - s.policyStore.ListAll() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } var nameToRule = make(map[string]v1.RuleStats, 0) for _, rule := range policyStatus.Rules { @@ -105,6 +112,112 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) s.cache.mu.Unlock() } +func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineResponse) { + s.cache.mu.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + var policyAverageExecutionTime time.Duration + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + newAverageExecutionTime := updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver) + policyAverageExecutionTime += newAverageExecutionTime + ruleStat.ExecutionTime = newAverageExecutionTime.String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + policyStatus.ResourcesBlockedCount++ + ruleStat.AppliedCount++ + ruleStat.ResourcesBlockedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() +} + +func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineResponse) { + s.cache.mu.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + var policyAverageExecutionTime time.Duration + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + newAverageExecutionTime := updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver) + policyAverageExecutionTime += newAverageExecutionTime + ruleStat.ExecutionTime = newAverageExecutionTime.String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() +} + func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { if averageOver == 0 { return newTime diff --git a/pkg/policystore/policystore.go b/pkg/policystore/policystore.go index 3d23b106e4..f8a8e30874 100644 --- a/pkg/policystore/policystore.go +++ b/pkg/policystore/policystore.go @@ -96,6 +96,10 @@ func (ps *PolicyStore) ListAll() ([]kyverno.ClusterPolicy, error) { return policies, nil } +func (ps *PolicyStore) Get(policyName string) (*kyverno.ClusterPolicy, error) { + return ps.pLister.Get(policyName) +} + //UnRegister Remove policy information func (ps *PolicyStore) UnRegister(policy kyverno.ClusterPolicy) error { ps.mu.Lock() From 592df74c57aa44800bc1f368dcea6dbddc65d759 Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 22 Feb 2020 23:35:02 +0530 Subject: [PATCH 06/34] 527 tested mutate needs further testing --- cmd/kyverno/main.go | 5 +- pkg/api/kyverno/v1/types.go | 20 +- pkg/policy/apply.go | 6 +- pkg/policy/controller.go | 76 +------ pkg/policy/existing.go | 2 +- pkg/policy/status.go | 399 ++++++++++++++++++++---------------- pkg/policy/status2.go | 230 --------------------- pkg/webhooks/generation.go | 1 + pkg/webhooks/mutation.go | 6 +- pkg/webhooks/server.go | 6 +- pkg/webhooks/validation.go | 36 +--- 11 files changed, 249 insertions(+), 538 deletions(-) delete mode 100644 pkg/policy/status2.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index ae9b53ce57..ee6644e91c 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -200,6 +200,8 @@ func main() { glog.Fatalf("Failed registering Admission Webhooks: %v\n", err) } + statusSync := policy.NewStatusSync(pclient, stopCh, policyMetaStore) + // WEBHOOOK // - https server to provide endpoints called based on rules defined in Mutating & Validation webhook configuration // - reports the results based on the response from the policy engine: @@ -215,7 +217,7 @@ func main() { kubeInformer.Rbac().V1().ClusterRoleBindings(), egen, webhookRegistrationClient, - policy.NewStatusSync(pclient, stopCh, policyMetaStore), + statusSync, configData, policyMetaStore, pvgen, @@ -238,6 +240,7 @@ func main() { go grc.Run(1, stopCh) go grcc.Run(1, stopCh) go pvgen.Run(1, stopCh) + go statusSync.Run() // verifys if the admission control is enabled and active // resync: 60 seconds diff --git a/pkg/api/kyverno/v1/types.go b/pkg/api/kyverno/v1/types.go index 1f020492f5..fa29e4be83 100644 --- a/pkg/api/kyverno/v1/types.go +++ b/pkg/api/kyverno/v1/types.go @@ -232,15 +232,15 @@ type PolicyStatus struct { // average time required to process the policy rules on a resource AvgExecutionTime string `json:"averageExecutionTime"` // Count of rules that failed - ViolationCount int `json:"violationCount"` + ViolationCount int `json:"violationCount,omitempty"` // Count of rules that were applied - RulesAppliedCount int `json:"rulesAppliedCount"` + RulesAppliedCount int `json:"rulesAppliedCount,omitempty"` // Count of resources that were blocked for failing a validate, across all rules - ResourcesBlockedCount int `json:"resourcesBlockedCount"` + ResourcesBlockedCount int `json:"resourcesBlockedCount,omitempty"` // Count of resources that were successfully mutated, across all rules - ResourcesMutatedCount int `json:"resourcesMutatedCount"` + ResourcesMutatedCount int `json:"resourcesMutatedCount,omitempty"` - Rules []RuleStats `json:"ruleStatus"` + Rules []RuleStats `json:"ruleStatus,omitempty"` } //RuleStats provides status per rule @@ -248,15 +248,15 @@ type RuleStats struct { // Rule name Name string `json:"ruleName"` // average time require to process the rule - ExecutionTime string `json:"averageExecutionTime"` + ExecutionTime string `json:"averageExecutionTime,omitempty"` // Count of rules that failed - ViolationCount int `json:"violationCount"` + ViolationCount int `json:"violationCount,omitempty"` // Count of rules that were applied - AppliedCount int `json:"appliedCount"` + AppliedCount int `json:"appliedCount,omitempty"` // Count of resources for whom update/create api requests were blocked as the resource did not satisfy the policy rules - ResourcesBlockedCount int `json:"resourcesBlockedCount"` + ResourcesBlockedCount int `json:"resourcesBlockedCount,omitempty"` // Count of resources that were successfully mutated - ResourcesMutatedCount int `json:"resourcesMutatedCount"` + ResourcesMutatedCount int `json:"resourcesMutatedCount,omitempty"` } // PolicyList is a list of Policy resources diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go index 5b79cc4bec..32d654346c 100644 --- a/pkg/policy/apply.go +++ b/pkg/policy/apply.go @@ -19,7 +19,7 @@ import ( // applyPolicy applies policy on a resource //TODO: generation rules -func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, policyStatus PolicyStatusInterface) (responses []response.EngineResponse) { +func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructured) (responses []response.EngineResponse) { startTime := time.Now() glog.V(4).Infof("Started apply policy %s on resource %s/%s/%s (%v)", policy.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), startTime) @@ -35,7 +35,7 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure ctx.AddResource(transformResource(resource)) //MUTATION - engineResponse, err = mutation(policy, resource, policyStatus, ctx) + engineResponse, err = mutation(policy, resource, ctx) engineResponses = append(engineResponses, engineResponse) if err != nil { glog.Errorf("unable to process mutation rules: %v", err) @@ -48,7 +48,7 @@ func applyPolicy(policy kyverno.ClusterPolicy, resource unstructured.Unstructure //TODO: GENERATION return engineResponses } -func mutation(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, policyStatus PolicyStatusInterface, ctx context.EvalInterface) (response.EngineResponse, error) { +func mutation(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, ctx context.EvalInterface) (response.EngineResponse, error) { engineResponse := engine.Mutate(engine.PolicyContext{Policy: policy, NewResource: resource, Context: ctx}) if !engineResponse.IsSuccesful() { diff --git a/pkg/policy/controller.go b/pkg/policy/controller.go index 1b62db5671..2f643f2745 100644 --- a/pkg/policy/controller.go +++ b/pkg/policy/controller.go @@ -2,7 +2,6 @@ package policy import ( "fmt" - "reflect" "time" "github.com/golang/glog" @@ -68,8 +67,6 @@ type PolicyController struct { rm resourceManager // helpers to validate against current loaded configuration configHandler config.Interface - // receives stats and aggregates details - statusAggregator *PolicyStatusAggregator // store to hold policy meta data for faster lookup pMetaStore policystore.UpdateInterface // policy violation generator @@ -145,10 +142,6 @@ func NewPolicyController(kyvernoClient *kyvernoclient.Clientset, //TODO: pass the time in seconds instead of converting it internally pc.rm = NewResourceManager(30) - // aggregator - // pc.statusAggregator = NewPolicyStatAggregator(kyvernoClient, pInformer) - pc.statusAggregator = NewPolicyStatAggregator(kyvernoClient) - return &pc, nil } @@ -265,9 +258,6 @@ func (pc *PolicyController) Run(workers int, stopCh <-chan struct{}) { for i := 0; i < workers; i++ { go wait.Until(pc.worker, time.Second, stopCh) } - // policy status aggregator - //TODO: workers required for aggergation - pc.statusAggregator.Run(1, stopCh) <-stopCh } @@ -329,8 +319,6 @@ func (pc *PolicyController) syncPolicy(key string) error { if err := pc.deleteNamespacedPolicyViolations(key); err != nil { return err } - // remove the recorded stats for the policy - pc.statusAggregator.RemovePolicyStats(key) // remove webhook configurations if there are no policies if err := pc.removeResourceWebhookConfiguration(); err != nil { @@ -346,23 +334,12 @@ func (pc *PolicyController) syncPolicy(key string) error { pc.resourceWebhookWatcher.RegisterResourceWebhook() - // cluster policy violations - cpvList, err := pc.getClusterPolicyViolationForPolicy(policy.Name) - if err != nil { - return err - } - // namespaced policy violation - nspvList, err := pc.getNamespacedPolicyViolationForPolicy(policy.Name) - if err != nil { - return err - } - // process policies on existing resources engineResponses := pc.processExistingResources(*policy) // report errors pc.cleanupAndReport(engineResponses) - // sync active - return pc.syncStatusOnly(policy, cpvList, nspvList) + + return nil } func (pc *PolicyController) deleteClusterPolicyViolations(policy string) error { @@ -391,39 +368,6 @@ func (pc *PolicyController) deleteNamespacedPolicyViolations(policy string) erro return nil } -//syncStatusOnly updates the policy status subresource -func (pc *PolicyController) syncStatusOnly(p *kyverno.ClusterPolicy, pvList []*kyverno.ClusterPolicyViolation, nspvList []*kyverno.PolicyViolation) error { - newStatus := pc.calculateStatus(p.Name, pvList, nspvList) - if reflect.DeepEqual(newStatus, p.Status) { - // no update to status - return nil - } - // update status - newPolicy := p - newPolicy.Status = newStatus - _, err := pc.kyvernoClient.KyvernoV1().ClusterPolicies().UpdateStatus(newPolicy) - return err -} - -func (pc *PolicyController) calculateStatus(policyName string, pvList []*kyverno.ClusterPolicyViolation, nspvList []*kyverno.PolicyViolation) kyverno.PolicyStatus { - violationCount := len(pvList) + len(nspvList) - status := kyverno.PolicyStatus{ - ViolationCount: violationCount, - } - // get stats - stats := pc.statusAggregator.GetPolicyStats(policyName) - if !reflect.DeepEqual(stats, (PolicyStatInfo{})) { - status.RulesAppliedCount = stats.RulesAppliedCount - status.ResourcesBlockedCount = stats.ResourceBlocked - status.AvgExecutionTimeMutation = stats.MutationExecutionTime.String() - status.AvgExecutionTimeValidation = stats.ValidationExecutionTime.String() - status.AvgExecutionTimeGeneration = stats.GenerationExecutionTime.String() - // update rule stats - status.Rules = convertRules(stats.Rules) - } - return status -} - func (pc *PolicyController) getNamespacedPolicyViolationForPolicy(policy string) ([]*kyverno.PolicyViolation, error) { policySelector, err := buildPolicyLabel(policy) if err != nil { @@ -459,19 +403,3 @@ func (r RealPVControl) DeleteClusterPolicyViolation(name string) error { func (r RealPVControl) DeleteNamespacedPolicyViolation(ns, name string) error { return r.Client.KyvernoV1().PolicyViolations(ns).Delete(name, &metav1.DeleteOptions{}) } - -// convertRules converts the internal rule stats to one used in policy.stats struct -func convertRules(rules []RuleStatinfo) []kyverno.RuleStats { - var stats []kyverno.RuleStats - for _, r := range rules { - stat := kyverno.RuleStats{ - Name: r.RuleName, - ExecutionTime: r.ExecutionTime.String(), - AppliedCount: r.RuleAppliedCount, - ViolationCount: r.RulesFailedCount, - MutationCount: r.MutationCount, - } - stats = append(stats, stat) - } - return stats -} diff --git a/pkg/policy/existing.go b/pkg/policy/existing.go index 9165978ca4..97c09affac 100644 --- a/pkg/policy/existing.go +++ b/pkg/policy/existing.go @@ -39,7 +39,7 @@ func (pc *PolicyController) processExistingResources(policy kyverno.ClusterPolic // apply the policy on each glog.V(4).Infof("apply policy %s with resource version %s on resource %s/%s/%s with resource version %s", policy.Name, policy.ResourceVersion, resource.GetKind(), resource.GetNamespace(), resource.GetName(), resource.GetResourceVersion()) - engineResponse := applyPolicy(policy, resource, pc.statusAggregator) + engineResponse := applyPolicy(policy, resource) // get engine response for mutation & validation independently engineResponses = append(engineResponses, engineResponse...) // post-processing, register the resource as processed diff --git a/pkg/policy/status.go b/pkg/policy/status.go index 8d4beb4f02..2f55fd252b 100644 --- a/pkg/policy/status.go +++ b/pkg/policy/status.go @@ -1,210 +1,249 @@ package policy import ( + "log" "sync" "time" - "github.com/golang/glog" - kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "github.com/nirmata/kyverno/pkg/policystore" + + "github.com/nirmata/kyverno/pkg/engine/response" + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/nirmata/kyverno/pkg/client/clientset/versioned" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) -//PolicyStatusAggregator stores information abt aggregation -type PolicyStatusAggregator struct { - // time since we start aggregating the stats - startTime time.Time - // channel to receive stats - ch chan PolicyStat - //TODO: lock based on key, possibly sync.Map ? - //sync RW for policyData - mux sync.RWMutex - // stores aggregated stats for policy - policyData map[string]PolicyStatInfo +type statusCache struct { + mu sync.RWMutex + data map[string]v1.PolicyStatus } -//NewPolicyStatAggregator returns a new policy status -func NewPolicyStatAggregator(client *kyvernoclient.Clientset) *PolicyStatusAggregator { - psa := PolicyStatusAggregator{ - startTime: time.Now(), - ch: make(chan PolicyStat), - policyData: map[string]PolicyStatInfo{}, - } - return &psa +type StatSync struct { + cache *statusCache + stop <-chan struct{} + client *versioned.Clientset + policyStore *policystore.PolicyStore } -//Run begins aggregator -func (psa *PolicyStatusAggregator) Run(workers int, stopCh <-chan struct{}) { - defer utilruntime.HandleCrash() - glog.V(4).Info("Started aggregator for policy status stats") - defer func() { - glog.V(4).Info("Shutting down aggregator for policy status stats") - }() - for i := 0; i < workers; i++ { - go wait.Until(psa.process, time.Second, stopCh) - } - <-stopCh -} - -func (psa *PolicyStatusAggregator) process() { - // As mutation and validation are handled separately - // ideally we need to combine the execution time from both for a policy - // but its tricky to detect here the type of rules policy contains - // so we dont combine the results, but instead compute the execution time for - // mutation & validation rules separately - for r := range psa.ch { - glog.V(4).Infof("received policy stats %v", r) - psa.aggregate(r) +func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { + return &StatSync{ + cache: &statusCache{ + mu: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + }, + stop: stopCh, + client: client, + policyStore: pMetaStore, } } -func (psa *PolicyStatusAggregator) aggregate(ps PolicyStat) { - func() { - glog.V(4).Infof("write lock update policy %s", ps.PolicyName) - psa.mux.Lock() - }() - defer func() { - glog.V(4).Infof("write Unlock update policy %s", ps.PolicyName) - psa.mux.Unlock() - }() - - if len(ps.Stats.Rules) == 0 { - glog.V(4).Infof("ignoring stats, as no rule was applied") - return - } - - info, ok := psa.policyData[ps.PolicyName] - if !ok { - psa.policyData[ps.PolicyName] = ps.Stats - glog.V(4).Infof("added stats for policy %s", ps.PolicyName) - return - } - // aggregate policy information - info.RulesAppliedCount = info.RulesAppliedCount + ps.Stats.RulesAppliedCount - if ps.Stats.ResourceBlocked == 1 { - info.ResourceBlocked++ - } - var zeroDuration time.Duration - if info.MutationExecutionTime != zeroDuration { - info.MutationExecutionTime = (info.MutationExecutionTime + ps.Stats.MutationExecutionTime) / 2 - glog.V(4).Infof("updated avg mutation time %v", info.MutationExecutionTime) - } else { - info.MutationExecutionTime = ps.Stats.MutationExecutionTime - } - if info.ValidationExecutionTime != zeroDuration { - info.ValidationExecutionTime = (info.ValidationExecutionTime + ps.Stats.ValidationExecutionTime) / 2 - glog.V(4).Infof("updated avg validation time %v", info.ValidationExecutionTime) - } else { - info.ValidationExecutionTime = ps.Stats.ValidationExecutionTime - } - if info.GenerationExecutionTime != zeroDuration { - info.GenerationExecutionTime = (info.GenerationExecutionTime + ps.Stats.GenerationExecutionTime) / 2 - glog.V(4).Infof("updated avg generation time %v", info.GenerationExecutionTime) - } else { - info.GenerationExecutionTime = ps.Stats.GenerationExecutionTime - } - // aggregate rule details - info.Rules = aggregateRules(info.Rules, ps.Stats.Rules) - // update - psa.policyData[ps.PolicyName] = info - glog.V(4).Infof("updated stats for policy %s", ps.PolicyName) +func (s *StatSync) Run() { + // update policy status every 10 seconds - waits for previous updateStatus to complete + wait.Until(s.updateStats, 1*time.Second, s.stop) + <-s.stop + s.updateStats() } -func aggregateRules(old []RuleStatinfo, update []RuleStatinfo) []RuleStatinfo { - var zeroDuration time.Duration - searchRule := func(list []RuleStatinfo, key string) *RuleStatinfo { - for _, v := range list { - if v.RuleName == key { - return &v - } +func (s *StatSync) updateStats() { + s.cache.mu.Lock() + var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) + for k, v := range s.cache.data { + nameToStatus[k] = v + } + s.cache.mu.Unlock() + + for policyName, status := range nameToStatus { + var policy = &v1.ClusterPolicy{} + policy, err := s.policyStore.Get(policyName) + if err != nil { + continue } - return nil - } - newRules := []RuleStatinfo{} - // search for new rules in old rules and update it - for _, updateR := range update { - if updateR.ExecutionTime != zeroDuration { - if rule := searchRule(old, updateR.RuleName); rule != nil { - rule.ExecutionTime = (rule.ExecutionTime + updateR.ExecutionTime) / 2 - rule.RuleAppliedCount = rule.RuleAppliedCount + updateR.RuleAppliedCount - rule.RulesFailedCount = rule.RulesFailedCount + updateR.RulesFailedCount - rule.MutationCount = rule.MutationCount + updateR.MutationCount - newRules = append(newRules, *rule) - } else { - newRules = append(newRules, updateR) - } + policy.Status = status + _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) + if err != nil { + log.Println(err) } } - return newRules } -//GetPolicyStats returns the policy stats -func (psa *PolicyStatusAggregator) GetPolicyStats(policyName string) PolicyStatInfo { - func() { - glog.V(4).Infof("read lock update policy %s", policyName) - psa.mux.RLock() - }() - defer func() { - glog.V(4).Infof("read Unlock update policy %s", policyName) - psa.mux.RUnlock() - }() - glog.V(4).Infof("read stats for policy %s", policyName) - return psa.policyData[policyName] +func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) { + s.cache.mu.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + policyStatus.ResourcesMutatedCount++ + ruleStat.AppliedCount++ + ruleStat.ResourcesMutatedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() } -//RemovePolicyStats rmves policy stats records -func (psa *PolicyStatusAggregator) RemovePolicyStats(policyName string) { - func() { - glog.V(4).Infof("write lock update policy %s", policyName) - psa.mux.Lock() - }() - defer func() { - glog.V(4).Infof("write Unlock update policy %s", policyName) - psa.mux.Unlock() - }() - glog.V(4).Infof("removing stats for policy %s", policyName) - delete(psa.policyData, policyName) +func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineResponse) { + s.cache.mu.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + ruleStat.AppliedCount++ + if response.PolicyResponse.ValidationFailureAction == "enforce" { + policyStatus.ResourcesBlockedCount++ + ruleStat.ResourcesBlockedCount++ + } + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() } -//PolicyStatusInterface provides methods to modify policyStatus -type PolicyStatusInterface interface { - SendStat(stat PolicyStat) - // UpdateViolationCount(policyName string, pvList []*kyverno.PolicyViolation) error +func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineResponse) { + s.cache.mu.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + if !exist { + policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range response.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.mu.Unlock() } -//PolicyStat stored stats for policy -type PolicyStat struct { - PolicyName string - Stats PolicyStatInfo -} - -//PolicyStatInfo provides statistics for policy -type PolicyStatInfo struct { - MutationExecutionTime time.Duration - ValidationExecutionTime time.Duration - GenerationExecutionTime time.Duration - RulesAppliedCount int - ResourceBlocked int - Rules []RuleStatinfo -} - -//RuleStatinfo provides statistics for rule -type RuleStatinfo struct { - RuleName string - ExecutionTime time.Duration - RuleAppliedCount int - RulesFailedCount int - MutationCount int -} - -//SendStat sends the stat information for aggregation -func (psa *PolicyStatusAggregator) SendStat(stat PolicyStat) { - glog.V(4).Infof("sending policy stats: %v", stat) - // Send over channel - psa.ch <- stat -} - -//GetPolicyStatusAggregator returns interface to send policy status stats -func (pc *PolicyController) GetPolicyStatusAggregator() PolicyStatusInterface { - return pc.statusAggregator +func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { + if averageOver == 0 { + return newTime + } + oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) + numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() + denominator := averageOver + 1 + newAverageTimeInNanoSeconds := numerator / denominator + return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond } diff --git a/pkg/policy/status2.go b/pkg/policy/status2.go deleted file mode 100644 index 41b4c0a90a..0000000000 --- a/pkg/policy/status2.go +++ /dev/null @@ -1,230 +0,0 @@ -package policy - -import ( - "sync" - "time" - - "github.com/nirmata/kyverno/pkg/policystore" - - "github.com/nirmata/kyverno/pkg/engine/response" - - "k8s.io/apimachinery/pkg/util/wait" - - "github.com/nirmata/kyverno/pkg/client/clientset/versioned" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" -) - -type statusCache struct { - mu sync.RWMutex - data map[string]v1.PolicyStatus -} - -type StatSync struct { - cache *statusCache - stop <-chan struct{} - client *versioned.Clientset - policyStore *policystore.PolicyStore -} - -func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { - return &StatSync{ - cache: &statusCache{ - mu: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), - }, - stop: stopCh, - client: client, - policyStore: pMetaStore, - } -} - -func (s *StatSync) Start() { - // update policy status every 10 seconds - waits for previous updateStatus to complete - wait.Until(s.updateStats, 1*time.Second, s.stop) - <-s.stop - s.updateStats() -} - -func (s *StatSync) updateStats() { - s.cache.mu.Lock() - for policyName, status := range s.cache.data { - var policy = &v1.ClusterPolicy{} - policy.Name = policyName - policy.Status = status - _, _ = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) - } - s.cache.data = make(map[string]v1.PolicyStatus) - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) { - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - var policyAverageExecutionTime time.Duration - for _, rule := range response.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) - newAverageExecutionTime := updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver) - policyAverageExecutionTime += newAverageExecutionTime - ruleStat.ExecutionTime = newAverageExecutionTime.String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - policyStatus.ResourcesMutatedCount++ - ruleStat.AppliedCount++ - ruleStat.ResourcesMutatedCount++ - } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - ruleStats = append(ruleStats, ruleStat) - } - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[response.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineResponse) { - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - var policyAverageExecutionTime time.Duration - for _, rule := range response.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) - newAverageExecutionTime := updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver) - policyAverageExecutionTime += newAverageExecutionTime - ruleStat.ExecutionTime = newAverageExecutionTime.String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - policyStatus.ResourcesBlockedCount++ - ruleStat.AppliedCount++ - ruleStat.ResourcesBlockedCount++ - } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - ruleStats = append(ruleStats, ruleStat) - } - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[response.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineResponse) { - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - var policyAverageExecutionTime time.Duration - for _, rule := range response.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) - newAverageExecutionTime := updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver) - policyAverageExecutionTime += newAverageExecutionTime - ruleStat.ExecutionTime = newAverageExecutionTime.String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - ruleStat.AppliedCount++ - } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - ruleStats = append(ruleStats, ruleStat) - } - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[response.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { - if averageOver == 0 { - return newTime - } - oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) - numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() - denominator := averageOver + 1 - newAverageTimeInNanoSeconds := numerator / denominator - return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond -} diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 2dddf71e1c..4f8a087977 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -58,6 +58,7 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic for _, policy := range policies { policyContext.Policy = policy engineResponse := engine.Generate(policyContext) + go ws.status.UpdateStatusWithGenerateStats(engineResponse) if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index dbdb8552c7..90bb14ccb9 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -1,6 +1,7 @@ package webhooks import ( + "log" "time" "github.com/golang/glog" @@ -56,13 +57,16 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou glog.V(2).Infof("Handling mutation for Kind=%s, Namespace=%s Name=%s UID=%s patchOperation=%s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), request.UID, request.Operation) policyContext.Policy = policy + if resource.GetKind() == "Pod" { + log.Println("some") + } engineResponse := engine.Mutate(policyContext) - go ws.status.UpdateStatusWithMutateStats(engineResponse) engineResponses = append(engineResponses, engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue } + go ws.status.UpdateStatusWithMutateStats(engineResponse) // gather patches patches = append(patches, engineResponse.GetPatches()...) glog.V(4).Infof("Mutation from policy %s has applied successfully to %s %s/%s", policy.Name, request.Kind.Kind, resource.GetNamespace(), resource.GetName()) diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 27dbff78ed..6e51fd692b 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -56,7 +56,7 @@ type WebhookServer struct { // webhook registration client webhookRegistrationClient *webhookconfig.WebhookRegistrationClient // API to send policy stats for aggregation - status policy.StatSync + status *policy.StatSync // helpers to validate against current loaded configuration configHandler config.Interface // channel for cleanup notification @@ -83,7 +83,7 @@ func NewWebhookServer( crbInformer rbacinformer.ClusterRoleBindingInformer, eventGen event.Interface, webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, - status *policy.StatSync, + statusSync *policy.StatSync, configHandler config.Interface, pMetaStore policystore.LookupInterface, pvGenerator policyviolation.GeneratorInterface, @@ -113,7 +113,7 @@ func NewWebhookServer( crbSynced: crbInformer.Informer().HasSynced, eventGen: eventGen, webhookRegistrationClient: webhookRegistrationClient, - status: status, + status: statusSync, configHandler: configHandler, cleanUp: cleanUp, lastReqTime: resourceWebhookWatcher.LastReqTime, diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index c67a012ea2..af397d2c17 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -9,9 +9,7 @@ import ( "github.com/nirmata/kyverno/pkg/engine" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" - policyctr "github.com/nirmata/kyverno/pkg/policy" "github.com/nirmata/kyverno/pkg/policyviolation" - "github.com/nirmata/kyverno/pkg/utils" v1beta1 "k8s.io/api/admission/v1beta1" ) @@ -22,36 +20,7 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol glog.V(4).Infof("Receive request in validating webhook: Kind=%s, Namespace=%s Name=%s UID=%s patchOperation=%s", request.Kind.Kind, request.Namespace, request.Name, request.UID, request.Operation) - var policyStats []policyctr.PolicyStat evalTime := time.Now() - // gather stats from the engine response - gatherStat := func(policyName string, policyResponse response.PolicyResponse) { - ps := policyctr.PolicyStat{} - ps.PolicyName = policyName - ps.Stats.ValidationExecutionTime = policyResponse.ProcessingTime - ps.Stats.RulesAppliedCount = policyResponse.RulesAppliedCount - // capture rule level stats - for _, rule := range policyResponse.Rules { - rs := policyctr.RuleStatinfo{} - rs.RuleName = rule.Name - rs.ExecutionTime = rule.RuleStats.ProcessingTime - if rule.Success { - rs.RuleAppliedCount++ - } else { - rs.RulesFailedCount++ - } - ps.Stats.Rules = append(ps.Stats.Rules, rs) - } - policyStats = append(policyStats, ps) - } - // send stats for aggregation - sendStat := func(blocked bool) { - for _, stat := range policyStats { - stat.Stats.ResourceBlocked = utils.Btoi(blocked) - //SEND - ws.policyStatus.SendStat(stat) - } - } // Get new and old resource newR, oldR, err := extractResources(patchedResource, request) @@ -100,12 +69,11 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) - // Gather policy application statistics - gatherStat(policy.Name, engineResponse.PolicyResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue } + go ws.status.UpdateStatusWithValidateStats(engineResponse) } glog.V(4).Infof("eval: %v %s/%s/%s ", time.Since(evalTime), request.Kind, request.Namespace, request.Name) // report time @@ -117,7 +85,6 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol blocked := toBlockResource(engineResponses) if blocked { glog.V(4).Infof("resource %s/%s/%s is blocked\n", newR.GetKind(), newR.GetNamespace(), newR.GetName()) - sendStat(true) return false, getEnforceFailureErrorMsg(engineResponses) } @@ -128,7 +95,6 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol // ADD EVENTS events := generateEvents(engineResponses, (request.Operation == v1beta1.Update)) ws.eventGen.Add(events...) - sendStat(false) // report time end glog.V(4).Infof("report: %v %s/%s/%s", time.Since(reportTime), request.Kind, request.Namespace, request.Name) return true, "" From ac37ec66f083ed60732a14108afcbee925a1b016 Mon Sep 17 00:00:00 2001 From: shravan Date: Sun, 23 Feb 2020 20:40:00 +0530 Subject: [PATCH 07/34] 527 minor fixes --- pkg/policy/status.go | 19 ++++++++++++++++--- pkg/webhooks/generation.go | 1 - pkg/webhooks/mutation.go | 2 +- pkg/webhooks/validation.go | 2 +- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/policy/status.go b/pkg/policy/status.go index 2f55fd252b..a3061629e2 100644 --- a/pkg/policy/status.go +++ b/pkg/policy/status.go @@ -2,6 +2,7 @@ package policy import ( "log" + "sort" "sync" "time" @@ -118,6 +119,10 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) ruleStats = append(ruleStats, ruleStat) } + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats @@ -154,13 +159,13 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons if rule.Success { policyStatus.RulesAppliedCount++ ruleStat.AppliedCount++ + } else { + policyStatus.ViolationCount++ + ruleStat.ViolationCount++ if response.PolicyResponse.ValidationFailureAction == "enforce" { policyStatus.ResourcesBlockedCount++ ruleStat.ResourcesBlockedCount++ } - } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ } nameToRule[rule.Name] = ruleStat @@ -176,6 +181,10 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons ruleStats = append(ruleStats, ruleStat) } + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats @@ -230,6 +239,10 @@ func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineRespons ruleStats = append(ruleStats, ruleStat) } + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 4f8a087977..2dddf71e1c 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -58,7 +58,6 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic for _, policy := range policies { policyContext.Policy = policy engineResponse := engine.Generate(policyContext) - go ws.status.UpdateStatusWithGenerateStats(engineResponse) if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 90bb14ccb9..e2dd090caa 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -62,11 +62,11 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou } engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) + go ws.status.UpdateStatusWithMutateStats(engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue } - go ws.status.UpdateStatusWithMutateStats(engineResponse) // gather patches patches = append(patches, engineResponse.GetPatches()...) glog.V(4).Infof("Mutation from policy %s has applied successfully to %s %s/%s", policy.Name, request.Kind.Kind, resource.GetNamespace(), resource.GetName()) diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index af397d2c17..41761dd071 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -69,11 +69,11 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) + go ws.status.UpdateStatusWithValidateStats(engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue } - go ws.status.UpdateStatusWithValidateStats(engineResponse) } glog.V(4).Infof("eval: %v %s/%s/%s ", time.Since(evalTime), request.Kind, request.Namespace, request.Name) // report time From d758a4ad4539ac8d007dad9deaf7dc8fb616f784 Mon Sep 17 00:00:00 2001 From: shravan Date: Sun, 23 Feb 2020 23:24:18 +0530 Subject: [PATCH 08/34] 527 added accurate violation Count --- cmd/kyverno/main.go | 7 ++- pkg/api/kyverno/v1/types.go | 8 +++- pkg/policy/status.go | 86 ++++++++++++++++++++++++++++++++----- 3 files changed, 88 insertions(+), 13 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index ee6644e91c..812337ac9b 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -200,7 +200,12 @@ func main() { glog.Fatalf("Failed registering Admission Webhooks: %v\n", err) } - statusSync := policy.NewStatusSync(pclient, stopCh, policyMetaStore) + statusSync := policy.NewStatusSync( + pclient, + stopCh, + policyMetaStore, + pInformer.Kyverno().V1().ClusterPolicyViolations().Lister(), + pInformer.Kyverno().V1().PolicyViolations().Lister()) // WEBHOOOK // - https server to provide endpoints called based on rules defined in Mutating & Validation webhook configuration diff --git a/pkg/api/kyverno/v1/types.go b/pkg/api/kyverno/v1/types.go index fa29e4be83..22e5f54468 100644 --- a/pkg/api/kyverno/v1/types.go +++ b/pkg/api/kyverno/v1/types.go @@ -231,8 +231,10 @@ type CloneFrom struct { type PolicyStatus struct { // average time required to process the policy rules on a resource AvgExecutionTime string `json:"averageExecutionTime"` - // Count of rules that failed + // number of violations related to the policy ViolationCount int `json:"violationCount,omitempty"` + // Count of rules that failed + RulesFailedCount int `json:"rulesFailedCount,omitempty"` // Count of rules that were applied RulesAppliedCount int `json:"rulesAppliedCount,omitempty"` // Count of resources that were blocked for failing a validate, across all rules @@ -249,8 +251,10 @@ type RuleStats struct { Name string `json:"ruleName"` // average time require to process the rule ExecutionTime string `json:"averageExecutionTime,omitempty"` - // Count of rules that failed + // number of violations related to this rule ViolationCount int `json:"violationCount,omitempty"` + // Count of rules that failed + FailedCount int `json:"failedCount,omitempty"` // Count of rules that were applied AppliedCount int `json:"appliedCount,omitempty"` // Count of resources for whom update/create api requests were blocked as the resource did not satisfy the policy rules diff --git a/pkg/policy/status.go b/pkg/policy/status.go index a3061629e2..a46fd74857 100644 --- a/pkg/policy/status.go +++ b/pkg/policy/status.go @@ -6,6 +6,8 @@ import ( "sync" "time" + v12 "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" + "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/engine/response" @@ -27,9 +29,17 @@ type StatSync struct { stop <-chan struct{} client *versioned.Clientset policyStore *policystore.PolicyStore + cpvLister v12.ClusterPolicyViolationLister + pvLister v12.PolicyViolationLister } -func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore) *StatSync { +func NewStatusSync( + client *versioned.Clientset, + stopCh <-chan struct{}, + pMetaStore *policystore.PolicyStore, + cpvLister v12.ClusterPolicyViolationLister, + pvLister v12.PolicyViolationLister, +) *StatSync { return &StatSync{ cache: &statusCache{ mu: sync.RWMutex{}, @@ -38,6 +48,8 @@ func NewStatusSync(client *versioned.Clientset, stopCh <-chan struct{}, pMetaSto stop: stopCh, client: client, policyStore: pMetaStore, + cpvLister: cpvLister, + pvLister: pvLister, } } @@ -57,6 +69,10 @@ func (s *StatSync) updateStats() { s.cache.mu.Unlock() for policyName, status := range nameToStatus { + cpvList, _ := s.getClusterPolicyViolationForPolicy(policyName) + pvList, _ := s.getNamespacedPolicyViolationForPolicy(policyName) + updateStatusWithViolationCount(&status, cpvList, pvList) + var policy = &v1.ClusterPolicy{} policy, err := s.policyStore.Get(policyName) if err != nil { @@ -90,7 +106,7 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) ruleStat.ExecutionTime = updateAverageTime( rule.ProcessingTime, ruleStat.ExecutionTime, @@ -102,8 +118,8 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) ruleStat.AppliedCount++ ruleStat.ResourcesMutatedCount++ } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ } nameToRule[rule.Name] = ruleStat @@ -150,7 +166,7 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) ruleStat.ExecutionTime = updateAverageTime( rule.ProcessingTime, ruleStat.ExecutionTime, @@ -160,8 +176,8 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons policyStatus.RulesAppliedCount++ ruleStat.AppliedCount++ } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ if response.PolicyResponse.ValidationFailureAction == "enforce" { policyStatus.ResourcesBlockedCount++ ruleStat.ResourcesBlockedCount++ @@ -212,7 +228,7 @@ func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineRespons ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name - averageOver := int64(ruleStat.AppliedCount + ruleStat.ViolationCount) + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) ruleStat.ExecutionTime = updateAverageTime( rule.ProcessingTime, ruleStat.ExecutionTime, @@ -222,8 +238,8 @@ func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineRespons policyStatus.RulesAppliedCount++ ruleStat.AppliedCount++ } else { - policyStatus.ViolationCount++ - ruleStat.ViolationCount++ + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ } nameToRule[rule.Name] = ruleStat @@ -260,3 +276,53 @@ func updateAverageTime(newTime time.Duration, oldAverageTimeString string, avera newAverageTimeInNanoSeconds := numerator / denominator return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond } + +func (s *StatSync) getClusterPolicyViolationForPolicy(policy string) ([]*v1.ClusterPolicyViolation, error) { + policySelector, err := buildPolicyLabel(policy) + if err != nil { + return nil, err + } + // Get List of cluster policy violation + cpvList, err := s.cpvLister.List(policySelector) + if err != nil { + return nil, err + } + return cpvList, nil +} + +func (s *StatSync) getNamespacedPolicyViolationForPolicy(policy string) ([]*v1.PolicyViolation, error) { + policySelector, err := buildPolicyLabel(policy) + if err != nil { + return nil, err + } + // Get List of cluster policy violation + nspvList, err := s.pvLister.List(policySelector) + if err != nil { + return nil, err + } + return nspvList, nil + +} + +func updateStatusWithViolationCount(status *v1.PolicyStatus, cpvList []*v1.ClusterPolicyViolation, pvList []*v1.PolicyViolation) { + + status.ViolationCount = len(cpvList) + len(pvList) + + var ruleNameToNumberOfViolations = make(map[string]int) + + for _, cpv := range cpvList { + for _, violatedRule := range cpv.Spec.ViolatedRules { + ruleNameToNumberOfViolations[violatedRule.Name]++ + } + } + + for _, pv := range pvList { + for _, violatedRule := range pv.Spec.ViolatedRules { + ruleNameToNumberOfViolations[violatedRule.Name]++ + } + } + + for i, rule := range status.Rules { + status.Rules[i].ViolationCount = ruleNameToNumberOfViolations[rule.Name] + } +} From d080aa18ce91bcb85236e895f66d22c0138b0046 Mon Sep 17 00:00:00 2001 From: shravan Date: Mon, 24 Feb 2020 20:12:39 +0530 Subject: [PATCH 09/34] 527 prototype changes to handle generate stats - also changes made to handle stats such as violation count and generated resources count - currently untested --- cmd/kyverno/main.go | 17 ++-- pkg/api/kyverno/v1/types.go | 8 +- pkg/engine/generation.go | 12 ++- pkg/generate/controller.go | 5 +- pkg/generate/status.go | 6 +- pkg/namespace/generation.go | 36 -------- pkg/policy/status.go | 135 ++++++++++++---------------- pkg/policyviolation/clusterpv.go | 9 ++ pkg/policyviolation/generator.go | 17 ++-- pkg/policyviolation/namespacedpv.go | 9 ++ pkg/webhooks/generation.go | 1 + 11 files changed, 121 insertions(+), 134 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 812337ac9b..68d1bbd857 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -135,12 +135,19 @@ func main() { client, pInformer.Kyverno().V1().ClusterPolicies()) + // Policy Status Handler - deals with all logic related to policy status + statusSync := policy.NewStatusSync( + pclient, + stopCh, + policyMetaStore) + // POLICY VIOLATION GENERATOR // -- generate policy violation pvgen := policyviolation.NewPVGenerator(pclient, client, pInformer.Kyverno().V1().ClusterPolicyViolations(), - pInformer.Kyverno().V1().PolicyViolations()) + pInformer.Kyverno().V1().PolicyViolations(), + statusSync) // POLICY CONTROLLER // - reconciliation policy and policy violation @@ -174,6 +181,7 @@ func main() { egen, pvgen, kubedynamicInformer, + statusSync, ) // GENERATE REQUEST CLEANUP // -- cleans up the generate requests that have not been processed(i.e. state = [Pending, Failed]) for more than defined timeout @@ -200,13 +208,6 @@ func main() { glog.Fatalf("Failed registering Admission Webhooks: %v\n", err) } - statusSync := policy.NewStatusSync( - pclient, - stopCh, - policyMetaStore, - pInformer.Kyverno().V1().ClusterPolicyViolations().Lister(), - pInformer.Kyverno().V1().PolicyViolations().Lister()) - // WEBHOOOK // - https server to provide endpoints called based on rules defined in Mutating & Validation webhook configuration // - reports the results based on the response from the policy engine: diff --git a/pkg/api/kyverno/v1/types.go b/pkg/api/kyverno/v1/types.go index 22e5f54468..84a3eb486c 100644 --- a/pkg/api/kyverno/v1/types.go +++ b/pkg/api/kyverno/v1/types.go @@ -231,7 +231,7 @@ type CloneFrom struct { type PolicyStatus struct { // average time required to process the policy rules on a resource AvgExecutionTime string `json:"averageExecutionTime"` - // number of violations related to the policy + // number of violations created by this policy ViolationCount int `json:"violationCount,omitempty"` // Count of rules that failed RulesFailedCount int `json:"rulesFailedCount,omitempty"` @@ -241,6 +241,8 @@ type PolicyStatus struct { ResourcesBlockedCount int `json:"resourcesBlockedCount,omitempty"` // Count of resources that were successfully mutated, across all rules ResourcesMutatedCount int `json:"resourcesMutatedCount,omitempty"` + // Count of resources that were successfully generated, across all rules + ResourcesGeneratedCount int `json:"resourcesGeneratedCount,omitempty"` Rules []RuleStats `json:"ruleStatus,omitempty"` } @@ -251,7 +253,7 @@ type RuleStats struct { Name string `json:"ruleName"` // average time require to process the rule ExecutionTime string `json:"averageExecutionTime,omitempty"` - // number of violations related to this rule + // number of violations created by this rule ViolationCount int `json:"violationCount,omitempty"` // Count of rules that failed FailedCount int `json:"failedCount,omitempty"` @@ -261,6 +263,8 @@ type RuleStats struct { ResourcesBlockedCount int `json:"resourcesBlockedCount,omitempty"` // Count of resources that were successfully mutated ResourcesMutatedCount int `json:"resourcesMutatedCount,omitempty"` + // Count of resources that were successfully generated + ResourcesGeneratedCount int `json:"resourcesGeneratedCount,omitempty"` } // PolicyList is a list of Policy resources diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go index 651943fc32..7b02fb3fa3 100644 --- a/pkg/engine/generation.go +++ b/pkg/engine/generation.go @@ -2,6 +2,7 @@ package engine import ( "fmt" + "time" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -29,6 +30,9 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission if !rule.HasGenerate() { return nil } + + startTime := time.Now() + if !rbac.MatchAdmissionInfo(rule, admissionInfo) { return nil } @@ -43,8 +47,12 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission } // build rule Response return &response.RuleResponse{ - Name: rule.Name, - Type: "Generation", + Name: rule.Name, + Type: "Generation", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Since(startTime), + }, } } diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index 944b8b49cb..31e12a68c4 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -4,6 +4,8 @@ import ( "fmt" "time" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -68,6 +70,7 @@ func NewController( eventGen event.Interface, pvGenerator policyviolation.GeneratorInterface, dynamicInformer dynamicinformer.DynamicSharedInformerFactory, + policyStatus *policy.StatSync, ) *Controller { c := Controller{ client: client, @@ -79,7 +82,7 @@ func NewController( queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(1, 30), "generate-request"), dynamicInformer: dynamicInformer, } - c.statusControl = StatusControl{client: kyvernoclient} + c.statusControl = StatusControl{client: kyvernoclient, policyStatus: policyStatus} pInformer.Informer().AddEventHandlerWithResyncPeriod(cache.ResourceEventHandlerFuncs{ UpdateFunc: c.updatePolicy, // We only handle updates to policy diff --git a/pkg/generate/status.go b/pkg/generate/status.go index 70d9539053..026a657457 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -4,6 +4,7 @@ import ( "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" + "github.com/nirmata/kyverno/pkg/policy" ) //StatusControlInterface provides interface to update status subresource @@ -14,7 +15,8 @@ type StatusControlInterface interface { // StatusControl is default implementaation of GRStatusControlInterface type StatusControl struct { - client kyvernoclient.Interface + client kyvernoclient.Interface + policyStatus *policy.StatSync } //Failed sets gr status.state to failed with message @@ -39,6 +41,8 @@ func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyver // Update Generated Resources gr.Status.GeneratedResources = genResources + go sc.policyStatus.UpdatePolicyStatusWithGeneratedResourceCount(gr) + _, err := sc.client.KyvernoV1().GenerateRequests("kyverno").UpdateStatus(&gr) if err != nil { glog.V(4).Infof("FAILED: updated gr %s status to %s", gr.Name, string(kyverno.Completed)) diff --git a/pkg/namespace/generation.go b/pkg/namespace/generation.go index 7ce302a046..efca33e9bf 100644 --- a/pkg/namespace/generation.go +++ b/pkg/namespace/generation.go @@ -12,7 +12,6 @@ import ( "github.com/nirmata/kyverno/pkg/engine/response" policyctr "github.com/nirmata/kyverno/pkg/policy" "github.com/nirmata/kyverno/pkg/policystore" - "github.com/nirmata/kyverno/pkg/utils" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" @@ -188,36 +187,6 @@ func listpolicies(ns unstructured.Unstructured, pMetaStore policystore.LookupInt } func applyPolicy(client *client.Client, resource unstructured.Unstructured, p kyverno.ClusterPolicy, policyStatus policyctr.PolicyStatusInterface) response.EngineResponse { - var policyStats []policyctr.PolicyStat - // gather stats from the engine response - gatherStat := func(policyName string, policyResponse response.PolicyResponse) { - ps := policyctr.PolicyStat{} - ps.PolicyName = policyName - ps.Stats.GenerationExecutionTime = policyResponse.ProcessingTime - ps.Stats.RulesAppliedCount = policyResponse.RulesAppliedCount - // capture rule level stats - for _, rule := range policyResponse.Rules { - rs := policyctr.RuleStatinfo{} - rs.RuleName = rule.Name - rs.ExecutionTime = rule.RuleStats.ProcessingTime - if rule.Success { - rs.RuleAppliedCount++ - } else { - rs.RulesFailedCount++ - } - ps.Stats.Rules = append(ps.Stats.Rules, rs) - } - policyStats = append(policyStats, ps) - } - // send stats for aggregation - sendStat := func(blocked bool) { - for _, stat := range policyStats { - stat.Stats.ResourceBlocked = utils.Btoi(blocked) - //SEND - policyStatus.SendStat(stat) - } - } - startTime := time.Now() glog.V(4).Infof("Started apply policy %s on resource %s/%s/%s (%v)", p.Name, resource.GetKind(), resource.GetNamespace(), resource.GetName(), startTime) defer func() { @@ -234,11 +203,6 @@ func applyPolicy(client *client.Client, resource unstructured.Unstructured, p ky Context: ctx, } engineResponse := engine.Generate(policyContext) - // gather stats - gatherStat(p.Name, engineResponse.PolicyResponse) - //send stats - sendStat(false) - return engineResponse } diff --git a/pkg/policy/status.go b/pkg/policy/status.go index a46fd74857..1b3be316a5 100644 --- a/pkg/policy/status.go +++ b/pkg/policy/status.go @@ -2,12 +2,11 @@ package policy import ( "log" + "reflect" "sort" "sync" "time" - v12 "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" - "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/engine/response" @@ -29,16 +28,12 @@ type StatSync struct { stop <-chan struct{} client *versioned.Clientset policyStore *policystore.PolicyStore - cpvLister v12.ClusterPolicyViolationLister - pvLister v12.PolicyViolationLister } func NewStatusSync( client *versioned.Clientset, stopCh <-chan struct{}, pMetaStore *policystore.PolicyStore, - cpvLister v12.ClusterPolicyViolationLister, - pvLister v12.PolicyViolationLister, ) *StatSync { return &StatSync{ cache: &statusCache{ @@ -48,8 +43,6 @@ func NewStatusSync( stop: stopCh, client: client, policyStore: pMetaStore, - cpvLister: cpvLister, - pvLister: pvLister, } } @@ -69,10 +62,6 @@ func (s *StatSync) updateStats() { s.cache.mu.Unlock() for policyName, status := range nameToStatus { - cpvList, _ := s.getClusterPolicyViolationForPolicy(policyName) - pvList, _ := s.getNamespacedPolicyViolationForPolicy(policyName) - updateStatusWithViolationCount(&status, cpvList, pvList) - var policy = &v1.ClusterPolicy{} policy, err := s.policyStore.Get(policyName) if err != nil { @@ -86,12 +75,44 @@ func (s *StatSync) updateStats() { } } -func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) { +func (s *StatSync) UpdatePolicyStatusWithViolationCount(policyName string, violatedRules []v1.ViolatedRule) { + s.cache.mu.Lock() + status := s.cache.data[policyName] + + var ruleNameToViolations = make(map[string]int) + for _, rule := range violatedRules { + ruleNameToViolations[rule.Name]++ + } + + for i := range status.Rules { + status.ViolationCount += ruleNameToViolations[status.Rules[i].Name] + status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] + } + + s.cache.data[policyName] = status + s.cache.mu.Unlock() +} + +func (s *StatSync) UpdatePolicyStatusWithGeneratedResourceCount(generateRequest v1.GenerateRequest) { + s.cache.mu.Lock() + status := s.cache.data[generateRequest.Spec.Policy] + + status.ResourcesGeneratedCount += len(generateRequest.Status.GeneratedResources) + + s.cache.data[generateRequest.Spec.Policy] = status + s.cache.mu.Unlock() +} + +func (s *StatSync) UpdateStatusWithMutateStats(resp response.EngineResponse) { + if reflect.DeepEqual(response.EngineResponse{}, resp) { + return + } + s.cache.mu.Lock() var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) if policy != nil { policyStatus = policy.Status } @@ -102,7 +123,7 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) nameToRule[rule.Name] = rule } - for _, rule := range response.PolicyResponse.Rules { + for _, rule := range resp.PolicyResponse.Rules { ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name @@ -142,16 +163,20 @@ func (s *StatSync) UpdateStatusWithMutateStats(response response.EngineResponse) policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats - s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.data[resp.PolicyResponse.Policy] = policyStatus s.cache.mu.Unlock() } -func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineResponse) { +func (s *StatSync) UpdateStatusWithValidateStats(resp response.EngineResponse) { + if reflect.DeepEqual(response.EngineResponse{}, resp) { + return + } + s.cache.mu.Lock() var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) if policy != nil { policyStatus = policy.Status } @@ -162,7 +187,7 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons nameToRule[rule.Name] = rule } - for _, rule := range response.PolicyResponse.Rules { + for _, rule := range resp.PolicyResponse.Rules { ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name @@ -178,7 +203,7 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons } else { policyStatus.RulesFailedCount++ ruleStat.FailedCount++ - if response.PolicyResponse.ValidationFailureAction == "enforce" { + if resp.PolicyResponse.ValidationFailureAction == "enforce" { policyStatus.ResourcesBlockedCount++ ruleStat.ResourcesBlockedCount++ } @@ -204,16 +229,20 @@ func (s *StatSync) UpdateStatusWithValidateStats(response response.EngineRespons policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats - s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.data[resp.PolicyResponse.Policy] = policyStatus s.cache.mu.Unlock() } -func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineResponse) { +func (s *StatSync) UpdateStatusWithGenerateStats(resp response.EngineResponse) { + if reflect.DeepEqual(response.EngineResponse{}, resp) { + return + } + s.cache.mu.Lock() var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[response.PolicyResponse.Policy] + policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] if !exist { - policy, _ := s.policyStore.Get(response.PolicyResponse.Policy) + policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) if policy != nil { policyStatus = policy.Status } @@ -224,7 +253,7 @@ func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineRespons nameToRule[rule.Name] = rule } - for _, rule := range response.PolicyResponse.Rules { + for _, rule := range resp.PolicyResponse.Rules { ruleStat := nameToRule[rule.Name] ruleStat.Name = rule.Name @@ -262,7 +291,7 @@ func (s *StatSync) UpdateStatusWithGenerateStats(response response.EngineRespons policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() policyStatus.Rules = ruleStats - s.cache.data[response.PolicyResponse.Policy] = policyStatus + s.cache.data[resp.PolicyResponse.Policy] = policyStatus s.cache.mu.Unlock() } @@ -276,53 +305,3 @@ func updateAverageTime(newTime time.Duration, oldAverageTimeString string, avera newAverageTimeInNanoSeconds := numerator / denominator return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond } - -func (s *StatSync) getClusterPolicyViolationForPolicy(policy string) ([]*v1.ClusterPolicyViolation, error) { - policySelector, err := buildPolicyLabel(policy) - if err != nil { - return nil, err - } - // Get List of cluster policy violation - cpvList, err := s.cpvLister.List(policySelector) - if err != nil { - return nil, err - } - return cpvList, nil -} - -func (s *StatSync) getNamespacedPolicyViolationForPolicy(policy string) ([]*v1.PolicyViolation, error) { - policySelector, err := buildPolicyLabel(policy) - if err != nil { - return nil, err - } - // Get List of cluster policy violation - nspvList, err := s.pvLister.List(policySelector) - if err != nil { - return nil, err - } - return nspvList, nil - -} - -func updateStatusWithViolationCount(status *v1.PolicyStatus, cpvList []*v1.ClusterPolicyViolation, pvList []*v1.PolicyViolation) { - - status.ViolationCount = len(cpvList) + len(pvList) - - var ruleNameToNumberOfViolations = make(map[string]int) - - for _, cpv := range cpvList { - for _, violatedRule := range cpv.Spec.ViolatedRules { - ruleNameToNumberOfViolations[violatedRule.Name]++ - } - } - - for _, pv := range pvList { - for _, violatedRule := range pv.Spec.ViolatedRules { - ruleNameToNumberOfViolations[violatedRule.Name]++ - } - } - - for i, rule := range status.Rules { - status.Rules[i].ViolationCount = ruleNameToNumberOfViolations[rule.Name] - } -} diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 8b974594ae..97409427c0 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -4,6 +4,8 @@ import ( "fmt" "reflect" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" @@ -20,16 +22,20 @@ type clusterPV struct { cpvLister kyvernolister.ClusterPolicyViolationLister // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface + // update policy stats with violationCount + policyStatus *policy.StatSync } func newClusterPV(dclient *client.Client, cpvLister kyvernolister.ClusterPolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, + policyStatus *policy.StatSync, ) *clusterPV { cpv := clusterPV{ dclient: dclient, cpvLister: cpvLister, kyvernoInterface: kyvernoInterface, + policyStatus: policyStatus, } return &cpv } @@ -93,6 +99,8 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { glog.V(4).Infof("failed to create Cluster Policy Violation: %v", err) return err } + + go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil } @@ -115,5 +123,6 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err } glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) + go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) return nil } diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index 7d820f980b..f2309aa82e 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -8,6 +8,8 @@ import ( "sync" "time" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -37,9 +39,10 @@ type Generator struct { // returns true if the cluster policy store has been synced at least once pvSynced cache.InformerSynced // returns true if the namespaced cluster policy store has been synced at at least once - nspvSynced cache.InformerSynced - queue workqueue.RateLimitingInterface - dataStore *dataStore + nspvSynced cache.InformerSynced + queue workqueue.RateLimitingInterface + dataStore *dataStore + policyStatus *policy.StatSync } //NewDataStore returns an instance of data store @@ -103,7 +106,8 @@ type GeneratorInterface interface { func NewPVGenerator(client *kyvernoclient.Clientset, dclient *dclient.Client, pvInformer kyvernoinformer.ClusterPolicyViolationInformer, - nspvInformer kyvernoinformer.PolicyViolationInformer) *Generator { + nspvInformer kyvernoinformer.PolicyViolationInformer, + policyStatus *policy.StatSync) *Generator { gen := Generator{ kyvernoInterface: client.KyvernoV1(), dclient: dclient, @@ -113,6 +117,7 @@ func NewPVGenerator(client *kyvernoclient.Clientset, nspvSynced: nspvInformer.Informer().HasSynced, queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName), dataStore: newDataStore(), + policyStatus: policyStatus, } return &gen } @@ -219,10 +224,10 @@ func (gen *Generator) syncHandler(info Info) error { builder := newPvBuilder() if info.Resource.GetNamespace() == "" { // cluster scope resource generate a clusterpolicy violation - handler = newClusterPV(gen.dclient, gen.cpvLister, gen.kyvernoInterface) + handler = newClusterPV(gen.dclient, gen.cpvLister, gen.kyvernoInterface, gen.policyStatus) } else { // namespaced resources generated a namespaced policy violation in the namespace of the resource - handler = newNamespacedPV(gen.dclient, gen.nspvLister, gen.kyvernoInterface) + handler = newNamespacedPV(gen.dclient, gen.nspvLister, gen.kyvernoInterface, gen.policyStatus) } failure := false diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 1967a0ba3f..4dc3cad759 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -4,6 +4,8 @@ import ( "fmt" "reflect" + "github.com/nirmata/kyverno/pkg/policy" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" @@ -20,16 +22,20 @@ type namespacedPV struct { nspvLister kyvernolister.PolicyViolationLister // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface + // update policy status with violationCount + policyStatus *policy.StatSync } func newNamespacedPV(dclient *client.Client, nspvLister kyvernolister.PolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, + policyStatus *policy.StatSync, ) *namespacedPV { nspv := namespacedPV{ dclient: dclient, nspvLister: nspvLister, kyvernoInterface: kyvernoInterface, + policyStatus: policyStatus, } return &nspv } @@ -92,6 +98,8 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { glog.V(4).Infof("failed to create Cluster Policy Violation: %v", err) return err } + + go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil } @@ -112,6 +120,7 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error return fmt.Errorf("failed to update namespaced policy violation: %v", err) } + go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil } diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 2dddf71e1c..6edf74a94e 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -61,6 +61,7 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) + go ws.status.UpdateStatusWithGenerateStats(engineResponse) } } // Adds Generate Request to a channel(queue size 1000) to generators From a14040648513b07069cf2dc73dfc3db719d7591f Mon Sep 17 00:00:00 2001 From: shravan Date: Mon, 24 Feb 2020 20:20:51 +0530 Subject: [PATCH 10/34] 527 pending changes from previous merge conflict resolution --- pkg/namespace/generation.go | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 pkg/namespace/generation.go diff --git a/pkg/namespace/generation.go b/pkg/namespace/generation.go deleted file mode 100644 index e69de29bb2..0000000000 From 1026fc52366103fa7a713bd60a8ed0aaabbf3be9 Mon Sep 17 00:00:00 2001 From: shravan Date: Tue, 25 Feb 2020 00:07:13 +0530 Subject: [PATCH 11/34] 527 clear cache to handle deleting policies --- pkg/policy/status.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/policy/status.go b/pkg/policy/status.go index 1b3be316a5..c7aceb9dbc 100644 --- a/pkg/policy/status.go +++ b/pkg/policy/status.go @@ -59,6 +59,7 @@ func (s *StatSync) updateStats() { for k, v := range s.cache.data { nameToStatus[k] = v } + s.cache.data = make(map[string]v1.PolicyStatus) s.cache.mu.Unlock() for policyName, status := range nameToStatus { From d32cd9363ee9cd034aa7390943b178fd6ec540d5 Mon Sep 17 00:00:00 2001 From: shravan Date: Tue, 25 Feb 2020 20:55:07 +0530 Subject: [PATCH 12/34] 527 save commit --- cmd/kyverno/main.go | 4 +- pkg/generate/controller.go | 5 +- pkg/generate/status.go | 4 +- pkg/policy/status.go | 308 --------------------- pkg/policyStatus/0_main.go | 81 ++++++ pkg/policyStatus/1_interface.go | 5 + pkg/policyStatus/2_functions.go | 14 + pkg/policyStatus/generateCount.go | 84 ++++++ pkg/policyStatus/generatedResourceCount.go | 25 ++ pkg/policyStatus/mutateStats.go | 87 ++++++ pkg/policyStatus/some_test.go | 7 + pkg/policyStatus/validateStats.go | 88 ++++++ pkg/policyStatus/violationCount.go | 35 +++ pkg/policyviolation/clusterpv.go | 7 +- pkg/policyviolation/generator.go | 7 +- pkg/policyviolation/namespacedpv.go | 7 +- pkg/webhooks/server.go | 7 +- 17 files changed, 445 insertions(+), 330 deletions(-) delete mode 100644 pkg/policy/status.go create mode 100644 pkg/policyStatus/0_main.go create mode 100644 pkg/policyStatus/1_interface.go create mode 100644 pkg/policyStatus/2_functions.go create mode 100644 pkg/policyStatus/generateCount.go create mode 100644 pkg/policyStatus/generatedResourceCount.go create mode 100644 pkg/policyStatus/mutateStats.go create mode 100644 pkg/policyStatus/some_test.go create mode 100644 pkg/policyStatus/validateStats.go create mode 100644 pkg/policyStatus/violationCount.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 68d1bbd857..98f2c735ab 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -5,6 +5,8 @@ import ( "flag" "time" + "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/checker" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -136,7 +138,7 @@ func main() { pInformer.Kyverno().V1().ClusterPolicies()) // Policy Status Handler - deals with all logic related to policy status - statusSync := policy.NewStatusSync( + statusSync := policyStatus.NewSync( pclient, stopCh, policyMetaStore) diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index 31e12a68c4..e5f4c2a8e8 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -4,8 +4,6 @@ import ( "fmt" "time" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -13,6 +11,7 @@ import ( kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" dclient "github.com/nirmata/kyverno/pkg/dclient" "github.com/nirmata/kyverno/pkg/event" + "github.com/nirmata/kyverno/pkg/policyStatus" "github.com/nirmata/kyverno/pkg/policyviolation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -70,7 +69,7 @@ func NewController( eventGen event.Interface, pvGenerator policyviolation.GeneratorInterface, dynamicInformer dynamicinformer.DynamicSharedInformerFactory, - policyStatus *policy.StatSync, + policyStatus *policyStatus.Sync, ) *Controller { c := Controller{ client: client, diff --git a/pkg/generate/status.go b/pkg/generate/status.go index 026a657457..6f14597229 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -4,7 +4,7 @@ import ( "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" - "github.com/nirmata/kyverno/pkg/policy" + "github.com/nirmata/kyverno/pkg/policyStatus" ) //StatusControlInterface provides interface to update status subresource @@ -16,7 +16,7 @@ type StatusControlInterface interface { // StatusControl is default implementaation of GRStatusControlInterface type StatusControl struct { client kyvernoclient.Interface - policyStatus *policy.StatSync + policyStatus *policyStatus.Sync } //Failed sets gr status.state to failed with message diff --git a/pkg/policy/status.go b/pkg/policy/status.go deleted file mode 100644 index c7aceb9dbc..0000000000 --- a/pkg/policy/status.go +++ /dev/null @@ -1,308 +0,0 @@ -package policy - -import ( - "log" - "reflect" - "sort" - "sync" - "time" - - "github.com/nirmata/kyverno/pkg/policystore" - - "github.com/nirmata/kyverno/pkg/engine/response" - - "k8s.io/apimachinery/pkg/util/wait" - - "github.com/nirmata/kyverno/pkg/client/clientset/versioned" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" -) - -type statusCache struct { - mu sync.RWMutex - data map[string]v1.PolicyStatus -} - -type StatSync struct { - cache *statusCache - stop <-chan struct{} - client *versioned.Clientset - policyStore *policystore.PolicyStore -} - -func NewStatusSync( - client *versioned.Clientset, - stopCh <-chan struct{}, - pMetaStore *policystore.PolicyStore, -) *StatSync { - return &StatSync{ - cache: &statusCache{ - mu: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), - }, - stop: stopCh, - client: client, - policyStore: pMetaStore, - } -} - -func (s *StatSync) Run() { - // update policy status every 10 seconds - waits for previous updateStatus to complete - wait.Until(s.updateStats, 1*time.Second, s.stop) - <-s.stop - s.updateStats() -} - -func (s *StatSync) updateStats() { - s.cache.mu.Lock() - var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) - for k, v := range s.cache.data { - nameToStatus[k] = v - } - s.cache.data = make(map[string]v1.PolicyStatus) - s.cache.mu.Unlock() - - for policyName, status := range nameToStatus { - var policy = &v1.ClusterPolicy{} - policy, err := s.policyStore.Get(policyName) - if err != nil { - continue - } - policy.Status = status - _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) - if err != nil { - log.Println(err) - } - } -} - -func (s *StatSync) UpdatePolicyStatusWithViolationCount(policyName string, violatedRules []v1.ViolatedRule) { - s.cache.mu.Lock() - status := s.cache.data[policyName] - - var ruleNameToViolations = make(map[string]int) - for _, rule := range violatedRules { - ruleNameToViolations[rule.Name]++ - } - - for i := range status.Rules { - status.ViolationCount += ruleNameToViolations[status.Rules[i].Name] - status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] - } - - s.cache.data[policyName] = status - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdatePolicyStatusWithGeneratedResourceCount(generateRequest v1.GenerateRequest) { - s.cache.mu.Lock() - status := s.cache.data[generateRequest.Spec.Policy] - - status.ResourcesGeneratedCount += len(generateRequest.Status.GeneratedResources) - - s.cache.data[generateRequest.Spec.Policy] = status - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithMutateStats(resp response.EngineResponse) { - if reflect.DeepEqual(response.EngineResponse{}, resp) { - return - } - - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - policyStatus.ResourcesMutatedCount++ - ruleStat.AppliedCount++ - ruleStat.ResourcesMutatedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[resp.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithValidateStats(resp response.EngineResponse) { - if reflect.DeepEqual(response.EngineResponse{}, resp) { - return - } - - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - ruleStat.AppliedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - if resp.PolicyResponse.ValidationFailureAction == "enforce" { - policyStatus.ResourcesBlockedCount++ - ruleStat.ResourcesBlockedCount++ - } - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[resp.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func (s *StatSync) UpdateStatusWithGenerateStats(resp response.EngineResponse) { - if reflect.DeepEqual(response.EngineResponse{}, resp) { - return - } - - s.cache.mu.Lock() - var policyStatus v1.PolicyStatus - policyStatus, exist := s.cache.data[resp.PolicyResponse.Policy] - if !exist { - policy, _ := s.policyStore.Get(resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - - var nameToRule = make(map[string]v1.RuleStats, 0) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - ruleStat.AppliedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - s.cache.data[resp.PolicyResponse.Policy] = policyStatus - s.cache.mu.Unlock() -} - -func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { - if averageOver == 0 { - return newTime - } - oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) - numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() - denominator := averageOver + 1 - newAverageTimeInNanoSeconds := numerator / denominator - return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond -} diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/0_main.go new file mode 100644 index 0000000000..aa008844db --- /dev/null +++ b/pkg/policyStatus/0_main.go @@ -0,0 +1,81 @@ +package policyStatus + +import ( + "sync" + "time" + + "github.com/golang/glog" + + "github.com/nirmata/kyverno/pkg/policystore" + + "k8s.io/apimachinery/pkg/util/wait" + + "github.com/nirmata/kyverno/pkg/client/clientset/versioned" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" +) + +type Sync struct { + cache *cache + listener chan statusUpdater + stop <-chan struct{} + client *versioned.Clientset + policyStore *policystore.PolicyStore +} + +type cache struct { + mutex sync.RWMutex + data map[string]v1.PolicyStatus +} + +func NewSync(c *versioned.Clientset, sc <-chan struct{}, pms *policystore.PolicyStore) *Sync { + return &Sync{ + cache: &cache{ + mutex: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + }, + stop: sc, + client: c, + policyStore: pms, + } +} + +func (s *Sync) Run() { + wait.Until(s.updatePolicyStatus, 5*time.Second, s.stop) + <-s.stop + s.updatePolicyStatus() +} + +func (s *Sync) updateStatusCache() { + for { + select { + case statusUpdater := <-s.listener: + statusUpdater.updateStatus() + case <-s.stop: + return + } + } +} + +func (s *Sync) updatePolicyStatus() { + s.cache.mutex.Lock() + var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) + for k, v := range s.cache.data { + nameToStatus[k] = v + } + s.cache.data = make(map[string]v1.PolicyStatus) + s.cache.mutex.Unlock() + + for policyName, status := range nameToStatus { + var policy = &v1.ClusterPolicy{} + policy, err := s.policyStore.Get(policyName) + if err != nil { + continue + } + policy.Status = status + _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) + if err != nil { + glog.V(4).Info(err) + } + } +} diff --git a/pkg/policyStatus/1_interface.go b/pkg/policyStatus/1_interface.go new file mode 100644 index 0000000000..942337fb8a --- /dev/null +++ b/pkg/policyStatus/1_interface.go @@ -0,0 +1,5 @@ +package policyStatus + +type statusUpdater interface { + updateStatus() +} diff --git a/pkg/policyStatus/2_functions.go b/pkg/policyStatus/2_functions.go new file mode 100644 index 0000000000..db18b57ea8 --- /dev/null +++ b/pkg/policyStatus/2_functions.go @@ -0,0 +1,14 @@ +package policyStatus + +import "time" + +func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { + if averageOver == 0 { + return newTime + } + oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) + numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() + denominator := averageOver + 1 + newAverageTimeInNanoSeconds := numerator / denominator + return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond +} diff --git a/pkg/policyStatus/generateCount.go b/pkg/policyStatus/generateCount.go new file mode 100644 index 0000000000..5c633d9eaa --- /dev/null +++ b/pkg/policyStatus/generateCount.go @@ -0,0 +1,84 @@ +package policyStatus + +import ( + "reflect" + "sort" + "time" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/response" +) + +type generateStats struct { + s *Sync + resp response.EngineResponse +} + +func (s *Sync) UpdateStatusWithGenerateStats(resp response.EngineResponse) { + s.listener <- &generateStats{ + s: s, + resp: resp, + } +} + +func (gs *generateStats) updateStatus() { + if reflect.DeepEqual(response.EngineResponse{}, gs.resp) { + return + } + + gs.s.cache.mutex.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := gs.s.cache.data[gs.resp.PolicyResponse.Policy] + if !exist { + policy, _ := gs.s.policyStore.Get(gs.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range gs.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + gs.s.cache.data[gs.resp.PolicyResponse.Policy] = policyStatus + gs.s.cache.mutex.Unlock() +} diff --git a/pkg/policyStatus/generatedResourceCount.go b/pkg/policyStatus/generatedResourceCount.go new file mode 100644 index 0000000000..9f5d013437 --- /dev/null +++ b/pkg/policyStatus/generatedResourceCount.go @@ -0,0 +1,25 @@ +package policyStatus + +import v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + +type generatedResourceCount struct { + sync *Sync + generateRequest v1.GenerateRequest +} + +func (s *Sync) UpdatePolicyStatusWithGeneratedResourceCount(generateRequest v1.GenerateRequest) { + s.listener <- &generatedResourceCount{ + sync: s, + generateRequest: generateRequest, + } +} + +func (vc *generatedResourceCount) updateStatus() { + vc.sync.cache.mutex.Lock() + status := vc.sync.cache.data[vc.generateRequest.Spec.Policy] + + status.ResourcesGeneratedCount += len(vc.generateRequest.Status.GeneratedResources) + + vc.sync.cache.data[vc.generateRequest.Spec.Policy] = status + vc.sync.cache.mutex.Unlock() +} diff --git a/pkg/policyStatus/mutateStats.go b/pkg/policyStatus/mutateStats.go new file mode 100644 index 0000000000..06052f0351 --- /dev/null +++ b/pkg/policyStatus/mutateStats.go @@ -0,0 +1,87 @@ +package policyStatus + +import ( + "reflect" + "sort" + "time" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/response" +) + +type mutateStats struct { + s *Sync + resp response.EngineResponse +} + +func (s *Sync) UpdateStatusWithMutateStats(resp response.EngineResponse) { + s.listener <- &mutateStats{ + s: s, + resp: resp, + } + +} + +func (ms *mutateStats) updateStatus() { + if reflect.DeepEqual(response.EngineResponse{}, ms.resp) { + return + } + + ms.s.cache.mutex.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := ms.s.cache.data[ms.resp.PolicyResponse.Policy] + if !exist { + policy, _ := ms.s.policyStore.Get(ms.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range ms.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + policyStatus.ResourcesMutatedCount++ + ruleStat.AppliedCount++ + ruleStat.ResourcesMutatedCount++ + } else { + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + ms.s.cache.data[ms.resp.PolicyResponse.Policy] = policyStatus + ms.s.cache.mutex.Unlock() +} diff --git a/pkg/policyStatus/some_test.go b/pkg/policyStatus/some_test.go new file mode 100644 index 0000000000..826c2c6eaa --- /dev/null +++ b/pkg/policyStatus/some_test.go @@ -0,0 +1,7 @@ +package policyStatus + +import "testing" + +func TestNewSync(t *testing.T) { + +} diff --git a/pkg/policyStatus/validateStats.go b/pkg/policyStatus/validateStats.go new file mode 100644 index 0000000000..96e204cac9 --- /dev/null +++ b/pkg/policyStatus/validateStats.go @@ -0,0 +1,88 @@ +package policyStatus + +import ( + "reflect" + "sort" + "time" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/engine/response" +) + +type validateStats struct { + s *Sync + resp response.EngineResponse +} + +func (s *Sync) UpdateStatusWithValidateStats(resp response.EngineResponse) { + s.listener <- &validateStats{ + s: s, + resp: resp, + } +} + +func (vs *validateStats) updateStatus() { + if reflect.DeepEqual(response.EngineResponse{}, vs.resp) { + return + } + + vs.s.cache.mutex.Lock() + var policyStatus v1.PolicyStatus + policyStatus, exist := vs.s.cache.data[vs.resp.PolicyResponse.Policy] + if !exist { + policy, _ := vs.s.policyStore.Get(vs.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } + } + + var nameToRule = make(map[string]v1.RuleStats, 0) + for _, rule := range policyStatus.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range vs.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + policyStatus.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + policyStatus.RulesFailedCount++ + ruleStat.FailedCount++ + if vs.resp.PolicyResponse.ValidationFailureAction == "enforce" { + policyStatus.ResourcesBlockedCount++ + ruleStat.ResourcesBlockedCount++ + } + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() + policyStatus.Rules = ruleStats + + vs.s.cache.data[vs.resp.PolicyResponse.Policy] = policyStatus + vs.s.cache.mutex.Unlock() +} diff --git a/pkg/policyStatus/violationCount.go b/pkg/policyStatus/violationCount.go new file mode 100644 index 0000000000..93f02fded1 --- /dev/null +++ b/pkg/policyStatus/violationCount.go @@ -0,0 +1,35 @@ +package policyStatus + +import v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + +type violationCount struct { + sync *Sync + policyName string + violatedRules []v1.ViolatedRule +} + +func (s *Sync) UpdatePolicyStatusWithViolationCount(policyName string, violatedRules []v1.ViolatedRule) { + s.listener <- &violationCount{ + sync: s, + policyName: policyName, + violatedRules: violatedRules, + } +} + +func (vc *violationCount) updateStatus() { + vc.sync.cache.mutex.Lock() + status := vc.sync.cache.data[vc.policyName] + + var ruleNameToViolations = make(map[string]int) + for _, rule := range vc.violatedRules { + ruleNameToViolations[rule.Name]++ + } + + for i := range status.Rules { + status.ViolationCount += ruleNameToViolations[status.Rules[i].Name] + status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] + } + + vc.sync.cache.data[vc.policyName] = status + vc.sync.cache.mutex.Unlock() +} diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 97409427c0..48d1ce4c30 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -4,13 +4,12 @@ import ( "fmt" "reflect" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" client "github.com/nirmata/kyverno/pkg/dclient" + "github.com/nirmata/kyverno/pkg/policyStatus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -23,13 +22,13 @@ type clusterPV struct { // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface // update policy stats with violationCount - policyStatus *policy.StatSync + policyStatus *policyStatus.Sync } func newClusterPV(dclient *client.Client, cpvLister kyvernolister.ClusterPolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, - policyStatus *policy.StatSync, + policyStatus *policyStatus.Sync, ) *clusterPV { cpv := clusterPV{ dclient: dclient, diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index f2309aa82e..22b538aeef 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -8,14 +8,13 @@ import ( "sync" "time" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernoinformer "github.com/nirmata/kyverno/pkg/client/informers/externalversions/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" + "github.com/nirmata/kyverno/pkg/policyStatus" dclient "github.com/nirmata/kyverno/pkg/dclient" unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -42,7 +41,7 @@ type Generator struct { nspvSynced cache.InformerSynced queue workqueue.RateLimitingInterface dataStore *dataStore - policyStatus *policy.StatSync + policyStatus *policyStatus.Sync } //NewDataStore returns an instance of data store @@ -107,7 +106,7 @@ func NewPVGenerator(client *kyvernoclient.Clientset, dclient *dclient.Client, pvInformer kyvernoinformer.ClusterPolicyViolationInformer, nspvInformer kyvernoinformer.PolicyViolationInformer, - policyStatus *policy.StatSync) *Generator { + policyStatus *policyStatus.Sync) *Generator { gen := Generator{ kyvernoInterface: client.KyvernoV1(), dclient: dclient, diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 4dc3cad759..8c24d62a83 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -4,13 +4,12 @@ import ( "fmt" "reflect" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" client "github.com/nirmata/kyverno/pkg/dclient" + "github.com/nirmata/kyverno/pkg/policyStatus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -23,13 +22,13 @@ type namespacedPV struct { // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface // update policy status with violationCount - policyStatus *policy.StatSync + policyStatus *policyStatus.Sync } func newNamespacedPV(dclient *client.Client, nspvLister kyvernolister.PolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, - policyStatus *policy.StatSync, + policyStatus *policyStatus.Sync, ) *namespacedPV { nspv := namespacedPV{ dclient: dclient, diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 6e51fd692b..e7733e5f72 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -10,8 +10,6 @@ import ( "net/http" "time" - "github.com/nirmata/kyverno/pkg/policy" - "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/checker" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -20,6 +18,7 @@ import ( "github.com/nirmata/kyverno/pkg/config" client "github.com/nirmata/kyverno/pkg/dclient" "github.com/nirmata/kyverno/pkg/event" + "github.com/nirmata/kyverno/pkg/policyStatus" "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/policyviolation" tlsutils "github.com/nirmata/kyverno/pkg/tls" @@ -56,7 +55,7 @@ type WebhookServer struct { // webhook registration client webhookRegistrationClient *webhookconfig.WebhookRegistrationClient // API to send policy stats for aggregation - status *policy.StatSync + status *policyStatus.Sync // helpers to validate against current loaded configuration configHandler config.Interface // channel for cleanup notification @@ -83,7 +82,7 @@ func NewWebhookServer( crbInformer rbacinformer.ClusterRoleBindingInformer, eventGen event.Interface, webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, - statusSync *policy.StatSync, + statusSync *policyStatus.Sync, configHandler config.Interface, pMetaStore policystore.LookupInterface, pvGenerator policyviolation.GeneratorInterface, From 4c573bd3c73064f9254ae4349c19b29c9a70a4fa Mon Sep 17 00:00:00 2001 From: shravan Date: Tue, 25 Feb 2020 21:07:00 +0530 Subject: [PATCH 13/34] 527 ci fixes --- cmd/kyverno/main.go | 2 +- pkg/policyStatus/0_main.go | 6 +++++- pkg/policyStatus/generateCount.go | 2 +- pkg/policyStatus/mutateStats.go | 2 +- pkg/policyStatus/some_test.go | 7 ------- pkg/policyStatus/validateStats.go | 2 +- 6 files changed, 9 insertions(+), 12 deletions(-) delete mode 100644 pkg/policyStatus/some_test.go diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 98f2c735ab..b60ebc77cb 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -248,7 +248,7 @@ func main() { go grc.Run(1, stopCh) go grcc.Run(1, stopCh) go pvgen.Run(1, stopCh) - go statusSync.Run() + go statusSync.Run(2) // verifys if the admission control is enabled and active // resync: 60 seconds diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/0_main.go index aa008844db..75e94b2c20 100644 --- a/pkg/policyStatus/0_main.go +++ b/pkg/policyStatus/0_main.go @@ -40,7 +40,11 @@ func NewSync(c *versioned.Clientset, sc <-chan struct{}, pms *policystore.Policy } } -func (s *Sync) Run() { +func (s *Sync) Run(workers int) { + for i := 0; i < workers; i++ { + go s.updateStatusCache() + } + wait.Until(s.updatePolicyStatus, 5*time.Second, s.stop) <-s.stop s.updatePolicyStatus() diff --git a/pkg/policyStatus/generateCount.go b/pkg/policyStatus/generateCount.go index 5c633d9eaa..570fa86e61 100644 --- a/pkg/policyStatus/generateCount.go +++ b/pkg/policyStatus/generateCount.go @@ -36,7 +36,7 @@ func (gs *generateStats) updateStatus() { } } - var nameToRule = make(map[string]v1.RuleStats, 0) + var nameToRule = make(map[string]v1.RuleStats) for _, rule := range policyStatus.Rules { nameToRule[rule.Name] = rule } diff --git a/pkg/policyStatus/mutateStats.go b/pkg/policyStatus/mutateStats.go index 06052f0351..c27176d626 100644 --- a/pkg/policyStatus/mutateStats.go +++ b/pkg/policyStatus/mutateStats.go @@ -37,7 +37,7 @@ func (ms *mutateStats) updateStatus() { } } - var nameToRule = make(map[string]v1.RuleStats, 0) + var nameToRule = make(map[string]v1.RuleStats) for _, rule := range policyStatus.Rules { nameToRule[rule.Name] = rule } diff --git a/pkg/policyStatus/some_test.go b/pkg/policyStatus/some_test.go deleted file mode 100644 index 826c2c6eaa..0000000000 --- a/pkg/policyStatus/some_test.go +++ /dev/null @@ -1,7 +0,0 @@ -package policyStatus - -import "testing" - -func TestNewSync(t *testing.T) { - -} diff --git a/pkg/policyStatus/validateStats.go b/pkg/policyStatus/validateStats.go index 96e204cac9..fc44b60740 100644 --- a/pkg/policyStatus/validateStats.go +++ b/pkg/policyStatus/validateStats.go @@ -36,7 +36,7 @@ func (vs *validateStats) updateStatus() { } } - var nameToRule = make(map[string]v1.RuleStats, 0) + var nameToRule = make(map[string]v1.RuleStats) for _, rule := range policyStatus.Rules { nameToRule[rule.Name] = rule } From 7df58b8a4f6babc42d0b8dead38d9f6549793d45 Mon Sep 17 00:00:00 2001 From: shravan Date: Tue, 25 Feb 2020 21:20:08 +0530 Subject: [PATCH 14/34] 527 corrected file name --- pkg/policyStatus/{generateCount.go => generateStats.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename pkg/policyStatus/{generateCount.go => generateStats.go} (100%) diff --git a/pkg/policyStatus/generateCount.go b/pkg/policyStatus/generateStats.go similarity index 100% rename from pkg/policyStatus/generateCount.go rename to pkg/policyStatus/generateStats.go From d0217a3faf4e1ce4464ca3ab19dd74e5d7419544 Mon Sep 17 00:00:00 2001 From: shravan Date: Tue, 25 Feb 2020 23:57:16 +0530 Subject: [PATCH 15/34] 527 minor fixes --- pkg/policyStatus/0_main.go | 1 + pkg/policyStatus/generatedResourceCount.go | 8 +++++++- pkg/policyStatus/violationCount.go | 8 +++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/0_main.go index 75e94b2c20..8f7987d882 100644 --- a/pkg/policyStatus/0_main.go +++ b/pkg/policyStatus/0_main.go @@ -37,6 +37,7 @@ func NewSync(c *versioned.Clientset, sc <-chan struct{}, pms *policystore.Policy stop: sc, client: c, policyStore: pms, + listener: make(chan statusUpdater), } } diff --git a/pkg/policyStatus/generatedResourceCount.go b/pkg/policyStatus/generatedResourceCount.go index 9f5d013437..f951cf759c 100644 --- a/pkg/policyStatus/generatedResourceCount.go +++ b/pkg/policyStatus/generatedResourceCount.go @@ -16,7 +16,13 @@ func (s *Sync) UpdatePolicyStatusWithGeneratedResourceCount(generateRequest v1.G func (vc *generatedResourceCount) updateStatus() { vc.sync.cache.mutex.Lock() - status := vc.sync.cache.data[vc.generateRequest.Spec.Policy] + status, exist := vc.sync.cache.data[vc.generateRequest.Spec.Policy] + if !exist { + policy, _ := vc.sync.policyStore.Get(vc.generateRequest.Spec.Policy) + if policy != nil { + status = policy.Status + } + } status.ResourcesGeneratedCount += len(vc.generateRequest.Status.GeneratedResources) diff --git a/pkg/policyStatus/violationCount.go b/pkg/policyStatus/violationCount.go index 93f02fded1..dd7ed30574 100644 --- a/pkg/policyStatus/violationCount.go +++ b/pkg/policyStatus/violationCount.go @@ -18,7 +18,13 @@ func (s *Sync) UpdatePolicyStatusWithViolationCount(policyName string, violatedR func (vc *violationCount) updateStatus() { vc.sync.cache.mutex.Lock() - status := vc.sync.cache.data[vc.policyName] + status, exist := vc.sync.cache.data[vc.policyName] + if !exist { + policy, _ := vc.sync.policyStore.Get(vc.policyName) + if policy != nil { + status = policy.Status + } + } var ruleNameToViolations = make(map[string]int) for _, rule := range vc.violatedRules { From 157694b4fdf267164c7a60ef537d0e9585f98225 Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 00:26:09 +0530 Subject: [PATCH 16/34] 527 skippings stats for sync pv --- pkg/policy/report.go | 4 ++++ pkg/policyviolation/clusterpv.go | 9 +++++++-- pkg/policyviolation/generator.go | 5 +++++ pkg/policyviolation/namespacedpv.go | 8 ++++++-- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/pkg/policy/report.go b/pkg/policy/report.go index 206a6ebd61..3614da9b32 100644 --- a/pkg/policy/report.go +++ b/pkg/policy/report.go @@ -18,6 +18,10 @@ func (pc *PolicyController) cleanupAndReport(engineResponses []response.EngineRe pc.eventGen.Add(eventInfos...) // create policy violation pvInfos := policyviolation.GeneratePVsFromEngineResponse(engineResponses) + for i := range pvInfos { + pvInfos[i].FromSync = true + } + pc.pvGenerator.Add(pvInfos...) // cleanup existing violations if any // if there is any error in clean up, we dont re-queue the resource diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 48d1ce4c30..42a5c45fbe 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -99,7 +99,10 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { return err } - go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + if newPv.Annotations["fromSync"] != "true" { + go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + } + glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil } @@ -122,6 +125,8 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err } glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) - go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + if newPv.Annotations["fromSync"] != "true" { + go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + } return nil } diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index 22b538aeef..3e49935e0f 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -81,6 +81,7 @@ type Info struct { PolicyName string Resource unstructured.Unstructured Rules []kyverno.ViolatedRule + FromSync bool } func (i Info) toKey() string { @@ -232,6 +233,10 @@ func (gen *Generator) syncHandler(info Info) error { failure := false pv := builder.generate(info) + if info.FromSync { + pv.Annotations["fromSync"] = "true" + } + // Create Policy Violations glog.V(3).Infof("Creating policy violation: %s", info.toKey()) if err := handler.create(pv); err != nil { diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 8c24d62a83..cc6440afaf 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -98,7 +98,9 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { return err } - go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + if newPv.Annotations["fromSync"] != "true" { + go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil } @@ -119,7 +121,9 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error return fmt.Errorf("failed to update namespaced policy violation: %v", err) } - go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + if newPv.Annotations["fromSync"] != "true" { + go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + } glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil } From 8a12ea30d6288b3f9248be73392c1140a981bb3c Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 07:03:43 +0530 Subject: [PATCH 17/34] 527 fixed violation count and generated count --- pkg/generate/status.go | 6 +++++- pkg/policyviolation/generator.go | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/generate/status.go b/pkg/generate/status.go index 6f14597229..b4c5b2c12c 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -36,12 +36,16 @@ func (sc StatusControl) Failed(gr kyverno.GenerateRequest, message string, genRe // Success sets the gr status.state to completed and clears message func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyverno.ResourceSpec) error { + grCopy := gr.DeepCopy() + gr.Status.State = kyverno.Completed gr.Status.Message = "" // Update Generated Resources gr.Status.GeneratedResources = genResources - go sc.policyStatus.UpdatePolicyStatusWithGeneratedResourceCount(gr) + if grCopy.Status.State != kyverno.Completed { + go sc.policyStatus.UpdatePolicyStatusWithGeneratedResourceCount(gr) + } _, err := sc.client.KyvernoV1().GenerateRequests("kyverno").UpdateStatus(&gr) if err != nil { diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index 3e49935e0f..6c491112ee 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -234,7 +234,9 @@ func (gen *Generator) syncHandler(info Info) error { pv := builder.generate(info) if info.FromSync { - pv.Annotations["fromSync"] = "true" + pv.Annotations = map[string]string{ + "fromSync": "true", + } } // Create Policy Violations From 2a9b6a8f692a56aa9631aaceee02062b2b8a18e3 Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 07:36:19 +0530 Subject: [PATCH 18/34] 527 removed uneeded statements --- pkg/webhooks/mutation.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index e2dd090caa..90f9ee70e0 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -1,7 +1,6 @@ package webhooks import ( - "log" "time" "github.com/golang/glog" @@ -57,9 +56,6 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou glog.V(2).Infof("Handling mutation for Kind=%s, Namespace=%s Name=%s UID=%s patchOperation=%s", resource.GetKind(), resource.GetNamespace(), resource.GetName(), request.UID, request.Operation) policyContext.Policy = policy - if resource.GetKind() == "Pod" { - log.Println("some") - } engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) go ws.status.UpdateStatusWithMutateStats(engineResponse) From b068733ac83128d2cb94e8172dc16e194145c784 Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 10:22:11 +0530 Subject: [PATCH 19/34] 527 removing uneeded statements --- pkg/policyStatus/0_main.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/0_main.go index 8f7987d882..5357a90252 100644 --- a/pkg/policyStatus/0_main.go +++ b/pkg/policyStatus/0_main.go @@ -46,7 +46,7 @@ func (s *Sync) Run(workers int) { go s.updateStatusCache() } - wait.Until(s.updatePolicyStatus, 5*time.Second, s.stop) + wait.Until(s.updatePolicyStatus, 2*time.Second, s.stop) <-s.stop s.updatePolicyStatus() } @@ -68,11 +68,9 @@ func (s *Sync) updatePolicyStatus() { for k, v := range s.cache.data { nameToStatus[k] = v } - s.cache.data = make(map[string]v1.PolicyStatus) s.cache.mutex.Unlock() for policyName, status := range nameToStatus { - var policy = &v1.ClusterPolicy{} policy, err := s.policyStore.Get(policyName) if err != nil { continue @@ -80,6 +78,9 @@ func (s *Sync) updatePolicyStatus() { policy.Status = status _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) if err != nil { + s.cache.mutex.Lock() + delete(s.cache.data, policyName) + s.cache.mutex.Unlock() glog.V(4).Info(err) } } From b7e551a5cbb2a9df80646b8afb4cca64764f237f Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 13:52:12 +0530 Subject: [PATCH 20/34] 527 added tests --- pkg/policyStatus/generateStats.go | 9 +- pkg/policyStatus/mutateStats.go | 9 +- pkg/policyStatus/status_test.go | 247 ++++++++++++++++++++++++++++++ pkg/policyStatus/validateStats.go | 9 +- 4 files changed, 262 insertions(+), 12 deletions(-) create mode 100644 pkg/policyStatus/status_test.go diff --git a/pkg/policyStatus/generateStats.go b/pkg/policyStatus/generateStats.go index 570fa86e61..eabe280af6 100644 --- a/pkg/policyStatus/generateStats.go +++ b/pkg/policyStatus/generateStats.go @@ -27,12 +27,13 @@ func (gs *generateStats) updateStatus() { } gs.s.cache.mutex.Lock() - var policyStatus v1.PolicyStatus policyStatus, exist := gs.s.cache.data[gs.resp.PolicyResponse.Policy] if !exist { - policy, _ := gs.s.policyStore.Get(gs.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status + if gs.s.policyStore != nil { + policy, _ := gs.s.policyStore.Get(gs.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } } } diff --git a/pkg/policyStatus/mutateStats.go b/pkg/policyStatus/mutateStats.go index c27176d626..7d402fe8ed 100644 --- a/pkg/policyStatus/mutateStats.go +++ b/pkg/policyStatus/mutateStats.go @@ -28,12 +28,13 @@ func (ms *mutateStats) updateStatus() { } ms.s.cache.mutex.Lock() - var policyStatus v1.PolicyStatus policyStatus, exist := ms.s.cache.data[ms.resp.PolicyResponse.Policy] if !exist { - policy, _ := ms.s.policyStore.Get(ms.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status + if ms.s.policyStore != nil { + policy, _ := ms.s.policyStore.Get(ms.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } } } diff --git a/pkg/policyStatus/status_test.go b/pkg/policyStatus/status_test.go new file mode 100644 index 0000000000..a17bf53224 --- /dev/null +++ b/pkg/policyStatus/status_test.go @@ -0,0 +1,247 @@ +package policyStatus + +import ( + "encoding/json" + "reflect" + "testing" + "time" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + + "github.com/nirmata/kyverno/pkg/engine/response" +) + +func Test_Stats(t *testing.T) { + testCase := struct { + mutateStats []response.EngineResponse + validateStats []response.EngineResponse + generateStats []response.EngineResponse + violationCountStats []struct { + policyName string + violatedRules []v1.ViolatedRule + } + generatedCountStats []v1.GenerateRequest + expectedOutput []byte + }{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"1.482µs","violationCount":1,"rulesFailedCount":3,"rulesAppliedCount":3,"resourcesBlockedCount":1,"resourcesMutatedCount":1,"resourcesGeneratedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"243ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"251ns","failedCount":1},{"ruleName":"rule3","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"251ns","violationCount":1,"failedCount":1,"resourcesBlockedCount":1},{"ruleName":"rule5","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"251ns","failedCount":1}]},"policy2":{"averageExecutionTime":"1.299µs","violationCount":1,"rulesFailedCount":3,"rulesAppliedCount":3,"resourcesMutatedCount":1,"resourcesGeneratedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"222ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"211ns","failedCount":1},{"ruleName":"rule3","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"211ns","violationCount":1,"failedCount":1},{"ruleName":"rule5","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"211ns","failedCount":1}]}}`), + generatedCountStats: []v1.GenerateRequest{ + { + Spec: v1.GenerateRequestSpec{ + Policy: "policy1", + }, + Status: v1.GenerateRequestStatus{ + GeneratedResources: make([]v1.ResourceSpec, 1, 1), + }, + }, + { + Spec: v1.GenerateRequestSpec{ + Policy: "policy2", + }, + Status: v1.GenerateRequestStatus{ + GeneratedResources: make([]v1.ResourceSpec, 1, 1), + }, + }, + }, + violationCountStats: []struct { + policyName string + violatedRules []v1.ViolatedRule + }{ + { + policyName: "policy1", + violatedRules: []v1.ViolatedRule{ + { + Name: "rule4", + }, + }, + }, + { + policyName: "policy2", + violatedRules: []v1.ViolatedRule{ + { + Name: "rule4", + }, + }, + }, + }, + mutateStats: []response.EngineResponse{ + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy1", + Rules: []response.RuleResponse{ + { + Name: "rule1", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 243, + }, + }, + { + Name: "rule2", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 251, + }, + }, + }, + }, + }, + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy2", + Rules: []response.RuleResponse{ + { + Name: "rule1", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 222, + }, + }, + { + Name: "rule2", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 211, + }, + }, + }, + }, + }, + }, + validateStats: []response.EngineResponse{ + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy1", + ValidationFailureAction: "enforce", + Rules: []response.RuleResponse{ + { + Name: "rule3", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 243, + }, + }, + { + Name: "rule4", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 251, + }, + }, + }, + }, + }, + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy2", + Rules: []response.RuleResponse{ + { + Name: "rule3", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 222, + }, + }, + { + Name: "rule4", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 211, + }, + }, + }, + }, + }, + }, + generateStats: []response.EngineResponse{ + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy1", + Rules: []response.RuleResponse{ + { + Name: "rule5", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 243, + }, + }, + { + Name: "rule6", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 251, + }, + }, + }, + }, + }, + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy2", + Rules: []response.RuleResponse{ + { + Name: "rule5", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 222, + }, + }, + { + Name: "rule6", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 211, + }, + }, + }, + }, + }, + }, + } + + s := NewSync(nil, nil, nil) + for _, mutateStat := range testCase.mutateStats { + receiver := &mutateStats{ + s: s, + resp: mutateStat, + } + receiver.updateStatus() + } + + for _, validateStat := range testCase.validateStats { + receiver := &validateStats{ + s: s, + resp: validateStat, + } + receiver.updateStatus() + } + + for _, generateStat := range testCase.generateStats { + receiver := &generateStats{ + s: s, + resp: generateStat, + } + receiver.updateStatus() + } + + for _, generateCountStat := range testCase.generatedCountStats { + receiver := &generatedResourceCount{ + sync: s, + generateRequest: generateCountStat, + } + receiver.updateStatus() + } + + for _, violationCountStat := range testCase.violationCountStats { + receiver := &violationCount{ + sync: s, + policyName: violationCountStat.policyName, + violatedRules: violationCountStat.violatedRules, + } + receiver.updateStatus() + } + + output, _ := json.Marshal(s.cache.data) + if !reflect.DeepEqual(output, testCase.expectedOutput) { + t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) + } +} diff --git a/pkg/policyStatus/validateStats.go b/pkg/policyStatus/validateStats.go index fc44b60740..b77238b35a 100644 --- a/pkg/policyStatus/validateStats.go +++ b/pkg/policyStatus/validateStats.go @@ -27,12 +27,13 @@ func (vs *validateStats) updateStatus() { } vs.s.cache.mutex.Lock() - var policyStatus v1.PolicyStatus policyStatus, exist := vs.s.cache.data[vs.resp.PolicyResponse.Policy] if !exist { - policy, _ := vs.s.policyStore.Get(vs.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status + if vs.s.policyStore != nil { + policy, _ := vs.s.policyStore.Get(vs.resp.PolicyResponse.Policy) + if policy != nil { + policyStatus = policy.Status + } } } From 8c84e9af2b0861ddbbcbd4d983c44b113c541e4d Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 26 Feb 2020 13:56:45 +0530 Subject: [PATCH 21/34] 527 fixed ci --- pkg/policyStatus/status_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/policyStatus/status_test.go b/pkg/policyStatus/status_test.go index a17bf53224..375e412ffa 100644 --- a/pkg/policyStatus/status_test.go +++ b/pkg/policyStatus/status_test.go @@ -30,7 +30,7 @@ func Test_Stats(t *testing.T) { Policy: "policy1", }, Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1, 1), + GeneratedResources: make([]v1.ResourceSpec, 1), }, }, { @@ -38,7 +38,7 @@ func Test_Stats(t *testing.T) { Policy: "policy2", }, Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1, 1), + GeneratedResources: make([]v1.ResourceSpec, 1), }, }, }, From 053ccde6b872af52ec74d57eb632e7c4e8296767 Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 29 Feb 2020 17:19:00 +0530 Subject: [PATCH 22/34] 527 stopCh changes --- cmd/kyverno/main.go | 2 +- pkg/generate/status.go | 4 ++-- pkg/policyStatus/0_main.go | 16 +++++++--------- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index b60ebc77cb..261787cca8 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -248,7 +248,7 @@ func main() { go grc.Run(1, stopCh) go grcc.Run(1, stopCh) go pvgen.Run(1, stopCh) - go statusSync.Run(2) + go statusSync.Run(1, stopCh) // verifys if the admission control is enabled and active // resync: 60 seconds diff --git a/pkg/generate/status.go b/pkg/generate/status.go index b4c5b2c12c..f685800dd1 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -36,14 +36,14 @@ func (sc StatusControl) Failed(gr kyverno.GenerateRequest, message string, genRe // Success sets the gr status.state to completed and clears message func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyverno.ResourceSpec) error { - grCopy := gr.DeepCopy() + oldState := gr.Status.State gr.Status.State = kyverno.Completed gr.Status.Message = "" // Update Generated Resources gr.Status.GeneratedResources = genResources - if grCopy.Status.State != kyverno.Completed { + if oldState != kyverno.Completed { go sc.policyStatus.UpdatePolicyStatusWithGeneratedResourceCount(gr) } diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/0_main.go index 5357a90252..991595cb08 100644 --- a/pkg/policyStatus/0_main.go +++ b/pkg/policyStatus/0_main.go @@ -18,7 +18,6 @@ import ( type Sync struct { cache *cache listener chan statusUpdater - stop <-chan struct{} client *versioned.Clientset policyStore *policystore.PolicyStore } @@ -28,35 +27,34 @@ type cache struct { data map[string]v1.PolicyStatus } -func NewSync(c *versioned.Clientset, sc <-chan struct{}, pms *policystore.PolicyStore) *Sync { +func NewSync(c *versioned.Clientset, pms *policystore.PolicyStore) *Sync { return &Sync{ cache: &cache{ mutex: sync.RWMutex{}, data: make(map[string]v1.PolicyStatus), }, - stop: sc, client: c, policyStore: pms, listener: make(chan statusUpdater), } } -func (s *Sync) Run(workers int) { +func (s *Sync) Run(workers int, stopCh <-chan struct{}) { for i := 0; i < workers; i++ { - go s.updateStatusCache() + go s.updateStatusCache(stopCh) } - wait.Until(s.updatePolicyStatus, 2*time.Second, s.stop) - <-s.stop + wait.Until(s.updatePolicyStatus, 2*time.Second, stopCh) + <-stopCh s.updatePolicyStatus() } -func (s *Sync) updateStatusCache() { +func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { for { select { case statusUpdater := <-s.listener: statusUpdater.updateStatus() - case <-s.stop: + case <-stopCh: return } } From 40e92ebacf67f619c3ef04aec6de311806dce6cc Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 29 Feb 2020 22:39:27 +0530 Subject: [PATCH 23/34] 527 decoupling sender and reciever --- cmd/kyverno/main.go | 1 - pkg/generate/policyStatus_test.go | 58 ++++ pkg/generate/status.go | 30 +- pkg/policyStatus/1_interface.go | 5 - pkg/policyStatus/2_functions.go | 14 - pkg/policyStatus/generateStats.go | 85 ----- pkg/policyStatus/generatedResourceCount.go | 31 -- pkg/policyStatus/{0_main.go => main.go} | 52 +-- pkg/policyStatus/mutateStats.go | 88 ----- pkg/policyStatus/validateStats.go | 89 ----- pkg/policyStatus/violationCount.go | 41 --- pkg/policyviolation/clusterpv.go | 8 +- pkg/policyviolation/common.go | 39 +++ pkg/policyviolation/namespacedpv.go | 8 +- pkg/policyviolation/policyStatus_test.go | 74 ++++ pkg/webhooks/generation.go | 95 ++++- pkg/webhooks/mutation.go | 85 ++++- .../policyStatus_test.go} | 326 ++++++++---------- pkg/webhooks/validation.go | 85 ++++- 19 files changed, 652 insertions(+), 562 deletions(-) create mode 100644 pkg/generate/policyStatus_test.go delete mode 100644 pkg/policyStatus/1_interface.go delete mode 100644 pkg/policyStatus/2_functions.go delete mode 100644 pkg/policyStatus/generateStats.go delete mode 100644 pkg/policyStatus/generatedResourceCount.go rename pkg/policyStatus/{0_main.go => main.go} (54%) delete mode 100644 pkg/policyStatus/mutateStats.go delete mode 100644 pkg/policyStatus/validateStats.go delete mode 100644 pkg/policyStatus/violationCount.go create mode 100644 pkg/policyviolation/policyStatus_test.go rename pkg/{policyStatus/status_test.go => webhooks/policyStatus_test.go} (54%) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 261787cca8..6a396b8f2e 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -140,7 +140,6 @@ func main() { // Policy Status Handler - deals with all logic related to policy status statusSync := policyStatus.NewSync( pclient, - stopCh, policyMetaStore) // POLICY VIOLATION GENERATOR diff --git a/pkg/generate/policyStatus_test.go b/pkg/generate/policyStatus_test.go new file mode 100644 index 0000000000..fad3eacd79 --- /dev/null +++ b/pkg/generate/policyStatus_test.go @@ -0,0 +1,58 @@ +package generate + +import ( + "encoding/json" + "reflect" + "testing" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + "github.com/nirmata/kyverno/pkg/policyStatus" +) + +type dummyStore struct { +} + +func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { + return &v1.ClusterPolicy{}, nil +} + +func Test_Stats(t *testing.T) { + testCase := struct { + generatedCountStats []v1.GenerateRequest + expectedOutput []byte + }{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"","resourcesGeneratedCount":1},"policy2":{"averageExecutionTime":"","resourcesGeneratedCount":1}}`), + generatedCountStats: []v1.GenerateRequest{ + { + Spec: v1.GenerateRequestSpec{ + Policy: "policy1", + }, + Status: v1.GenerateRequestStatus{ + GeneratedResources: make([]v1.ResourceSpec, 1), + }, + }, + { + Spec: v1.GenerateRequestSpec{ + Policy: "policy2", + }, + Status: v1.GenerateRequestStatus{ + GeneratedResources: make([]v1.ResourceSpec, 1), + }, + }, + }, + } + + s := policyStatus.NewSync(nil, &dummyStore{}) + + for _, generateCountStat := range testCase.generatedCountStats { + receiver := &generatedResourceCount{ + generateRequest: generateCountStat, + } + receiver.UpdateStatus(s) + } + + output, _ := json.Marshal(s.Cache.Data) + if !reflect.DeepEqual(output, testCase.expectedOutput) { + t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) + } +} diff --git a/pkg/generate/status.go b/pkg/generate/status.go index f685800dd1..f68e564f52 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -44,7 +44,9 @@ func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyver gr.Status.GeneratedResources = genResources if oldState != kyverno.Completed { - go sc.policyStatus.UpdatePolicyStatusWithGeneratedResourceCount(gr) + go func() { + sc.policyStatus.Listener <- updatePolicyStatusWithGeneratedResourceCount(gr) + }() } _, err := sc.client.KyvernoV1().GenerateRequests("kyverno").UpdateStatus(&gr) @@ -55,3 +57,29 @@ func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyver glog.V(4).Infof("updated gr %s status to %s", gr.Name, string(kyverno.Completed)) return nil } + +type generatedResourceCount struct { + generateRequest kyverno.GenerateRequest +} + +func updatePolicyStatusWithGeneratedResourceCount(generateRequest kyverno.GenerateRequest) *generatedResourceCount { + return &generatedResourceCount{ + generateRequest: generateRequest, + } +} + +func (vc *generatedResourceCount) UpdateStatus(s *policyStatus.Sync) { + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[vc.generateRequest.Spec.Policy] + if !exist { + policy, _ := s.PolicyStore.Get(vc.generateRequest.Spec.Policy) + if policy != nil { + status = policy.Status + } + } + + status.ResourcesGeneratedCount += len(vc.generateRequest.Status.GeneratedResources) + + s.Cache.Data[vc.generateRequest.Spec.Policy] = status + s.Cache.Mutex.Unlock() +} diff --git a/pkg/policyStatus/1_interface.go b/pkg/policyStatus/1_interface.go deleted file mode 100644 index 942337fb8a..0000000000 --- a/pkg/policyStatus/1_interface.go +++ /dev/null @@ -1,5 +0,0 @@ -package policyStatus - -type statusUpdater interface { - updateStatus() -} diff --git a/pkg/policyStatus/2_functions.go b/pkg/policyStatus/2_functions.go deleted file mode 100644 index db18b57ea8..0000000000 --- a/pkg/policyStatus/2_functions.go +++ /dev/null @@ -1,14 +0,0 @@ -package policyStatus - -import "time" - -func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { - if averageOver == 0 { - return newTime - } - oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) - numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() - denominator := averageOver + 1 - newAverageTimeInNanoSeconds := numerator / denominator - return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond -} diff --git a/pkg/policyStatus/generateStats.go b/pkg/policyStatus/generateStats.go deleted file mode 100644 index eabe280af6..0000000000 --- a/pkg/policyStatus/generateStats.go +++ /dev/null @@ -1,85 +0,0 @@ -package policyStatus - -import ( - "reflect" - "sort" - "time" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/response" -) - -type generateStats struct { - s *Sync - resp response.EngineResponse -} - -func (s *Sync) UpdateStatusWithGenerateStats(resp response.EngineResponse) { - s.listener <- &generateStats{ - s: s, - resp: resp, - } -} - -func (gs *generateStats) updateStatus() { - if reflect.DeepEqual(response.EngineResponse{}, gs.resp) { - return - } - - gs.s.cache.mutex.Lock() - policyStatus, exist := gs.s.cache.data[gs.resp.PolicyResponse.Policy] - if !exist { - if gs.s.policyStore != nil { - policy, _ := gs.s.policyStore.Get(gs.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - } - - var nameToRule = make(map[string]v1.RuleStats) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range gs.resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - ruleStat.AppliedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - gs.s.cache.data[gs.resp.PolicyResponse.Policy] = policyStatus - gs.s.cache.mutex.Unlock() -} diff --git a/pkg/policyStatus/generatedResourceCount.go b/pkg/policyStatus/generatedResourceCount.go deleted file mode 100644 index f951cf759c..0000000000 --- a/pkg/policyStatus/generatedResourceCount.go +++ /dev/null @@ -1,31 +0,0 @@ -package policyStatus - -import v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - -type generatedResourceCount struct { - sync *Sync - generateRequest v1.GenerateRequest -} - -func (s *Sync) UpdatePolicyStatusWithGeneratedResourceCount(generateRequest v1.GenerateRequest) { - s.listener <- &generatedResourceCount{ - sync: s, - generateRequest: generateRequest, - } -} - -func (vc *generatedResourceCount) updateStatus() { - vc.sync.cache.mutex.Lock() - status, exist := vc.sync.cache.data[vc.generateRequest.Spec.Policy] - if !exist { - policy, _ := vc.sync.policyStore.Get(vc.generateRequest.Spec.Policy) - if policy != nil { - status = policy.Status - } - } - - status.ResourcesGeneratedCount += len(vc.generateRequest.Status.GeneratedResources) - - vc.sync.cache.data[vc.generateRequest.Spec.Policy] = status - vc.sync.cache.mutex.Unlock() -} diff --git a/pkg/policyStatus/0_main.go b/pkg/policyStatus/main.go similarity index 54% rename from pkg/policyStatus/0_main.go rename to pkg/policyStatus/main.go index 991595cb08..efa7e30d4a 100644 --- a/pkg/policyStatus/0_main.go +++ b/pkg/policyStatus/main.go @@ -6,8 +6,6 @@ import ( "github.com/golang/glog" - "github.com/nirmata/kyverno/pkg/policystore" - "k8s.io/apimachinery/pkg/util/wait" "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -15,27 +13,35 @@ import ( v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) +type statusUpdater interface { + UpdateStatus(s *Sync) +} + +type policyStore interface { + Get(policyName string) (*v1.ClusterPolicy, error) +} + type Sync struct { - cache *cache - listener chan statusUpdater + Cache *cache + Listener chan statusUpdater client *versioned.Clientset - policyStore *policystore.PolicyStore + PolicyStore policyStore } type cache struct { - mutex sync.RWMutex - data map[string]v1.PolicyStatus + Mutex sync.RWMutex + Data map[string]v1.PolicyStatus } -func NewSync(c *versioned.Clientset, pms *policystore.PolicyStore) *Sync { +func NewSync(c *versioned.Clientset, p policyStore) *Sync { return &Sync{ - cache: &cache{ - mutex: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), + Cache: &cache{ + Mutex: sync.RWMutex{}, + Data: make(map[string]v1.PolicyStatus), }, client: c, - policyStore: pms, - listener: make(chan statusUpdater), + PolicyStore: p, + Listener: make(chan statusUpdater), } } @@ -52,8 +58,8 @@ func (s *Sync) Run(workers int, stopCh <-chan struct{}) { func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { for { select { - case statusUpdater := <-s.listener: - statusUpdater.updateStatus() + case statusUpdater := <-s.Listener: + statusUpdater.UpdateStatus(s) case <-stopCh: return } @@ -61,24 +67,24 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { } func (s *Sync) updatePolicyStatus() { - s.cache.mutex.Lock() - var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) - for k, v := range s.cache.data { + s.Cache.Mutex.Lock() + var nameToStatus = make(map[string]v1.PolicyStatus, len(s.Cache.Data)) + for k, v := range s.Cache.Data { nameToStatus[k] = v } - s.cache.mutex.Unlock() + s.Cache.Mutex.Unlock() for policyName, status := range nameToStatus { - policy, err := s.policyStore.Get(policyName) + policy, err := s.PolicyStore.Get(policyName) if err != nil { continue } policy.Status = status _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) if err != nil { - s.cache.mutex.Lock() - delete(s.cache.data, policyName) - s.cache.mutex.Unlock() + s.Cache.Mutex.Lock() + delete(s.Cache.Data, policyName) + s.Cache.Mutex.Unlock() glog.V(4).Info(err) } } diff --git a/pkg/policyStatus/mutateStats.go b/pkg/policyStatus/mutateStats.go deleted file mode 100644 index 7d402fe8ed..0000000000 --- a/pkg/policyStatus/mutateStats.go +++ /dev/null @@ -1,88 +0,0 @@ -package policyStatus - -import ( - "reflect" - "sort" - "time" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/response" -) - -type mutateStats struct { - s *Sync - resp response.EngineResponse -} - -func (s *Sync) UpdateStatusWithMutateStats(resp response.EngineResponse) { - s.listener <- &mutateStats{ - s: s, - resp: resp, - } - -} - -func (ms *mutateStats) updateStatus() { - if reflect.DeepEqual(response.EngineResponse{}, ms.resp) { - return - } - - ms.s.cache.mutex.Lock() - policyStatus, exist := ms.s.cache.data[ms.resp.PolicyResponse.Policy] - if !exist { - if ms.s.policyStore != nil { - policy, _ := ms.s.policyStore.Get(ms.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - } - - var nameToRule = make(map[string]v1.RuleStats) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range ms.resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - policyStatus.ResourcesMutatedCount++ - ruleStat.AppliedCount++ - ruleStat.ResourcesMutatedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - ms.s.cache.data[ms.resp.PolicyResponse.Policy] = policyStatus - ms.s.cache.mutex.Unlock() -} diff --git a/pkg/policyStatus/validateStats.go b/pkg/policyStatus/validateStats.go deleted file mode 100644 index b77238b35a..0000000000 --- a/pkg/policyStatus/validateStats.go +++ /dev/null @@ -1,89 +0,0 @@ -package policyStatus - -import ( - "reflect" - "sort" - "time" - - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/response" -) - -type validateStats struct { - s *Sync - resp response.EngineResponse -} - -func (s *Sync) UpdateStatusWithValidateStats(resp response.EngineResponse) { - s.listener <- &validateStats{ - s: s, - resp: resp, - } -} - -func (vs *validateStats) updateStatus() { - if reflect.DeepEqual(response.EngineResponse{}, vs.resp) { - return - } - - vs.s.cache.mutex.Lock() - policyStatus, exist := vs.s.cache.data[vs.resp.PolicyResponse.Policy] - if !exist { - if vs.s.policyStore != nil { - policy, _ := vs.s.policyStore.Get(vs.resp.PolicyResponse.Policy) - if policy != nil { - policyStatus = policy.Status - } - } - } - - var nameToRule = make(map[string]v1.RuleStats) - for _, rule := range policyStatus.Rules { - nameToRule[rule.Name] = rule - } - - for _, rule := range vs.resp.PolicyResponse.Rules { - ruleStat := nameToRule[rule.Name] - ruleStat.Name = rule.Name - - averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) - ruleStat.ExecutionTime = updateAverageTime( - rule.ProcessingTime, - ruleStat.ExecutionTime, - averageOver).String() - - if rule.Success { - policyStatus.RulesAppliedCount++ - ruleStat.AppliedCount++ - } else { - policyStatus.RulesFailedCount++ - ruleStat.FailedCount++ - if vs.resp.PolicyResponse.ValidationFailureAction == "enforce" { - policyStatus.ResourcesBlockedCount++ - ruleStat.ResourcesBlockedCount++ - } - } - - nameToRule[rule.Name] = ruleStat - } - - var policyAverageExecutionTime time.Duration - var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) - for _, ruleStat := range nameToRule { - executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) - if err == nil { - policyAverageExecutionTime += executionTime - } - ruleStats = append(ruleStats, ruleStat) - } - - sort.Slice(ruleStats, func(i, j int) bool { - return ruleStats[i].Name < ruleStats[j].Name - }) - - policyStatus.AvgExecutionTime = policyAverageExecutionTime.String() - policyStatus.Rules = ruleStats - - vs.s.cache.data[vs.resp.PolicyResponse.Policy] = policyStatus - vs.s.cache.mutex.Unlock() -} diff --git a/pkg/policyStatus/violationCount.go b/pkg/policyStatus/violationCount.go deleted file mode 100644 index dd7ed30574..0000000000 --- a/pkg/policyStatus/violationCount.go +++ /dev/null @@ -1,41 +0,0 @@ -package policyStatus - -import v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - -type violationCount struct { - sync *Sync - policyName string - violatedRules []v1.ViolatedRule -} - -func (s *Sync) UpdatePolicyStatusWithViolationCount(policyName string, violatedRules []v1.ViolatedRule) { - s.listener <- &violationCount{ - sync: s, - policyName: policyName, - violatedRules: violatedRules, - } -} - -func (vc *violationCount) updateStatus() { - vc.sync.cache.mutex.Lock() - status, exist := vc.sync.cache.data[vc.policyName] - if !exist { - policy, _ := vc.sync.policyStore.Get(vc.policyName) - if policy != nil { - status = policy.Status - } - } - - var ruleNameToViolations = make(map[string]int) - for _, rule := range vc.violatedRules { - ruleNameToViolations[rule.Name]++ - } - - for i := range status.Rules { - status.ViolationCount += ruleNameToViolations[status.Rules[i].Name] - status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] - } - - vc.sync.cache.data[vc.policyName] = status - vc.sync.cache.mutex.Unlock() -} diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 42a5c45fbe..3b57ff2597 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -100,7 +100,9 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + go func() { + cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + }() } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) @@ -126,7 +128,9 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) if newPv.Annotations["fromSync"] != "true" { - go cpv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + go func() { + cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + }() } return nil } diff --git a/pkg/policyviolation/common.go b/pkg/policyviolation/common.go index dfb4a704ac..44bf0e46ae 100644 --- a/pkg/policyviolation/common.go +++ b/pkg/policyviolation/common.go @@ -4,9 +4,12 @@ import ( "fmt" "time" + "github.com/nirmata/kyverno/pkg/policyStatus" + backoff "github.com/cenkalti/backoff" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" client "github.com/nirmata/kyverno/pkg/dclient" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -70,3 +73,39 @@ func converLabelToSelector(labelMap map[string]string) (labels.Selector, error) return policyViolationSelector, nil } + +type violationCount struct { + policyName string + violatedRules []v1.ViolatedRule +} + +func updatePolicyStatusWithViolationCount(policyName string, violatedRules []kyverno.ViolatedRule) *violationCount { + return &violationCount{ + policyName: policyName, + violatedRules: violatedRules, + } +} + +func (vc *violationCount) UpdateStatus(s *policyStatus.Sync) { + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[vc.policyName] + if !exist { + policy, _ := s.PolicyStore.Get(vc.policyName) + if policy != nil { + status = policy.Status + } + } + + var ruleNameToViolations = make(map[string]int) + for _, rule := range vc.violatedRules { + ruleNameToViolations[rule.Name]++ + } + + for i := range status.Rules { + status.ViolationCount += ruleNameToViolations[status.Rules[i].Name] + status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] + } + + s.Cache.Data[vc.policyName] = status + s.Cache.Mutex.Unlock() +} diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index cc6440afaf..10e92ef24a 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -99,7 +99,9 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + go func() { + nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + }() } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil @@ -122,7 +124,9 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error } if newPv.Annotations["fromSync"] != "true" { - go nspv.policyStatus.UpdatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + go func() { + nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + }() } glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil diff --git a/pkg/policyviolation/policyStatus_test.go b/pkg/policyviolation/policyStatus_test.go new file mode 100644 index 0000000000..599dd42aa3 --- /dev/null +++ b/pkg/policyviolation/policyStatus_test.go @@ -0,0 +1,74 @@ +package policyviolation + +import ( + "encoding/json" + "reflect" + "testing" + + "github.com/nirmata/kyverno/pkg/policyStatus" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" +) + +type dummyStore struct { +} + +func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { + return &v1.ClusterPolicy{ + Status: v1.PolicyStatus{ + Rules: []v1.RuleStats{ + { + Name: "rule4", + }, + }, + }, + }, nil +} + +func Test_Stats(t *testing.T) { + testCase := struct { + violationCountStats []struct { + policyName string + violatedRules []v1.ViolatedRule + } + expectedOutput []byte + }{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"","violationCount":1,"ruleStatus":[{"ruleName":"rule4","violationCount":1}]},"policy2":{"averageExecutionTime":"","violationCount":1,"ruleStatus":[{"ruleName":"rule4","violationCount":1}]}}`), + violationCountStats: []struct { + policyName string + violatedRules []v1.ViolatedRule + }{ + { + policyName: "policy1", + violatedRules: []v1.ViolatedRule{ + { + Name: "rule4", + }, + }, + }, + { + policyName: "policy2", + violatedRules: []v1.ViolatedRule{ + { + Name: "rule4", + }, + }, + }, + }, + } + + s := policyStatus.NewSync(nil, &dummyStore{}) + + for _, violationCountStat := range testCase.violationCountStats { + receiver := &violationCount{ + policyName: violationCountStat.policyName, + violatedRules: violationCountStat.violatedRules, + } + receiver.UpdateStatus(s) + } + + output, _ := json.Marshal(s.Cache.Data) + if !reflect.DeepEqual(output, testCase.expectedOutput) { + t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) + } +} diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 6edf74a94e..b62de1bddb 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -1,8 +1,15 @@ package webhooks import ( + "reflect" + "sort" + "time" + + "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" @@ -61,7 +68,9 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) - go ws.status.UpdateStatusWithGenerateStats(engineResponse) + go func() { + ws.status.Listener <- updateStatusWithGenerateStats(engineResponse) + }() } } // Adds Generate Request to a channel(queue size 1000) to generators @@ -103,3 +112,87 @@ func transform(userRequestInfo kyverno.RequestInfo, er response.EngineResponse) } return gr } + +type generateStats struct { + resp response.EngineResponse +} + +func updateStatusWithGenerateStats(resp response.EngineResponse) *generateStats { + return &generateStats{ + resp: resp, + } +} + +func (gs *generateStats) UpdateStatus(s *policyStatus.Sync) { + if reflect.DeepEqual(response.EngineResponse{}, gs.resp) { + return + } + + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[gs.resp.PolicyResponse.Policy] + if !exist { + if s.PolicyStore != nil { + policy, _ := s.PolicyStore.Get(gs.resp.PolicyResponse.Policy) + if policy != nil { + status = policy.Status + } + } + } + + var nameToRule = make(map[string]v1.RuleStats) + for _, rule := range status.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range gs.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + status.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + status.RulesFailedCount++ + ruleStat.FailedCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + status.AvgExecutionTime = policyAverageExecutionTime.String() + status.Rules = ruleStats + + s.Cache.Data[gs.resp.PolicyResponse.Policy] = status + s.Cache.Mutex.Unlock() +} + +func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { + if averageOver == 0 { + return newTime + } + oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) + numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() + denominator := averageOver + 1 + newAverageTimeInNanoSeconds := numerator / denominator + return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond +} diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 90f9ee70e0..cb23ca2703 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -1,10 +1,15 @@ package webhooks import ( + "reflect" + "sort" "time" + "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" @@ -58,7 +63,9 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) - go ws.status.UpdateStatusWithMutateStats(engineResponse) + go func() { + ws.status.Listener <- updateStatusWithMutateStats(engineResponse) + }() if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue @@ -106,3 +113,79 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou // patches holds all the successful patches, if no patch is created, it returns nil return engineutils.JoinPatches(patches) } + +type mutateStats struct { + resp response.EngineResponse +} + +func updateStatusWithMutateStats(resp response.EngineResponse) *mutateStats { + return &mutateStats{ + resp: resp, + } + +} + +func (ms *mutateStats) UpdateStatus(s *policyStatus.Sync) { + if reflect.DeepEqual(response.EngineResponse{}, ms.resp) { + return + } + + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[ms.resp.PolicyResponse.Policy] + if !exist { + if s.PolicyStore != nil { + policy, _ := s.PolicyStore.Get(ms.resp.PolicyResponse.Policy) + if policy != nil { + status = policy.Status + } + } + } + + var nameToRule = make(map[string]v1.RuleStats) + for _, rule := range status.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range ms.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + status.RulesAppliedCount++ + status.ResourcesMutatedCount++ + ruleStat.AppliedCount++ + ruleStat.ResourcesMutatedCount++ + } else { + status.RulesFailedCount++ + ruleStat.FailedCount++ + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + status.AvgExecutionTime = policyAverageExecutionTime.String() + status.Rules = ruleStats + + s.Cache.Data[ms.resp.PolicyResponse.Policy] = status + s.Cache.Mutex.Unlock() +} diff --git a/pkg/policyStatus/status_test.go b/pkg/webhooks/policyStatus_test.go similarity index 54% rename from pkg/policyStatus/status_test.go rename to pkg/webhooks/policyStatus_test.go index 375e412ffa..e85c8522f6 100644 --- a/pkg/policyStatus/status_test.go +++ b/pkg/webhooks/policyStatus_test.go @@ -1,4 +1,4 @@ -package policyStatus +package webhooks import ( "encoding/json" @@ -7,151 +7,23 @@ import ( "time" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/engine/response" + "github.com/nirmata/kyverno/pkg/policyStatus" ) -func Test_Stats(t *testing.T) { +type dummyStore struct { +} + +func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { + return &v1.ClusterPolicy{}, nil +} + +func Test_GenerateStats(t *testing.T) { testCase := struct { - mutateStats []response.EngineResponse - validateStats []response.EngineResponse - generateStats []response.EngineResponse - violationCountStats []struct { - policyName string - violatedRules []v1.ViolatedRule - } - generatedCountStats []v1.GenerateRequest - expectedOutput []byte + generateStats []response.EngineResponse + expectedOutput []byte }{ - expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"1.482µs","violationCount":1,"rulesFailedCount":3,"rulesAppliedCount":3,"resourcesBlockedCount":1,"resourcesMutatedCount":1,"resourcesGeneratedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"243ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"251ns","failedCount":1},{"ruleName":"rule3","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"251ns","violationCount":1,"failedCount":1,"resourcesBlockedCount":1},{"ruleName":"rule5","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"251ns","failedCount":1}]},"policy2":{"averageExecutionTime":"1.299µs","violationCount":1,"rulesFailedCount":3,"rulesAppliedCount":3,"resourcesMutatedCount":1,"resourcesGeneratedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"222ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"211ns","failedCount":1},{"ruleName":"rule3","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"211ns","violationCount":1,"failedCount":1},{"ruleName":"rule5","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"211ns","failedCount":1}]}}`), - generatedCountStats: []v1.GenerateRequest{ - { - Spec: v1.GenerateRequestSpec{ - Policy: "policy1", - }, - Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1), - }, - }, - { - Spec: v1.GenerateRequestSpec{ - Policy: "policy2", - }, - Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1), - }, - }, - }, - violationCountStats: []struct { - policyName string - violatedRules []v1.ViolatedRule - }{ - { - policyName: "policy1", - violatedRules: []v1.ViolatedRule{ - { - Name: "rule4", - }, - }, - }, - { - policyName: "policy2", - violatedRules: []v1.ViolatedRule{ - { - Name: "rule4", - }, - }, - }, - }, - mutateStats: []response.EngineResponse{ - { - PolicyResponse: response.PolicyResponse{ - Policy: "policy1", - Rules: []response.RuleResponse{ - { - Name: "rule1", - Success: true, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 243, - }, - }, - { - Name: "rule2", - Success: false, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 251, - }, - }, - }, - }, - }, - { - PolicyResponse: response.PolicyResponse{ - Policy: "policy2", - Rules: []response.RuleResponse{ - { - Name: "rule1", - Success: true, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 222, - }, - }, - { - Name: "rule2", - Success: false, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 211, - }, - }, - }, - }, - }, - }, - validateStats: []response.EngineResponse{ - { - PolicyResponse: response.PolicyResponse{ - Policy: "policy1", - ValidationFailureAction: "enforce", - Rules: []response.RuleResponse{ - { - Name: "rule3", - Success: true, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 243, - }, - }, - { - Name: "rule4", - Success: false, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 251, - }, - }, - }, - }, - }, - { - PolicyResponse: response.PolicyResponse{ - Policy: "policy2", - Rules: []response.RuleResponse{ - { - Name: "rule3", - Success: true, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 222, - }, - }, - { - Name: "rule4", - Success: false, - RuleStats: response.RuleStats{ - ProcessingTime: time.Nanosecond * 211, - }, - }, - }, - }, - }, - }, + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"494ns","rulesFailedCount":1,"rulesAppliedCount":1,"ruleStatus":[{"ruleName":"rule5","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"251ns","failedCount":1}]},"policy2":{"averageExecutionTime":"433ns","rulesFailedCount":1,"rulesAppliedCount":1,"ruleStatus":[{"ruleName":"rule5","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule6","averageExecutionTime":"211ns","failedCount":1}]}}`), generateStats: []response.EngineResponse{ { PolicyResponse: response.PolicyResponse{ @@ -198,49 +70,149 @@ func Test_Stats(t *testing.T) { }, } - s := NewSync(nil, nil, nil) - for _, mutateStat := range testCase.mutateStats { - receiver := &mutateStats{ - s: s, - resp: mutateStat, - } - receiver.updateStatus() - } - - for _, validateStat := range testCase.validateStats { - receiver := &validateStats{ - s: s, - resp: validateStat, - } - receiver.updateStatus() - } + s := policyStatus.NewSync(nil, &dummyStore{}) for _, generateStat := range testCase.generateStats { receiver := &generateStats{ - s: s, resp: generateStat, } - receiver.updateStatus() + receiver.UpdateStatus(s) } - for _, generateCountStat := range testCase.generatedCountStats { - receiver := &generatedResourceCount{ - sync: s, - generateRequest: generateCountStat, - } - receiver.updateStatus() - } - - for _, violationCountStat := range testCase.violationCountStats { - receiver := &violationCount{ - sync: s, - policyName: violationCountStat.policyName, - violatedRules: violationCountStat.violatedRules, - } - receiver.updateStatus() - } - - output, _ := json.Marshal(s.cache.data) + output, _ := json.Marshal(s.Cache.Data) + if !reflect.DeepEqual(output, testCase.expectedOutput) { + t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) + } +} + +func Test_MutateStats(t *testing.T) { + testCase := struct { + mutateStats []response.EngineResponse + expectedOutput []byte + }{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"494ns","rulesFailedCount":1,"rulesAppliedCount":1,"resourcesMutatedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"243ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"251ns","failedCount":1}]},"policy2":{"averageExecutionTime":"433ns","rulesFailedCount":1,"rulesAppliedCount":1,"resourcesMutatedCount":1,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"222ns","appliedCount":1,"resourcesMutatedCount":1},{"ruleName":"rule2","averageExecutionTime":"211ns","failedCount":1}]}}`), + mutateStats: []response.EngineResponse{ + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy1", + Rules: []response.RuleResponse{ + { + Name: "rule1", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 243, + }, + }, + { + Name: "rule2", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 251, + }, + }, + }, + }, + }, + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy2", + Rules: []response.RuleResponse{ + { + Name: "rule1", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 222, + }, + }, + { + Name: "rule2", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 211, + }, + }, + }, + }, + }, + }, + } + + s := policyStatus.NewSync(nil, &dummyStore{}) + for _, mutateStat := range testCase.mutateStats { + receiver := &mutateStats{ + resp: mutateStat, + } + receiver.UpdateStatus(s) + } + + output, _ := json.Marshal(s.Cache.Data) + if !reflect.DeepEqual(output, testCase.expectedOutput) { + t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) + } +} + +func Test_ValidateStats(t *testing.T) { + testCase := struct { + validateStats []response.EngineResponse + expectedOutput []byte + }{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"494ns","rulesFailedCount":1,"rulesAppliedCount":1,"resourcesBlockedCount":1,"ruleStatus":[{"ruleName":"rule3","averageExecutionTime":"243ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"251ns","failedCount":1,"resourcesBlockedCount":1}]},"policy2":{"averageExecutionTime":"433ns","rulesFailedCount":1,"rulesAppliedCount":1,"ruleStatus":[{"ruleName":"rule3","averageExecutionTime":"222ns","appliedCount":1},{"ruleName":"rule4","averageExecutionTime":"211ns","failedCount":1}]}}`), + validateStats: []response.EngineResponse{ + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy1", + ValidationFailureAction: "enforce", + Rules: []response.RuleResponse{ + { + Name: "rule3", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 243, + }, + }, + { + Name: "rule4", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 251, + }, + }, + }, + }, + }, + { + PolicyResponse: response.PolicyResponse{ + Policy: "policy2", + Rules: []response.RuleResponse{ + { + Name: "rule3", + Success: true, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 222, + }, + }, + { + Name: "rule4", + Success: false, + RuleStats: response.RuleStats{ + ProcessingTime: time.Nanosecond * 211, + }, + }, + }, + }, + }, + }, + } + + s := policyStatus.NewSync(nil, &dummyStore{}) + for _, validateStat := range testCase.validateStats { + receiver := &validateStats{ + resp: validateStat, + } + receiver.UpdateStatus(s) + } + + output, _ := json.Marshal(s.Cache.Data) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index 41761dd071..5f8051ff0f 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -2,10 +2,14 @@ package webhooks import ( "reflect" + "sort" "time" + "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine" "github.com/nirmata/kyverno/pkg/engine/context" "github.com/nirmata/kyverno/pkg/engine/response" @@ -69,7 +73,9 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) - go ws.status.UpdateStatusWithValidateStats(engineResponse) + go func() { + ws.status.Listener <- updateStatusWithValidateStats(engineResponse) + }() if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue @@ -99,3 +105,80 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol glog.V(4).Infof("report: %v %s/%s/%s", time.Since(reportTime), request.Kind, request.Namespace, request.Name) return true, "" } + +type validateStats struct { + resp response.EngineResponse +} + +func updateStatusWithValidateStats(resp response.EngineResponse) *validateStats { + return &validateStats{ + resp: resp, + } +} + +func (vs *validateStats) UpdateStatus(s *policyStatus.Sync) { + if reflect.DeepEqual(response.EngineResponse{}, vs.resp) { + return + } + + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[vs.resp.PolicyResponse.Policy] + if !exist { + if s.PolicyStore != nil { + policy, _ := s.PolicyStore.Get(vs.resp.PolicyResponse.Policy) + if policy != nil { + status = policy.Status + } + } + } + + var nameToRule = make(map[string]v1.RuleStats) + for _, rule := range status.Rules { + nameToRule[rule.Name] = rule + } + + for _, rule := range vs.resp.PolicyResponse.Rules { + ruleStat := nameToRule[rule.Name] + ruleStat.Name = rule.Name + + averageOver := int64(ruleStat.AppliedCount + ruleStat.FailedCount) + ruleStat.ExecutionTime = updateAverageTime( + rule.ProcessingTime, + ruleStat.ExecutionTime, + averageOver).String() + + if rule.Success { + status.RulesAppliedCount++ + ruleStat.AppliedCount++ + } else { + status.RulesFailedCount++ + ruleStat.FailedCount++ + if vs.resp.PolicyResponse.ValidationFailureAction == "enforce" { + status.ResourcesBlockedCount++ + ruleStat.ResourcesBlockedCount++ + } + } + + nameToRule[rule.Name] = ruleStat + } + + var policyAverageExecutionTime time.Duration + var ruleStats = make([]v1.RuleStats, 0, len(nameToRule)) + for _, ruleStat := range nameToRule { + executionTime, err := time.ParseDuration(ruleStat.ExecutionTime) + if err == nil { + policyAverageExecutionTime += executionTime + } + ruleStats = append(ruleStats, ruleStat) + } + + sort.Slice(ruleStats, func(i, j int) bool { + return ruleStats[i].Name < ruleStats[j].Name + }) + + status.AvgExecutionTime = policyAverageExecutionTime.String() + status.Rules = ruleStats + + s.Cache.Data[vs.resp.PolicyResponse.Policy] = status + s.Cache.Mutex.Unlock() +} From fdb1cc36ac4e437fbca4290e020a7d7814ee3a3d Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 4 Mar 2020 13:11:48 +0530 Subject: [PATCH 24/34] 527 getting generate stats from sync --- pkg/generate/controller.go | 5 ++- pkg/generate/generate.go | 66 +++++++++++++++++++++++++++++-- pkg/generate/policyStatus_test.go | 2 +- pkg/generate/status.go | 38 +----------------- pkg/policyStatus/main.go | 1 - 5 files changed, 69 insertions(+), 43 deletions(-) diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index e5f4c2a8e8..6d877d9448 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -58,6 +58,8 @@ type Controller struct { //TODO: list of generic informers // only support Namespaces for re-evalutation on resource updates nsInformer informers.GenericInformer + + policyStatus *policyStatus.Sync } //NewController returns an instance of the Generate-Request Controller @@ -80,8 +82,9 @@ func NewController( // as we dont want a deleted GR to be re-queue queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(1, 30), "generate-request"), dynamicInformer: dynamicInformer, + policyStatus: policyStatus, } - c.statusControl = StatusControl{client: kyvernoclient, policyStatus: policyStatus} + c.statusControl = StatusControl{client: kyvernoclient} pInformer.Informer().AddEventHandlerWithResyncPeriod(cache.ResourceEventHandlerFuncs{ UpdateFunc: c.updatePolicy, // We only handle updates to policy diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index cd9ab1b2fd..48d53cae4c 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -3,6 +3,9 @@ package generate import ( "encoding/json" "fmt" + "time" + + "github.com/nirmata/kyverno/pkg/policyStatus" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -80,7 +83,7 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern } // Apply the generate rule on resource - return applyGeneratePolicy(c.client, policyContext) + return c.applyGeneratePolicy(policyContext, gr) } func updateStatus(statusControl StatusControlInterface, gr kyverno.GenerateRequest, err error, genResources []kyverno.ResourceSpec) error { @@ -92,7 +95,7 @@ func updateStatus(statusControl StatusControlInterface, gr kyverno.GenerateReque return statusControl.Success(gr, genResources) } -func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyContext) ([]kyverno.ResourceSpec, error) { +func (c *Controller) applyGeneratePolicy(policyContext engine.PolicyContext, gr kyverno.GenerateRequest) ([]kyverno.ResourceSpec, error) { // List of generatedResources var genResources []kyverno.ResourceSpec // Get the response as the actions to be performed on the resource @@ -107,20 +110,77 @@ func applyGeneratePolicy(client *dclient.Client, policyContext engine.PolicyCont return rcreationTime.Before(&pcreationTime) }() + ruleNameToProcessingTime := make(map[string]time.Duration) for _, rule := range policy.Spec.Rules { if !rule.HasGenerate() { continue } - genResource, err := applyRule(client, rule, resource, ctx, processExisting) + + startTime := time.Now() + genResource, err := applyRule(c.client, rule, resource, ctx, processExisting) if err != nil { return nil, err } + + ruleNameToProcessingTime[rule.Name] = time.Since(startTime) genResources = append(genResources, genResource) } + if gr.Status.State == "" { + go func() { + c.policyStatus.Listener <- &generateSyncStats{ + policyName: policy.Name, + ruleNameToProcessingTime: ruleNameToProcessingTime, + } + }() + } + return genResources, nil } +type generateSyncStats struct { + policyName string + ruleNameToProcessingTime map[string]time.Duration +} + +func (vc *generateSyncStats) UpdateStatus(s *policyStatus.Sync) { + s.Cache.Mutex.Lock() + status, exist := s.Cache.Data[vc.policyName] + if !exist { + policy, _ := s.PolicyStore.Get(vc.policyName) + if policy != nil { + status = policy.Status + } + } + + for i := range status.Rules { + if executionTime, exist := vc.ruleNameToProcessingTime[status.Rules[i].Name]; exist { + status.ResourcesGeneratedCount += 1 + status.Rules[i].ResourcesGeneratedCount += 1 + averageOver := int64(status.Rules[i].AppliedCount + status.Rules[i].FailedCount) + status.Rules[i].ExecutionTime = updateGenerateExecutionTime( + executionTime, + status.Rules[i].ExecutionTime, + averageOver, + ).String() + } + } + + s.Cache.Data[vc.policyName] = status + s.Cache.Mutex.Unlock() +} + +func updateGenerateExecutionTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { + if averageOver == 0 { + return newTime + } + oldAverageExecutionTime, _ := time.ParseDuration(oldAverageTimeString) + numerator := (oldAverageExecutionTime.Nanoseconds() * averageOver) + newTime.Nanoseconds() + denominator := averageOver + newAverageTimeInNanoSeconds := numerator / denominator + return time.Duration(newAverageTimeInNanoSeconds) * time.Nanosecond +} + func applyRule(client *dclient.Client, rule kyverno.Rule, resource unstructured.Unstructured, ctx context.EvalInterface, processExisting bool) (kyverno.ResourceSpec, error) { var rdata map[string]interface{} var err error diff --git a/pkg/generate/policyStatus_test.go b/pkg/generate/policyStatus_test.go index fad3eacd79..624c912d86 100644 --- a/pkg/generate/policyStatus_test.go +++ b/pkg/generate/policyStatus_test.go @@ -45,7 +45,7 @@ func Test_Stats(t *testing.T) { s := policyStatus.NewSync(nil, &dummyStore{}) for _, generateCountStat := range testCase.generatedCountStats { - receiver := &generatedResourceCount{ + receiver := &generateSyncStats{ generateRequest: generateCountStat, } receiver.UpdateStatus(s) diff --git a/pkg/generate/status.go b/pkg/generate/status.go index f68e564f52..70d9539053 100644 --- a/pkg/generate/status.go +++ b/pkg/generate/status.go @@ -4,7 +4,6 @@ import ( "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" - "github.com/nirmata/kyverno/pkg/policyStatus" ) //StatusControlInterface provides interface to update status subresource @@ -15,8 +14,7 @@ type StatusControlInterface interface { // StatusControl is default implementaation of GRStatusControlInterface type StatusControl struct { - client kyvernoclient.Interface - policyStatus *policyStatus.Sync + client kyvernoclient.Interface } //Failed sets gr status.state to failed with message @@ -36,19 +34,11 @@ func (sc StatusControl) Failed(gr kyverno.GenerateRequest, message string, genRe // Success sets the gr status.state to completed and clears message func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyverno.ResourceSpec) error { - oldState := gr.Status.State - gr.Status.State = kyverno.Completed gr.Status.Message = "" // Update Generated Resources gr.Status.GeneratedResources = genResources - if oldState != kyverno.Completed { - go func() { - sc.policyStatus.Listener <- updatePolicyStatusWithGeneratedResourceCount(gr) - }() - } - _, err := sc.client.KyvernoV1().GenerateRequests("kyverno").UpdateStatus(&gr) if err != nil { glog.V(4).Infof("FAILED: updated gr %s status to %s", gr.Name, string(kyverno.Completed)) @@ -57,29 +47,3 @@ func (sc StatusControl) Success(gr kyverno.GenerateRequest, genResources []kyver glog.V(4).Infof("updated gr %s status to %s", gr.Name, string(kyverno.Completed)) return nil } - -type generatedResourceCount struct { - generateRequest kyverno.GenerateRequest -} - -func updatePolicyStatusWithGeneratedResourceCount(generateRequest kyverno.GenerateRequest) *generatedResourceCount { - return &generatedResourceCount{ - generateRequest: generateRequest, - } -} - -func (vc *generatedResourceCount) UpdateStatus(s *policyStatus.Sync) { - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[vc.generateRequest.Spec.Policy] - if !exist { - policy, _ := s.PolicyStore.Get(vc.generateRequest.Spec.Policy) - if policy != nil { - status = policy.Status - } - } - - status.ResourcesGeneratedCount += len(vc.generateRequest.Status.GeneratedResources) - - s.Cache.Data[vc.generateRequest.Spec.Policy] = status - s.Cache.Mutex.Unlock() -} diff --git a/pkg/policyStatus/main.go b/pkg/policyStatus/main.go index efa7e30d4a..a1507cac81 100644 --- a/pkg/policyStatus/main.go +++ b/pkg/policyStatus/main.go @@ -52,7 +52,6 @@ func (s *Sync) Run(workers int, stopCh <-chan struct{}) { wait.Until(s.updatePolicyStatus, 2*time.Second, stopCh) <-stopCh - s.updatePolicyStatus() } func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { From 38b92a0d34f6b61286a7fc0b3cb532910ccb6bbc Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 4 Mar 2020 13:35:49 +0530 Subject: [PATCH 25/34] 527 making status listner into a buffered channel instead of go routines --- pkg/generate/generate.go | 10 ++++------ pkg/policyStatus/main.go | 2 +- pkg/policyviolation/clusterpv.go | 8 ++------ pkg/policyviolation/namespacedpv.go | 8 ++------ pkg/webhooks/generation.go | 4 +--- pkg/webhooks/mutation.go | 4 +--- pkg/webhooks/validation.go | 4 +--- 7 files changed, 12 insertions(+), 28 deletions(-) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 48d53cae4c..8391b37978 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -127,12 +127,10 @@ func (c *Controller) applyGeneratePolicy(policyContext engine.PolicyContext, gr } if gr.Status.State == "" { - go func() { - c.policyStatus.Listener <- &generateSyncStats{ - policyName: policy.Name, - ruleNameToProcessingTime: ruleNameToProcessingTime, - } - }() + c.policyStatus.Listener <- &generateSyncStats{ + policyName: policy.Name, + ruleNameToProcessingTime: ruleNameToProcessingTime, + } } return genResources, nil diff --git a/pkg/policyStatus/main.go b/pkg/policyStatus/main.go index a1507cac81..7e85ada691 100644 --- a/pkg/policyStatus/main.go +++ b/pkg/policyStatus/main.go @@ -41,7 +41,7 @@ func NewSync(c *versioned.Clientset, p policyStore) *Sync { }, client: c, PolicyStore: p, - Listener: make(chan statusUpdater), + Listener: make(chan statusUpdater, 20), } } diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 3b57ff2597..4d42464ba8 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -100,9 +100,7 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - go func() { - cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) - }() + cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) @@ -128,9 +126,7 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) if newPv.Annotations["fromSync"] != "true" { - go func() { - cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) - }() + cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) } return nil } diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 10e92ef24a..acc5095572 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -99,9 +99,7 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - go func() { - nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) - }() + nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil @@ -124,9 +122,7 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error } if newPv.Annotations["fromSync"] != "true" { - go func() { - nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) - }() + nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) } glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index b62de1bddb..5cd692de6e 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -68,9 +68,7 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) - go func() { - ws.status.Listener <- updateStatusWithGenerateStats(engineResponse) - }() + ws.status.Listener <- updateStatusWithGenerateStats(engineResponse) } } // Adds Generate Request to a channel(queue size 1000) to generators diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index dfaa8c2fa7..fdf5862cb8 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -63,9 +63,7 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) - go func() { - ws.status.Listener <- updateStatusWithMutateStats(engineResponse) - }() + ws.status.Listener <- updateStatusWithMutateStats(engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index b539bfef9d..2ab599793b 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -73,9 +73,7 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) - go func() { - ws.status.Listener <- updateStatusWithValidateStats(engineResponse) - }() + ws.status.Listener <- updateStatusWithValidateStats(engineResponse) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue From 6206852262c6e9549e95282c7e959e680b742e43 Mon Sep 17 00:00:00 2001 From: shravan Date: Wed, 4 Mar 2020 15:45:20 +0530 Subject: [PATCH 26/34] 527 redesigned implementation so that package variables are not used across packages --- pkg/generate/generate.go | 21 +++------ pkg/generate/policyStatus_test.go | 57 +++++++++++------------- pkg/policyStatus/main.go | 49 ++++++++++++-------- pkg/policyviolation/clusterpv.go | 4 +- pkg/policyviolation/common.go | 22 ++------- pkg/policyviolation/namespacedpv.go | 4 +- pkg/policyviolation/policyStatus_test.go | 40 ++++++++--------- pkg/webhooks/generation.go | 30 ++++--------- pkg/webhooks/mutation.go | 31 ++++--------- pkg/webhooks/policyStatus_test.go | 32 +++++-------- pkg/webhooks/validation.go | 30 ++++--------- 11 files changed, 128 insertions(+), 192 deletions(-) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 8391b37978..512c76bcdf 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -5,8 +5,6 @@ import ( "fmt" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" dclient "github.com/nirmata/kyverno/pkg/dclient" @@ -127,7 +125,7 @@ func (c *Controller) applyGeneratePolicy(policyContext engine.PolicyContext, gr } if gr.Status.State == "" { - c.policyStatus.Listener <- &generateSyncStats{ + c.policyStatus.Listener <- generateSyncStats{ policyName: policy.Name, ruleNameToProcessingTime: ruleNameToProcessingTime, } @@ -141,15 +139,11 @@ type generateSyncStats struct { ruleNameToProcessingTime map[string]time.Duration } -func (vc *generateSyncStats) UpdateStatus(s *policyStatus.Sync) { - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[vc.policyName] - if !exist { - policy, _ := s.PolicyStore.Get(vc.policyName) - if policy != nil { - status = policy.Status - } - } +func (vc generateSyncStats) PolicyName() string { + return vc.policyName +} + +func (vc generateSyncStats) UpdateStatus(status kyverno.PolicyStatus) kyverno.PolicyStatus { for i := range status.Rules { if executionTime, exist := vc.ruleNameToProcessingTime[status.Rules[i].Name]; exist { @@ -164,8 +158,7 @@ func (vc *generateSyncStats) UpdateStatus(s *policyStatus.Sync) { } } - s.Cache.Data[vc.policyName] = status - s.Cache.Mutex.Unlock() + return status } func updateGenerateExecutionTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { diff --git a/pkg/generate/policyStatus_test.go b/pkg/generate/policyStatus_test.go index 624c912d86..b0fa04b591 100644 --- a/pkg/generate/policyStatus_test.go +++ b/pkg/generate/policyStatus_test.go @@ -4,54 +4,49 @@ import ( "encoding/json" "reflect" "testing" + "time" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" - "github.com/nirmata/kyverno/pkg/policyStatus" ) -type dummyStore struct { -} - -func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { - return &v1.ClusterPolicy{}, nil -} - func Test_Stats(t *testing.T) { testCase := struct { - generatedCountStats []v1.GenerateRequest - expectedOutput []byte + generatedSyncStats []generateSyncStats + expectedOutput []byte + existingStatus map[string]v1.PolicyStatus }{ - expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"","resourcesGeneratedCount":1},"policy2":{"averageExecutionTime":"","resourcesGeneratedCount":1}}`), - generatedCountStats: []v1.GenerateRequest{ + expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"","resourcesGeneratedCount":2,"ruleStatus":[{"ruleName":"rule1","averageExecutionTime":"23ns","resourcesGeneratedCount":1},{"ruleName":"rule2","averageExecutionTime":"44ns","resourcesGeneratedCount":1},{"ruleName":"rule3"}]}}`), + generatedSyncStats: []generateSyncStats{ { - Spec: v1.GenerateRequestSpec{ - Policy: "policy1", - }, - Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1), + policyName: "policy1", + ruleNameToProcessingTime: map[string]time.Duration{ + "rule1": time.Nanosecond * 23, + "rule2": time.Nanosecond * 44, }, }, - { - Spec: v1.GenerateRequestSpec{ - Policy: "policy2", - }, - Status: v1.GenerateRequestStatus{ - GeneratedResources: make([]v1.ResourceSpec, 1), + }, + existingStatus: map[string]v1.PolicyStatus{ + "policy1": { + Rules: []v1.RuleStats{ + { + Name: "rule1", + }, + { + Name: "rule2", + }, + { + Name: "rule3", + }, }, }, }, } - s := policyStatus.NewSync(nil, &dummyStore{}) - - for _, generateCountStat := range testCase.generatedCountStats { - receiver := &generateSyncStats{ - generateRequest: generateCountStat, - } - receiver.UpdateStatus(s) + for _, generateSyncStat := range testCase.generatedSyncStats { + testCase.existingStatus[generateSyncStat.PolicyName()] = generateSyncStat.UpdateStatus(testCase.existingStatus[generateSyncStat.PolicyName()]) } - output, _ := json.Marshal(s.Cache.Data) + output, _ := json.Marshal(testCase.existingStatus) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } diff --git a/pkg/policyStatus/main.go b/pkg/policyStatus/main.go index 7e85ada691..498ab2a0fc 100644 --- a/pkg/policyStatus/main.go +++ b/pkg/policyStatus/main.go @@ -14,7 +14,8 @@ import ( ) type statusUpdater interface { - UpdateStatus(s *Sync) + PolicyName() string + UpdateStatus(status v1.PolicyStatus) v1.PolicyStatus } type policyStore interface { @@ -22,25 +23,25 @@ type policyStore interface { } type Sync struct { - Cache *cache + cache *cache Listener chan statusUpdater client *versioned.Clientset - PolicyStore policyStore + policyStore policyStore } type cache struct { - Mutex sync.RWMutex - Data map[string]v1.PolicyStatus + mutex sync.RWMutex + data map[string]v1.PolicyStatus } func NewSync(c *versioned.Clientset, p policyStore) *Sync { return &Sync{ - Cache: &cache{ - Mutex: sync.RWMutex{}, - Data: make(map[string]v1.PolicyStatus), + cache: &cache{ + mutex: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), }, client: c, - PolicyStore: p, + policyStore: p, Listener: make(chan statusUpdater, 20), } } @@ -58,7 +59,19 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { for { select { case statusUpdater := <-s.Listener: - statusUpdater.UpdateStatus(s) + s.cache.mutex.Lock() + + status, exist := s.cache.data[statusUpdater.PolicyName()] + if !exist { + policy, _ := s.policyStore.Get(statusUpdater.PolicyName()) + if policy != nil { + status = policy.Status + } + } + + s.cache.data[statusUpdater.PolicyName()] = statusUpdater.UpdateStatus(status) + + s.cache.mutex.Unlock() case <-stopCh: return } @@ -66,24 +79,24 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { } func (s *Sync) updatePolicyStatus() { - s.Cache.Mutex.Lock() - var nameToStatus = make(map[string]v1.PolicyStatus, len(s.Cache.Data)) - for k, v := range s.Cache.Data { + s.cache.mutex.Lock() + var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) + for k, v := range s.cache.data { nameToStatus[k] = v } - s.Cache.Mutex.Unlock() + s.cache.mutex.Unlock() for policyName, status := range nameToStatus { - policy, err := s.PolicyStore.Get(policyName) + policy, err := s.policyStore.Get(policyName) if err != nil { continue } policy.Status = status _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) if err != nil { - s.Cache.Mutex.Lock() - delete(s.Cache.Data, policyName) - s.Cache.Mutex.Unlock() + s.cache.mutex.Lock() + delete(s.cache.data, policyName) + s.cache.mutex.Unlock() glog.V(4).Info(err) } } diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index 4d42464ba8..d041c32828 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -100,7 +100,7 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + cpv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) @@ -126,7 +126,7 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) if newPv.Annotations["fromSync"] != "true" { - cpv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + cpv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} } return nil } diff --git a/pkg/policyviolation/common.go b/pkg/policyviolation/common.go index 44bf0e46ae..6f077b25b8 100644 --- a/pkg/policyviolation/common.go +++ b/pkg/policyviolation/common.go @@ -4,8 +4,6 @@ import ( "fmt" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - backoff "github.com/cenkalti/backoff" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -79,22 +77,11 @@ type violationCount struct { violatedRules []v1.ViolatedRule } -func updatePolicyStatusWithViolationCount(policyName string, violatedRules []kyverno.ViolatedRule) *violationCount { - return &violationCount{ - policyName: policyName, - violatedRules: violatedRules, - } +func (vc violationCount) PolicyName() string { + return vc.policyName } -func (vc *violationCount) UpdateStatus(s *policyStatus.Sync) { - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[vc.policyName] - if !exist { - policy, _ := s.PolicyStore.Get(vc.policyName) - if policy != nil { - status = policy.Status - } - } +func (vc violationCount) UpdateStatus(status kyverno.PolicyStatus) kyverno.PolicyStatus { var ruleNameToViolations = make(map[string]int) for _, rule := range vc.violatedRules { @@ -106,6 +93,5 @@ func (vc *violationCount) UpdateStatus(s *policyStatus.Sync) { status.Rules[i].ViolationCount += ruleNameToViolations[status.Rules[i].Name] } - s.Cache.Data[vc.policyName] = status - s.Cache.Mutex.Unlock() + return status } diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index acc5095572..80496312c9 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -99,7 +99,7 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + nspv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil @@ -122,7 +122,7 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error } if newPv.Annotations["fromSync"] != "true" { - nspv.policyStatus.Listener <- updatePolicyStatusWithViolationCount(newPv.Spec.Policy, newPv.Spec.ViolatedRules) + nspv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} } glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil diff --git a/pkg/policyviolation/policyStatus_test.go b/pkg/policyviolation/policyStatus_test.go index 599dd42aa3..8db26ae4a6 100644 --- a/pkg/policyviolation/policyStatus_test.go +++ b/pkg/policyviolation/policyStatus_test.go @@ -5,26 +5,9 @@ import ( "reflect" "testing" - "github.com/nirmata/kyverno/pkg/policyStatus" - v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) -type dummyStore struct { -} - -func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { - return &v1.ClusterPolicy{ - Status: v1.PolicyStatus{ - Rules: []v1.RuleStats{ - { - Name: "rule4", - }, - }, - }, - }, nil -} - func Test_Stats(t *testing.T) { testCase := struct { violationCountStats []struct { @@ -32,7 +15,24 @@ func Test_Stats(t *testing.T) { violatedRules []v1.ViolatedRule } expectedOutput []byte + existingCache map[string]v1.PolicyStatus }{ + existingCache: map[string]v1.PolicyStatus{ + "policy1": { + Rules: []v1.RuleStats{ + { + Name: "rule4", + }, + }, + }, + "policy2": { + Rules: []v1.RuleStats{ + { + Name: "rule4", + }, + }, + }, + }, expectedOutput: []byte(`{"policy1":{"averageExecutionTime":"","violationCount":1,"ruleStatus":[{"ruleName":"rule4","violationCount":1}]},"policy2":{"averageExecutionTime":"","violationCount":1,"ruleStatus":[{"ruleName":"rule4","violationCount":1}]}}`), violationCountStats: []struct { policyName string @@ -57,17 +57,17 @@ func Test_Stats(t *testing.T) { }, } - s := policyStatus.NewSync(nil, &dummyStore{}) + policyNameToStatus := testCase.existingCache for _, violationCountStat := range testCase.violationCountStats { receiver := &violationCount{ policyName: violationCountStat.policyName, violatedRules: violationCountStat.violatedRules, } - receiver.UpdateStatus(s) + policyNameToStatus[receiver.PolicyName()] = receiver.UpdateStatus(policyNameToStatus[receiver.PolicyName()]) } - output, _ := json.Marshal(s.Cache.Data) + output, _ := json.Marshal(policyNameToStatus) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 5cd692de6e..4e06dd9b09 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -5,8 +5,6 @@ import ( "sort" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -68,7 +66,9 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- updateStatusWithGenerateStats(engineResponse) + ws.status.Listener <- generateStats{ + resp: engineResponse, + } } } // Adds Generate Request to a channel(queue size 1000) to generators @@ -115,26 +115,13 @@ type generateStats struct { resp response.EngineResponse } -func updateStatusWithGenerateStats(resp response.EngineResponse) *generateStats { - return &generateStats{ - resp: resp, - } +func (gs generateStats) PolicyName() string { + return gs.resp.PolicyResponse.Policy } -func (gs *generateStats) UpdateStatus(s *policyStatus.Sync) { +func (gs generateStats) UpdateStatus(status kyverno.PolicyStatus) kyverno.PolicyStatus { if reflect.DeepEqual(response.EngineResponse{}, gs.resp) { - return - } - - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[gs.resp.PolicyResponse.Policy] - if !exist { - if s.PolicyStore != nil { - policy, _ := s.PolicyStore.Get(gs.resp.PolicyResponse.Policy) - if policy != nil { - status = policy.Status - } - } + return status } var nameToRule = make(map[string]v1.RuleStats) @@ -180,8 +167,7 @@ func (gs *generateStats) UpdateStatus(s *policyStatus.Sync) { status.AvgExecutionTime = policyAverageExecutionTime.String() status.Rules = ruleStats - s.Cache.Data[gs.resp.PolicyResponse.Policy] = status - s.Cache.Mutex.Unlock() + return status } func updateAverageTime(newTime time.Duration, oldAverageTimeString string, averageOver int64) time.Duration { diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index fdf5862cb8..09e4fe80ba 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -5,8 +5,6 @@ import ( "sort" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -63,7 +61,9 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- updateStatusWithMutateStats(engineResponse) + ws.status.Listener <- mutateStats{ + resp: engineResponse, + } if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue @@ -122,27 +122,13 @@ type mutateStats struct { resp response.EngineResponse } -func updateStatusWithMutateStats(resp response.EngineResponse) *mutateStats { - return &mutateStats{ - resp: resp, - } - +func (ms mutateStats) PolicyName() string { + return ms.resp.PolicyResponse.Policy } -func (ms *mutateStats) UpdateStatus(s *policyStatus.Sync) { +func (ms mutateStats) UpdateStatus(status kyverno.PolicyStatus) kyverno.PolicyStatus { if reflect.DeepEqual(response.EngineResponse{}, ms.resp) { - return - } - - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[ms.resp.PolicyResponse.Policy] - if !exist { - if s.PolicyStore != nil { - policy, _ := s.PolicyStore.Get(ms.resp.PolicyResponse.Policy) - if policy != nil { - status = policy.Status - } - } + return status } var nameToRule = make(map[string]v1.RuleStats) @@ -190,6 +176,5 @@ func (ms *mutateStats) UpdateStatus(s *policyStatus.Sync) { status.AvgExecutionTime = policyAverageExecutionTime.String() status.Rules = ruleStats - s.Cache.Data[ms.resp.PolicyResponse.Policy] = status - s.Cache.Mutex.Unlock() + return status } diff --git a/pkg/webhooks/policyStatus_test.go b/pkg/webhooks/policyStatus_test.go index e85c8522f6..6c71fc6222 100644 --- a/pkg/webhooks/policyStatus_test.go +++ b/pkg/webhooks/policyStatus_test.go @@ -8,16 +8,8 @@ import ( v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" "github.com/nirmata/kyverno/pkg/engine/response" - "github.com/nirmata/kyverno/pkg/policyStatus" ) -type dummyStore struct { -} - -func (d *dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { - return &v1.ClusterPolicy{}, nil -} - func Test_GenerateStats(t *testing.T) { testCase := struct { generateStats []response.EngineResponse @@ -70,16 +62,16 @@ func Test_GenerateStats(t *testing.T) { }, } - s := policyStatus.NewSync(nil, &dummyStore{}) + policyNameToStatus := map[string]v1.PolicyStatus{} for _, generateStat := range testCase.generateStats { - receiver := &generateStats{ + receiver := generateStats{ resp: generateStat, } - receiver.UpdateStatus(s) + policyNameToStatus[receiver.PolicyName()] = receiver.UpdateStatus(policyNameToStatus[receiver.PolicyName()]) } - output, _ := json.Marshal(s.Cache.Data) + output, _ := json.Marshal(policyNameToStatus) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } @@ -137,15 +129,15 @@ func Test_MutateStats(t *testing.T) { }, } - s := policyStatus.NewSync(nil, &dummyStore{}) + policyNameToStatus := map[string]v1.PolicyStatus{} for _, mutateStat := range testCase.mutateStats { - receiver := &mutateStats{ + receiver := mutateStats{ resp: mutateStat, } - receiver.UpdateStatus(s) + policyNameToStatus[receiver.PolicyName()] = receiver.UpdateStatus(policyNameToStatus[receiver.PolicyName()]) } - output, _ := json.Marshal(s.Cache.Data) + output, _ := json.Marshal(policyNameToStatus) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } @@ -204,15 +196,15 @@ func Test_ValidateStats(t *testing.T) { }, } - s := policyStatus.NewSync(nil, &dummyStore{}) + policyNameToStatus := map[string]v1.PolicyStatus{} for _, validateStat := range testCase.validateStats { - receiver := &validateStats{ + receiver := validateStats{ resp: validateStat, } - receiver.UpdateStatus(s) + policyNameToStatus[receiver.PolicyName()] = receiver.UpdateStatus(policyNameToStatus[receiver.PolicyName()]) } - output, _ := json.Marshal(s.Cache.Data) + output, _ := json.Marshal(policyNameToStatus) if !reflect.DeepEqual(output, testCase.expectedOutput) { t.Errorf("\n\nTestcase has failed\nExpected:\n%v\nGot:\n%v\n\n", string(testCase.expectedOutput), string(output)) } diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index 2ab599793b..04f76a08f5 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -5,8 +5,6 @@ import ( "sort" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1" v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" @@ -73,7 +71,9 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- updateStatusWithValidateStats(engineResponse) + ws.status.Listener <- validateStats{ + resp: engineResponse, + } if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue @@ -117,26 +117,13 @@ type validateStats struct { resp response.EngineResponse } -func updateStatusWithValidateStats(resp response.EngineResponse) *validateStats { - return &validateStats{ - resp: resp, - } +func (vs validateStats) PolicyName() string { + return vs.resp.PolicyResponse.Policy } -func (vs *validateStats) UpdateStatus(s *policyStatus.Sync) { +func (vs validateStats) UpdateStatus(status kyverno.PolicyStatus) kyverno.PolicyStatus { if reflect.DeepEqual(response.EngineResponse{}, vs.resp) { - return - } - - s.Cache.Mutex.Lock() - status, exist := s.Cache.Data[vs.resp.PolicyResponse.Policy] - if !exist { - if s.PolicyStore != nil { - policy, _ := s.PolicyStore.Get(vs.resp.PolicyResponse.Policy) - if policy != nil { - status = policy.Status - } - } + return status } var nameToRule = make(map[string]v1.RuleStats) @@ -186,6 +173,5 @@ func (vs *validateStats) UpdateStatus(s *policyStatus.Sync) { status.AvgExecutionTime = policyAverageExecutionTime.String() status.Rules = ruleStats - s.Cache.Data[vs.resp.PolicyResponse.Policy] = status - s.Cache.Mutex.Unlock() + return status } From 879826a80d75e7597a04e05ddc99a3c77198804c Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Wed, 4 Mar 2020 15:25:56 -0800 Subject: [PATCH 27/34] Add clusterrole for policyviolation --- definitions/install.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/definitions/install.yaml b/definitions/install.yaml index 5672a6ba82..e47bcc12dc 100644 --- a/definitions/install.yaml +++ b/definitions/install.yaml @@ -469,6 +469,16 @@ metadata: name: kyverno-service-account namespace: kyverno --- +apiVersion: rbac.authorization.k8s.io/v1beta1 +kind: ClusterRole +metadata: + name: policyviolation +rules: +- apiGroups: ["kyverno.io"] + resources: + - policyviolations + verbs: ["get", "list", "watch"] +--- kind: ClusterRoleBinding apiVersion: rbac.authorization.k8s.io/v1 metadata: From 023ac5ecf5936ad4caae0aee316da27fd673c12b Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Wed, 4 Mar 2020 16:04:02 -0800 Subject: [PATCH 28/34] rename to kyverno:policyviolations --- definitions/install.yaml | 2 +- documentation/installation.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/definitions/install.yaml b/definitions/install.yaml index e47bcc12dc..8584bd4ad3 100644 --- a/definitions/install.yaml +++ b/definitions/install.yaml @@ -472,7 +472,7 @@ metadata: apiVersion: rbac.authorization.k8s.io/v1beta1 kind: ClusterRole metadata: - name: policyviolation + name: kyverno:policyviolations rules: - apiGroups: ["kyverno.io"] resources: diff --git a/documentation/installation.md b/documentation/installation.md index 1e59e13a5c..0bac6466b5 100644 --- a/documentation/installation.md +++ b/documentation/installation.md @@ -168,7 +168,7 @@ Here is a script that generates a self-signed CA, a TLS certificate-key pair, an # Configure a namespace admin to access policy violations -During Kyverno installation, it creates a ClusterRole `policyviolation` which has the `list,get,watch` operation on resource `policyviolations`. To grant access to a namespace admin, configure the following YAML file then apply to the cluster. +During Kyverno installation, it creates a ClusterRole `kyverno:policyviolations` which has the `list,get,watch` operation on resource `policyviolations`. To grant access to a namespace admin, configure the following YAML file then apply to the cluster. - Replace `metadata.namespace` with namespace of the admin - Configure `subjects` field to bind admin's role to the ClusterRole `policyviolation` From f4cc5d30fc602e316717969e5f34b6b5f9d1f5db Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Wed, 4 Mar 2020 17:37:51 -0800 Subject: [PATCH 29/34] Add rules to disallow default namespace for pod controllers. --- .../disallow_default_namespace.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/samples/best_practices/disallow_default_namespace.yaml b/samples/best_practices/disallow_default_namespace.yaml index 64b1fe8844..477a9659b8 100644 --- a/samples/best_practices/disallow_default_namespace.yaml +++ b/samples/best_practices/disallow_default_namespace.yaml @@ -3,6 +3,7 @@ kind: ClusterPolicy metadata: name: disallow-default-namespace annotations: + pod-policies.kyverno.io/autogen-controllers: none policies.kyverno.io/category: Workload Isolation policies.kyverno.io/description: Kubernetes namespaces are an optional feature that provide a way to segment and isolate cluster resources across multiple @@ -31,4 +32,30 @@ spec: pattern: metadata: namespace: "?*" + - name: validate-podcontroller-namespace + match: + resources: + kinds: + - DaemonSet + - Deployment + - Job + - StatefulSet + validate: + message: "Using 'default' namespace is not allowed for podcontrollers" + pattern: + metadata: + namespace: "!default" + - name: require-podcontroller-namespace + match: + resources: + kinds: + - DaemonSet + - Deployment + - Job + - StatefulSet + validate: + message: "A namespace is required for podcontrollers" + pattern: + metadata: + namespace: "?*" From c0eda74b98794fa224b96f8e527833bbc6fa7f8c Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Wed, 4 Mar 2020 17:40:33 -0800 Subject: [PATCH 30/34] update doc --- samples/DisallowDefaultNamespace.md | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/samples/DisallowDefaultNamespace.md b/samples/DisallowDefaultNamespace.md index 13bbf98861..d73ee4417e 100644 --- a/samples/DisallowDefaultNamespace.md +++ b/samples/DisallowDefaultNamespace.md @@ -11,6 +11,14 @@ apiVersion: kyverno.io/v1 kind: ClusterPolicy metadata: name: disallow-default-namespace + annotations: + pod-policies.kyverno.io/autogen-controllers: none + policies.kyverno.io/category: Workload Isolation + policies.kyverno.io/description: Kubernetes namespaces are an optional feature + that provide a way to segment and isolate cluster resources across multiple + applications and users. As a best practice, workloads should be isolated with + namespaces. Namespaces should be required and the default (empty) namespace + should not be used. spec: rules: - name: validate-namespace @@ -33,4 +41,30 @@ spec: pattern: metadata: namespace: "?*" + - name: validate-podcontroller-namespace + match: + resources: + kinds: + - DaemonSet + - Deployment + - Job + - StatefulSet + validate: + message: "Using 'default' namespace is not allowed for podcontrollers" + pattern: + metadata: + namespace: "!default" + - name: require-podcontroller-namespace + match: + resources: + kinds: + - DaemonSet + - Deployment + - Job + - StatefulSet + validate: + message: "A namespace is required for podcontrollers" + pattern: + metadata: + namespace: "?*" ```` From 9656975b5a659b4fc0d06264e1e33e66b96e8f9d Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 7 Mar 2020 12:53:37 +0530 Subject: [PATCH 31/34] 527 renamed package and send listner instead of entire sync object --- cmd/kyverno/main.go | 11 ++++--- pkg/generate/controller.go | 12 ++++---- pkg/generate/generate.go | 4 +-- pkg/{policyStatus => policystatus}/main.go | 10 +++++-- pkg/policyviolation/clusterpv.go | 18 ++++++------ pkg/policyviolation/generator.go | 34 +++++++++++----------- pkg/policyviolation/namespacedpv.go | 18 ++++++------ pkg/webhooks/generation.go | 4 +-- pkg/webhooks/mutation.go | 4 +-- pkg/webhooks/server.go | 8 ++--- pkg/webhooks/validation.go | 4 +-- 11 files changed, 65 insertions(+), 62 deletions(-) rename pkg/{policyStatus => policystatus}/main.go (94%) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index 6a396b8f2e..5066591e1e 100644 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -5,8 +5,6 @@ import ( "flag" "time" - "github.com/nirmata/kyverno/pkg/policyStatus" - "github.com/golang/glog" "github.com/nirmata/kyverno/pkg/checker" kyvernoclient "github.com/nirmata/kyverno/pkg/client/clientset/versioned" @@ -17,6 +15,7 @@ import ( "github.com/nirmata/kyverno/pkg/generate" generatecleanup "github.com/nirmata/kyverno/pkg/generate/cleanup" "github.com/nirmata/kyverno/pkg/policy" + "github.com/nirmata/kyverno/pkg/policystatus" "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/policyviolation" "github.com/nirmata/kyverno/pkg/signal" @@ -138,7 +137,7 @@ func main() { pInformer.Kyverno().V1().ClusterPolicies()) // Policy Status Handler - deals with all logic related to policy status - statusSync := policyStatus.NewSync( + statusSync := policystatus.NewSync( pclient, policyMetaStore) @@ -148,7 +147,7 @@ func main() { client, pInformer.Kyverno().V1().ClusterPolicyViolations(), pInformer.Kyverno().V1().PolicyViolations(), - statusSync) + statusSync.Listener) // POLICY CONTROLLER // - reconciliation policy and policy violation @@ -182,7 +181,7 @@ func main() { egen, pvgen, kubedynamicInformer, - statusSync, + statusSync.Listener, ) // GENERATE REQUEST CLEANUP // -- cleans up the generate requests that have not been processed(i.e. state = [Pending, Failed]) for more than defined timeout @@ -224,7 +223,7 @@ func main() { kubeInformer.Rbac().V1().ClusterRoleBindings(), egen, webhookRegistrationClient, - statusSync, + statusSync.Listener, configData, policyMetaStore, pvgen, diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index 6d877d9448..d40ed2af0d 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -11,7 +11,7 @@ import ( kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" dclient "github.com/nirmata/kyverno/pkg/dclient" "github.com/nirmata/kyverno/pkg/event" - "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/nirmata/kyverno/pkg/policystatus" "github.com/nirmata/kyverno/pkg/policyviolation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -59,7 +59,7 @@ type Controller struct { // only support Namespaces for re-evalutation on resource updates nsInformer informers.GenericInformer - policyStatus *policyStatus.Sync + policyStatusListener policystatus.Listener } //NewController returns an instance of the Generate-Request Controller @@ -71,7 +71,7 @@ func NewController( eventGen event.Interface, pvGenerator policyviolation.GeneratorInterface, dynamicInformer dynamicinformer.DynamicSharedInformerFactory, - policyStatus *policyStatus.Sync, + policyStatus policystatus.Listener, ) *Controller { c := Controller{ client: client, @@ -80,9 +80,9 @@ func NewController( pvGenerator: pvGenerator, //TODO: do the math for worst case back off and make sure cleanup runs after that // as we dont want a deleted GR to be re-queue - queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(1, 30), "generate-request"), - dynamicInformer: dynamicInformer, - policyStatus: policyStatus, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(1, 30), "generate-request"), + dynamicInformer: dynamicInformer, + policyStatusListener: policyStatus, } c.statusControl = StatusControl{client: kyvernoclient} diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 512c76bcdf..bff1dbbca6 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -125,10 +125,10 @@ func (c *Controller) applyGeneratePolicy(policyContext engine.PolicyContext, gr } if gr.Status.State == "" { - c.policyStatus.Listener <- generateSyncStats{ + c.policyStatusListener.Send(generateSyncStats{ policyName: policy.Name, ruleNameToProcessingTime: ruleNameToProcessingTime, - } + }) } return genResources, nil diff --git a/pkg/policyStatus/main.go b/pkg/policystatus/main.go similarity index 94% rename from pkg/policyStatus/main.go rename to pkg/policystatus/main.go index 498ab2a0fc..ff3caaa7cc 100644 --- a/pkg/policyStatus/main.go +++ b/pkg/policystatus/main.go @@ -1,4 +1,4 @@ -package policyStatus +package policystatus import ( "sync" @@ -22,9 +22,15 @@ type policyStore interface { Get(policyName string) (*v1.ClusterPolicy, error) } +type Listener chan statusUpdater + +func (l Listener) Send(s statusUpdater) { + l <- s +} + type Sync struct { cache *cache - Listener chan statusUpdater + Listener Listener client *versioned.Clientset policyStore policyStore } diff --git a/pkg/policyviolation/clusterpv.go b/pkg/policyviolation/clusterpv.go index d041c32828..c9336f8059 100644 --- a/pkg/policyviolation/clusterpv.go +++ b/pkg/policyviolation/clusterpv.go @@ -9,7 +9,7 @@ import ( kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" client "github.com/nirmata/kyverno/pkg/dclient" - "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/nirmata/kyverno/pkg/policystatus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -22,19 +22,19 @@ type clusterPV struct { // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface // update policy stats with violationCount - policyStatus *policyStatus.Sync + policyStatusListener policystatus.Listener } func newClusterPV(dclient *client.Client, cpvLister kyvernolister.ClusterPolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, - policyStatus *policyStatus.Sync, + policyStatus policystatus.Listener, ) *clusterPV { cpv := clusterPV{ - dclient: dclient, - cpvLister: cpvLister, - kyvernoInterface: kyvernoInterface, - policyStatus: policyStatus, + dclient: dclient, + cpvLister: cpvLister, + kyvernoInterface: kyvernoInterface, + policyStatusListener: policyStatus, } return &cpv } @@ -100,7 +100,7 @@ func (cpv *clusterPV) createPV(newPv *kyverno.ClusterPolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - cpv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} + cpv.policyStatusListener.Send(violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules}) } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) @@ -126,7 +126,7 @@ func (cpv *clusterPV) updatePV(newPv, oldPv *kyverno.ClusterPolicyViolation) err glog.Infof("cluster policy violation updated for resource %v", newPv.Spec.ResourceSpec) if newPv.Annotations["fromSync"] != "true" { - cpv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} + cpv.policyStatusListener.Send(violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules}) } return nil } diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index 6c491112ee..db5ca63fcd 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -14,7 +14,7 @@ import ( kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernoinformer "github.com/nirmata/kyverno/pkg/client/informers/externalversions/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" - "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/nirmata/kyverno/pkg/policystatus" dclient "github.com/nirmata/kyverno/pkg/dclient" unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -38,10 +38,10 @@ type Generator struct { // returns true if the cluster policy store has been synced at least once pvSynced cache.InformerSynced // returns true if the namespaced cluster policy store has been synced at at least once - nspvSynced cache.InformerSynced - queue workqueue.RateLimitingInterface - dataStore *dataStore - policyStatus *policyStatus.Sync + nspvSynced cache.InformerSynced + queue workqueue.RateLimitingInterface + dataStore *dataStore + policyStatusListener policystatus.Listener } //NewDataStore returns an instance of data store @@ -107,17 +107,17 @@ func NewPVGenerator(client *kyvernoclient.Clientset, dclient *dclient.Client, pvInformer kyvernoinformer.ClusterPolicyViolationInformer, nspvInformer kyvernoinformer.PolicyViolationInformer, - policyStatus *policyStatus.Sync) *Generator { + policyStatus policystatus.Listener) *Generator { gen := Generator{ - kyvernoInterface: client.KyvernoV1(), - dclient: dclient, - cpvLister: pvInformer.Lister(), - pvSynced: pvInformer.Informer().HasSynced, - nspvLister: nspvInformer.Lister(), - nspvSynced: nspvInformer.Informer().HasSynced, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName), - dataStore: newDataStore(), - policyStatus: policyStatus, + kyvernoInterface: client.KyvernoV1(), + dclient: dclient, + cpvLister: pvInformer.Lister(), + pvSynced: pvInformer.Informer().HasSynced, + nspvLister: nspvInformer.Lister(), + nspvSynced: nspvInformer.Informer().HasSynced, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName), + dataStore: newDataStore(), + policyStatusListener: policyStatus, } return &gen } @@ -224,10 +224,10 @@ func (gen *Generator) syncHandler(info Info) error { builder := newPvBuilder() if info.Resource.GetNamespace() == "" { // cluster scope resource generate a clusterpolicy violation - handler = newClusterPV(gen.dclient, gen.cpvLister, gen.kyvernoInterface, gen.policyStatus) + handler = newClusterPV(gen.dclient, gen.cpvLister, gen.kyvernoInterface, gen.policyStatusListener) } else { // namespaced resources generated a namespaced policy violation in the namespace of the resource - handler = newNamespacedPV(gen.dclient, gen.nspvLister, gen.kyvernoInterface, gen.policyStatus) + handler = newNamespacedPV(gen.dclient, gen.nspvLister, gen.kyvernoInterface, gen.policyStatusListener) } failure := false diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 80496312c9..ff32748d0b 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -9,7 +9,7 @@ import ( kyvernov1 "github.com/nirmata/kyverno/pkg/client/clientset/versioned/typed/kyverno/v1" kyvernolister "github.com/nirmata/kyverno/pkg/client/listers/kyverno/v1" client "github.com/nirmata/kyverno/pkg/dclient" - "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/nirmata/kyverno/pkg/policystatus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -22,19 +22,19 @@ type namespacedPV struct { // policy violation interface kyvernoInterface kyvernov1.KyvernoV1Interface // update policy status with violationCount - policyStatus *policyStatus.Sync + policyStatusListener policystatus.Listener } func newNamespacedPV(dclient *client.Client, nspvLister kyvernolister.PolicyViolationLister, kyvernoInterface kyvernov1.KyvernoV1Interface, - policyStatus *policyStatus.Sync, + policyStatus policystatus.Listener, ) *namespacedPV { nspv := namespacedPV{ - dclient: dclient, - nspvLister: nspvLister, - kyvernoInterface: kyvernoInterface, - policyStatus: policyStatus, + dclient: dclient, + nspvLister: nspvLister, + kyvernoInterface: kyvernoInterface, + policyStatusListener: policyStatus, } return &nspv } @@ -99,7 +99,7 @@ func (nspv *namespacedPV) createPV(newPv *kyverno.PolicyViolation) error { } if newPv.Annotations["fromSync"] != "true" { - nspv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} + nspv.policyStatusListener.Send(violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules}) } glog.Infof("policy violation created for resource %v", newPv.Spec.ResourceSpec) return nil @@ -122,7 +122,7 @@ func (nspv *namespacedPV) updatePV(newPv, oldPv *kyverno.PolicyViolation) error } if newPv.Annotations["fromSync"] != "true" { - nspv.policyStatus.Listener <- violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules} + nspv.policyStatusListener.Send(violationCount{policyName: newPv.Spec.Policy, violatedRules: newPv.Spec.ViolatedRules}) } glog.Infof("namespaced policy violation updated for resource %v", newPv.Spec.ResourceSpec) return nil diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 4e06dd9b09..956c568cac 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -66,9 +66,9 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic if len(engineResponse.PolicyResponse.Rules) > 0 { // some generate rules do apply to the resource engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- generateStats{ + ws.statusListener.Send(generateStats{ resp: engineResponse, - } + }) } } // Adds Generate Request to a channel(queue size 1000) to generators diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index 09e4fe80ba..2723b00110 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -61,9 +61,7 @@ func (ws *WebhookServer) HandleMutation(request *v1beta1.AdmissionRequest, resou policyContext.Policy = policy engineResponse := engine.Mutate(policyContext) engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- mutateStats{ - resp: engineResponse, - } + ws.statusListener.Send(mutateStats{resp: engineResponse}) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, resource.GetNamespace(), resource.GetName()) continue diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index e7733e5f72..b0ca988449 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -18,7 +18,7 @@ import ( "github.com/nirmata/kyverno/pkg/config" client "github.com/nirmata/kyverno/pkg/dclient" "github.com/nirmata/kyverno/pkg/event" - "github.com/nirmata/kyverno/pkg/policyStatus" + "github.com/nirmata/kyverno/pkg/policystatus" "github.com/nirmata/kyverno/pkg/policystore" "github.com/nirmata/kyverno/pkg/policyviolation" tlsutils "github.com/nirmata/kyverno/pkg/tls" @@ -55,7 +55,7 @@ type WebhookServer struct { // webhook registration client webhookRegistrationClient *webhookconfig.WebhookRegistrationClient // API to send policy stats for aggregation - status *policyStatus.Sync + statusListener policystatus.Listener // helpers to validate against current loaded configuration configHandler config.Interface // channel for cleanup notification @@ -82,7 +82,7 @@ func NewWebhookServer( crbInformer rbacinformer.ClusterRoleBindingInformer, eventGen event.Interface, webhookRegistrationClient *webhookconfig.WebhookRegistrationClient, - statusSync *policyStatus.Sync, + statusSync policystatus.Listener, configHandler config.Interface, pMetaStore policystore.LookupInterface, pvGenerator policyviolation.GeneratorInterface, @@ -112,7 +112,7 @@ func NewWebhookServer( crbSynced: crbInformer.Informer().HasSynced, eventGen: eventGen, webhookRegistrationClient: webhookRegistrationClient, - status: statusSync, + statusListener: statusSync, configHandler: configHandler, cleanUp: cleanUp, lastReqTime: resourceWebhookWatcher.LastReqTime, diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index 04f76a08f5..54a7fbdf3e 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -71,9 +71,9 @@ func (ws *WebhookServer) HandleValidation(request *v1beta1.AdmissionRequest, pol continue } engineResponses = append(engineResponses, engineResponse) - ws.status.Listener <- validateStats{ + ws.statusListener.Send(validateStats{ resp: engineResponse, - } + }) if !engineResponse.IsSuccesful() { glog.V(4).Infof("Failed to apply policy %s on resource %s/%s\n", policy.Name, newR.GetNamespace(), newR.GetName()) continue From 2c3931a671d20edf491a34926c1481ba1062a016 Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 7 Mar 2020 14:56:42 +0530 Subject: [PATCH 32/34] 527 optimised lock implementation --- pkg/policystatus/keyToMutex.go | 27 ++++++++++++++++++ pkg/policystatus/main.go | 30 ++++++++++++-------- pkg/policystatus/status_test.go | 50 +++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 pkg/policystatus/keyToMutex.go create mode 100644 pkg/policystatus/status_test.go diff --git a/pkg/policystatus/keyToMutex.go b/pkg/policystatus/keyToMutex.go new file mode 100644 index 0000000000..a7b533a0e6 --- /dev/null +++ b/pkg/policystatus/keyToMutex.go @@ -0,0 +1,27 @@ +package policystatus + +import "sync" + +type keyToMutex struct { + mu sync.RWMutex + keyMu map[string]*sync.RWMutex +} + +func newKeyToMutex() *keyToMutex { + return &keyToMutex{ + mu: sync.RWMutex{}, + keyMu: make(map[string]*sync.RWMutex), + } +} + +func (k *keyToMutex) Get(key string) *sync.RWMutex { + k.mu.Lock() + defer k.mu.Unlock() + mutex := k.keyMu[key] + if mutex == nil { + mutex = &sync.RWMutex{} + k.keyMu[key] = mutex + } + + return mutex +} diff --git a/pkg/policystatus/main.go b/pkg/policystatus/main.go index ff3caaa7cc..cf99d45f71 100644 --- a/pkg/policystatus/main.go +++ b/pkg/policystatus/main.go @@ -36,15 +36,17 @@ type Sync struct { } type cache struct { - mutex sync.RWMutex - data map[string]v1.PolicyStatus + dataMu sync.RWMutex + data map[string]v1.PolicyStatus + keyToMutex *keyToMutex } func NewSync(c *versioned.Clientset, p policyStore) *Sync { return &Sync{ cache: &cache{ - mutex: sync.RWMutex{}, - data: make(map[string]v1.PolicyStatus), + dataMu: sync.RWMutex{}, + data: make(map[string]v1.PolicyStatus), + keyToMutex: newKeyToMutex(), }, client: c, policyStore: p, @@ -65,9 +67,11 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { for { select { case statusUpdater := <-s.Listener: - s.cache.mutex.Lock() + s.cache.keyToMutex.Get(statusUpdater.PolicyName()).Lock() + s.cache.dataMu.RLock() status, exist := s.cache.data[statusUpdater.PolicyName()] + s.cache.dataMu.RUnlock() if !exist { policy, _ := s.policyStore.Get(statusUpdater.PolicyName()) if policy != nil { @@ -75,9 +79,13 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { } } - s.cache.data[statusUpdater.PolicyName()] = statusUpdater.UpdateStatus(status) + updatedStatus := statusUpdater.UpdateStatus(status) - s.cache.mutex.Unlock() + s.cache.dataMu.Lock() + s.cache.data[statusUpdater.PolicyName()] = updatedStatus + s.cache.dataMu.Unlock() + + s.cache.keyToMutex.Get(statusUpdater.PolicyName()).Unlock() case <-stopCh: return } @@ -85,12 +93,12 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { } func (s *Sync) updatePolicyStatus() { - s.cache.mutex.Lock() + s.cache.dataMu.Lock() var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) for k, v := range s.cache.data { nameToStatus[k] = v } - s.cache.mutex.Unlock() + s.cache.dataMu.Unlock() for policyName, status := range nameToStatus { policy, err := s.policyStore.Get(policyName) @@ -100,9 +108,9 @@ func (s *Sync) updatePolicyStatus() { policy.Status = status _, err = s.client.KyvernoV1().ClusterPolicies().UpdateStatus(policy) if err != nil { - s.cache.mutex.Lock() + s.cache.dataMu.Lock() delete(s.cache.data, policyName) - s.cache.mutex.Unlock() + s.cache.dataMu.Unlock() glog.V(4).Info(err) } } diff --git a/pkg/policystatus/status_test.go b/pkg/policystatus/status_test.go new file mode 100644 index 0000000000..310852cf2f --- /dev/null +++ b/pkg/policystatus/status_test.go @@ -0,0 +1,50 @@ +package policystatus + +import ( + "encoding/json" + "testing" + "time" + + v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" +) + +type dummyStore struct { +} + +func (d dummyStore) Get(policyName string) (*v1.ClusterPolicy, error) { + return &v1.ClusterPolicy{}, nil +} + +type dummyStatusUpdater struct { +} + +func (d dummyStatusUpdater) UpdateStatus(status v1.PolicyStatus) v1.PolicyStatus { + status.RulesAppliedCount++ + return status +} + +func (d dummyStatusUpdater) PolicyName() string { + return "policy1" +} + +func TestKeyToMutex(t *testing.T) { + expectedCache := `{"policy1":{"averageExecutionTime":"","rulesAppliedCount":100}}` + + stopCh := make(chan struct{}) + s := NewSync(nil, dummyStore{}) + for i := 0; i < 100; i++ { + go s.updateStatusCache(stopCh) + } + + for i := 0; i < 100; i++ { + go s.Listener.Send(dummyStatusUpdater{}) + } + + <-time.After(time.Second * 3) + stopCh <- struct{}{} + + cacheRaw, _ := json.Marshal(s.cache.data) + if string(cacheRaw) != expectedCache { + t.Errorf("\nTestcase Failed\nGot:\n%v\nExpected:\n%v\n", string(cacheRaw), expectedCache) + } +} From 1906841ad5ed93d29d88e28ea34b5da816f29fdf Mon Sep 17 00:00:00 2001 From: shravan Date: Sat, 7 Mar 2020 16:23:17 +0530 Subject: [PATCH 33/34] 527 added comments and added a log --- pkg/policystatus/keyToMutex.go | 4 ++++ pkg/policystatus/main.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/pkg/policystatus/keyToMutex.go b/pkg/policystatus/keyToMutex.go index a7b533a0e6..801db2981a 100644 --- a/pkg/policystatus/keyToMutex.go +++ b/pkg/policystatus/keyToMutex.go @@ -2,6 +2,10 @@ package policystatus import "sync" +// keyToMutex allows status to be updated +//for different policies at the same time +//while ensuring the status for same policies +//are updated one at a time. type keyToMutex struct { mu sync.RWMutex keyMu map[string]*sync.RWMutex diff --git a/pkg/policystatus/main.go b/pkg/policystatus/main.go index cf99d45f71..3e633aaf82 100644 --- a/pkg/policystatus/main.go +++ b/pkg/policystatus/main.go @@ -1,6 +1,7 @@ package policystatus import ( + "encoding/json" "sync" "time" @@ -13,6 +14,22 @@ import ( v1 "github.com/nirmata/kyverno/pkg/api/kyverno/v1" ) +// Policy status implementation works in the following way, +//Currently policy status maintains a cache of the status of +//each policy. +//Every x unit of time the status of policy is updated using +//the data from the cache. +//The sync exposes a listener which accepts a statusUpdater +//interface which dictates how the status should be updated. +//The status is updated by a worker that receives the interface +//on a channel. +//The worker then updates the current status using the methods +//exposed by the interface. +//Current implementation is designed to be threadsafe with optimised +//locking for each policy. + +// statusUpdater defines a type to have a method which +//updates the given status type statusUpdater interface { PolicyName() string UpdateStatus(status v1.PolicyStatus) v1.PolicyStatus @@ -28,6 +45,10 @@ func (l Listener) Send(s statusUpdater) { l <- s } +// Sync is the object which is used to initialize +//the policyStatus sync, can be considered the parent object +//since it contains access to all the persistant data present +//in this package. type Sync struct { cache *cache Listener Listener @@ -63,6 +84,8 @@ func (s *Sync) Run(workers int, stopCh <-chan struct{}) { <-stopCh } +// updateStatusCache is a worker which updates the current status +//using the statusUpdater interface func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { for { select { @@ -86,12 +109,18 @@ func (s *Sync) updateStatusCache(stopCh <-chan struct{}) { s.cache.dataMu.Unlock() s.cache.keyToMutex.Get(statusUpdater.PolicyName()).Unlock() + oldStatus, _ := json.Marshal(status) + newStatus, _ := json.Marshal(updatedStatus) + + glog.V(4).Infof("\nupdated status of policy - %v\noldStatus:\n%v\nnewStatus:\n%v\n", statusUpdater.PolicyName(), string(oldStatus), string(newStatus)) case <-stopCh: return } } } +// updatePolicyStatus updates the status in the policy resource definition +//from the status cache, syncing them func (s *Sync) updatePolicyStatus() { s.cache.dataMu.Lock() var nameToStatus = make(map[string]v1.PolicyStatus, len(s.cache.data)) From 8480276f23a4f273e168537fa1158b08b14f1a6e Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Mon, 9 Mar 2020 11:15:03 -0700 Subject: [PATCH 34/34] tag v1.1.4-rc1 --- definitions/install.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/definitions/install.yaml b/definitions/install.yaml index 8584bd4ad3..be688d3d97 100644 --- a/definitions/install.yaml +++ b/definitions/install.yaml @@ -683,10 +683,10 @@ spec: serviceAccountName: kyverno-service-account initContainers: - name: kyverno-pre - image: nirmata/kyvernopre:v1.1.3 + image: nirmata/kyvernopre:v1.1.4-rc1 containers: - name: kyverno - image: nirmata/kyverno:v1.1.3 + image: nirmata/kyverno:v1.1.4-rc1 args: - "--filterK8Resources=[Event,*,*][*,kube-system,*][*,kube-public,*][*,kube-node-lease,*][Node,*,*][APIService,*,*][TokenReview,*,*][SubjectAccessReview,*,*][*,kyverno,*]" # customize webhook timout