Fix kyverno#1506; Resolve path reference in entire rule instead of ju…

…st pattern/overlay

Signed-off-by: Max Goncharenko <kacejot@fex.net>

pkg/engine/anchor/common/common.go

@@ -1,5 +1,10 @@
 package common
 
+import (
+	"path"
+	"strings"
+)
+
 // IsAnchor is a function handler
 type IsAnchor func(str string) bool
 
@@ -72,6 +77,24 @@ func RemoveAnchor(key string) (string, string) {
 	return key, ""
 }
 
+// RemoveAnchorsFromPath removes all anchor from path string
+func RemoveAnchorsFromPath(str string) string {
+	components := strings.Split(str, "/")
+	if components[0] == "" {
+		components = components[1:]
+	}
+
+	for i, component := range components {
+		components[i], _ = RemoveAnchor(component)
+	}
+
+	newPath := path.Join(components...)
+	if path.IsAbs(str) {
+		newPath = "/" + newPath
+	}
+	return newPath
+}
+
 // AddAnchor adds an anchor with the supplied prefix.
 // The suffix is assumed to be ")".
 func AddAnchor(key, anchorPrefix string) string {

pkg/engine/anchor/common/common_test.go

@@ -56,3 +56,13 @@ func TestIsExistenceAnchor_OnlyHat(t *testing.T) {
 func TestIsExistenceAnchor_ConditionAnchor(t *testing.T) {
 	assert.Assert(t, !IsExistenceAnchor("(abc)"))
 }
+
+func TestRemoveAnchorsFromPath_WorksWithAbsolutePath(t *testing.T) {
+	newPath := RemoveAnchorsFromPath("/path/(to)/X(anchors)")
+	assert.Equal(t, newPath, "/path/to/anchors")
+}
+
+func TestRemoveAnchorsFromPath_WorksWithRelativePath(t *testing.T) {
+	newPath := RemoveAnchorsFromPath("path/(to)/X(anchors)")
+	assert.Equal(t, newPath, "path/to/anchors")
+}

pkg/engine/common/utils.go

@@ -0,0 +1,19 @@
+package common
+
+// CopyMap creates a full copy of the target map
+func CopyMap(m map[string]interface{}) map[string]interface{} {
+	mapCopy := make(map[string]interface{})
+	for k, v := range m {
+		mapCopy[k] = v
+	}
+
+	return mapCopy
+}
+
+// CopySlice creates a full copy of the target slice
+func CopySlice(s []interface{}) []interface{} {
+	sliceCopy := make([]interface{}, len(s))
+	copy(sliceCopy, s)
+
+	return sliceCopy
+}

pkg/engine/common/utils_test.go

@@ -0,0 +1,37 @@
+package common
+
+import (
+	"testing"
+
+	"gotest.tools/assert"
+)
+
+func Test_OriginalMapMustNotBeChanged(t *testing.T) {
+	// no variables
+	originalMap := map[string]interface{}{
+		"rsc": 3711,
+		"r":   2138,
+		"gri": 1908,
+		"adg": 912,
+	}
+
+	mapCopy := CopyMap(originalMap)
+	mapCopy["r"] = 1
+
+	assert.Equal(t, originalMap["r"], 2138)
+}
+
+func Test_OriginalSliceMustNotBeChanged(t *testing.T) {
+	// no variables
+	originalSlice := []interface{}{
+		3711,
+		2138,
+		1908,
+		912,
+	}
+
+	sliceCopy := CopySlice(originalSlice)
+	sliceCopy[0] = 1
+
+	assert.Equal(t, originalSlice[0], 3711)
+}

pkg/engine/forceMutate.go

@@ -1,9 +1,7 @@
 package engine
 
 import (
-	"encoding/json"
 	"fmt"
-	"regexp"
 
 	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
 	"github.com/kyverno/kyverno/pkg/engine/context"
@@ -63,17 +61,15 @@ func ForceMutate(ctx context.EvalInterface, policy kyverno.ClusterPolicy, resour
 			continue
 		}
 
+		rule, err = variables.SubstituteAllForceMutate(log.Log, ctx, rule)
+		if err != nil {
+			return unstructured.Unstructured{}, err
+		}
+
 		mutation := rule.Mutation.DeepCopy()
 
 		if mutation.Overlay != nil {
 			overlay := mutation.Overlay
-			if ctx != nil {
-				if overlay, err = variables.SubstituteVars(log.Log, ctx, overlay); err != nil {
-					return unstructured.Unstructured{}, err
-				}
-			} else {
-				overlay = replaceSubstituteVariables(overlay)
-			}
 
 			resource, err = mutateResourceWithOverlay(resource, overlay)
 			if err != nil {
@@ -93,27 +89,3 @@ func ForceMutate(ctx context.EvalInterface, policy kyverno.ClusterPolicy, resour
 
 	return resource, nil
 }
-
-func replaceSubstituteVariables(overlay interface{}) interface{} {
-	overlayRaw, err := json.Marshal(overlay)
-	if err != nil {
-		return overlay
-	}
-
-	regex := regexp.MustCompile(`\{\{([^{}]*)\}\}`)
-	for {
-		if len(regex.FindAllStringSubmatch(string(overlayRaw), -1)) > 0 {
-			overlayRaw = regex.ReplaceAll(overlayRaw, []byte(`placeholderValue`))
-		} else {
-			break
-		}
-	}
-
-	var output interface{}
-	err = json.Unmarshal(overlayRaw, &output)
-	if err != nil {
-		return overlay
-	}
-
-	return output
-}

pkg/engine/forceMutate_test.go

@@ -0,0 +1,150 @@
+package engine
+
+import (
+	"encoding/json"
+	"testing"
+
+	kyverno "github.com/kyverno/kyverno/pkg/api/kyverno/v1"
+	"github.com/kyverno/kyverno/pkg/engine/context"
+	"github.com/kyverno/kyverno/pkg/engine/utils"
+	"gotest.tools/assert"
+)
+
+var rawPolicy = []byte(`
+{
+	"apiVersion": "kyverno.io/v1",
+	"kind": "ClusterPolicy",
+	"metadata": {
+		"name": "add-label"
+	},
+	"spec": {
+		"rules": [
+			{
+				"name": "add-name-label",
+				"match": {
+					"resources": {
+						"kinds": [
+							"Pod"
+						]
+					}
+				},
+				"mutate": {
+					"overlay": {
+						"metadata": {
+							"labels": {
+								"appname": "{{request.object.metadata.name}}"
+							}
+						}
+					}
+				}
+			}
+		]
+	}
+}
+`)
+
+var rawResource = []byte(`
+{
+	"apiVersion": "v1",
+	"kind": "Pod",
+	"metadata": {
+		"name": "check-root-user"
+	},
+	"spec": {
+		"containers": [
+			{
+				"name": "check-root-user",
+				"image": "nginxinc/nginx-unprivileged",
+				"securityContext": {
+					"runAsNonRoot": true
+				}
+			}
+		]
+	}
+}
+`)
+
+func Test_ForceMutateSubstituteVars(t *testing.T) {
+	expectedRawResource := []byte(`
+	{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "check-root-user",
+			"labels": {
+				"appname": "check-root-user"
+			}
+		},
+		"spec": {
+			"containers": [
+				{
+					"name": "check-root-user",
+					"image": "nginxinc/nginx-unprivileged",
+					"securityContext": {
+						"runAsNonRoot": true
+					}
+				}
+			]
+		}
+	}
+	`)
+
+	var expectedResource interface{}
+	assert.NilError(t, json.Unmarshal(expectedRawResource, &expectedResource))
+
+	var policy kyverno.ClusterPolicy
+	err := json.Unmarshal(rawPolicy, &policy)
+	assert.NilError(t, err)
+
+	resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
+	assert.NilError(t, err)
+	ctx := context.NewContext()
+	err = ctx.AddResource(rawResource)
+	assert.NilError(t, err)
+
+	mutatedResource, err := ForceMutate(ctx, policy, *resourceUnstructured)
+	assert.NilError(t, err)
+
+	assert.DeepEqual(t, expectedResource, mutatedResource.UnstructuredContent())
+}
+
+func Test_ForceMutateSubstituteVarsWithNilContext(t *testing.T) {
+	expectedRawResource := []byte(`
+	{
+		"apiVersion": "v1",
+		"kind": "Pod",
+		"metadata": {
+			"name": "check-root-user",
+			"labels": {
+				"appname": "placeholderValue"
+			}
+		},
+		"spec": {
+			"containers": [
+				{
+					"name": "check-root-user",
+					"image": "nginxinc/nginx-unprivileged",
+					"securityContext": {
+						"runAsNonRoot": true
+					}
+				}
+			]
+		}
+	}
+	`)
+
+	var expectedResource interface{}
+	assert.NilError(t, json.Unmarshal(expectedRawResource, &expectedResource))
+
+	var policy kyverno.ClusterPolicy
+	err := json.Unmarshal(rawPolicy, &policy)
+	assert.NilError(t, err)
+
+	resourceUnstructured, err := utils.ConvertToUnstructured(rawResource)
+	assert.NilError(t, err)
+
+	mutatedResource, err := ForceMutate(nil, policy, *resourceUnstructured)
+	assert.NilError(t, err)
+
+	assert.DeepEqual(t, expectedResource, mutatedResource.UnstructuredContent())
+}