From 9f9e0d749f1afbe7cea2ec73e14b0dd448c1b59b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 30 Mar 2022 16:28:09 +0200 Subject: [PATCH] refactor: use policy interface in policycache package (#3503) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Charles-Edouard Brétéché --- pkg/policycache/cache_test.go | 14 ++++++------- pkg/policycache/informer.go | 16 ++++----------- pkg/policycache/policy_cache.go | 35 +++++++++++++++++---------------- pkg/policycache/policy_map.go | 13 ++++++------ pkg/webhooks/common.go | 2 +- pkg/webhooks/generation.go | 8 ++++---- pkg/webhooks/mutation.go | 12 +++++------ pkg/webhooks/server.go | 2 +- pkg/webhooks/validation.go | 8 ++++---- pkg/webhooks/verify_images.go | 4 ++-- 10 files changed, 54 insertions(+), 60 deletions(-) diff --git a/pkg/policycache/cache_test.go b/pkg/policycache/cache_test.go index 70f2c29de2..15a99987c1 100644 --- a/pkg/policycache/cache_test.go +++ b/pkg/policycache/cache_test.go @@ -473,7 +473,7 @@ func newAnyPolicy(t *testing.T) *kyverno.ClusterPolicy { return policy } -func newNsPolicy(t *testing.T) *kyverno.ClusterPolicy { +func newNsPolicy(t *testing.T) kyverno.PolicyInterface { rawPolicy := []byte(`{ "metadata": { "name": "test-policy", @@ -577,7 +577,7 @@ func newNsPolicy(t *testing.T) *kyverno.ClusterPolicy { err := json.Unmarshal(rawPolicy, &policy) assert.NilError(t, err) - return convertPolicyToClusterPolicy(policy) + return policy } func newGVKPolicy(t *testing.T) *kyverno.ClusterPolicy { @@ -637,7 +637,7 @@ func newGVKPolicy(t *testing.T) *kyverno.ClusterPolicy { return policy } -func newUserTestPolicy(t *testing.T) *kyverno.ClusterPolicy { +func newUserTestPolicy(t *testing.T) kyverno.PolicyInterface { rawPolicy := []byte(`{ "apiVersion": "kyverno.io/v1", "kind": "Policy", @@ -676,7 +676,7 @@ func newUserTestPolicy(t *testing.T) *kyverno.ClusterPolicy { err := json.Unmarshal(rawPolicy, &policy) assert.NilError(t, err) - return convertPolicyToClusterPolicy(policy) + return policy } func newgenratePolicy(t *testing.T) *kyverno.ClusterPolicy { @@ -771,7 +771,7 @@ func newMutatePolicy(t *testing.T) *kyverno.ClusterPolicy { return policy } -func newNsMutatePolicy(t *testing.T) *kyverno.ClusterPolicy { +func newNsMutatePolicy(t *testing.T) kyverno.PolicyInterface { rawPolicy := []byte(`{ "metadata": { "name": "logger-sidecar", @@ -814,7 +814,7 @@ func newNsMutatePolicy(t *testing.T) *kyverno.ClusterPolicy { err := json.Unmarshal(rawPolicy, &policy) assert.NilError(t, err) - return convertPolicyToClusterPolicy(policy) + return policy } func newValidateAuditPolicy(t *testing.T) *kyverno.ClusterPolicy { @@ -990,7 +990,7 @@ func Test_Ns_Add_Validate_Audit(t *testing.T) { pCache.add(policy) pCache.add(policy) nspace := policy.GetNamespace() - policy.Spec.ValidationFailureAction = "audit" + policy.GetSpec().ValidationFailureAction = "audit" pCache.add(policy) pCache.add(policy) for _, rule := range autogen.ComputeRules(policy) { diff --git a/pkg/policycache/informer.go b/pkg/policycache/informer.go index 5a69b2876a..51f157d292 100644 --- a/pkg/policycache/informer.go +++ b/pkg/policycache/informer.go @@ -52,13 +52,6 @@ func NewPolicyCacheController( return &pc } -// convertPolicyToClusterPolicy - convert Policy to ClusterPolicy -// This will retain the kind of Policy and convert type to ClusterPolicy -func convertPolicyToClusterPolicy(nsPolicies *kyverno.Policy) *kyverno.ClusterPolicy { - cpol := kyverno.ClusterPolicy(*nsPolicies) - return &cpol -} - func (c *Controller) addPolicy(obj interface{}) { p := obj.(*kyverno.ClusterPolicy) c.Cache.add(p) @@ -67,7 +60,6 @@ func (c *Controller) addPolicy(obj interface{}) { func (c *Controller) updatePolicy(old, cur interface{}) { pOld := old.(*kyverno.ClusterPolicy) pNew := cur.(*kyverno.ClusterPolicy) - if reflect.DeepEqual(pOld.Spec, pNew.Spec) { return } @@ -83,7 +75,7 @@ func (c *Controller) deletePolicy(obj interface{}) { // addNsPolicy - Add Policy to cache func (c *Controller) addNsPolicy(obj interface{}) { p := obj.(*kyverno.Policy) - c.Cache.add(convertPolicyToClusterPolicy(p)) + c.Cache.add(p) } // updateNsPolicy - Update Policy of cache @@ -93,14 +85,14 @@ func (c *Controller) updateNsPolicy(old, cur interface{}) { if reflect.DeepEqual(npOld.Spec, npNew.Spec) { return } - c.Cache.remove(convertPolicyToClusterPolicy(npOld)) - c.Cache.add(convertPolicyToClusterPolicy(npNew)) + c.Cache.remove(npOld) + c.Cache.add(npNew) } // deleteNsPolicy - Delete Policy from cache func (c *Controller) deleteNsPolicy(obj interface{}) { p := obj.(*kyverno.Policy) - c.Cache.remove(convertPolicyToClusterPolicy(p)) + c.Cache.remove(p) } // Run waits until policy informer to be synced diff --git a/pkg/policycache/policy_cache.go b/pkg/policycache/policy_cache.go index 16947662cd..80186684ac 100644 --- a/pkg/policycache/policy_cache.go +++ b/pkg/policycache/policy_cache.go @@ -5,7 +5,7 @@ import ( kyverno "github.com/kyverno/kyverno/api/kyverno/v1" kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1" "github.com/kyverno/kyverno/pkg/common" - policy2 "github.com/kyverno/kyverno/pkg/policy" + "github.com/kyverno/kyverno/pkg/policy" ) // Interface ... @@ -13,15 +13,15 @@ import ( type Interface interface { // GetPolicies returns all policies that apply to a namespace, including cluster-wide policies // If the namespace is empty, only cluster-wide policies are returned - GetPolicies(pkey PolicyType, kind string, nspace string) []*kyverno.ClusterPolicy + GetPolicies(PolicyType, string, string) []kyverno.PolicyInterface // add adds a policy to the cache - add(policy *kyverno.ClusterPolicy) + add(kyverno.PolicyInterface) // remove removes a policy from the cache - remove(policy *kyverno.ClusterPolicy) + remove(kyverno.PolicyInterface) - get(pkey PolicyType, kind string, nspace string) []string + get(PolicyType, string, string) []string } // policyCache ... @@ -58,7 +58,7 @@ func newPolicyCache(log logr.Logger, pLister kyvernolister.ClusterPolicyLister, } // Add a policy to cache -func (pc *policyCache) add(policy *kyverno.ClusterPolicy) { +func (pc *policyCache) add(policy kyverno.PolicyInterface) { pc.pMap.add(policy) pc.logger.V(4).Info("policy is added to cache", "name", policy.GetName()) } @@ -68,7 +68,7 @@ func (pc *policyCache) get(pkey PolicyType, kind, nspace string) []string { return pc.pMap.get(pkey, kind, nspace) } -func (pc *policyCache) GetPolicies(pkey PolicyType, kind, nspace string) []*kyverno.ClusterPolicy { +func (pc *policyCache) GetPolicies(pkey PolicyType, kind, nspace string) []kyverno.PolicyInterface { policies := pc.getPolicyObject(pkey, kind, "") if nspace == "" { return policies @@ -78,28 +78,29 @@ func (pc *policyCache) GetPolicies(pkey PolicyType, kind, nspace string) []*kyve } // Remove a policy from cache -func (pc *policyCache) remove(policy *kyverno.ClusterPolicy) { - pc.pMap.remove(policy) - pc.logger.V(4).Info("policy is removed from cache", "name", policy.GetName()) +func (pc *policyCache) remove(p kyverno.PolicyInterface) { + pc.pMap.remove(p) + pc.logger.V(4).Info("policy is removed from cache", "name", p.GetName()) } -func (pc *policyCache) getPolicyObject(key PolicyType, gvk string, nspace string) (policyObject []*kyverno.ClusterPolicy) { +func (pc *policyCache) getPolicyObject(key PolicyType, gvk string, nspace string) (policyObject []kyverno.PolicyInterface) { _, kind := common.GetKindFromGVK(gvk) policyNames := pc.pMap.get(key, kind, nspace) wildcardPolicies := pc.pMap.get(key, "*", nspace) policyNames = append(policyNames, wildcardPolicies...) for _, policyName := range policyNames { - var policy *kyverno.ClusterPolicy - ns, key, isNamespacedPolicy := policy2.ParseNamespacedPolicy(policyName) + var p kyverno.PolicyInterface + ns, key, isNamespacedPolicy := policy.ParseNamespacedPolicy(policyName) if !isNamespacedPolicy { - policy, _ = pc.pLister.Get(key) + p, _ = pc.pLister.Get(key) } else { if ns == nspace { - nspolicy, _ := pc.npLister.Policies(ns).Get(key) - policy = policy2.ConvertPolicyToClusterPolicy(nspolicy) + p, _ = pc.npLister.Policies(ns).Get(key) } } - policyObject = append(policyObject, policy) + if p != nil { + policyObject = append(policyObject, p) + } } return policyObject } diff --git a/pkg/policycache/policy_map.go b/pkg/policycache/policy_map.go index 6e798e905b..e4622b6611 100644 --- a/pkg/policycache/policy_map.go +++ b/pkg/policycache/policy_map.go @@ -7,7 +7,7 @@ import ( kyverno "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/autogen" "github.com/kyverno/kyverno/pkg/common" - policy2 "github.com/kyverno/kyverno/pkg/policy" + "github.com/kyverno/kyverno/pkg/policy" ) type pMap struct { @@ -24,12 +24,13 @@ type pMap struct { nameCacheMap map[PolicyType]map[string]bool } -func (m *pMap) add(policy *kyverno.ClusterPolicy) { +func (m *pMap) add(policy kyverno.PolicyInterface) { m.lock.Lock() defer m.lock.Unlock() - enforcePolicy := policy.Spec.ValidationFailureAction == kyverno.Enforce - for _, k := range policy.Spec.ValidationFailureActionOverrides { + spec := policy.GetSpec() + enforcePolicy := spec.ValidationFailureAction == kyverno.Enforce + for _, k := range spec.ValidationFailureActionOverrides { if k.Action == kyverno.Enforce { enforcePolicy = true break @@ -75,7 +76,7 @@ func (m *pMap) get(key PolicyType, gvk, namespace string) (names []string) { defer m.lock.RUnlock() _, kind := common.GetKindFromGVK(gvk) for _, policyName := range m.kindDataMap[kind][key] { - ns, key, isNamespacedPolicy := policy2.ParseNamespacedPolicy(policyName) + ns, key, isNamespacedPolicy := policy.ParseNamespacedPolicy(policyName) if !isNamespacedPolicy && namespace == "" { names = append(names, key) } else { @@ -87,7 +88,7 @@ func (m *pMap) get(key PolicyType, gvk, namespace string) (names []string) { return names } -func (m *pMap) remove(policy *kyverno.ClusterPolicy) { +func (m *pMap) remove(policy kyverno.PolicyInterface) { m.lock.Lock() defer m.lock.Unlock() var pName = policy.GetName() diff --git a/pkg/webhooks/common.go b/pkg/webhooks/common.go index e1a9095648..f540af7c3a 100644 --- a/pkg/webhooks/common.go +++ b/pkg/webhooks/common.go @@ -140,7 +140,7 @@ func processResourceWithPatches(patch []byte, resource []byte, log logr.Logger) return resource } -func containsRBACInfo(policies ...[]*kyverno.ClusterPolicy) bool { +func containsRBACInfo(policies ...[]kyverno.PolicyInterface) bool { for _, policySlice := range policies { for _, policy := range policySlice { for _, rule := range autogen.ComputeRules(policy) { diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 5551e6bde2..fe4047d6d5 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -34,7 +34,7 @@ import ( "k8s.io/apimachinery/pkg/labels" ) -func (ws *WebhookServer) applyGeneratePolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []*kyverno.ClusterPolicy, ts int64, logger logr.Logger) { +func (ws *WebhookServer) applyGeneratePolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []kyverno.PolicyInterface, ts int64, logger logr.Logger) { admissionReviewCompletionLatencyChannel := make(chan int64, 1) generateEngineResponsesSenderForAdmissionReviewDurationMetric := make(chan []*response.EngineResponse, 1) generateEngineResponsesSenderForAdmissionRequestsCountMetric := make(chan []*response.EngineResponse, 1) @@ -47,7 +47,7 @@ func (ws *WebhookServer) applyGeneratePolicies(request *v1beta1.AdmissionRequest //handleGenerate handles admission-requests for policies with generate rules func (ws *WebhookServer) handleGenerate( request *v1beta1.AdmissionRequest, - policies []*kyverno.ClusterPolicy, + policies []kyverno.PolicyInterface, ctx *context.Context, userRequestInfo kyverno.RequestInfo, dynamicConfig config.Interface, @@ -148,7 +148,7 @@ func (ws *WebhookServer) registerPolicyExecutionDurationMetricGenerate(logger lo } //handleUpdatesForGenerateRules handles admission-requests for update -func (ws *WebhookServer) handleUpdatesForGenerateRules(request *v1beta1.AdmissionRequest, policies []*kyverno.ClusterPolicy) { +func (ws *WebhookServer) handleUpdatesForGenerateRules(request *v1beta1.AdmissionRequest, policies []kyverno.PolicyInterface) { if request.Operation != v1beta1.Update { return } @@ -220,7 +220,7 @@ func (ws *WebhookServer) updateAnnotationInGR(gr *kyverno.GenerateRequest, logge } //handleUpdateGenerateTargetResource - handles update of target resource for generate policy -func (ws *WebhookServer) handleUpdateGenerateTargetResource(request *v1beta1.AdmissionRequest, policies []*kyverno.ClusterPolicy, resLabels map[string]string, logger logr.Logger) { +func (ws *WebhookServer) handleUpdateGenerateTargetResource(request *v1beta1.AdmissionRequest, policies []kyverno.PolicyInterface, resLabels map[string]string, logger logr.Logger) { enqueueBool := false newRes, err := enginutils.ConvertToUnstructured(request.Object.Raw) if err != nil { diff --git a/pkg/webhooks/mutation.go b/pkg/webhooks/mutation.go index bf6d525183..6eeeda2f67 100644 --- a/pkg/webhooks/mutation.go +++ b/pkg/webhooks/mutation.go @@ -22,7 +22,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -func (ws *WebhookServer) applyMutatePolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []*kyverno.ClusterPolicy, ts int64, logger logr.Logger) []byte { +func (ws *WebhookServer) applyMutatePolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []kyverno.PolicyInterface, ts int64, logger logr.Logger) []byte { var mutateEngineResponses []*response.EngineResponse mutatePatches, mutateEngineResponses := ws.handleMutation(request, policyContext, policies) @@ -40,7 +40,7 @@ func (ws *WebhookServer) applyMutatePolicies(request *v1beta1.AdmissionRequest, func (ws *WebhookServer) handleMutation( request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, - policies []*kyverno.ClusterPolicy) ([]byte, []*response.EngineResponse) { + policies []kyverno.PolicyInterface) ([]byte, []*response.EngineResponse) { if len(policies) == 0 { return nil, nil @@ -74,11 +74,11 @@ func (ws *WebhookServer) handleMutation( var engineResponses []*response.EngineResponse for _, policy := range policies { - if !policy.HasMutate() { + spec := policy.GetSpec() + if !spec.HasMutate() { continue } - - logger.V(3).Info("applying policy mutate rules", "policy", policy.Name) + logger.V(3).Info("applying policy mutate rules", "policy", policy.GetName()) policyContext.Policy = policy engineResponse, policyPatches, err := ws.applyMutation(request, policyContext, logger) if err != nil { @@ -91,7 +91,7 @@ func (ws *WebhookServer) handleMutation( patches = append(patches, policyPatches...) rules := engineResponse.GetSuccessRules() if len(rules) != 0 { - logger.Info("mutation rules from policy applied successfully", "policy", policy.Name, "rules", rules) + logger.Info("mutation rules from policy applied successfully", "policy", policy.GetName(), "rules", rules) } } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index bcfe4b860e..efafdd8358 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -509,7 +509,7 @@ func (ws *WebhookServer) resourceValidation(request *v1beta1.AdmissionRequest) * if len(generatePolicies) == 0 && request.Operation == v1beta1.Update { // handle generate source resource updates - go ws.handleUpdatesForGenerateRules(request, []*v1.ClusterPolicy{}) + go ws.handleUpdatesForGenerateRules(request, []v1.PolicyInterface{}) } var roles, clusterRoles []string diff --git a/pkg/webhooks/validation.go b/pkg/webhooks/validation.go index 486e8f1302..c8e5bdabe6 100644 --- a/pkg/webhooks/validation.go +++ b/pkg/webhooks/validation.go @@ -33,7 +33,7 @@ type validationHandler struct { func (v *validationHandler) handleValidation( promConfig *metrics.PromConfig, request *v1beta1.AdmissionRequest, - policies []*v1.ClusterPolicy, + policies []v1.PolicyInterface, policyContext *engine.PolicyContext, namespaceLabels map[string]string, admissionRequestTimestamp int64) (bool, string) { @@ -58,7 +58,7 @@ func (v *validationHandler) handleValidation( var engineResponses []*response.EngineResponse for _, policy := range policies { - logger.V(3).Info("evaluating policy", "policy", policy.Name) + logger.V(3).Info("evaluating policy", "policy", policy.GetName()) policyContext.Policy = policy policyContext.NamespaceLabels = namespaceLabels engineResponse := engine.Validate(policyContext) @@ -75,12 +75,12 @@ func (v *validationHandler) handleValidation( engineResponses = append(engineResponses, engineResponse) if !engineResponse.IsSuccessful() { - logger.V(2).Info("validation failed", "policy", policy.Name, "failed rules", engineResponse.GetFailedRules()) + logger.V(2).Info("validation failed", "policy", policy.GetName(), "failed rules", engineResponse.GetFailedRules()) continue } if len(engineResponse.GetSuccessRules()) > 0 { - logger.V(2).Info("validation passed", "policy", policy.Name) + logger.V(2).Info("validation passed", "policy", policy.GetName()) } } diff --git a/pkg/webhooks/verify_images.go b/pkg/webhooks/verify_images.go index 505173dca0..f85816136a 100644 --- a/pkg/webhooks/verify_images.go +++ b/pkg/webhooks/verify_images.go @@ -12,7 +12,7 @@ import ( "k8s.io/api/admission/v1beta1" ) -func (ws *WebhookServer) applyImageVerifyPolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []*v1.ClusterPolicy, logger logr.Logger) ([]byte, error) { +func (ws *WebhookServer) applyImageVerifyPolicies(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, policies []v1.PolicyInterface, logger logr.Logger) ([]byte, error) { ok, message, imagePatches := ws.handleVerifyImages(request, policyContext, policies) if !ok { return nil, errors.New(message) @@ -24,7 +24,7 @@ func (ws *WebhookServer) applyImageVerifyPolicies(request *v1beta1.AdmissionRequ func (ws *WebhookServer) handleVerifyImages(request *v1beta1.AdmissionRequest, policyContext *engine.PolicyContext, - policies []*v1.ClusterPolicy) (bool, string, []byte) { + policies []v1.PolicyInterface) (bool, string, []byte) { if len(policies) == 0 { return true, "", nil