Update mutate overlay to handle keys with slashes for labels (like annotations). Added debug V4 logs for mutate flows. (#972)

pkg/engine/forceMutate.go

@@ -16,10 +16,13 @@ import (
 )
 
 func mutateResourceWithOverlay(resource unstructured.Unstructured, overlay interface{}) (unstructured.Unstructured, error) {
+	logger := log.Log.WithValues("resource", resource.GetKind(), "overlay", overlay)
 	patches, err := mutate.MutateResourceWithOverlay(resource.UnstructuredContent(), overlay)
 	if err != nil {
-		return unstructured.Unstructured{}, err
+		logger.V(4).Info("failed to mutate resource with overlay")
+		return resource, err
 	}
+
 	if len(patches) == 0 {
 		return resource, nil
 	}
@@ -27,21 +30,25 @@ func mutateResourceWithOverlay(resource unstructured.Unstructured, overlay inter
 	// convert to RAW
 	resourceRaw, err := resource.MarshalJSON()
 	if err != nil {
-		return unstructured.Unstructured{}, err
+		logger.V(4).Info("failed to marshall resource JSON")
+		return resource, err
 	}
 
 	var patchResource []byte
 	patchResource, err = utils.ApplyPatches(resourceRaw, patches)
 	if err != nil {
-		return unstructured.Unstructured{}, err
+		logger.V(4).Info("failed to apply patches")
+		return resource, err
 	}
 
 	resource = unstructured.Unstructured{}
 	err = resource.UnmarshalJSON(patchResource)
 	if err != nil {
-		return unstructured.Unstructured{}, err
+		logger.V(4).Info("failed to unmarshal patched resource JSON")
+		return resource, err
 	}
 
+	logger.V(4).Info("mutated resource with overlay")
 	return resource, nil
 }
 
@@ -67,7 +74,8 @@ func ForceMutate(ctx context.EvalInterface, policy kyverno.ClusterPolicy, resour
 
 			resource, err = mutateResourceWithOverlay(resource, overlay)
 			if err != nil {
-				return unstructured.Unstructured{}, fmt.Errorf("could not mutate resource with overlay on rule %v:%v", rule.Name, err)
+				detailedErr := fmt.Errorf("failed to mutate resource %s with overlay rule %v:%v", resource.GetKind(), rule.Name, err)
+				return unstructured.Unstructured{}, detailedErr
 			}
 		}
 

pkg/engine/mutate/overlay.go

@@ -351,7 +351,8 @@ func processSubtree(overlay interface{}, path string, op string) ([]byte, error)
 
 	// explicitly handle boolean type in annotation
 	// keep the type boolean as it is in any other fields
-	if strings.Contains(path, "/metadata/annotations") {
+	if strings.Contains(path, "/metadata/annotations")  ||
+		strings.Contains(path, "/metadata/labels"){
 		patchStr = wrapBoolean(patchStr)
 	}
 
@@ -366,16 +367,27 @@ func processSubtree(overlay interface{}, path string, op string) ([]byte, error)
 
 func preparePath(path string) string {
 	if path == "" {
-		path = "/"
+		return "/"
 	}
 
-	annPath := "/metadata/annotations/"
-	// escape slash in annotation patch
-	if strings.Contains(path, annPath) {
-		idx := strings.Index(path, annPath)
-		p := path[idx+len(annPath):]
-		path = path[:idx+len(annPath)] + strings.ReplaceAll(p, "/", "~1")
+	// TODO - handle all map key paths
+	// The path for a maps needs to be updated to handle keys with slashes.
+	// We currently do this for known map types. Ideally we can check the
+	// target schema and generically update for any map type.
+	path = replaceSlashes(path, "/metadata/annotations/")
+	path = replaceSlashes(path, "/metadata/labels/")
+	return path
+}
+
+// escape slash in paths for maps (labels, annotations, etc.
+func replaceSlashes(path, prefix string) string {
+	if !strings.Contains(path, prefix) {
+		return path
 	}
+
+	idx := strings.Index(path, prefix)
+	p := path[idx+len(prefix):]
+	path = path[:idx+len(prefix)] + strings.ReplaceAll(p, "/", "~1")
 	return path
 }
 

pkg/engine/utils/utils.go

@@ -4,6 +4,7 @@ import (
 	jsonpatch "github.com/evanphx/json-patch"
 	"github.com/nirmata/kyverno/pkg/engine/anchor"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 //RuleType defines the type for rule
@@ -35,14 +36,17 @@ func ApplyPatches(resource []byte, patches [][]byte) ([]byte, error) {
 	joinedPatches := JoinPatches(patches)
 	patch, err := jsonpatch.DecodePatch(joinedPatches)
 	if err != nil {
+		log.Log.V(4).Info("failed to decode JSON patch", "patch", patch)
 		return resource, err
 	}
 
 	patchedDocument, err := patch.Apply(resource)
 	if err != nil {
+		log.Log.V(4).Info("failed to apply JSON patch", "patch", patch)
 		return resource, err
 	}
 
+	log.Log.V(4).Info("applied JSON patch", "patch", patch)
 	return patchedDocument, err
 }
 

pkg/kyverno/apply/command.go

@@ -69,9 +69,10 @@ func Command() *cobra.Command {
 			for _, policy := range policies {
 				err := policy2.Validate(utils.MarshalPolicy(*policy), nil, true, openAPIController)
 				if err != nil {
-					fmt.Printf("Policy %v is not valid\n", policy.Name)
+					fmt.Printf("Policy %v is not valid: %v\n", policy.Name, err)
 					os.Exit(3)
 				}
+
 				if policyHasVariables(*policy) {
 					return sanitizedError.NewWithError(fmt.Sprintf("invalid policy %s. 'apply' does not support policies with variables", policy.Name), err)
 				}

pkg/openapi/validation.go

@@ -128,17 +128,19 @@ func (o *Controller) ValidatePolicyMutation(policy v1.ClusterPolicy) error {
 		newPolicy := *policy.DeepCopy()
 		newPolicy.Spec.Rules = rules
 		resource, _ := o.generateEmptyResource(o.definitions[o.kindToDefinitionName[kind]]).(map[string]interface{})
-		if resource == nil {
-			log.Log.V(4).Info(fmt.Sprintf("Cannot Validate policy: openApi definition now found for %v", kind))
+		if resource == nil  || len(resource) == 0 {
+			log.Log.V(2).Info("unable to validate resource. OpenApi definition not found", "kind", kind)
 			return nil
 		}
+
 		newResource := unstructured.Unstructured{Object: resource}
 		newResource.SetKind(kind)
 
-		patchedResource, err := engine.ForceMutate(nil, *newPolicy.DeepCopy(), newResource)
+		patchedResource, err := engine.ForceMutate(nil, newPolicy, newResource)
 		if err != nil {
 			return err
 		}
+
 		err = o.ValidateResource(*patchedResource.DeepCopy(), kind)
 		if err != nil {
 			return err