added conversion of overlay to patch strategic merge (#1138)

* added conversion of overlay to patch strategic merge and modified unittest for the same

* updated best practice policy

pkg/api/kyverno/v1/types.go

@@ -270,7 +270,7 @@ type Mutation struct {
 	// Patches is preserved for backwards compatibility and will be removed in Kyverno 1.5+
 	Patches []Patch `json:"patches,omitempty" yaml:"patches,omitempty"`
 
-	PatchStrategicMerge interface{} `json:"patchStrategicMerge,omitempty" yaml:"patchesStrategicMerge,omitempty"`
+	PatchStrategicMerge interface{} `json:"patchStrategicMerge,omitempty" yaml:"patchStrategicMerge,omitempty"`
 	PatchesJSON6902     string      `json:"patchesJson6902,omitempty" yaml:"patchesJson6902,omitempty"`
 }
 

pkg/engine/mutate/mutation.go

@@ -22,7 +22,11 @@ func CreateMutateHandler(ruleName string, mutate *kyverno.Mutation, patchedResou
 	case isPatchesJSON6902(mutate):
 		return newPatchesJSON6902Handler(ruleName, mutate, patchedResource, logger)
 	case isOverlay(mutate):
-		return newOverlayHandler(ruleName, mutate, patchedResource, context, logger)
+		// return newOverlayHandler(ruleName, mutate, patchedResource, context, logger)
+		mutate.PatchStrategicMerge = mutate.Overlay
+		var a interface{}
+		mutate.Overlay = a
+		return newpatchStrategicMergeHandler(ruleName, mutate, patchedResource, context, logger)
 	case isPatches(mutate):
 		return newpatchesHandler(ruleName, mutate, patchedResource, context, logger)
 	default:

pkg/engine/mutation_test.go

@@ -64,7 +64,7 @@ func Test_VariableSubstitutionOverlay(t *testing.T) {
 		}
 	}
 	`)
-	expectedPatch := []byte(`{ "op": "add", "path": "/metadata/labels", "value":{"appname":"check-root-user"} }`)
+	expectedPatch := []byte(`{"op":"add","path":"/metadata/labels","value":{"appname":"check-root-user"}}`)
 
 	var policy kyverno.ClusterPolicy
 	err := json.Unmarshal(rawPolicy, &policy)

pkg/policymutation/policymutation.go

@@ -2,6 +2,7 @@ package policymutation
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 	"strconv"
@@ -56,6 +57,19 @@ func GenerateJSONPatchesForDefaults(policy *kyverno.ClusterPolicy, log logr.Logg
 
 	patches = append(patches, convertPatch...)
 
+	overlaySMPPatches, errs := convertOverlayToStrategicMerge(policy, log)
+
+	if len(errs) > 0 {
+		var errMsgs []string
+		for _, err := range errs {
+			errMsgs = append(errMsgs, err.Error())
+			log.Error(err, "failed to generate pod controller rule")
+		}
+		updateMsgs = append(updateMsgs, strings.Join(errMsgs, ";"))
+	}
+
+	patches = append(patches, overlaySMPPatches...)
+
 	return utils.JoinPatches(patches), updateMsgs
 }
 
@@ -96,6 +110,45 @@ func convertPatchToJSON6902(policy *kyverno.ClusterPolicy, log logr.Logger) (pat
 	return patches, errs
 }
 
+func convertOverlayToStrategicMerge(policy *kyverno.ClusterPolicy, log logr.Logger) (patches [][]byte, errs []error) {
+	patches = make([][]byte, 0)
+	if len(policy.Spec.Rules) == 0 {
+		return patches, []error{
+			errors.New("a policy should have at least one rule"),
+		}
+	}
+
+	for i, rule := range policy.Spec.Rules {
+		if !reflect.DeepEqual(rule.Mutation, kyverno.Mutation{}) {
+			if !reflect.DeepEqual(rule.Mutation.Overlay, kyverno.Mutation{}.Overlay) {
+				mutation := rule.Mutation
+				mutation.PatchStrategicMerge = mutation.Overlay
+				var a interface{}
+				mutation.Overlay = a
+
+				jsonPatch := struct {
+					Path  string            `json:"path"`
+					Op    string            `json:"op"`
+					Value *kyverno.Mutation `json:"value"`
+				}{
+					fmt.Sprintf("/spec/rules/%s/mutate", strconv.Itoa(i)),
+					"replace",
+					&mutation,
+				}
+
+				patchByte, err := json.Marshal(jsonPatch)
+				if err != nil {
+					errs = append(errs, fmt.Errorf("failed to convert overlay to patchStrategicMerge for policy '%s': %v", policy.Name, err))
+				}
+
+				patches = append(patches, patchByte)
+			}
+		}
+	}
+
+	return patches, errs
+}
+
 func defaultBackgroundFlag(policy *kyverno.ClusterPolicy, log logr.Logger) ([]byte, string) {
 	// set 'Background' flag to 'true' if not specified
 	defaultVal := true
@@ -377,7 +430,7 @@ func generateRuleForControllers(rule kyverno.Rule, controllers string, log logr.
 
 	if rule.Mutation.Overlay != nil {
 		newMutation := &kyverno.Mutation{
-			Overlay: map[string]interface{}{
+			PatchStrategicMerge: map[string]interface{}{
 				"spec": map[string]interface{}{
 					"template": rule.Mutation.Overlay,
 				},

pkg/webhooks/policymutation_test.go

@@ -95,7 +95,7 @@ func TestGeneratePodControllerRule_DisableFeature(t *testing.T) {
 				}
 			  },
 			  "mutate": {
-				"overlay": {
+				"patchStrategicMerge": {
 				  "metadata": {
 					"annotations": {
 					  "+(cluster-autoscaler.kubernetes.io/safe-to-evict)": "true"
@@ -146,7 +146,7 @@ func TestGeneratePodControllerRule_Mutate(t *testing.T) {
 				}
 			  },
 			  "mutate": {
-				"overlay": {
+				"patchStrategicMerge": {
 				  "metadata": {
 					"annotations": {
 					  "+(cluster-autoscaler.kubernetes.io/safe-to-evict)": "true"
@@ -197,7 +197,7 @@ func TestGeneratePodControllerRule_Mutate(t *testing.T) {
 				}
 			  },
 			  "mutate": {
-				"overlay": {
+				"patchStrategicMerge": {
 				  "metadata": {
 					"annotations": {
 					  "+(cluster-autoscaler.kubernetes.io/safe-to-evict)": "true"
@@ -227,7 +227,7 @@ func TestGeneratePodControllerRule_Mutate(t *testing.T) {
 				}
 			  },
 			  "mutate": {
-				"overlay": {
+				"patchStrategicMerge": {
 				  "spec": {
 					"template": {
 					  "metadata": {
@@ -258,7 +258,7 @@ func TestGeneratePodControllerRule_Mutate(t *testing.T) {
 				}
 			  },
 			  "mutate": {
-				"overlay": {
+				"patchStrategicMerge": {
 				  "spec": {
 					"jobTemplate": {
 					  "spec": {

samples/AddSafeToEvict.md

@@ -25,10 +25,10 @@ spec:
         kinds: 
         - Pod
     mutate: 
-      overlay:
+      patchStrategicMerge:
         metadata:
           annotations:
-            +(cluster-autoscaler.kubernetes.io/safe-to-evict): true
+            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
         spec:          
           volumes: 
           - (emptyDir): {}
@@ -38,10 +38,10 @@ spec:
         kinds: 
         - Pod
     mutate: 
-      overlay:
+      patchStrategicMerge:
         metadata:
           annotations:
-            +(cluster-autoscaler.kubernetes.io/safe-to-evict): true
+            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
         spec:          
           volumes: 
           - (hostPath):

samples/best_practices/add_safe_to_evict.yaml

@@ -15,10 +15,10 @@ spec:
         kinds: 
         - Pod
     mutate: 
-      overlay:
+      patchStrategicMerge:
         metadata:
           annotations:
-            +(cluster-autoscaler.kubernetes.io/safe-to-evict): true
+            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
         spec:          
           volumes: 
           - (emptyDir): {}
@@ -28,10 +28,10 @@ spec:
         kinds: 
         - Pod
     mutate: 
-      overlay:
+      patchStrategicMerge:
         metadata:
           annotations:
-            +(cluster-autoscaler.kubernetes.io/safe-to-evict): true
+            +(cluster-autoscaler.kubernetes.io/safe-to-evict): "true"
         spec:          
           volumes: 
           - (hostPath):

test/output/output_mutate_pod_spec.yaml

@@ -1,6 +1,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
+  creationTimestamp: "2020-09-21T12:56:35Z"
   name: nginx-deployment
   labels:
     app: nginx
@@ -12,6 +13,7 @@ spec:
       app: nginx
   template:
     metadata:
+      creationTimestamp: "2020-09-21T12:56:35Z"
       labels:
         app: nginx
     spec:

test/output/output_mutate_validate_qos.yaml

@@ -1,7 +1,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
-  creationTimestamp: 
+  creationTimestamp: "2020-09-21T12:56:35Z"
   labels:
     test: qos
   name: qos-demo
@@ -13,7 +13,7 @@ spec:
   strategy: {}
   template:
     metadata:
-      creationTimestamp: 
+      creationTimestamp: "2020-09-21T12:56:35Z"
       labels:
         app: nginx
     spec:

test/resources/resource_mutate_pod_spec.yaml

@@ -1,6 +1,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
+  creationTimestamp: "2020-09-21T12:56:35Z"
   name: nginx-deployment
   labels:
     app: nginx
@@ -12,6 +13,7 @@ spec:
       app: nginx
   template:
     metadata:
+      creationTimestamp: "2020-09-21T12:56:35Z"
       labels:
         app: nginx
     spec:

test/resources/resource_mutate_validate_qos.yaml

@@ -1,6 +1,7 @@
 apiVersion: apps/v1
 kind: Deployment
 metadata:
+  creationTimestamp: "2020-09-21T12:56:35Z"
   name: qos-demo
   labels:
     test: qos
@@ -11,6 +12,7 @@ spec:
       app: nginx
   template:
     metadata:
+      creationTimestamp: "2020-09-21T12:56:35Z"
       labels:
         app: nginx
     spec:

test/scenarios/other/scenario_mutate_pod_spec.yaml

@@ -16,4 +16,4 @@ expected:
         - name: disable-servicelink-and-token
           type: Mutation
           success: true
-          message: successfully processed overlay
+          message: successfully processed stragetic merge patch

test/scenarios/other/scenario_mutate_validate_qos.yaml

@@ -16,7 +16,7 @@ expected:
         - name: add-memory-limit
           type: Mutation
           success: true
-          message: successfully processed overlay
+          message: successfully processed stragetic merge patch
   validation:
     policyresponse:
       policy: policy-qos

test/scenarios/samples/best_practices/add_safe_to_evict.yaml

@@ -16,4 +16,4 @@ expected:
         - name: annotate-empty-dir
           type: Mutation
           success: true
-          message: "successfully processed overlay"
+          message: "successfully processed stragetic merge patch"

test/scenarios/samples/best_practices/add_safe_to_evict2.yaml

@@ -16,4 +16,4 @@ expected:
         - name: annotate-host-path
           type: Mutation
           success: true
-          message: "successfully processed overlay"
+          message: "successfully processed stragetic merge patch"