Repository: kyverno/kyverno
Pull request #1471 (2021-01-14 05:56:05)

support nested JMESPATH var substitution
## Related issue

#1466 

<!--
Please link the GitHub issue this pull request resolves in the format of `#1234`. If you discussed this change
with a maintainer, please mention her/him using the `@` syntax (e.g. `@JimBugwadia`).

If this change neither resolves an existing issue nor has sign-off from one of the maintainers, there is a
chance substantial changes will be requested or that the changes will be rejected.

You can discuss changes with maintainers in the [Kyvrno Slack Channel](https://kubernetes.slack.com/).
-->

**What type of PR is this?**
<!--

> Uncomment only one ` /kind <>` line, hit enter to put that in a new line, and remove leading whitespaces from that line:
>
> /kind api-change
> /kind bug
> /kind cleanup
> /kind design
> /kind documentation
> /kind failing-test
> /kind feature
-->

## Proposed changes

<!--
Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
-->

## Checklist

<!--
Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of
them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
-->

- [ x] I have read the [contributing guidelines](../blob/master/CONTRIBUTING.md).
- [x ] I have added tests that prove my fix is effective or that my feature
      works.
- [ ] I have added or changed [the documentation](documentation/).

## Further comments

<!--
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution
you did and what alternatives you considered, etc...
-->



Patch:
diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go
index c035df839..f058e12b0 100644
--- a/pkg/engine/variables/vars.go
+++ b/pkg/engine/variables/vars.go
@@ -89,34 +89,39 @@ func (n NotFoundVariableErr) Error() string {
 func subValR(log logr.Logger, ctx context.EvalInterface, valuePattern string, path string) (interface{}, error) {
 	originalPattern := valuePattern
 	vars := regexVariables.FindAllString(valuePattern, -1)
-	for _, v := range vars {
-		variable := strings.ReplaceAll(v, "{{", "")
-		variable = strings.ReplaceAll(variable, "}}", "")
-		variable = strings.TrimSpace(variable)
-		substitutedVar, err := ctx.Query(variable)
-		if err != nil {
-			return nil, fmt.Errorf("failed to resolve %v at path %s", variable, path)
-		}
+	for len(vars) > 0 {
+		for _, v := range vars {
+			variable := strings.ReplaceAll(v, "{{", "")
+			variable = strings.ReplaceAll(variable, "}}", "")
+			variable = strings.TrimSpace(variable)
+			substitutedVar, err := ctx.Query(variable)
+			if err != nil {
+				return nil, fmt.Errorf("failed to resolve %v at path %s", variable, path)
+			}
 
-		log.V(3).Info("variable substituted", "variable", v, "value", substitutedVar, "path", path)
+			log.V(3).Info("variable substituted", "variable", v, "value", substitutedVar, "path", path)
 
-		if val, ok := substitutedVar.(string); ok {
-			valuePattern = strings.Replace(valuePattern, v, val, -1)
-			continue
-		}
+			if val, ok := substitutedVar.(string); ok {
+				valuePattern = strings.Replace(valuePattern, v, val, -1)
+				continue
+			}
+
+			if substitutedVar != nil {
+				if originalPattern == v {
+					return substitutedVar, nil
+				}
 
-		if substitutedVar != nil {
-			if originalPattern == v {
-				return substitutedVar, nil
+				return nil, fmt.Errorf("failed to resolve %v at path %s", variable, path)
 			}
 
-			return nil, fmt.Errorf("failed to resolve %v at path %s", variable, path)
+			return nil, NotFoundVariableErr{
+				variable: variable,
+				path:     path,
+			}
 		}
 
-		return nil, NotFoundVariableErr{
-			variable: variable,
-			path:     path,
-		}
+		// check for nested variables in strings
+		vars = regexVariables.FindAllString(valuePattern, -1)
 	}
 
 	return valuePattern, nil


Test patch:
diff --git a/pkg/engine/variables/vars_test.go b/pkg/engine/variables/vars_test.go
index 6aacadf93..0d1fef4d3 100644
--- a/pkg/engine/variables/vars_test.go
+++ b/pkg/engine/variables/vars_test.go
@@ -131,26 +131,74 @@ func Test_subVars_failed(t *testing.T) {
 	}
 }
 
-func Test_SubvarRecursive(t *testing.T) {
-	patternRaw := []byte(`"{{request.object.metadata.{{request.object.metadata.test}}}}"`)
-	var pattern interface{}
-	assert.Assert(t, json.Unmarshal(patternRaw, &pattern))
-
-	resourceRaw := []byte(`
+var resourceRaw = []byte(`
 	{
 		"metadata": {
 			"name": "temp",
 			"namespace": "n1",
-			"test":"name"
+			"annotations": {
+			  "test": "name"
+            }
 		},
 		"spec": {
 			"namespace": "n1",
 			"name": "temp1"
 		}
 	}
-		`)
+`)
+
+func Test_SubstituteSuccess(t *testing.T) {
+	ctx := context.NewContext()
+	assert.Assert(t, ctx.AddResource(resourceRaw))
+
+	var pattern interface{}
+	patternRaw := []byte(`"{{request.object.metadata.annotations.test}}"`)
+	assert.Assert(t, json.Unmarshal(patternRaw, &pattern))
+	results, err := subValR(log.Log, ctx, string(patternRaw), "/")
+	if err != nil {
+		t.Errorf("substitution failed: %v", err.Error())
+		return
+	}
+
+	if results.(string) != `"name"` {
+		t.Errorf("expected %s received %v", "name", results)
+	}
+}
+
+func Test_SubstituteRecursiveErrors(t *testing.T) {
+	ctx := context.NewContext()
+	assert.Assert(t, ctx.AddResource(resourceRaw))
 
+	var pattern interface{}
+	patternRaw := []byte(`"{{request.object.metadata.{{request.object.metadata.annotations.test2}}}}"`)
+	assert.Assert(t, json.Unmarshal(patternRaw, &pattern))
+	results, err := subValR(log.Log, ctx, string(patternRaw), "/")
+	if err == nil {
+		t.Errorf("expected error but received: %v", results)
+	}
+
+	patternRaw = []byte(`"{{request.object.metadata2.{{request.object.metadata.annotations.test}}}}"`)
+	assert.Assert(t, json.Unmarshal(patternRaw, &pattern))
+	results, err = subValR(log.Log, ctx, string(patternRaw), "/")
+	if err == nil {
+		t.Errorf("expected error but received: %v", results)
+	}
+}
+
+func Test_SubstituteRecursive(t *testing.T) {
 	ctx := context.NewContext()
 	assert.Assert(t, ctx.AddResource(resourceRaw))
-	subValR(log.Log, ctx, string(patternRaw), "/")
+
+	var pattern interface{}
+	patternRaw := []byte(`"{{request.object.metadata.{{request.object.metadata.annotations.test}}}}"`)
+	assert.Assert(t, json.Unmarshal(patternRaw, &pattern))
+	results, err := subValR(log.Log, ctx, string(patternRaw), "/")
+	if err != nil {
+		t.Errorf("substitution failed: %v", err.Error())
+		return
+	}
+
+	if results.(string) != `"temp"` {
+		t.Errorf("expected %s received %v", "temp", results)
+	}
 }