fix generate clone/data check

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 : {{<variable_name}}
 	// - if there is variables that are not defined the context -> 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 {

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
 }
 

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

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