Repository: kyverno/chainsaw (go)

Issue:
[Bug] JMESPath conditions for name, namespace or labels don't work
### chainsaw version Version

v0.2.11

### Description

JMESPath conditions for name or namespace are ignored. When a JMESPath condition is used for a label, *all* labels are ignored. In our setup this resulted in strange sporadic failures where totally different resources from other tests were matched.

The problem is that `ResourceRef` assumes that the values for those properties are string and silently removes them as a side-effect when they aren't, removing all labels if any label value isn't a string.

This could be considered an unsupported feature, but because of the strange behaviour and the clear cause I found in the code I think it can be considered a bug.

I already created a fix for which I will submit a pull request.

### Steps to reproduce

1. Run the following test:
  ```
  apiVersion: chainsaw.kyverno.io/v1alpha1
  kind: Test
  metadata:
    name: metadata-template-conditions
  spec:
    steps:
      - try:
          - apply:
              resource:
                apiVersion: v1
                kind: ConfigMap
                metadata:
                  labels:
                    color: green
                    type: peugeot
                    top_speed: '130'
                  name: peugeot-206
          - assert:
              resource:
                apiVersion: v1
                kind: ConfigMap
                metadata:
                  labels:
                    color: red
                    type: ferrari
                    top_speed:
                      (to_number(@) > `200`): true
                  name:
                    (starts_with(@, 'ferrari-')): true
  ```

### Expected behavior

The test should fail because four conditions don't match, of which two are not even JMESPath conditions.

### Screenshots

_No response_

### Logs

_No response_

### Slack discussion

_No response_

### Troubleshooting

- [X] I have searched other issues in this repository and mine is not recorded.

Pull request:
fix: template-functions now correctly work for name, namespace etc.
…bels

## Explanation

JMESPath conditions for name or namespace are ignored. When a JMESPath condition is used for a label, all labels are ignored. In our setup this resulted in strange sporadic failures where totally different resources from other tests were matched.

## Related issue

Fixes #2138

## Proposed Changes

The current code uses the getters and setters from Unstructured, i.e.:

```
	temp.SetNamespace(obj.GetNamespace())
	temp.SetLabels(obj.GetLabels())
	
	... perform templating on temp
	
	obj.SetNamespace(temp.GetNamespace())
	obj.SetLabels(temp.GetLabels())
```

The problem here is that the `Get...` methods will return empty values if the values are not strings, and by setting them back to `obj` the existing structured values are overwritten.

The fix involves two changes:
1. Instead of using `obj.GetLabels()`, which returns `nil` if any label value is not a string, filter all `string` labels ignoring any other label.
2. Instead of copying the properties back using the setters, use `obj.Object = maps.Merge(obj.Object, merged.UnstructuredContent())` to avoid that structured values will be removed from `obj` simply because they are absent in the templated object.

## Checklist

- [x ] I have read the [contributing guidelines](https://github.com/kyverno/kyverno/blob/main/CONTRIBUTING.md).
- [ x] I have read the [PR documentation guide](https://github.com/kyverno/kyverno/blob/main/.github/pr_documentation.md) and followed the process including adding proof manifests to this PR.
- [ x] This is a bug fix and I have added unit tests that prove my fix is effective.

## Further Comments


Patch:
diff --git a/pkg/engine/templating/resource_ref.go b/pkg/engine/templating/resource_ref.go
index 8f21430f..1fb34238 100644
--- a/pkg/engine/templating/resource_ref.go
+++ b/pkg/engine/templating/resource_ref.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/kyverno/chainsaw/pkg/apis"
 	"github.com/kyverno/chainsaw/pkg/apis/v1alpha1"
+	"github.com/kyverno/chainsaw/pkg/utils/maps"
 	"github.com/kyverno/kyverno-json/pkg/core/compilers"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 )
@@ -24,17 +25,27 @@ func ResourceRef(ctx context.Context, c compilers.Compilers, obj *unstructured.U
 	temp.SetKind(kind)
 	temp.SetName(obj.GetName())
 	temp.SetNamespace(obj.GetNamespace())
-	temp.SetLabels(obj.GetLabels())
+	temp.SetLabels(getStringLabels(obj))
 	template := v1alpha1.NewProjection(temp.UnstructuredContent())
 	if merged, err := TemplateAndMerge(ctx, c, temp, bindings, template); err != nil {
 		return err
 	} else {
-		temp = merged
+		obj.Object = maps.Merge(obj.Object, merged.UnstructuredContent())
+	}
+	return nil
+}
+
+func getStringLabels(obj *unstructured.Unstructured) map[string]string {
+	if labels, ok, _ := unstructured.NestedFieldNoCopy(obj.UnstructuredContent(), "metadata", "labels"); ok {
+		if labelsMap, ok := labels.(map[string]interface{}); ok {
+			tempLabels := make(map[string]string)
+			for k, v := range labelsMap {
+				if stringValue, ok := v.(string); ok {
+					tempLabels[k] = stringValue
+				}
+			}
+			return tempLabels
+		}
 	}
-	obj.SetAPIVersion(temp.GetAPIVersion())
-	obj.SetKind(temp.GetKind())
-	obj.SetName(temp.GetName())
-	obj.SetNamespace(temp.GetNamespace())
-	obj.SetLabels(temp.GetLabels())
 	return nil
 }


Test patch:
diff --git a/pkg/engine/templating/resource_ref_test.go b/pkg/engine/templating/resource_ref_test.go
index c9f0994f..388bb0bb 100644
--- a/pkg/engine/templating/resource_ref_test.go
+++ b/pkg/engine/templating/resource_ref_test.go
@@ -33,6 +33,40 @@ func TestResourceRef(t *testing.T) {
 			"($foo)": "($bar)",
 		},
 	)
+
+	withNonStringValues := unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "namespace",
+			"metadata": map[string]interface{}{
+				"name":      map[string]interface{}{"k": "v"},
+				"namespace": map[string]interface{}{"k": "v"},
+				"labels": map[string]interface{}{
+					"plain":           "value",
+					"templated":       "($foo)",
+					"templated-empty": "($empty)",
+					"nested":          map[string]interface{}{"k": "v"},
+				},
+			},
+		},
+	}
+	expectedForNonStringValues := unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "v1",
+			"kind":       "namespace",
+			"metadata": map[string]interface{}{
+				"name":      map[string]interface{}{"k": "v"},
+				"namespace": map[string]interface{}{"k": "v"},
+				"labels": map[string]interface{}{
+					"plain":           "value",
+					"templated":       "foo",
+					"templated-empty": "",
+					"nested":          map[string]interface{}{"k": "v"},
+				},
+			},
+		},
+	}
+
 	tests := []struct {
 		name     string
 		obj      *unstructured.Unstructured
@@ -75,6 +109,12 @@ func TestResourceRef(t *testing.T) {
 		bindings: apis.NewBindings(),
 		wantErr:  true,
 		want:     &binds,
+	}, {
+		name:     "retain non-string values",
+		obj:      withNonStringValues.DeepCopy(),
+		bindings: apis.NewBindings().Register("$foo", apis.NewBinding("foo")).Register("$empty", apis.NewBinding("")),
+		wantErr:  false,
+		want:     &expectedForNonStringValues,
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/testdata/e2e/examples/jmespath-label-condition/README.md b/testdata/e2e/examples/jmespath-label-condition/README.md
new file mode 100644
index 00000000..0b81698c
--- /dev/null
+++ b/testdata/e2e/examples/jmespath-label-condition/README.md
@@ -0,0 +1,23 @@
+# Test: `jmespath-label-condition`
+
+*No description*
+
+## Steps
+
+| # | Name | Bindings | Try | Catch | Finally | Cleanup |
+|:-:|---|:-:|:-:|:-:|:-:|
+| 1 | [step-1](#step-step-1) | 0 | 2 | 0 | 0 | 0 |
+
+### Step: `step-1`
+
+*No description*
+
+#### Try
+
+| # | Operation | Bindings | Outputs | Description |
+|:-:|---|:-:|:-:|---|
+| 1 | `apply` | 0 | 0 | *No description* |
+| 2 | `assert` | 0 | 0 | *No description* |
+
+---
+
diff --git a/testdata/e2e/examples/jmespath-label-condition/chainsaw-test.yaml b/testdata/e2e/examples/jmespath-label-condition/chainsaw-test.yaml
new file mode 100644
index 00000000..fe0155de
--- /dev/null
+++ b/testdata/e2e/examples/jmespath-label-condition/chainsaw-test.yaml
@@ -0,0 +1,24 @@
+# yaml-language-server: $schema=../../../../.schemas/json/test-chainsaw-v1alpha1.json
+apiVersion: chainsaw.kyverno.io/v1alpha1
+kind: Test
+metadata:
+  name: jmespath-label-condition
+spec:
+  steps:
+  - try:
+    - apply:
+        resource:
+          apiVersion: v1
+          kind: ConfigMap
+          metadata:
+            name: jmespath-label-condition
+            labels:
+              type: my-first-configmap
+    - assert:
+        resource:
+          apiVersion: v1
+          kind: ConfigMap
+          metadata:
+            labels:
+              type:
+                (starts_with(@, 'my-first-')): true