From bff7229678b93f1cac4412dea2bcc222a101b472 Mon Sep 17 00:00:00 2001 From: Pooja Singh <36136335+NoSkillGirl@users.noreply.github.com> Date: Tue, 15 Dec 2020 04:22:13 +0530 Subject: [PATCH] 1345 use GR lister (#1387) * improved log message * added lister for GR * added label to GR * added wait for cache is sync --- cmd/kyverno/main.go | 5 +-- pkg/generate/cleanup/controller.go | 4 +-- pkg/generate/generate.go | 14 +++++--- pkg/generate/generate_controller.go | 3 +- pkg/webhooks/generate/generate.go | 51 ++++++++++++++++++++++------- pkg/webhooks/generation.go | 15 ++++++--- pkg/webhooks/server.go | 11 ++++++- 7 files changed, 78 insertions(+), 25 deletions(-) diff --git a/cmd/kyverno/main.go b/cmd/kyverno/main.go index d9c33a824a..ab39ffae77 100755 --- a/cmd/kyverno/main.go +++ b/cmd/kyverno/main.go @@ -220,7 +220,7 @@ func main() { } // GENERATE REQUEST GENERATOR - grgen := webhookgenerate.NewGenerator(pclient, stopCh, log.Log.WithName("GenerateRequestGenerator")) + grgen := webhookgenerate.NewGenerator(pclient, pInformer.Kyverno().V1().GenerateRequests(), stopCh, log.Log.WithName("GenerateRequestGenerator")) // GENERATE CONTROLLER // - applies generate rules on resources based on generate requests created by webhook @@ -308,6 +308,7 @@ func main() { pclient, client, tlsPair, + pInformer.Kyverno().V1().GenerateRequests(), pInformer.Kyverno().V1().ClusterPolicies(), kubeInformer.Rbac().V1().RoleBindings(), kubeInformer.Rbac().V1().ClusterRoleBindings(), @@ -341,7 +342,7 @@ func main() { go reportReqGen.Run(2, stopCh) go prgen.Run(1, stopCh) - go grgen.Run(1) + go grgen.Run(1, stopCh) go configData.Run(stopCh) go policyCtrl.Run(2, stopCh) go eventGenerator.Run(3, stopCh) diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go index def7462f31..0b7e17a80b 100644 --- a/pkg/generate/cleanup/controller.go +++ b/pkg/generate/cleanup/controller.go @@ -185,13 +185,13 @@ func (c *Controller) deleteGR(obj interface{}) { for _, resource := range gr.Status.GeneratedResources { r, err := c.client.GetResource(resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) if err != nil && !apierrors.IsNotFound(err) { - logger.Error(err, "Generated resource is not deleted", "Resource", resource.Name) + logger.Error(err, "failed to fetch generated resource", "resource", resource.Name) return } if r != nil && r.GetLabels()["policy.kyverno.io/synchronize"] == "enable" { if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil && !apierrors.IsNotFound(err) { - logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) + logger.Error(err, "failed to delete the generated resource", "resource", r.GetName()) return } } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 0b8a926bda..e61926fe46 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -20,6 +20,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" ) func (c *Controller) processGR(gr *kyverno.GenerateRequest) error { @@ -135,15 +136,20 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern // Removing GR if rule is failed. Used when the generate condition failed but gr exist for _, r := range engineResponse.PolicyResponse.Rules { if !r.Success { - logger.V(4).Info("querying all generate requests") - grList, err := c.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).List(contextdefault.TODO(), metav1.ListOptions{}) + selector := labels.SelectorFromSet(labels.Set(map[string]string{ + "policyName": engineResponse.PolicyResponse.Policy, + "resourceName": engineResponse.PolicyResponse.Resource.Name, + "resourceKind": engineResponse.PolicyResponse.Resource.Kind, + "ResourceNamespace": engineResponse.PolicyResponse.Resource.Namespace, + })) + grList, err := c.grLister.List(selector) if err != nil { - logger.Error(err, "failed to list generate requests") + logger.Error(err, "failed to get generate request for the resource", "kind", engineResponse.PolicyResponse.Resource.Kind, "name", engineResponse.PolicyResponse.Resource.Name, "namespace", engineResponse.PolicyResponse.Resource.Namespace) continue } - for _, v := range grList.Items { + for _, v := range grList { if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace { err := c.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Delete(contextdefault.TODO(), v.GetName(), metav1.DeleteOptions{}) if err != nil { diff --git a/pkg/generate/generate_controller.go b/pkg/generate/generate_controller.go index 2ac96202da..c3ed44e992 100644 --- a/pkg/generate/generate_controller.go +++ b/pkg/generate/generate_controller.go @@ -56,6 +56,7 @@ type Controller struct { // grSynced returns true if the Generate Request store has been synced at least once grSynced cache.InformerSynced + // dynamic shared informer factory dynamicInformer dynamicinformer.DynamicSharedInformerFactory @@ -112,7 +113,7 @@ func NewController( c.grLister = grInformer.Lister().GenerateRequests(config.KyvernoNamespace) c.policySynced = policyInformer.Informer().HasSynced - c.grSynced = policyInformer.Informer().HasSynced + c.grSynced = grInformer.Informer().HasSynced //TODO: dynamic registration // Only supported for namespaces diff --git a/pkg/webhooks/generate/generate.go b/pkg/webhooks/generate/generate.go index 51b44f89f8..fd600f238d 100644 --- a/pkg/webhooks/generate/generate.go +++ b/pkg/webhooks/generate/generate.go @@ -6,15 +6,20 @@ import ( "time" backoff "github.com/cenkalti/backoff" + "github.com/gardener/controller-manager-library/pkg/logger" "github.com/go-logr/logr" kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1" kyvernoclient "github.com/kyverno/kyverno/pkg/client/clientset/versioned" + kyvernoinformer "github.com/kyverno/kyverno/pkg/client/informers/externalversions/kyverno/v1" + kyvernolister "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/constant" "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/tools/cache" ) // GenerateRequests provides interface to manage generate requests @@ -35,15 +40,20 @@ type Generator struct { client *kyvernoclient.Clientset stopCh <-chan struct{} log logr.Logger + // grLister can list/get generate request from the shared informer's store + grLister kyvernolister.GenerateRequestNamespaceLister + grSynced cache.InformerSynced } // NewGenerator returns a new instance of Generate-Request resource generator -func NewGenerator(client *kyvernoclient.Clientset, stopCh <-chan struct{}, log logr.Logger) *Generator { +func NewGenerator(client *kyvernoclient.Clientset, grInformer kyvernoinformer.GenerateRequestInformer, stopCh <-chan struct{}, log logr.Logger) *Generator { gen := &Generator{ - ch: make(chan GeneratorChannel, 1000), - client: client, - stopCh: stopCh, - log: log, + ch: make(chan GeneratorChannel, 1000), + client: client, + stopCh: stopCh, + log: log, + grLister: grInformer.Lister().GenerateRequests(config.KyvernoNamespace), + grSynced: grInformer.Informer().HasSynced, } return gen } @@ -67,13 +77,19 @@ func (g *Generator) Apply(gr kyverno.GenerateRequestSpec, action v1beta1.Operati } // Run starts the generate request spec -func (g *Generator) Run(workers int) { +func (g *Generator) Run(workers int, stopCh <-chan struct{}) { logger := g.log defer utilruntime.HandleCrash() logger.V(4).Info("starting") defer func() { logger.V(4).Info("shutting down") }() + + if !cache.WaitForCacheSync(stopCh, g.grSynced) { + logger.Info("failed to sync informer cache") + return + } + for i := 0; i < workers; i++ { go wait.Until(g.processApply, constant.GenerateControllerResync, g.stopCh) } @@ -93,7 +109,7 @@ func (g *Generator) processApply() { func (g *Generator) generate(grSpec kyverno.GenerateRequestSpec, action v1beta1.Operation) error { // create/update a generate request - if err := retryApplyResource(g.client, grSpec, g.log, action); err != nil { + if err := retryApplyResource(g.client, grSpec, g.log, action, g.grLister); err != nil { return err } return nil @@ -106,6 +122,7 @@ func retryApplyResource(client *kyvernoclient.Clientset, grSpec kyverno.GenerateRequestSpec, log logr.Logger, action v1beta1.Operation, + grLister kyvernolister.GenerateRequestNamespaceLister, ) error { var i int var err error @@ -122,14 +139,20 @@ func retryApplyResource(client *kyvernoclient.Clientset, // generate requests created in kyverno namespace isExist := false if action == v1beta1.Create || action == v1beta1.Update { - log.V(4).Info("querying all generate requests") - grList, err := client.KyvernoV1().GenerateRequests(config.KyvernoNamespace).List(context.TODO(), metav1.ListOptions{}) + selector := labels.SelectorFromSet(labels.Set(map[string]string{ + "policyName": grSpec.Policy, + "resourceName": grSpec.Resource.Name, + "resourceKind": grSpec.Resource.Kind, + "ResourceNamespace": grSpec.Resource.Namespace, + })) + grList, err := grLister.List(selector) if err != nil { + logger.Error(err, "failed to get generate request for the resource", "kind", grSpec.Resource.Kind, "name", grSpec.Resource.Name, "namespace", grSpec.Resource.Namespace) return err } - for i, v := range grList.Items { + for _, v := range grList { if grSpec.Policy == v.Spec.Policy && grSpec.Resource.Name == v.Spec.Resource.Name && grSpec.Resource.Kind == v.Spec.Resource.Kind && grSpec.Resource.Namespace == v.Spec.Resource.Namespace { gr.SetLabels(map[string]string{ "resources-update": "true", @@ -138,7 +161,7 @@ func retryApplyResource(client *kyvernoclient.Clientset, v.Spec.Context = gr.Spec.Context v.Spec.Policy = gr.Spec.Policy v.Spec.Resource = gr.Spec.Resource - _, err = client.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Update(context.TODO(), &grList.Items[i], metav1.UpdateOptions{}) + _, err = client.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Update(context.TODO(), v, metav1.UpdateOptions{}) if err != nil { return err } @@ -147,6 +170,12 @@ func retryApplyResource(client *kyvernoclient.Clientset, } if !isExist { gr.SetGenerateName("gr-") + gr.SetLabels(map[string]string{ + "policyName": grSpec.Policy, + "resourceName": grSpec.Resource.Name, + "resourceKind": grSpec.Resource.Kind, + "ResourceNamespace": grSpec.Resource.Namespace, + }) _, err = client.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Create(context.TODO(), &gr, metav1.CreateOptions{}) if err != nil { return err diff --git a/pkg/webhooks/generation.go b/pkg/webhooks/generation.go index 4569969b95..9f4d9c12de 100644 --- a/pkg/webhooks/generation.go +++ b/pkg/webhooks/generation.go @@ -20,6 +20,7 @@ import ( v1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" ) //HandleGenerate handles admission-requests for policies with generate rules @@ -54,14 +55,20 @@ func (ws *WebhookServer) HandleGenerate(request *v1beta1.AdmissionRequest, polic engineResponse := engine.Generate(policyContext) for _, rule := range engineResponse.PolicyResponse.Rules { if !rule.Success { - ws.log.V(4).Info("querying all generate requests") - grList, err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).List(contextdefault.TODO(), metav1.ListOptions{}) + selector := labels.SelectorFromSet(labels.Set(map[string]string{ + "policyName": engineResponse.PolicyResponse.Policy, + "resourceName": engineResponse.PolicyResponse.Resource.Name, + "resourceKind": engineResponse.PolicyResponse.Resource.Kind, + "ResourceNamespace": engineResponse.PolicyResponse.Resource.Namespace, + })) + grList, err := ws.grLister.List(selector) if err != nil { - logger.Error(err, "failed to list generate request") + logger.Error(err, "failed to get generate request for the resource", "kind", engineResponse.PolicyResponse.Resource.Kind, "name", engineResponse.PolicyResponse.Resource.Name, "namespace", engineResponse.PolicyResponse.Resource.Namespace) + continue } - for _, v := range grList.Items { + for _, v := range grList { if engineResponse.PolicyResponse.Policy == v.Spec.Policy && engineResponse.PolicyResponse.Resource.Name == v.Spec.Resource.Name && engineResponse.PolicyResponse.Resource.Kind == v.Spec.Resource.Kind && engineResponse.PolicyResponse.Resource.Namespace == v.Spec.Resource.Namespace { err := ws.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).Delete(contextdefault.TODO(), v.GetName(), metav1.DeleteOptions{}) if err != nil { diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index 0bd31a177a..99baab4698 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -43,6 +43,12 @@ type WebhookServer struct { client *client.Client kyvernoClient *kyvernoclient.Clientset + // grLister can list/get generate request from the shared informer's store + grLister kyvernolister.GenerateRequestNamespaceLister + + // grSynced returns true if the Generate Request store has been synced at least once + grSynced cache.InformerSynced + // list/get cluster policy resource pLister kyvernolister.ClusterPolicyLister @@ -118,6 +124,7 @@ func NewWebhookServer( kyvernoClient *kyvernoclient.Clientset, client *client.Client, tlsPair *tlsutils.PemPair, + grInformer kyvernoinformer.GenerateRequestInformer, pInformer kyvernoinformer.ClusterPolicyInformer, rbInformer rbacinformer.RoleBindingInformer, crbInformer rbacinformer.ClusterRoleBindingInformer, @@ -153,6 +160,8 @@ func NewWebhookServer( ws := &WebhookServer{ client: client, kyvernoClient: kyvernoClient, + grLister: grInformer.Lister().GenerateRequests(config.KyvernoNamespace), + grSynced: grInformer.Informer().HasSynced, pLister: pInformer.Lister(), pSynced: pInformer.Informer().HasSynced, rbLister: rbInformer.Lister(), @@ -460,7 +469,7 @@ func (ws *WebhookServer) resourceValidation(request *v1beta1.AdmissionRequest) * // RunAsync TLS server in separate thread and returns control immediately func (ws *WebhookServer) RunAsync(stopCh <-chan struct{}) { logger := ws.log - if !cache.WaitForCacheSync(stopCh, ws.pSynced, ws.rbSynced, ws.crbSynced, ws.rSynced, ws.crSynced) { + if !cache.WaitForCacheSync(stopCh, ws.grSynced, ws.pSynced, ws.rbSynced, ws.crbSynced, ws.rSynced, ws.crSynced) { logger.Info("failed to sync informer cache") }