From 9e0f39efcfead7e4339f8829962458b00d0e559e Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Wed, 13 Nov 2019 12:34:55 -0800 Subject: [PATCH] remove GetOwners() --- pkg/policy/cleanup.go | 5 +-- pkg/policyviolation/controller.go | 6 ++-- pkg/policyviolation/generator.go | 33 +++----------------- pkg/policyviolation/helpers.go | 28 ++--------------- pkg/policyviolation/namespacedpv.go | 2 +- pkg/policyviolation/namespacepvcontroller.go | 6 ++-- 6 files changed, 17 insertions(+), 63 deletions(-) diff --git a/pkg/policy/cleanup.go b/pkg/policy/cleanup.go index 7a445bbad7..427cef4fd9 100644 --- a/pkg/policy/cleanup.go +++ b/pkg/policy/cleanup.go @@ -69,11 +69,12 @@ func getPVonOwnerRef(pvLister kyvernolister.ClusterPolicyViolationLister, dclien } // get owners // getOwners returns nil if there is any error - owners := policyviolation.GetOwners(dclient, *resource) + owners := map[kyverno.ResourceSpec]interface{}{} + policyviolation.GetOwner(dclient, owners, *resource) // as we can have multiple top level owners to a resource // check if pv exists on each one // does not check for cycles - for _, owner := range owners { + for owner := range owners { pv, err := getPVOnResource(pvLister, policyName, owner.Kind, owner.Namespace, owner.Name) if err != nil { glog.Errorf("error while fetching resource owners: %v", err) diff --git a/pkg/policyviolation/controller.go b/pkg/policyviolation/controller.go index 9942da2a3f..3c9793d0e1 100644 --- a/pkg/policyviolation/controller.go +++ b/pkg/policyviolation/controller.go @@ -20,7 +20,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/cache" - "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" ) @@ -269,10 +268,11 @@ func (pvc *PolicyViolationController) syncBlockedResource(curPv *kyverno.Cluster for _, resource := range resources.Items { glog.V(4).Infof("getting owners for %s/%s/%s\n", resource.GetKind(), resource.GetNamespace(), resource.GetName()) - owners := GetOwners(pvc.client, resource) + owners := map[kyverno.ResourceSpec]interface{}{} + GetOwner(pvc.client, owners, resource) // owner of resource matches violation resourceSpec // remove policy violation as the blocked request got created - if containsOwner(owners, curPv.Spec.ResourceSpec) { + if _, ok := owners[curPv.Spec.ResourceSpec]; ok { // pod -> replicaset1; deploy -> replicaset2 // if replicaset1 == replicaset2, the pod is // no longer an active child of deploy, skip removing pv diff --git a/pkg/policyviolation/generator.go b/pkg/policyviolation/generator.go index 3fef575261..e07ba6a97a 100644 --- a/pkg/policyviolation/generator.go +++ b/pkg/policyviolation/generator.go @@ -339,7 +339,7 @@ func buildPVWithOwners(dclient *client.Client, info Info) []kyverno.ClusterPolic var pvs []kyverno.ClusterPolicyViolation // as its blocked resource, the violation is created on owner ownerMap := map[kyverno.ResourceSpec]interface{}{} - getOwner(dclient, ownerMap, info.Resource) + GetOwner(dclient, ownerMap, info.Resource) // standaloneresource, set pvResourceSpec with resource itself if len(ownerMap) == 0 { @@ -359,33 +359,8 @@ func buildPVWithOwners(dclient *client.Client, info Info) []kyverno.ClusterPolic return pvs } -//getOwners pass in unstr rather than using the client to get the unstr -// as if name is empty then GetResource panic as it returns a list -func getOwnersOld(dclient *dclient.Client, unstr unstructured.Unstructured) []kyverno.ResourceSpec { - resourceOwners := unstr.GetOwnerReferences() - if len(resourceOwners) == 0 { - return []kyverno.ResourceSpec{kyverno.ResourceSpec{ - Kind: unstr.GetKind(), - Namespace: unstr.GetNamespace(), - Name: unstr.GetName(), - }} - } - var owners []kyverno.ResourceSpec - for _, resourceOwner := range resourceOwners { - // TODO(shuting): when owner is replicaset, the replicaset even not create, too fast - unstrParent, err := dclient.GetResource(resourceOwner.Kind, unstr.GetNamespace(), resourceOwner.Name) - if err != nil { - glog.Errorf("Failed to get resource owner for %s/%s/%s, err: %v", resourceOwner.Kind, unstr.GetNamespace(), resourceOwner.Name, err) - return nil - } - - owners = append(owners, GetOwners(dclient, *unstrParent)...) - } - return owners -} - -// get owners of a resource by iterating over ownerReferences -func getOwner(dclient *client.Client, ownerMap map[kyverno.ResourceSpec]interface{}, resource unstructured.Unstructured) { +// GetOwner of a resource by iterating over ownerReferences +func GetOwner(dclient *client.Client, ownerMap map[kyverno.ResourceSpec]interface{}, resource unstructured.Unstructured) { var emptyInterface interface{} resourceSpec := kyverno.ResourceSpec{ Kind: resource.GetKind(), @@ -413,6 +388,6 @@ func getOwner(dclient *client.Client, ownerMap map[kyverno.ResourceSpec]interfac // as we want to process other owners continue } - getOwner(dclient, ownerMap, *owner) + GetOwner(dclient, ownerMap, *owner) } } diff --git a/pkg/policyviolation/helpers.go b/pkg/policyviolation/helpers.go index 34539c8cab..f619eafd55 100644 --- a/pkg/policyviolation/helpers.go +++ b/pkg/policyviolation/helpers.go @@ -3,10 +3,10 @@ package policyviolation import ( "fmt" "reflect" + "time" "github.com/golang/glog" kyverno "github.com/nirmata/kyverno/pkg/api/kyverno/v1alpha1" - dclient "github.com/nirmata/kyverno/pkg/dclient" v1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" unstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -30,30 +30,6 @@ func converLabelToSelector(labelMap map[string]string) (labels.Selector, error) return policyViolationSelector, nil } -//GetOwners pass in unstr rather than using the client to get the unstr -// as if name is empty then GetResource panic as it returns a list -func GetOwners(dclient *dclient.Client, unstr unstructured.Unstructured) []kyverno.ResourceSpec { - resourceOwners := unstr.GetOwnerReferences() - if len(resourceOwners) == 0 { - return []kyverno.ResourceSpec{kyverno.ResourceSpec{ - Kind: unstr.GetKind(), - Namespace: unstr.GetNamespace(), - Name: unstr.GetName(), - }} - } - var owners []kyverno.ResourceSpec - for _, resourceOwner := range resourceOwners { - unstrParent, err := dclient.GetResource(resourceOwner.Kind, unstr.GetNamespace(), resourceOwner.Name) - if err != nil { - glog.Errorf("Failed to get resource owner for %s/%s/%s, err: %v", resourceOwner.Kind, unstr.GetNamespace(), resourceOwner.Name, err) - return nil - } - - owners = append(owners, GetOwners(dclient, *unstrParent)...) - } - return owners -} - func containsOwner(owners []kyverno.ResourceSpec, pvResourceSpec kyverno.ResourceSpec) bool { curOwner := kyverno.ResourceSpec{ Kind: pvResourceSpec.Kind, @@ -87,11 +63,13 @@ func validDependantForDeployment(client appsv1.AppsV1Interface, pvResourceSpec k Name: pvResourceSpec.Name, } + start := time.Now() deploy, err := client.Deployments(owner.Namespace).Get(owner.Name, metav1.GetOptions{}) if err != nil { glog.Errorf("failed to get resourceOwner deployment %s/%s/%s: %v", owner.Kind, owner.Namespace, owner.Name, err) return false } + glog.V(4).Infof("Time getting deployment %v", time.Since(start)) // TODO(shuting): replace typed client AppsV1Interface expectReplicaset, err := deployutil.GetNewReplicaSet(deploy, client) diff --git a/pkg/policyviolation/namespacedpv.go b/pkg/policyviolation/namespacedpv.go index 37187632f0..06035a3d58 100644 --- a/pkg/policyviolation/namespacedpv.go +++ b/pkg/policyviolation/namespacedpv.go @@ -44,7 +44,7 @@ func buildNamespacedPVObj(policy string, resource kyverno.ResourceSpec, fRules [ func buildNamespacedPVWithOwner(dclient *dclient.Client, info Info) (pvs []kyverno.NamespacedPolicyViolation) { // create violation on resource owner (if exist) when action is set to enforce ownerMap := map[kyverno.ResourceSpec]interface{}{} - getOwner(dclient, ownerMap, info.Resource) + GetOwner(dclient, ownerMap, info.Resource) // standaloneresource, set pvResourceSpec with resource itself if len(ownerMap) == 0 { diff --git a/pkg/policyviolation/namespacepvcontroller.go b/pkg/policyviolation/namespacepvcontroller.go index 8057e0fd70..299ffd42cc 100644 --- a/pkg/policyviolation/namespacepvcontroller.go +++ b/pkg/policyviolation/namespacepvcontroller.go @@ -267,10 +267,10 @@ func (pvc *NamespacedPolicyViolationController) syncBlockedResource(curPv *kyver for _, resource := range resources.Items { glog.V(4).Infof("getting owners for %s/%s/%s\n", resource.GetKind(), resource.GetNamespace(), resource.GetName()) - owners := GetOwners(pvc.client, resource) - // owner of resource matches violation resourceSpec + owners := map[kyverno.ResourceSpec]interface{}{} + GetOwner(pvc.client, owners, resource) // owner of resource matches violation resourceSpec // remove policy violation as the blocked request got created - if containsOwner(owners, curPv.Spec.ResourceSpec) { + if _, ok := owners[curPv.Spec.ResourceSpec]; ok { // pod -> replicaset1; deploy -> replicaset2 // if replicaset1 == replicaset2, the pod is // no longer an active child of deploy, skip removing pv