Browse Source

fix: do not include the last element of the path in the iteration (#5588)

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Co-authored-by: Jakob Möller <jakob.moeller@sap.com>
Gergely Brautigam 6 months ago
parent
commit
1b95bfd551
2 changed files with 78 additions and 1 deletions
  1. 3 1
      runtime/template/v2/template.go
  2. 75 0
      runtime/template/v2/template_test.go

+ 3 - 1
runtime/template/v2/template.go

@@ -333,7 +333,9 @@ func applyParsedToPath(parsed any, target string, obj client.Object) error {
 		// navigate to the last element of the path and apply the entire struct at that location.
 		// build up the entire map structure that we are eventually going to apply.
 		current := unstructured
-		for _, part := range parts {
+		// this STOPS at the last part! That is important. for _, part := range parts does _include_ the last part
+		for i := 0; i < len(parts)-1; i++ {
+			part := parts[i]
 			if current[part] == nil {
 				current[part] = make(map[string]any)
 			}

+ 75 - 0
runtime/template/v2/template_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 
 	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
 )
@@ -1189,3 +1190,77 @@ func TestConfigMapDataNotBase64Encoded(t *testing.T) {
 	assert.Equal(t, "5432", configMap.Data["port"], "port should be plain text, not base64")
 	assert.Equal(t, "mydb", configMap.Data["database"], "database should be plain text, not base64")
 }
+
+func TestNestedPathTargeting(t *testing.T) {
+	tests := []struct {
+		name     string
+		target   string
+		scope    esapi.TemplateScope
+		tpl      map[string][]byte
+		data     map[string][]byte
+		verify   func(t *testing.T, obj map[string]interface{})
+		wantErr  bool
+		errorMsg string
+	}{
+		{
+			name:   "nested path spec.slack with template variables",
+			target: "spec.slack",
+			scope:  esapi.TemplateScopeKeysAndValues,
+			tpl: map[string][]byte{
+				"slack-config": []byte(`
+api_url: {{ .url }}
+webhook_url: {{ .webhook }}
+`),
+			},
+			data: map[string][]byte{
+				"url":     []byte("https://hooks.slack.com/services/XXX"),
+				"webhook": []byte("https://hooks.slack.com/services/YYY"),
+			},
+			verify: func(t *testing.T, obj map[string]interface{}) {
+				specMap := obj["spec"].(map[string]interface{})
+				slackMap := specMap["slack"].(map[string]interface{})
+
+				// Should NOT have spec.slack.api_url.url (the bug with template variables)
+				apiURL := slackMap["api_url"]
+				assert.Equal(t, "https://hooks.slack.com/services/XXX", apiURL, "api_url should be string, not nested map")
+
+				webhookURL := slackMap["webhook_url"]
+				assert.Equal(t, "https://hooks.slack.com/services/YYY", webhookURL, "webhook_url should be string, not nested map")
+
+				// Verify the fix: should NOT have duplicate 'slack' in the path
+				_, hasDuplicateSlack := slackMap["slack"]
+				assert.False(t, hasDuplicateSlack, "should not have spec.slack.slack (the bug)")
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Use an unstructured object to test generic target behavior
+			obj := &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "example.com/v1",
+					"kind":       "TestResource",
+					"metadata": map[string]interface{}{
+						"name":      "test-resource",
+						"namespace": "default",
+					},
+				},
+			}
+
+			err := Execute(tt.tpl, tt.data, tt.scope, tt.target, obj)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errorMsg != "" {
+					assert.Contains(t, err.Error(), tt.errorMsg)
+				}
+			} else {
+				require.NoError(t, err)
+				if tt.verify != nil {
+					tt.verify(t, obj.Object)
+				}
+			}
+		})
+	}
+}