Allow setting new value in Helm values file

* This was previously supported, but broke when the function was refactored to preserve order of yaml file * Behavior does not change if overriding an existing value * If the values file is empty or does not have a previous value, add the value instead of erroring Signed-off-by: Joshua Novick <[email protected]>
argoproj-labs · Aug 22, 2024 · 94ee4ad · 94ee4ad
1 parent 43dbd63
commit 94ee4ad
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 29 deletions.
diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go
@@ -465,11 +465,11 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 					return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion)
 				}
 
-				err = setHelmValue(helmNewValues, helmAnnotationParamName, helmParamName.Value)
+				err = setHelmValue(&helmNewValues, helmAnnotationParamName, helmParamName.Value)
 				if err != nil {
 					return nil, fmt.Errorf("failed to set image parameter name value: %v", err)
 				}
-				err = setHelmValue(helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value)
+				err = setHelmValue(&helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value)
 				if err != nil {
 					return nil, fmt.Errorf("failed to set image parameter version value: %v", err)
 				}
@@ -542,33 +542,43 @@ func findHelmValuesKey(m yaml.MapSlice, key string) (int, bool) {
 }
 
 // set value of the parameter passed from the annotations.
-func setHelmValue(m yaml.MapSlice, key string, value interface{}) error {
+func setHelmValue(currentValues *yaml.MapSlice, key string, value interface{}) error {
 	// Check if the full key exists
-	if idx, found := findHelmValuesKey(m, key); found {
-		m[idx].Value = value
+	if idx, found := findHelmValuesKey(*currentValues, key); found {
+		(*currentValues)[idx].Value = value
 		return nil
 	}
 
 	var err error
 	keys := strings.Split(key, ".")
-	current := m
+	current := currentValues
+	var parent *yaml.MapSlice
+	var parentIdx int
 
 	for i, k := range keys {
-		if idx, found := findHelmValuesKey(current, k); found {
+		if idx, found := findHelmValuesKey(*current, k); found {
 			if i == len(keys)-1 {
 				// If we're at the final key, set the value and return
-				current[idx].Value = value
+				(*current)[idx].Value = value
 				return nil
 			} else {
 				// Navigate deeper into the map
-				if nestedMap, ok := current[idx].Value.(yaml.MapSlice); ok {
-					current = nestedMap
+				if nestedMap, ok := (*current)[idx].Value.(yaml.MapSlice); ok {
+					parent = current
+					parentIdx = idx
+					current = &nestedMap
 				} else {
-					return fmt.Errorf("unexpected type %T for key %s", current[idx].Value, k)
+					return fmt.Errorf("unexpected type %T for key %s", (*current)[idx].Value, k)
 				}
 			}
 		} else {
-			err = fmt.Errorf("key %s not found in the map", k)
+			new := append(*current, yaml.MapItem{Key: k, Value: value})
+
+			if parent == nil {
+				*currentValues = new
+			} else {
+				(*parent)[parentIdx].Value = new
+			}
 			break
 		}
 	}

diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go
@@ -1472,6 +1472,13 @@ replicas: 1
 	})
 
 	t.Run("Failed to setValue image parameter name", func(t *testing.T) {
+		expected := `
+image:
+  name: nginx
+  tag: v1.0.0
+replicas: 1
+`
+
 		app := v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
 				Name: "testapp",
@@ -1514,16 +1521,24 @@ replicas: 1
 		}
 
 		originalData := []byte(`
-image_name: nginx
-image.tag: v0.0.0
+image:
+  name: nginx
 replicas: 1
 `)
-		_, err := marshalParamsOverride(&app, originalData)
-		assert.Error(t, err)
-		assert.Equal(t, "failed to set image parameter name value: key image not found in the map", err.Error())
+
+		yaml, err := marshalParamsOverride(&app, originalData)
+		require.NoError(t, err)
+		assert.NotEmpty(t, yaml)
+		assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", "  ")), strings.TrimSpace(string(yaml)))
 	})
 
 	t.Run("Failed to setValue image parameter version", func(t *testing.T) {
+		expected := `
+image:
+  tag: v1.0.0
+  name: nginx
+replicas: 1
+`
 		app := v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
 				Name: "testapp",
@@ -1566,13 +1581,15 @@ replicas: 1
 		}
 
 		originalData := []byte(`
-image.name: nginx
-image_tag: v0.0.0
+image:
+  tag: v0.0.0
 replicas: 1
 `)
-		_, err := marshalParamsOverride(&app, originalData)
-		assert.Error(t, err)
-		assert.Equal(t, "failed to set image parameter version value: key image not found in the map", err.Error())
+
+		yaml, err := marshalParamsOverride(&app, originalData)
+		require.NoError(t, err)
+		assert.NotEmpty(t, yaml)
+		assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", "  ")), strings.TrimSpace(string(yaml)))
 	})
 
 	t.Run("Missing annotation image-tag for helmvalues write-back-target", func(t *testing.T) {
@@ -1865,7 +1882,7 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		require.NoError(t, err)
 		assert.Equal(t, expected, input)
 	})
@@ -1881,23 +1898,53 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		require.NoError(t, err)
 		assert.Equal(t, expected, input)
 	})
 
 	t.Run("Key not found", func(t *testing.T) {
+		expected := yaml.MapSlice{
+			{Key: "image", Value: yaml.MapSlice{
+				{Key: "attributes", Value: yaml.MapSlice{
+					{Key: "name", Value: "repo-name"},
+					{Key: "tag", Value: "v2.0.0"},
+				}},
+			}},
+		}
+
 		input := yaml.MapSlice{
 			{Key: "image", Value: yaml.MapSlice{
-				{Key: "tag", Value: "v1.0.0"},
+				{Key: "attributes", Value: yaml.MapSlice{
+					{Key: "name", Value: "repo-name"},
+				}},
 			}},
 		}
+
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
-		assert.Error(t, err)
-		assert.Equal(t, "key attributes not found in the map", err.Error())
+		err := setHelmValue(&input, key, value)
+		require.NoError(t, err)
+		assert.Equal(t, expected, input)
+	})
+
+	t.Run("Root key not found", func(t *testing.T) {
+		expected := yaml.MapSlice{
+			{Key: "name", Value: "repo-name"},
+			{Key: "tag", Value: "v2.0.0"},
+		}
+
+		input := yaml.MapSlice{
+			{Key: "name", Value: "repo-name"},
+		}
+
+		key := "tag"
+		value := "v2.0.0"
+
+		err := setHelmValue(&input, key, value)
+		require.NoError(t, err)
+		assert.Equal(t, expected, input)
 	})
 
 	t.Run("Unexpected type for key", func(t *testing.T) {
@@ -1909,7 +1956,7 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		assert.Error(t, err)
 		assert.Equal(t, "unexpected type string for key attributes", err.Error())
 	})