From 942f0f5ac34e5a5878d7df864815b20d08825d7f Mon Sep 17 00:00:00 2001 From: shivkumar dudhani Date: Mon, 8 Jul 2019 15:34:21 -0700 Subject: [PATCH] get resource using kind & add cache invalidate mechanism and retry --- pkg/dclient/client.go | 98 ++++++++++++++++----------------- pkg/dclient/client_test.go | 18 +++--- pkg/dclient/utils.go | 17 +++--- pkg/engine/generation_new.go | 14 ++--- pkg/event/controller.go | 3 +- pkg/gencontroller/controller.go | 1 - pkg/testrunner/test.go | 7 ++- pkg/violation/builder.go | 50 +---------------- 8 files changed, 76 insertions(+), 132 deletions(-) diff --git a/pkg/dclient/client.go b/pkg/dclient/client.go index 2fee1e7d82..c473221c59 100644 --- a/pkg/dclient/client.go +++ b/pkg/dclient/client.go @@ -51,6 +51,8 @@ func NewClient(config *rest.Config) (*Client, error) { kclient: kclient, } // Set discovery client + // + discoveryClient := ServerPreferredResources{memory.NewMemCacheClient(kclient.Discovery())} client.SetDiscovery(discoveryClient) return &client, nil @@ -58,7 +60,7 @@ func NewClient(config *rest.Config) (*Client, error) { //GetKubePolicyDeployment returns kube policy depoyment value func (c *Client) GetKubePolicyDeployment() (*apps.Deployment, error) { - kubePolicyDeployment, err := c.GetResource("deployments", config.KubePolicyNamespace, config.KubePolicyDeploymentName) + kubePolicyDeployment, err := c.GetResource("Deployment", config.KubePolicyNamespace, config.KubePolicyDeploymentName) if err != nil { return nil, err } @@ -85,9 +87,9 @@ func (c *Client) getInterface(resource string) dynamic.NamespaceableResourceInte return c.client.Resource(c.getGroupVersionMapper(resource)) } -func (c *Client) getResourceInterface(resource string, namespace string) dynamic.ResourceInterface { - // Get the resource interface - namespaceableInterface := c.getInterface(resource) +func (c *Client) getResourceInterface(kind string, namespace string) dynamic.ResourceInterface { + // Get the resource interface from kind + namespaceableInterface := c.getInterface(kind) // Get the namespacable interface var resourceInteface dynamic.ResourceInterface if namespace != "" { @@ -99,73 +101,70 @@ func (c *Client) getResourceInterface(resource string, namespace string) dynamic } // Keep this a stateful as the resource list will be based on the kubernetes version we connect to -func (c *Client) getGroupVersionMapper(resource string) schema.GroupVersionResource { - //TODO: add checks to see if the resource is supported - //TODO: build the resource list dynamically( by querying the registered resources) - //TODO: the error scenarios - return c.DiscoveryClient.getGVR(resource) +func (c *Client) getGroupVersionMapper(kind string) schema.GroupVersionResource { + return c.DiscoveryClient.GetGVRFromKind(kind) } // GetResource returns the resource in unstructured/json format -func (c *Client) GetResource(resource string, namespace string, name string, subresources ...string) (*unstructured.Unstructured, error) { - return c.getResourceInterface(resource, namespace).Get(name, meta.GetOptions{}, subresources...) +func (c *Client) GetResource(kind string, namespace string, name string, subresources ...string) (*unstructured.Unstructured, error) { + return c.getResourceInterface(kind, namespace).Get(name, meta.GetOptions{}, subresources...) } // ListResource returns the list of resources in unstructured/json format // Access items using []Items -func (c *Client) ListResource(resource string, namespace string, lselector *meta.LabelSelector) (*unstructured.UnstructuredList, error) { +func (c *Client) ListResource(kind string, namespace string, lselector *meta.LabelSelector) (*unstructured.UnstructuredList, error) { options := meta.ListOptions{} if lselector != nil { options = meta.ListOptions{LabelSelector: helperv1.FormatLabelSelector(lselector)} } - return c.getResourceInterface(resource, namespace).List(options) + return c.getResourceInterface(kind, namespace).List(options) } // DeleteResouce deletes the specified resource -func (c *Client) DeleteResouce(resource string, namespace string, name string, dryRun bool) error { +func (c *Client) DeleteResouce(kind string, namespace string, name string, dryRun bool) error { options := meta.DeleteOptions{} if dryRun { options = meta.DeleteOptions{DryRun: []string{meta.DryRunAll}} } - return c.getResourceInterface(resource, namespace).Delete(name, &options) + return c.getResourceInterface(kind, namespace).Delete(name, &options) } // CreateResource creates object for the specified resource/namespace -func (c *Client) CreateResource(resource string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { +func (c *Client) CreateResource(kind string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { options := meta.CreateOptions{} if dryRun { options = meta.CreateOptions{DryRun: []string{meta.DryRunAll}} } // convert typed to unstructured obj if unstructuredObj := convertToUnstructured(obj); unstructuredObj != nil { - return c.getResourceInterface(resource, namespace).Create(unstructuredObj, options) + return c.getResourceInterface(kind, namespace).Create(unstructuredObj, options) } return nil, fmt.Errorf("Unable to create resource ") } // UpdateResource updates object for the specified resource/namespace -func (c *Client) UpdateResource(resource string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { +func (c *Client) UpdateResource(kind string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { options := meta.UpdateOptions{} if dryRun { options = meta.UpdateOptions{DryRun: []string{meta.DryRunAll}} } // convert typed to unstructured obj if unstructuredObj := convertToUnstructured(obj); unstructuredObj != nil { - return c.getResourceInterface(resource, namespace).Update(unstructuredObj, options) + return c.getResourceInterface(kind, namespace).Update(unstructuredObj, options) } return nil, fmt.Errorf("Unable to update resource ") } // UpdateStatusResource updates the resource "status" subresource -func (c *Client) UpdateStatusResource(resource string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { +func (c *Client) UpdateStatusResource(kind string, namespace string, obj interface{}, dryRun bool) (*unstructured.Unstructured, error) { options := meta.UpdateOptions{} if dryRun { options = meta.UpdateOptions{DryRun: []string{meta.DryRunAll}} } // convert typed to unstructured obj if unstructuredObj := convertToUnstructured(obj); unstructuredObj != nil { - return c.getResourceInterface(resource, namespace).UpdateStatus(unstructuredObj, options) + return c.getResourceInterface(kind, namespace).UpdateStatus(unstructuredObj, options) } return nil, fmt.Errorf("Unable to update resource ") } @@ -182,7 +181,6 @@ func convertToUnstructured(obj interface{}) *unstructured.Unstructured { // GenerateResource creates resource of the specified kind(supports 'clone' & 'data') func (c *Client) GenerateResource(generator types.Generation, namespace string, processExistingResources bool) error { var err error - rGVR := c.DiscoveryClient.GetGVRFromKind(generator.Kind) resource := &unstructured.Unstructured{} var rdata map[string]interface{} @@ -196,7 +194,7 @@ func (c *Client) GenerateResource(generator types.Generation, namespace string, } // clone -> copy from existing resource if generator.Clone != nil { - resource, err = c.GetResource(rGVR.Resource, generator.Clone.Namespace, generator.Clone.Name) + resource, err = c.GetResource(generator.Kind, generator.Clone.Namespace, generator.Clone.Name) if err != nil { return err } @@ -213,7 +211,7 @@ func (c *Client) GenerateResource(generator types.Generation, namespace string, glog.Errorf("Can't create a resource %s: %v", generator.Name, err) return nil } - _, err = c.CreateResource(rGVR.Resource, namespace, resource, false) + _, err = c.CreateResource(generator.Kind, namespace, resource, false) if err != nil { return err } @@ -254,7 +252,6 @@ func (c *Client) waitUntilNamespaceIsCreated(name string) error { } type IDiscovery interface { - getGVR(resource string) schema.GroupVersionResource GetGVRFromKind(kind string) schema.GroupVersionResource } @@ -266,48 +263,47 @@ type ServerPreferredResources struct { cachedClient discovery.CachedDiscoveryInterface } -func (c ServerPreferredResources) getGVR(resource string) schema.GroupVersionResource { - emptyGVR := schema.GroupVersionResource{} - serverresources, err := c.cachedClient.ServerPreferredResources() - if err != nil { - glog.Error(err) - return emptyGVR - } - resources, err := discovery.GroupVersionResources(serverresources) - if err != nil { - glog.Error(err) - return emptyGVR - } - //TODO using cached client to support cache validation and invalidation - // iterate over the key to compare the resource - for gvr := range resources { - if gvr.Resource == resource { +//GetGVRFromKind get the Group Version Resource from kind +// if kind is not found in first attempt we invalidate the cache, +// the retry will then fetch the new registered resources and check again +// if not found after 2 attempts, we declare kind is not found +// kind is Case sensitive +func (c ServerPreferredResources) GetGVRFromKind(kind string) schema.GroupVersionResource { + var gvr schema.GroupVersionResource + var err error + gvr, err = loadServerResources(kind, c.cachedClient) + if err != nil && !c.cachedClient.Fresh() { + + // invalidate cahce & re-try once more + c.cachedClient.Invalidate() + gvr, err = loadServerResources(kind, c.cachedClient) + if err == nil { return gvr } } - return emptyGVR + return gvr } -//To-do: measure performance -//To-do: evaluate DefaultRESTMapper to fetch kind->resource mapping -func (c ServerPreferredResources) GetGVRFromKind(kind string) schema.GroupVersionResource { +func loadServerResources(k string, cdi discovery.CachedDiscoveryInterface) (schema.GroupVersionResource, error) { + serverresources, err := cdi.ServerPreferredResources() emptyGVR := schema.GroupVersionResource{} - serverresources, err := c.cachedClient.ServerPreferredResources() if err != nil { glog.Error(err) - return emptyGVR + return emptyGVR, err } for _, serverresource := range serverresources { for _, resource := range serverresource.APIResources { - if resource.Kind == kind && !strings.Contains(resource.Name, "/") { + // skip the resource names with "/", to avoid comparison with subresources + + if resource.Kind == k && !strings.Contains(resource.Name, "/") { gv, err := schema.ParseGroupVersion(serverresource.GroupVersion) if err != nil { glog.Error(err) - return emptyGVR + return emptyGVR, err } - return gv.WithResource(resource.Name) + return gv.WithResource(resource.Name), nil } } } - return emptyGVR + return emptyGVR, fmt.Errorf("kind '%s' not found", k) } diff --git a/pkg/dclient/client_test.go b/pkg/dclient/client_test.go index 6c96105d74..db4396f1c0 100644 --- a/pkg/dclient/client_test.go +++ b/pkg/dclient/client_test.go @@ -64,32 +64,32 @@ func newFixture(t *testing.T) *fixture { func TestCRUDResource(t *testing.T) { f := newFixture(t) // Get Resource - _, err := f.client.GetResource("thekinds", "ns-foo", "name-foo") + _, err := f.client.GetResource("thekind", "ns-foo", "name-foo") if err != nil { t.Errorf("GetResource not working: %s", err) } // List Resources - _, err = f.client.ListResource("thekinds", "ns-foo", nil) + _, err = f.client.ListResource("thekind", "ns-foo", nil) if err != nil { t.Errorf("ListResource not working: %s", err) } // DeleteResouce - err = f.client.DeleteResouce("thekinds", "ns-foo", "name-bar", false) + err = f.client.DeleteResouce("thekind", "ns-foo", "name-bar", false) if err != nil { t.Errorf("DeleteResouce not working: %s", err) } // CreateResource - _, err = f.client.CreateResource("thekinds", "ns-foo", newUnstructured("group/version", "TheKind", "ns-foo", "name-foo1"), false) + _, err = f.client.CreateResource("thekind", "ns-foo", newUnstructured("group/version", "TheKind", "ns-foo", "name-foo1"), false) if err != nil { t.Errorf("CreateResource not working: %s", err) } // UpdateResource - _, err = f.client.UpdateResource("thekinds", "ns-foo", newUnstructuredWithSpec("group/version", "TheKind", "ns-foo", "name-foo1", map[string]interface{}{"foo": "bar"}), false) + _, err = f.client.UpdateResource("thekind", "ns-foo", newUnstructuredWithSpec("group/version", "TheKind", "ns-foo", "name-foo1", map[string]interface{}{"foo": "bar"}), false) if err != nil { t.Errorf("UpdateResource not working: %s", err) } // UpdateStatusResource - _, err = f.client.UpdateStatusResource("thekinds", "ns-foo", newUnstructuredWithSpec("group/version", "TheKind", "ns-foo", "name-foo1", map[string]interface{}{"foo": "status"}), false) + _, err = f.client.UpdateStatusResource("thekind", "ns-foo", newUnstructuredWithSpec("group/version", "TheKind", "ns-foo", "name-foo1", map[string]interface{}{"foo": "status"}), false) if err != nil { t.Errorf("UpdateStatusResource not working: %s", err) } @@ -124,7 +124,7 @@ func TestGenerateResource(t *testing.T) { // 1 create namespace // 2 generate resource // create namespace - ns, err := f.client.CreateResource("namespaces", "", newUnstructured("v1", "Namespace", "", "ns1"), false) + ns, err := f.client.CreateResource("Namespace", "", newUnstructured("v1", "Namespace", "", "ns1"), false) if err != nil { t.Errorf("CreateResource not working: %s", err) } @@ -135,7 +135,7 @@ func TestGenerateResource(t *testing.T) { if err != nil { t.Errorf("GenerateResource not working: %s", err) } - _, err = f.client.GetResource("thekinds", "ns1", "gen-kind") + _, err = f.client.GetResource("TheKind", "ns1", "gen-kind") if err != nil { t.Errorf("GetResource not working: %s", err) } @@ -147,7 +147,7 @@ func TestGenerateResource(t *testing.T) { if err != nil { t.Errorf("GenerateResource not working: %s", err) } - _, err = f.client.GetResource("thekinds", "ns1", "name2-baz-new") + _, err = f.client.GetResource("TheKind", "ns1", "name2-baz-new") if err != nil { t.Errorf("GetResource not working: %s", err) } diff --git a/pkg/dclient/utils.go b/pkg/dclient/utils.go index f1a28ba52d..6492187038 100644 --- a/pkg/dclient/utils.go +++ b/pkg/dclient/utils.go @@ -12,14 +12,15 @@ import ( ) const ( - //CSRs certificatesigningrequests - CSRs string = "certificatesigningrequests" - // Secrets secrets - Secrets string = "secrets" - // ConfigMaps configmaps - ConfigMaps string = "configmaps" - // Namespaces namespaces - Namespaces string = "namespaces" + // Kind names are case sensitive + //CSRs CertificateSigningRequest + CSRs string = "CertificateSigningRequest" + // Secrets Secret + Secrets string = "Secret" + // ConfigMaps ConfigMap + ConfigMaps string = "ConfigMap" + // Namespaces Namespace + Namespaces string = "Namespace" ) const namespaceCreationMaxWaitTime time.Duration = 30 * time.Second const namespaceCreationWaitInterval time.Duration = 100 * time.Millisecond diff --git a/pkg/engine/generation_new.go b/pkg/engine/generation_new.go index 04350968c5..f67df2927f 100644 --- a/pkg/engine/generation_new.go +++ b/pkg/engine/generation_new.go @@ -3,7 +3,6 @@ package engine import ( "encoding/json" "errors" - "fmt" "github.com/golang/glog" v1alpha1 "github.com/nirmata/kyverno/pkg/apis/policy/v1alpha1" @@ -40,15 +39,10 @@ func applyRuleGeneratorNew(client *client.Client, ns *corev1.Namespace, gen *v1a var err error resource := &unstructured.Unstructured{} var rdata map[string]interface{} - // get resource from kind - rGVR := client.DiscoveryClient.GetGVRFromKind(gen.Kind) - if rGVR.Resource == "" { - return fmt.Errorf("Kind to Resource Name conversion failed for %s", gen.Kind) - } if gen.Data != nil { // 1> Check if resource exists - obj, err := client.GetResource(rGVR.Resource, ns.Name, gen.Name) + obj, err := client.GetResource(gen.Kind, ns.Name, gen.Name) if err == nil { // 2> If already exsists, then verify the content is contained // found the resource @@ -70,12 +64,12 @@ func applyRuleGeneratorNew(client *client.Client, ns *corev1.Namespace, gen *v1a } if gen.Clone != nil { // 1> Check if resource exists - _, err := client.GetResource(rGVR.Resource, ns.Name, gen.Name) + _, err := client.GetResource(gen.Kind, ns.Name, gen.Name) if err == nil { return nil } // 2> If already exists return - resource, err = client.GetResource(rGVR.Resource, gen.Clone.Namespace, gen.Clone.Name) + resource, err = client.GetResource(gen.Kind, gen.Clone.Namespace, gen.Clone.Name) if err != nil { return err } @@ -87,7 +81,7 @@ func applyRuleGeneratorNew(client *client.Client, ns *corev1.Namespace, gen *v1a // Reset resource version resource.SetResourceVersion("") - _, err = client.CreateResource(rGVR.Resource, ns.Name, resource, false) + _, err = client.CreateResource(gen.Kind, ns.Name, resource, false) if err != nil { return err } diff --git a/pkg/event/controller.go b/pkg/event/controller.go index 17095053f6..2e85a8ee27 100644 --- a/pkg/event/controller.go +++ b/pkg/event/controller.go @@ -139,8 +139,7 @@ func (c *controller) SyncHandler(key Info) error { return err } default: - resource := c.client.DiscoveryClient.GetGVRFromKind(key.Kind).Resource - robj, err = c.client.GetResource(resource, key.Namespace, key.Name) + robj, err = c.client.GetResource(key.Kind, key.Namespace, key.Name) if err != nil { glog.Errorf("unable to create event for resource %s, will retry ", key.Namespace+"/"+key.Name) return err diff --git a/pkg/gencontroller/controller.go b/pkg/gencontroller/controller.go index c04aa7ed1d..7f872f530d 100644 --- a/pkg/gencontroller/controller.go +++ b/pkg/gencontroller/controller.go @@ -148,7 +148,6 @@ func (c *Controller) syncHandler(obj interface{}) error { } } - glog.Info("apply generation policy to resources :)") //TODO: need to find a way to store the policy such that we can directly queury the // policies with generation policies // PolicyListerExpansion diff --git a/pkg/testrunner/test.go b/pkg/testrunner/test.go index 5230444e5b..f76ba343b1 100644 --- a/pkg/testrunner/test.go +++ b/pkg/testrunner/test.go @@ -1,6 +1,7 @@ package testrunner import ( + "fmt" "strconv" "testing" @@ -41,7 +42,7 @@ func (t *test) run() { // assuming its namespaces creation decode := kscheme.Codecs.UniversalDeserializer().Decode obj, _, err := decode([]byte(t.tResource.rawResource), nil, nil) - _, err = client.CreateResource(getResourceFromKind(t.tResource.gvk.Kind), "", obj, false) + _, err = client.CreateResource(t.tResource.gvk.Kind, "", obj, false) if err != nil { t.t.Errorf("error while creating namespace %s", err) } @@ -68,6 +69,8 @@ func (t *test) checkMutationResult(pr *resourceInfo, policyInfo *info.PolicyInfo } // patched resource if !compareResource(pr, t.patchedResource) { + fmt.Println(string(t.patchedResource.rawResource)) + fmt.Println(string(pr.rawResource)) glog.Warningf("Expected resource %s ", string(pr.rawResource)) t.t.Error("Patched resources not as expected") } @@ -148,7 +151,7 @@ func (t *test) checkGenerationResult(client *client.Client, policyInfo *info.Pol for _, r := range t.genResources { n := ParseNameFromObject(r.rawResource) ns := ParseNamespaceFromObject(r.rawResource) - _, err := client.GetResource(getResourceFromKind(r.gvk.Kind), ns, n) + _, err := client.GetResource(r.gvk.Kind, ns, n) if err != nil { t.t.Errorf("Resource %s/%s of kinf %s not found", ns, n, r.gvk.Kind) } diff --git a/pkg/violation/builder.go b/pkg/violation/builder.go index d3dec2ac39..e6075728b0 100644 --- a/pkg/violation/builder.go +++ b/pkg/violation/builder.go @@ -55,7 +55,7 @@ func (b *builder) processViolation(info *Info) error { statusMap := map[string]interface{}{} var ok bool //TODO: hack get from client - p1, err := b.client.GetResource("policies", "", info.Policy, "status") + p1, err := b.client.GetResource("Policy", "", info.Policy, "status") if err != nil { return err } @@ -106,54 +106,6 @@ func (b *builder) processViolation(info *Info) error { return err } return nil - // modifiedViolations := []types.Violation{} - // modifiedViolations = append(modifiedViolations, types.Violation{Name: "name", Kind: "Deploymeny"}) - // unstr["status"] = modifiedViolations - // p1.SetUnstructuredContent(unstr) - // rdata, err := p1.MarshalJSON() - // if err != nil { - // glog.Info(err) - // } - // glog.Info(string(rdata)) - // _, err = b.client.UpdateStatusResource("policies", "", p1, false) - // if err != nil { - // glog.Info(err) - // } - - // p, err := b.policyLister.Get(info.Policy) - // if err != nil { - // glog.Error(err) - // return err - // } - - // glog.Info(p.TypeMeta.Kind) - // glog.Info(p.Kind) - // modifiedPolicy := p.DeepCopy() - // glog.Info(modifiedPolicy.Kind) - // // Create new violation - // newViolation := info.Violation - - // for _, violation := range modifiedPolicy.Status.Violations { - // ok, err := b.isActive(info.Kind, violation.Name) - // if err != nil { - // glog.Error(err) - // continue - // } - // if !ok { - // glog.Info("removed violation") - // } - // } - // // If violation already exists for this rule, we update the violation - // //TODO: update violation, instead of re-creating one every time - // modifiedViolations = append(modifiedViolations, newViolation) - // modifiedPolicy.Status.Violations = modifiedViolations - // // Violations are part of the status sub resource, so we can use the Update Status api instead of updating the policy object - // _, err = b.client.UpdateStatusResource("policies", "", *modifiedPolicy, false) - // if err != nil { - // glog.Info(err) - // return err - // } - // return nil } func (b *builder) isActive(kind string, rname string, rnamespace string) (bool, error) {