feat: Support nested objects in merge generator using go templates (Issue #12836)

applicationset/generators/merge.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	"dario.cat/mergo"
@@ -66,13 +67,13 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl
 		return nil, fmt.Errorf("error getting param sets from generators: %w", err)
 	}
 
-	baseParamSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSetsFromGenerators[0])
+	baseParamSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSetsFromGenerators[0], appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
 	if err != nil {
 		return nil, fmt.Errorf("error getting param sets by merge key: %w", err)
 	}
 
 	for _, paramSets := range paramSetsFromGenerators[1:] {
-		paramSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSets)
+		paramSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSets, appSet.Spec.GoTemplate, appSet.Spec.GoTemplateOptions)
 		if err != nil {
 			return nil, fmt.Errorf("error getting param sets by merge key: %w", err)
 		}
@@ -106,38 +107,84 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl
 }
 
 // getParamSetsByMergeKey converts the given list of parameter sets to a map of parameter sets where the key is the
-// unique key of the parameter set as determined by the given mergeKeys. If any two parameter sets share the same merge
+// unique representation of the parameter set as determined by the given mergeKeys. If any two parameter sets share the same merge
 // key, getParamSetsByMergeKey will throw NonUniqueParamSets.
-func getParamSetsByMergeKey(mergeKeys []string, paramSets []map[string]any) (map[string]map[string]any, error) {
+func getParamSetsByMergeKey(mergeKeys []string, paramSets []map[string]any, useGoTemplate bool, goTemplateOptions []string) (map[string]map[string]any, error) {
 	if len(mergeKeys) < 1 {
 		return nil, ErrNoMergeKeys
 	}
 
+	// Just deduplicate the merge keys.
+	// A merge key may be just a duplicate, we only need it once.
 	deDuplicatedMergeKeys := make(map[string]bool, len(mergeKeys))
 	for _, mergeKey := range mergeKeys {
 		deDuplicatedMergeKeys[mergeKey] = false
 	}
 
 	paramSetsByMergeKey := make(map[string]map[string]any, len(paramSets))
-	for _, paramSet := range paramSets {
-		paramSetKey := make(map[string]any)
-		for mergeKey := range deDuplicatedMergeKeys {
-			paramSetKey[mergeKey] = paramSet[mergeKey]
+	for i, paramSet := range paramSets {
+
+		paramSetRepr, err := generateParamSetRepr(deDuplicatedMergeKeys, paramSet, useGoTemplate, goTemplateOptions)
+		if err != nil {
+			return nil, fmt.Errorf("failed generating paramSetRepr for paramSet %v: %w", i, err)
 		}
-		paramSetKeyJson, err := json.Marshal(paramSetKey)
+
+		// Convert it to json and back to a string
+		// This is necessary, as the value indexed by this key may be a complex
+		// object. We want to index with this representation -> String.
+		paramSetReprJson, err := json.Marshal(paramSetRepr)
 		if err != nil {
 			return nil, fmt.Errorf("error marshalling param set key json: %w", err)
 		}
-		paramSetKeyString := string(paramSetKeyJson)
-		if _, exists := paramSetsByMergeKey[paramSetKeyString]; exists {
-			return nil, fmt.Errorf("%w. Duplicate key was %s", ErrNonUniqueParamSets, paramSetKeyString)
+		paramSetReprString := string(paramSetReprJson)
+
+		// If this was already in the map, we have a duplicate value with respect
+		// to these merge keys and these paramSets are not distinguishable.
+		if _, exists := paramSetsByMergeKey[paramSetReprString]; exists {
+			return nil, fmt.Errorf("%w. Duplicate key was %s", ErrNonUniqueParamSets, paramSetReprString)
 		}
-		paramSetsByMergeKey[paramSetKeyString] = paramSet
+		paramSetsByMergeKey[paramSetReprString] = paramSet
 	}
 
 	return paramSetsByMergeKey, nil
 }
 
+// generateparamSetRepr uses the keys to generate a representation for a paramSet.
+//
+// This can be thought of a modulo operation: 'paramSet % keys' leaves a
+// representation of the paramSet with respect to the keys.
+func generateParamSetRepr(keys map[string]bool, paramSet map[string]any, useGoTemplate bool, goTemplateOptions []string) (paramSetRepr map[string]any, err error) {
+	paramSetRepr = make(map[string]any)
+	// For each part of these keys, fetch the value from the paramSet
+	for mergeKey := range keys {
+		if useGoTemplate {
+			keyTemplate := mergeKey
+			// We treat every merge key as a template. But maybe, it is just a 'bare'
+			// key, not containing any golang templating.
+			// If so, we just surround the whole key with brackets to make
+			// go templating evaluate it.
+			// To avoid key collisions (e.g. a string containing a valid map
+			// representation), we additionally record the real type of the
+			// key used.
+			if !strings.Contains(mergeKey, "{{") && !strings.Contains(mergeKey, "}}") {
+				keyTemplate = fmt.Sprintf("{{ kindOf .%s }}:{{ .%s }}", mergeKey, mergeKey)
+			}
+
+			// Now, this can be templated into a value with respect to the current paramSet
+			templatedMergeKey, err := replaceTemplatedString(keyTemplate, paramSet, useGoTemplate, goTemplateOptions)
+			if err != nil {
+				return nil, fmt.Errorf("failed to template merge key: %w", err)
+			}
+
+			paramSetRepr[mergeKey] = templatedMergeKey
+		} else {
+			paramSetRepr[mergeKey] = paramSet[mergeKey]
+		}
+	}
+
+	return paramSetRepr, nil
+}
+
 // getParams get the parameters generated by this generator.
 func (m *MergeGenerator) getParams(appSetBaseGenerator argoprojiov1alpha1.ApplicationSetNestedGenerator, appSet *argoprojiov1alpha1.ApplicationSet, client client.Client) ([]map[string]any, error) {
 	matrixGen, err := getMatrixGenerator(appSetBaseGenerator)

applicationset/generators/merge_test.go

@@ -56,6 +56,7 @@
 		mergeKeys      []string
 		expectedErr    error
 		expected       []map[string]any
+		useGoTemplate  bool
 	}{
 		{
 			name:           "no generators",
@@ -83,6 +84,19 @@
 				{"a": "2_1", "b": "same", "c": "1_3"},
 			},
 		},
+		{
+			name: "happy flow templated - generate paramSets",
+			baseGenerators: []argoprojiov1alpha1.ApplicationSetNestedGenerator{
+				*getNestedListGenerator(`{"a": "1_1","b": "same"}`),
+				*getNestedListGenerator(`{"a": "1_1","b": "same","c": "2_3"}`),
+				*getNestedListGenerator(`{"a": "3_1","b": "different","c": "3_3"}`), // gets ignored because its merge key value isn't in the base params set
+			},
+			mergeKeys: []string{"{{ .a }}-{{ .b }}"},
+			expected: []map[string]interface{}{
+				{"a": "1_1", "b": "same", "c": "2_3"},
+			},
+			useGoTemplate: true,
+		},
 		{
 			name: "merge keys absent - do not merge",
 			baseGenerators: []argoprojiov1alpha1.ApplicationSetNestedGenerator{
@@ -155,6 +169,7 @@
 			t.Parallel()
 
 			appSet := &argoprojiov1alpha1.ApplicationSet{}
+			appSet.Spec.GoTemplate = testCaseCopy.useGoTemplate
 
 			mergeGenerator := NewMergeGenerator(
 				map[string]Generator{
@@ -211,22 +226,36 @@
 
 func TestParamSetsAreUniqueByMergeKeys(t *testing.T) {
 	testCases := []struct {
-		name        string
-		mergeKeys   []string
-		paramSets   []map[string]any
-		expectedErr error
-		expected    map[string]map[string]any
+		name              string
+		mergeKeys         []string
+		paramSets         []map[string]any
+		expectedErr       error
+		expected          map[string]map[string]any
+		useGoTemplate     bool
+		goTemplateOptions []string
 	}{
 		{
 			name:        "no merge keys",
 			mergeKeys:   []string{},
 			expectedErr: ErrNoMergeKeys,
 		},
+		{
+			name:          "no merge keys templated",
+			mergeKeys:     []string{},
+			expectedErr:   ErrNoMergeKeys,
+			useGoTemplate: true,
+		},
 		{
 			name:      "no paramSets",
 			mergeKeys: []string{"key"},
 			expected:  make(map[string]map[string]any),
 		},
+		{
+			name:          "no paramSets templated",
+			mergeKeys:     []string{"key"},
+			expected:      make(map[string]map[string]interface{}),
+			useGoTemplate: true,
+		},
 		{
 			name:      "simple key, unique paramSets",
 			mergeKeys: []string{"key"},
@@ -236,6 +265,26 @@
 				`{"key":"b"}`: {"key": "b"},
 			},
 		},
+		{
+			name:      "templated simple key, unique paramSets",
+			mergeKeys: []string{"key"},
+			paramSets: []map[string]interface{}{{"key": "a"}, {"key": "b"}},
+			expected: map[string]map[string]interface{}{
+				`{"key":"string:a"}`: {"key": "a"},
+				`{"key":"string:b"}`: {"key": "b"},
+			},
+			useGoTemplate: true,
+		},
+		{
+			name:      "templated multi-key, unique paramSets",
+			mergeKeys: []string{"{{ .key }}-{{ .key }}"},
+			paramSets: []map[string]interface{}{{"key": "a"}, {"key": "b"}},
+			expected: map[string]map[string]interface{}{
+				`{"{{ .key }}-{{ .key }}":"a-a"}`: {"key": "a"},
+				`{"{{ .key }}-{{ .key }}":"b-b"}`: {"key": "b"},
+			},
+			useGoTemplate: true,
+		},
 		{
 			name:      "simple key object, unique paramSets",
 			mergeKeys: []string{"key"},
@@ -308,6 +357,47 @@
 				`{"key1":"b","key2":"a"}`: {"key1": "b", "key2": "a"},
 			},
 		},
+		{
+			name:      "compound templated duplicate key, unique nested paramSets",
+			mergeKeys: []string{"key1", "key2", "{{ .key3.key4 }}", "key2"},
+			paramSets: []map[string]interface{}{
+				{"key1": "a", "key2": "a", "key3": map[string]interface{}{"key4": "a"}},
+				{"key1": "a", "key2": "b", "key3": map[string]interface{}{"key4": "a", "key5": "b"}},
+				{"key1": "b", "key2": "a", "key3": map[string]interface{}{"key4": "b"}},
+			},
+			expected: map[string]map[string]interface{}{
+				`{"key1":"string:a","key2":"string:a","{{ .key3.key4 }}":"a"}`: {"key1": "a", "key2": "a", "key3": map[string]interface{}{"key4": "a"}},
+				`{"key1":"string:a","key2":"string:b","{{ .key3.key4 }}":"a"}`: {"key1": "a", "key2": "b", "key3": map[string]interface{}{"key4": "a", "key5": "b"}},
+				`{"key1":"string:b","key2":"string:a","{{ .key3.key4 }}":"b"}`: {"key1": "b", "key2": "a", "key3": map[string]interface{}{"key4": "b"}},
+			},
+			useGoTemplate: true,
+		},
+		{
+			name:      "compound templated ordered key",
+			mergeKeys: []string{"key1", "key2"},
+			paramSets: []map[string]interface{}{
+				{"key1": "a", "key2": map[string]interface{}{"key3": "a", "key4": "b"}},
+				{"key1": "b", "key2": map[string]interface{}{"key4": "b", "key3": "a"}},
+			},
+			expected: map[string]map[string]interface{}{
+				`{"key1":"string:a","key2":"map:map[key3:a key4:b]"}`: {"key1": "a", "key2": map[string]interface{}{"key3": "a", "key4": "b"}},
+				`{"key1":"string:b","key2":"map:map[key3:a key4:b]"}`: {"key1": "b", "key2": map[string]interface{}{"key4": "b", "key3": "a"}},
+			},
+			useGoTemplate: true,
+		},
+		{
+			name:      "compound templated hierarchical key",
+			mergeKeys: []string{"key1"},
+			paramSets: []map[string]interface{}{
+				{"key1": map[string]interface{}{"key2": "a"}},
+				{"key1": "map[key2:a]"},
+			},
+			expected: map[string]map[string]interface{}{
+				`{"key1":"map:map[key2:a]"}`:    {"key1": map[string]interface{}{"key2": "a"}},
+				`{"key1":"string:map[key2:a]"}`: {"key1": "map[key2:a]"},
+			},
+			useGoTemplate: true,
+		},
 		{
 			name:      "compound key, non-unique paramSets",
 			mergeKeys: []string{"key1", "key2"},
@@ -336,7 +426,7 @@
 		t.Run(testCaseCopy.name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := getParamSetsByMergeKey(testCaseCopy.mergeKeys, testCaseCopy.paramSets)
+			got, err := getParamSetsByMergeKey(testCaseCopy.mergeKeys, testCaseCopy.paramSets, testCaseCopy.useGoTemplate, testCaseCopy.goTemplateOptions)
 
 			if testCaseCopy.expectedErr != nil {
 				require.EqualError(t, err, testCaseCopy.expectedErr.Error())

docs/operator-manual/applicationset/Generators-Merge.md

@@ -131,8 +131,6 @@ spec:
     # Use the selector set by both child generators to combine them.
     - merge:
         mergeKeys:
-          # Note that this would not work with goTemplate enabled,
-          # nested merge keys are not supported there.
           - values.selector
         generators:
           # Assuming, all configured clusters have a label for their location:
@@ -211,15 +209,6 @@ Assuming a cluster named `germany01` with the label `metadata.labels.location=Ge
                               elements:
                                 - # (...)
 
-1. Merging on nested values while using `goTemplate: true` is currently not supported, this will not work
-
-        spec:
-          goTemplate: true
-          generators:
-          - merge:
-              mergeKeys:
-                - values.merge
-
 1. When using a Merge generator nested inside another Matrix or Merge generator, [Post Selectors](Generators-Post-Selector.md) for this nested generator's generators will only be applied when enabled via `spec.applyNestedSelectors`.
 
         - merge:
@@ -230,3 +219,25 @@ Assuming a cluster named `germany01` with the label `metadata.labels.location=Ge
                         elements:
                           - # (...)
                       selector: { } # Only applied when applyNestedSelectors is true
+
+1. Using complex merge keys with `goTemplate: true` might be error prone:
+
+    ```yaml
+    # ...
+    mergeKeys:
+      - "{{ .values }}"
+    generators:
+      - list:
+          elements:
+            - values:
+                data: "some"
+      - list:
+          elements:
+            - values: "map[data:some]"
+    ```
+    Those two generators, although having different structures, evaluate to
+    the same merge key. To avoid this, use a merge key like
+    `"{{ kindOf .values }}:{{ .values }}"` including type information.
+
+    For simple merge keys like `values` (without any brackets), this is
+    done automatically.