From b25a0371135a7a57da6ea0a299a5d5be81f1385d Mon Sep 17 00:00:00 2001 From: Jim Bugwadia Date: Mon, 14 Dec 2020 02:43:16 -0800 Subject: [PATCH] fix generate clone/data check --- pkg/generate/generate.go | 85 ++++++++++++++++------------- pkg/generate/generate_controller.go | 7 ++- pkg/policy/generate/validate.go | 8 +-- pkg/webhooks/generate/generate.go | 12 ++-- 4 files changed, 61 insertions(+), 51 deletions(-) diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 7d2077752d..3b347d7f12 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -15,7 +15,6 @@ import ( "github.com/kyverno/kyverno/pkg/engine" "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/utils" - "github.com/kyverno/kyverno/pkg/engine/validate" "github.com/kyverno/kyverno/pkg/engine/variables" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -295,6 +294,7 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou if err != nil { return noGenResource, err } + // Variable substitutions // format : {{ results in error and rule is not applied @@ -310,6 +310,8 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou return noGenResource, err } + logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName) + // Resource to be generated newGenResource := kyverno.ResourceSpec{ APIVersion: genAPIVersion, @@ -318,32 +320,34 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou Name: genName, } - genData, found, err := unstructured.NestedMap(genUnst.Object, "data") + genData, _, err := unstructured.NestedMap(genUnst.Object, "data") if err != nil { - return noGenResource, err + return noGenResource, fmt.Errorf("failed to read `data`: %v", err.Error()) } genClone, _, err := unstructured.NestedMap(genUnst.Object, "clone") if err != nil { - return noGenResource, err + return noGenResource, fmt.Errorf("failed to read `clone`: %v", err.Error()) } - if genClone != nil { - rdata, mode, err = manageClone(log, genAPIVersion, genKind, genNamespace, genName, genClone, client, resource) + if genClone != nil && len(genClone) != 0 { + rdata, mode, err = manageClone(logger, genAPIVersion, genKind, genNamespace, genName, genClone, client) } else { - if found { - rdata, mode, err = manageData(log, genAPIVersion, genKind, genNamespace, genName, genData, client, resource) - } else { - log.V(3).Info("generate rule has no data and clone") - } + rdata, mode, err = manageData(logger, genAPIVersion, genKind, genNamespace, genName, genData, client) } - logger := log.WithValues("genKind", genKind, "genAPIVersion", genAPIVersion, "genNamespace", genNamespace, "genName", genName) + if err != nil { + logger.Error(err, "failed to generate resource", "data", rdata, "mode", mode) + return newGenResource, err + } - if rdata == nil { - // existing resource contains the configuration + logger.V(2).Info("applying generate rule", "data", rdata, "mode", mode) + + if rdata == nil && mode == Update { + logger.V(4).Info("no changes required for target resource") return newGenResource, nil } + if processExisting { return noGenResource, nil } @@ -373,6 +377,7 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou } else { label["policy.kyverno.io/synchronize"] = "disable" } + // Reset resource version newResource.SetResourceVersion("") // Create the resource @@ -381,7 +386,7 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou return noGenResource, err } - logger.V(2).Info("created resource") + logger.V(2).Info("generated target resource") } else if mode == Update { label := newResource.GetLabels() @@ -399,49 +404,60 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou logger.Error(err, "updating existing resource") return noGenResource, err } - logger.V(2).Info("updated generated resource") + logger.V(2).Info("updated target resource") } } + return newGenResource, nil } -func manageData(log logr.Logger, apiVersion, kind, namespace, name string, data map[string]interface{}, client *dclient.Client, resource unstructured.Unstructured) (map[string]interface{}, ResourceMode, error) { +func manageData(log logr.Logger, apiVersion, kind, namespace, name string, data map[string]interface{}, client *dclient.Client) (map[string]interface{}, ResourceMode, error) { obj, err := client.GetResource(apiVersion, kind, namespace, name) if err != nil { if apierrors.IsNotFound(err) { - log.V(3).Info("resource does not exist, will try to create", "genKind", kind, "genAPIVersion", apiVersion, "genNamespace", namespace, "genName", name) return data, Create, nil } + log.Error(err, "failed to get resource") return nil, Skip, err } + log.Info("found target resource", "resource", obj) + if data == nil { + log.Info("data is nil - skipping update", "resource", obj) + return nil, Skip, nil + } + updateObj := &unstructured.Unstructured{} updateObj.SetUnstructuredContent(data) updateObj.SetResourceVersion(obj.GetResourceVersion()) return updateObj.UnstructuredContent(), Update, nil } -func manageClone(log logr.Logger, apiVersion, kind, namespace, name string, clone map[string]interface{}, client *dclient.Client, resource unstructured.Unstructured) (map[string]interface{}, ResourceMode, error) { - newRNs, _, err := unstructured.NestedString(clone, "namespace") +func manageClone(log logr.Logger, apiVersion, kind, namespace, name string, clone map[string]interface{}, client *dclient.Client) (map[string]interface{}, ResourceMode, error) { + rNamespace, _, err := unstructured.NestedString(clone, "namespace") if err != nil { - return nil, Skip, err - } - newRName, _, err := unstructured.NestedString(clone, "name") - if err != nil { - return nil, Skip, err + return nil, Skip, fmt.Errorf("failed to find source namespace: %v", err) } - // Short-circuit if the resource to be generated and the clone is the same - if newRNs == namespace && newRName == name { - // attempting to clone it self, this will fail -> short-ciruit it + rName, _, err := unstructured.NestedString(clone, "name") + if err != nil { + return nil, Skip, fmt.Errorf("failed to find source name: %v", err) + } + + if rName == "" && rNamespace == "" { + return nil, Skip, fmt.Errorf("invalid source", "clone", clone) + } + + if rNamespace == namespace && rName == name { + log.V(4).Info("skip resource self-clone") return nil, Skip, nil } // check if the resource as reference in clone exists? - obj, err := client.GetResource(apiVersion, kind, newRNs, newRName) + obj, err := client.GetResource(apiVersion, kind, rNamespace, rName) if err != nil { - return nil, Skip, fmt.Errorf("reference clone resource %s/%s/%s/%s not found. %v", apiVersion, kind, newRNs, newRName, err) + return nil, Skip, fmt.Errorf("source resource %s %s/%s/%s not found. %v", apiVersion, kind, rNamespace, rName, err) } // check if resource to be generated exists @@ -475,15 +491,6 @@ const ( Update = "UPDATE" ) -func checkResource(log logr.Logger, newResourceSpec interface{}, resource *unstructured.Unstructured) error { - // check if the resource spec if a subset of the resource - if path, err := validate.ValidateResourceWithPattern(log, resource.Object, newResourceSpec); err != nil { - log.Error(err, "Failed to match the resource ", "path", path) - return err - } - return nil -} - func getUnstrRule(rule *kyverno.Generation) (*unstructured.Unstructured, error) { ruleData, err := json.Marshal(rule) if err != nil { diff --git a/pkg/generate/generate_controller.go b/pkg/generate/generate_controller.go index 2ac96202da..aead2c65b4 100644 --- a/pkg/generate/generate_controller.go +++ b/pkg/generate/generate_controller.go @@ -1,6 +1,7 @@ package generate import ( + "sigs.k8s.io/controller-runtime/pkg/log" "time" "github.com/go-logr/logr" @@ -262,15 +263,19 @@ func (c *Controller) Run(workers int, stopCh <-chan struct{}) { logger.Info("failed to sync informer cache") return } + for i := 0; i < workers; i++ { go wait.Until(c.worker, constant.GenerateControllerResync, stopCh) } + <-stopCh } // worker runs a worker thread that just dequeues items, processes them, and marks them done. // It enforces that the syncHandler is never invoked concurrently with the same key. func (c *Controller) worker() { + log.Log.Info("starting new worker...") + for c.processNextWorkItem() { } } @@ -280,10 +285,10 @@ func (c *Controller) processNextWorkItem() bool { if quit { return false } + defer c.queue.Done(key) err := c.syncGenerateRequest(key.(string)) c.handleErr(err, key) - return true } diff --git a/pkg/policy/generate/validate.go b/pkg/policy/generate/validate.go index 038bef5556..a491048b87 100644 --- a/pkg/policy/generate/validate.go +++ b/pkg/policy/generate/validate.go @@ -36,14 +36,8 @@ func NewGenerateFactory(client *dclient.Client, rule kyverno.Generation, log log //Validate validates the 'generate' rule func (g *Generate) Validate() (string, error) { rule := g.rule - if rule.Data == nil && rule.Clone == (kyverno.CloneFrom{}) { - // generate rules without data can be used to create objects - // which should not be updated (e.g. service accounts). - g.log.Info("generate rule has no data or clone") - } - if rule.Data != nil && rule.Clone != (kyverno.CloneFrom{}) { - return "", fmt.Errorf("only one operation allowed per generate rule(data or clone)") + return "", fmt.Errorf("only one of data or clone can be specified") } kind, name, namespace := rule.Kind, rule.Name, rule.Namespace diff --git a/pkg/webhooks/generate/generate.go b/pkg/webhooks/generate/generate.go index 51b44f89f8..93c0f1ebc5 100644 --- a/pkg/webhooks/generate/generate.go +++ b/pkg/webhooks/generate/generate.go @@ -28,7 +28,7 @@ type GeneratorChannel struct { action v1beta1.Operation } -// Generator defines the implmentation to mange generate request resource +// Generator defines the implementation to mange generate request resource type Generator struct { // channel to receive request ch chan GeneratorChannel @@ -48,15 +48,17 @@ func NewGenerator(client *kyvernoclient.Clientset, stopCh <-chan struct{}, log l return gen } -// Apply creates generate request resoruce (blocking call if channel is full) +// Apply creates generate request resource (blocking call if channel is full) func (g *Generator) Apply(gr kyverno.GenerateRequestSpec, action v1beta1.Operation) error { logger := g.log logger.V(4).Info("creating Generate Request", "request", gr) + // Update to channel message := GeneratorChannel{ action: action, spec: gr, } + select { case g.ch <- message: return nil @@ -70,13 +72,16 @@ func (g *Generator) Apply(gr kyverno.GenerateRequestSpec, action v1beta1.Operati func (g *Generator) Run(workers int) { logger := g.log defer utilruntime.HandleCrash() + logger.V(4).Info("starting") defer func() { logger.V(4).Info("shutting down") }() + for i := 0; i < workers; i++ { go wait.Until(g.processApply, constant.GenerateControllerResync, g.stopCh) } + <-g.stopCh } @@ -105,8 +110,7 @@ func (g *Generator) generate(grSpec kyverno.GenerateRequestSpec, action v1beta1. func retryApplyResource(client *kyvernoclient.Clientset, grSpec kyverno.GenerateRequestSpec, log logr.Logger, - action v1beta1.Operation, -) error { + action v1beta1.Operation) error { var i int var err error