From 6bbd77c202f961783b3f9a04fda6bad81bad6b0d Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Sun, 2 Mar 2025 14:48:06 +0000 Subject: [PATCH 1/6] feat: switch to yaml.v3 In order to support round-tripping of files with multiline yaml and anchors, move to yaml.v3. This lower-level interface preserves comments and doesn't follow aliases by default. Support for aliases in Helm values files is fairly minimal and I need to add tests to demonstrate that this works; it only follows one level of alias to avoid getting stuck in loops. A lot of tests needed updated because it seems yaml.v3 switched to four spaces instead of two when marshalling by default. Signed-off-by: Brian Ewins --- go.mod | 4 +- pkg/argocd/update.go | 114 ++++++----- pkg/argocd/update_test.go | 413 ++++++++++++++++++++++++++++---------- 3 files changed, 382 insertions(+), 149 deletions(-) diff --git a/go.mod b/go.mod index b4162b5f..d490e940 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( golang.org/x/oauth2 v0.25.0 golang.org/x/sync v0.11.0 google.golang.org/grpc v1.70.0 - gopkg.in/yaml.v2 v2.4.0 + gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.31.0 k8s.io/apimachinery v0.31.0 k8s.io/client-go v0.31.0 @@ -154,7 +154,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect k8s.io/apiextensions-apiserver v0.31.2 // indirect k8s.io/apiserver v0.31.0 // indirect k8s.io/cli-runtime v0.31.0 // indirect diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 58548b5c..3486817b 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -21,7 +21,7 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) // Stores some statistics about the results of a run @@ -459,7 +459,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) { images := GetImagesAndAliasesFromApplication(app) - helmNewValues := yaml.MapSlice{} + helmNewValues := yaml.Node{} err = yaml.Unmarshal(originalData, &helmNewValues) if err != nil { return nil, err @@ -505,7 +505,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } } - override, err = yaml.Marshal(helmNewValues) + override, err = yaml.Marshal(&helmNewValues) } else { var params helmOverride newParams := helmOverride{ @@ -572,72 +572,94 @@ func mergeKustomizeOverride(t *kustomizeOverride, o *kustomizeOverride) { } } -// Check if a key exists in a MapSlice and return its index and value -func findHelmValuesKey(m yaml.MapSlice, key string) (int, bool) { - for i, item := range m { - if item.Key == key { - return i, true +// Check if a key exists in a MappingNode and return the index of its value +func findHelmValuesKey(m *yaml.Node, key string) (int, bool) { + for i, item := range m.Content { + if i%2 == 0 && item.Value == key { + return i + 1, true } } return -1, false } +func nodeKindString(k yaml.Kind) string { + return map[yaml.Kind]string{ + yaml.DocumentNode: "DocumentNode", + yaml.SequenceNode: "SequenceNode", + yaml.MappingNode: "MappingNode", + yaml.ScalarNode: "ScalarNode", + yaml.AliasNode: "AliasNode", + }[k] +} + // set value of the parameter passed from the annotations. -func setHelmValue(currentValues *yaml.MapSlice, key string, value interface{}) error { +func setHelmValue(currentValues *yaml.Node, key string, value interface{}) error { + current := currentValues + + // an unmarshalled document has a DocumentNode at the root, but + // we navigate from a MappingNode. + if current.Kind == yaml.DocumentNode { + current = current.Content[0] + } + + if current.Kind != yaml.MappingNode { + return fmt.Errorf("unexpected type %s for root", nodeKindString(current.Kind)) + } + // Check if the full key exists - if idx, found := findHelmValuesKey(*currentValues, key); found { - (*currentValues)[idx].Value = value + if idx, found := findHelmValuesKey(current, key); found { + (*current).Content[idx].Value = value.(string) return nil } var err error keys := strings.Split(key, ".") - current := currentValues - var parent *yaml.MapSlice - parentIdx := -1 for i, k := range keys { - if idx, found := findHelmValuesKey(*current, k); found { + if idx, found := findHelmValuesKey(current, k); found { + // Navigate deeper into the map + current = (*current).Content[idx] + // unpack one level of alias; an alias of an alias is not supported + if current.Kind == yaml.AliasNode { + current = current.Alias + } if i == len(keys)-1 { // If we're at the final key, set the value and return - (*current)[idx].Value = value - return nil - } else { - // Navigate deeper into the map - if nestedMap, ok := (*current)[idx].Value.(yaml.MapSlice); ok { - parent = current - parentIdx = idx - current = &nestedMap + if current.Kind == yaml.ScalarNode { + current.Value = value.(string) } else { - return fmt.Errorf("unexpected type %T for key %s", (*current)[idx].Value, k) + return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k) } + return nil + } else if current.Kind != yaml.MappingNode { + return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k) } } else { - newCurrent := yaml.MapSlice{} - var newParent yaml.MapSlice - if i == len(keys)-1 { - newParent = append(*current, yaml.MapItem{Key: k, Value: value}) - } else { - newParent = append(*current, yaml.MapItem{Key: k, Value: newCurrent}) - } - - if parent == nil { - *currentValues = newParent + current.Content = append(current.Content, + &yaml.Node{ + Kind: yaml.ScalarNode, + Value: k, + }, + &yaml.Node{ + Kind: yaml.ScalarNode, + Value: value.(string), + }, + ) + return nil } else { - // if parentIdx has not been set (parent element is also new), set it to the last element - if parentIdx == -1 { - parentIdx = len(*parent) - 1 - if parentIdx < 0 { - parentIdx = 0 - } - } - (*parent)[parentIdx].Value = newParent + current.Content = append(current.Content, + &yaml.Node{ + Kind: yaml.ScalarNode, + Value: k, + }, + &yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{}, + }, + ) + current = current.Content[len(current.Content)-1] } - - parent = &newParent - current = &newCurrent - parentIdx = -1 } } diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 9dbeadae..6cbdddf1 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" "github.com/argoproj-labs/argocd-image-updater/ext/git" gitmock "github.com/argoproj-labs/argocd-image-updater/ext/git/mocks" @@ -1179,10 +1179,10 @@ func Test_MarshalParamsOverride(t *testing.T) { t.Run("Valid Kustomize source", func(t *testing.T) { expected := ` kustomize: - images: - - baz - - foo - - bar + images: + - baz + - foo + - bar ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1210,8 +1210,8 @@ kustomize: } originalData := []byte(` kustomize: - images: - - baz + images: + - baz `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1222,10 +1222,10 @@ kustomize: t.Run("Merge images param", func(t *testing.T) { expected := ` kustomize: - images: - - existing:latest - - updated:latest - - new + images: + - existing:latest + - updated:latest + - new ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1252,9 +1252,9 @@ kustomize: } originalData := []byte(` kustomize: - images: - - existing:latest - - updated:old + images: + - existing:latest + - updated:old `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1291,16 +1291,16 @@ kustomize: t.Run("Valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: baz - value: baz - forcestring: false - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: baz + value: baz + forcestring: false + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1351,13 +1351,13 @@ helm: t.Run("Empty originalData error with valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1402,13 +1402,13 @@ helm: t.Run("Invalid unmarshal originalData error with valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1588,8 +1588,8 @@ replicas: 1 expected = ` test-value1: one image: - spec: - foo: nginx:v1.0.0 + spec: + foo: nginx:v1.0.0 ` yaml, err = marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1690,13 +1690,13 @@ replicas: 1 expected = ` test-value1: one nginx: - image: - tag: v1.0.0 - name: nginx + image: + tag: v1.0.0 + name: nginx redis: - image: - tag: v1.0.0 - name: redis + image: + tag: v1.0.0 + name: redis ` yaml, err = marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1811,8 +1811,8 @@ replicas: 1 expected := ` test-value1: one image: - name: nginx - tag: v1.0.0 + name: nginx + tag: v1.0.0 replicas: 1 ` @@ -1860,7 +1860,7 @@ replicas: 1 originalData := []byte(` test-value1: one image: - name: nginx + name: nginx replicas: 1 `) @@ -1873,8 +1873,8 @@ replicas: 1 t.Run("Failed to setValue image parameter version", func(t *testing.T) { expected := ` image: - tag: v1.0.0 - name: nginx + tag: v1.0.0 + name: nginx replicas: 1 ` app := v1alpha1.Application{ @@ -2199,23 +2199,86 @@ replicas: 1 func Test_SetHelmValue(t *testing.T) { t.Run("Update existing Key", 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: "attributes", Value: yaml.MapSlice{ - {Key: "name", Value: "repo-name"}, - {Key: "tag", Value: "v1.0.0"}, - }}, - }}, + expected := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + { + Kind: yaml.ScalarNode, + Value: "tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v2.0.0", + }, + }, + }, + }, + }, + }, + } + + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + { + Kind: yaml.ScalarNode, + Value: "tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v1.0.0", + }, + }, + }, + }, + }, + }, } + key := "image.attributes.tag" value := "v2.0.0" @@ -2225,13 +2288,34 @@ func Test_SetHelmValue(t *testing.T) { }) t.Run("Update Key with dots", func(t *testing.T) { - expected := yaml.MapSlice{ - {Key: "image.attributes.tag", Value: "v2.0.0"}, + expected := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image.attributes.tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v2.0.0", + }, + }, } - input := yaml.MapSlice{ - {Key: "image.attributes.tag", Value: "v1.0.0"}, + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image.attributes.tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v1.0.0", + }, + }, } + key := "image.attributes.tag" value := "v2.0.0" @@ -2241,21 +2325,76 @@ func Test_SetHelmValue(t *testing.T) { }) 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"}, - }}, - }}, + expected := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + { + Kind: yaml.ScalarNode, + Value: "tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v2.0.0", + }, + }, + }, + }, + }, + }, } - input := yaml.MapSlice{ - {Key: "image", Value: yaml.MapSlice{ - {Key: "attributes", Value: yaml.MapSlice{ - {Key: "name", Value: "repo-name"}, - }}, - }}, + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + }, + }, + }, + }, + }, } key := "image.attributes.tag" @@ -2267,13 +2406,40 @@ func Test_SetHelmValue(t *testing.T) { }) t.Run("Root key not found", func(t *testing.T) { - expected := yaml.MapSlice{ - {Key: "name", Value: "repo-name"}, - {Key: "tag", Value: "v2.0.0"}, + expected := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + { + Kind: yaml.ScalarNode, + Value: "tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v2.0.0", + }, + }, } - input := yaml.MapSlice{ - {Key: "name", Value: "repo-name"}, + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "name", + }, + { + Kind: yaml.ScalarNode, + Value: "repo-name", + }, + }, } key := "tag" @@ -2285,15 +2451,42 @@ func Test_SetHelmValue(t *testing.T) { }) t.Run("Empty values with deep key", func(t *testing.T) { - expected := yaml.MapSlice{ - {Key: "image", Value: yaml.MapSlice{ - {Key: "attributes", Value: yaml.MapSlice{ - {Key: "tag", Value: "v2.0.0"}, - }}, - }}, + expected := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "tag", + }, + { + Kind: yaml.ScalarNode, + Value: "v2.0.0", + }, + }, + }, + }, + }, + }, } - input := yaml.MapSlice{} + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{}, + } key := "image.attributes.tag" value := "v2.0.0" @@ -2304,17 +2497,35 @@ func Test_SetHelmValue(t *testing.T) { }) t.Run("Unexpected type for key", func(t *testing.T) { - input := yaml.MapSlice{ - {Key: "image", Value: yaml.MapSlice{ - {Key: "attributes", Value: "v1.0.0"}, - }}, + input := yaml.Node{ + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "image", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "attributes", + }, + { + Kind: yaml.ScalarNode, + Value: "v1.0.0", + }, + }, + }, + }, } + key := "image.attributes.tag" value := "v2.0.0" err := setHelmValue(&input, key, value) assert.Error(t, err) - assert.Equal(t, "unexpected type string for key attributes", err.Error()) + assert.Equal(t, "unexpected type ScalarNode for key attributes", err.Error()) }) } From c1f323c60a9a14d7874c351604196b75749427dd Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Mon, 3 Mar 2025 12:42:35 +0000 Subject: [PATCH 2/6] fix: use yaml test fixtures not structs This change makes for less verbose code, also what we care about is the transformation of the yaml, not the internal representation. Signed-off-by: Brian Ewins --- pkg/argocd/update_test.go | 360 +++++++++----------------------------- 1 file changed, 84 insertions(+), 276 deletions(-) diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 6cbdddf1..8cef59dd 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -2199,331 +2199,139 @@ replicas: 1 func Test_SetHelmValue(t *testing.T) { t.Run("Update existing Key", func(t *testing.T) { - expected := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - { - Kind: yaml.ScalarNode, - Value: "tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v2.0.0", - }, - }, - }, - }, - }, - }, - } + expected := ` +image: + attributes: + name: repo-name + tag: v2.0.0 +` - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - { - Kind: yaml.ScalarNode, - Value: "tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v1.0.0", - }, - }, - }, - }, - }, - }, - } + inputData := []byte(` +image: + attributes: + name: repo-name + tag: v1.0.0 +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) key := "image.attributes.tag" value := "v2.0.0" - err := setHelmValue(&input, key, value) + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := yaml.Marshal(&input) require.NoError(t, err) - assert.Equal(t, expected, input) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) t.Run("Update Key with dots", func(t *testing.T) { - expected := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image.attributes.tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v2.0.0", - }, - }, - } + expected := `image.attributes.tag: v2.0.0` - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image.attributes.tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v1.0.0", - }, - }, - } + inputData := []byte(`image.attributes.tag: v1.0.0`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) key := "image.attributes.tag" value := "v2.0.0" - err := setHelmValue(&input, key, value) + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := yaml.Marshal(&input) require.NoError(t, err) - assert.Equal(t, expected, input) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) t.Run("Key not found", func(t *testing.T) { - expected := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - { - Kind: yaml.ScalarNode, - Value: "tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v2.0.0", - }, - }, - }, - }, - }, - }, - } + expected := ` +image: + attributes: + name: repo-name + tag: v2.0.0 +` - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - }, - }, - }, - }, - }, - } + inputData := []byte(` +image: + attributes: + name: repo-name +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) 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) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) t.Run("Root key not found", func(t *testing.T) { - expected := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - { - Kind: yaml.ScalarNode, - Value: "tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v2.0.0", - }, - }, - } + expected := ` +name: repo-name +tag: v2.0.0 +` - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "name", - }, - { - Kind: yaml.ScalarNode, - Value: "repo-name", - }, - }, - } + inputData := []byte(` +name: repo-name +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) key := "tag" value := "v2.0.0" - err := setHelmValue(&input, key, value) + err = setHelmValue(&input, key, value) require.NoError(t, err) - assert.Equal(t, expected, input) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) t.Run("Empty values with deep key", func(t *testing.T) { - expected := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "tag", - }, - { - Kind: yaml.ScalarNode, - Value: "v2.0.0", - }, - }, - }, - }, - }, - }, - } + // this uses inline syntax because the input data + // needed is an empty map, which can only be expressed as {}. + expected := `{image: {attributes: {tag: v2.0.0}}}` - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{}, - } + inputData := []byte(`{}`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) 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) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) t.Run("Unexpected type for key", func(t *testing.T) { - input := yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "image", - }, - { - Kind: yaml.MappingNode, - Content: []*yaml.Node{ - { - Kind: yaml.ScalarNode, - Value: "attributes", - }, - { - Kind: yaml.ScalarNode, - Value: "v1.0.0", - }, - }, - }, - }, - } + inputData := []byte(` +image: + attributes: v1.0.0 +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) 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 ScalarNode for key attributes", err.Error()) }) From 81c0e65f91dbb13f0bcc62856c891876a41aa4b1 Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Mon, 3 Mar 2025 13:32:33 +0000 Subject: [PATCH 3/6] fix: add tests for comments, multiline, alias features When round-tripping helm yaml, we want comments to be preserved. Multiline strings should retain their formatting (this is to allow for, for example, scripts in k8s commands). Aliases should be preserved, and followed, both for mappings and scalars. Signed-off-by: Brian Ewins --- pkg/argocd/update_test.go | 106 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 8cef59dd..fb20e3d4 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -2335,6 +2335,112 @@ image: assert.Error(t, err) assert.Equal(t, "unexpected type ScalarNode for key attributes", err.Error()) }) + + t.Run("Aliases, comments, and multiline strings are preserved", func(t *testing.T) { + expected := ` +image: + attributes: + name: &repo repo-name + tag: v2.0.0 + # this is a comment + multiline: | + one + two + three + alias: *repo +` + + inputData := []byte(` +image: + attributes: + name: &repo repo-name + tag: v1.0.0 + # this is a comment + multiline: | + one + two + three + alias: *repo +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) + + key := "image.attributes.tag" + value := "v2.0.0" + + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + }) + + t.Run("Aliases to mappings are followed", func(t *testing.T) { + expected := ` +global: + attributes: &attrs + name: &repo repo-name + tag: v2.0.0 +image: + attributes: *attrs +` + + inputData := []byte(` +global: + attributes: &attrs + name: &repo repo-name + tag: v1.0.0 +image: + attributes: *attrs +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) + + key := "image.attributes.tag" + value := "v2.0.0" + + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + }) + + t.Run("Aliases to scalars are followed", func(t *testing.T) { + expected := ` +image: + attributes: + name: repo-name + version: &ver v2.0.0 + tag: *ver +` + + inputData := []byte(` +image: + attributes: + name: repo-name + version: &ver v1.0.0 + tag: *ver +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) + + key := "image.attributes.tag" + value := "v2.0.0" + + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := yaml.Marshal(&input) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + }) + } func Test_GetWriteBackConfig(t *testing.T) { From 91b359f2481bcddac6220204e72e3addf7dcb391 Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Mon, 3 Mar 2025 15:32:19 +0000 Subject: [PATCH 4/6] fix: always set str tag on string scalars When yaml.v3 parses yaml, string scalars are tagged "!!str". It is possible that we could parse a values file which sets the version to eg 1.23 (a float), so that when we later update the value to "v2.4" it can't be serialized. To protect against this, set the tag whenever we create or update scalars that are explicitly intended to be strings. Signed-off-by: Brian Ewins --- pkg/argocd/update.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 3486817b..a5330f6e 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -627,6 +627,7 @@ func setHelmValue(currentValues *yaml.Node, key string, value interface{}) error // If we're at the final key, set the value and return if current.Kind == yaml.ScalarNode { current.Value = value.(string) + current.Tag = "!!str" } else { return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k) } @@ -640,10 +641,12 @@ func setHelmValue(currentValues *yaml.Node, key string, value interface{}) error &yaml.Node{ Kind: yaml.ScalarNode, Value: k, + Tag: "!!str", }, &yaml.Node{ Kind: yaml.ScalarNode, Value: value.(string), + Tag: "!!str", }, ) return nil @@ -652,6 +655,7 @@ func setHelmValue(currentValues *yaml.Node, key string, value interface{}) error &yaml.Node{ Kind: yaml.ScalarNode, Value: k, + Tag: "!!str", }, &yaml.Node{ Kind: yaml.MappingNode, From b49a6c375c856dc305d2d0c2bddd3627f7af16d8 Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Thu, 6 Mar 2025 09:52:21 +0000 Subject: [PATCH 5/6] fix: prefer 2 space indent yaml.v3 changed the default indent to 4 spaces, see: https://github.com/go-yaml/yaml/commit/3c39489ee0e38df5db7b55f4d2a5bb0ffa2af2da in order to avoid unnecessary whitespace changes in code previously formatted with argo_image_updater, use a 2-space indent instead. Signed-off-by: Brian Ewins --- pkg/argocd/update.go | 29 ++++-- pkg/argocd/update_test.go | 212 +++++++++++++++++++------------------- 2 files changed, 127 insertions(+), 114 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index a5330f6e..86f3f16c 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -1,6 +1,7 @@ package argocd import ( + "bytes" "context" "fmt" "path/filepath" @@ -418,6 +419,20 @@ func setAppImage(app *v1alpha1.Application, img *image.ContainerImage) error { return err } +func marshalWithIndent(in interface{}, indent int) (out []byte, err error) { + var b bytes.Buffer + encoder := yaml.NewEncoder(&b) + defer encoder.Close() + encoder.SetIndent(indent) + if err = encoder.Encode(in); err != nil { + return nil, err + } + if err = encoder.Close(); err != nil { + return nil, err + } + return b.Bytes(), nil +} + // marshalParamsOverride marshals the parameter overrides of a given application // into YAML bytes func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { @@ -441,16 +456,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } if len(originalData) == 0 { - override, err = yaml.Marshal(newParams) + override, err = marshalWithIndent(newParams, 2) break } err = yaml.Unmarshal(originalData, ¶ms) if err != nil { - override, err = yaml.Marshal(newParams) + override, err = marshalWithIndent(newParams, 2) break } mergeKustomizeOverride(¶ms, &newParams) - override, err = yaml.Marshal(params) + override, err = marshalWithIndent(params, 2) case ApplicationTypeHelm: if appSource.Helm == nil { return []byte{}, nil @@ -505,7 +520,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } } - override, err = yaml.Marshal(&helmNewValues) + override, err = marshalWithIndent(&helmNewValues, 2) } else { var params helmOverride newParams := helmOverride{ @@ -518,16 +533,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by log.WithContext().AddField("application", app).Debugf("values: '%s'", outputParams) if len(originalData) == 0 { - override, err = yaml.Marshal(newParams) + override, err = marshalWithIndent(newParams, 2) break } err = yaml.Unmarshal(originalData, ¶ms) if err != nil { - override, err = yaml.Marshal(newParams) + override, err = marshalWithIndent(newParams, 2) break } mergeHelmOverride(¶ms, &newParams) - override, err = yaml.Marshal(params) + override, err = marshalWithIndent(params, 2) } default: err = fmt.Errorf("unsupported application type") diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index fb20e3d4..4c921ab3 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -1179,10 +1179,10 @@ func Test_MarshalParamsOverride(t *testing.T) { t.Run("Valid Kustomize source", func(t *testing.T) { expected := ` kustomize: - images: - - baz - - foo - - bar + images: + - baz + - foo + - bar ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1210,8 +1210,8 @@ kustomize: } originalData := []byte(` kustomize: - images: - - baz + images: + - baz `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1222,10 +1222,10 @@ kustomize: t.Run("Merge images param", func(t *testing.T) { expected := ` kustomize: - images: - - existing:latest - - updated:latest - - new + images: + - existing:latest + - updated:latest + - new ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1252,9 +1252,9 @@ kustomize: } originalData := []byte(` kustomize: - images: - - existing:latest - - updated:old + images: + - existing:latest + - updated:old `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1291,16 +1291,16 @@ kustomize: t.Run("Valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: baz - value: baz - forcestring: false - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: baz + value: baz + forcestring: false + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1351,13 +1351,13 @@ helm: t.Run("Empty originalData error with valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1402,13 +1402,13 @@ helm: t.Run("Invalid unmarshal originalData error with valid Helm source", func(t *testing.T) { expected := ` helm: - parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + parameters: + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1588,8 +1588,8 @@ replicas: 1 expected = ` test-value1: one image: - spec: - foo: nginx:v1.0.0 + spec: + foo: nginx:v1.0.0 ` yaml, err = marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1690,13 +1690,13 @@ replicas: 1 expected = ` test-value1: one nginx: - image: - tag: v1.0.0 - name: nginx + image: + tag: v1.0.0 + name: nginx redis: - image: - tag: v1.0.0 - name: redis + image: + tag: v1.0.0 + name: redis ` yaml, err = marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1811,8 +1811,8 @@ replicas: 1 expected := ` test-value1: one image: - name: nginx - tag: v1.0.0 + name: nginx + tag: v1.0.0 replicas: 1 ` @@ -1860,7 +1860,7 @@ replicas: 1 originalData := []byte(` test-value1: one image: - name: nginx + name: nginx replicas: 1 `) @@ -1873,8 +1873,8 @@ replicas: 1 t.Run("Failed to setValue image parameter version", func(t *testing.T) { expected := ` image: - tag: v1.0.0 - name: nginx + tag: v1.0.0 + name: nginx replicas: 1 ` app := v1alpha1.Application{ @@ -2201,16 +2201,16 @@ func Test_SetHelmValue(t *testing.T) { t.Run("Update existing Key", func(t *testing.T) { expected := ` image: - attributes: - name: repo-name - tag: v2.0.0 + attributes: + name: repo-name + tag: v2.0.0 ` inputData := []byte(` image: - attributes: - name: repo-name - tag: v1.0.0 + attributes: + name: repo-name + tag: v1.0.0 `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2222,7 +2222,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2241,7 +2241,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2249,15 +2249,15 @@ image: t.Run("Key not found", func(t *testing.T) { expected := ` image: - attributes: - name: repo-name - tag: v2.0.0 + attributes: + name: repo-name + tag: v2.0.0 ` inputData := []byte(` image: - attributes: - name: repo-name + attributes: + name: repo-name `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2269,7 +2269,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2280,9 +2280,7 @@ name: repo-name tag: v2.0.0 ` - inputData := []byte(` -name: repo-name -`) + inputData := []byte(`name: repo-name`) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) require.NoError(t, err) @@ -2293,7 +2291,7 @@ name: repo-name err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2314,7 +2312,7 @@ name: repo-name err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2322,7 +2320,7 @@ name: repo-name t.Run("Unexpected type for key", func(t *testing.T) { inputData := []byte(` image: - attributes: v1.0.0 + attributes: v1.0.0 `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2339,28 +2337,28 @@ image: t.Run("Aliases, comments, and multiline strings are preserved", func(t *testing.T) { expected := ` image: - attributes: - name: &repo repo-name - tag: v2.0.0 - # this is a comment - multiline: | - one - two - three - alias: *repo + attributes: + name: &repo repo-name + tag: v2.0.0 + # this is a comment + multiline: | + one + two + three + alias: *repo ` inputData := []byte(` image: - attributes: - name: &repo repo-name - tag: v1.0.0 - # this is a comment - multiline: | - one - two - three - alias: *repo + attributes: + name: &repo repo-name + tag: v1.0.0 + # this is a comment + multiline: | + one + two + three + alias: *repo `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2372,7 +2370,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2380,20 +2378,20 @@ image: t.Run("Aliases to mappings are followed", func(t *testing.T) { expected := ` global: - attributes: &attrs - name: &repo repo-name - tag: v2.0.0 + attributes: &attrs + name: &repo repo-name + tag: v2.0.0 image: - attributes: *attrs + attributes: *attrs ` inputData := []byte(` global: - attributes: &attrs - name: &repo repo-name - tag: v1.0.0 + attributes: &attrs + name: &repo repo-name + tag: v1.0.0 image: - attributes: *attrs + attributes: *attrs `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2405,7 +2403,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2413,18 +2411,18 @@ image: t.Run("Aliases to scalars are followed", func(t *testing.T) { expected := ` image: - attributes: - name: repo-name - version: &ver v2.0.0 - tag: *ver + attributes: + name: repo-name + version: &ver v2.0.0 + tag: *ver ` inputData := []byte(` image: - attributes: - name: repo-name - version: &ver v1.0.0 - tag: *ver + attributes: + name: repo-name + version: &ver v1.0.0 + tag: *ver `) input := yaml.Node{} err := yaml.Unmarshal(inputData, &input) @@ -2436,7 +2434,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := yaml.Marshal(&input) + output, err := marshalWithIndent(&input, 2) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) From b43859dde5d14a11bd8a7703f567d2fd264b3444 Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Thu, 6 Mar 2025 11:22:58 +0000 Subject: [PATCH 6/6] fix: preserve indent if possible It's possible to preserve the indent in the original yaml when we Unmarshal to a yaml.Node, and the structure of the original yaml allows us to guess the indent. The two easiest cases are when we have a nested block-style mapping, or a sequence inside a mapping and the sequence is indented. For those, the node tells us the parsed column (starting at 1) and we use that indent. However: if we parse to some other data structure (as we do with Kustomize) or the original yaml has some structure where the indentation is unclear, eg: root: - unguessable indent - deeper: guessable: but requires deeper traversal ... then we just guess '2'. This should generate _fewer_ useless whitespace changes, but cannot prevent them all. Signed-off-by: Brian Ewins --- pkg/argocd/update.go | 28 +++++++++++++++++- pkg/argocd/update_test.go | 62 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 86f3f16c..562fa5a6 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -423,6 +423,7 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) { var b bytes.Buffer encoder := yaml.NewEncoder(&b) defer encoder.Close() + // note: yaml.v3 will only respect indents from 1 to 9 inclusive. encoder.SetIndent(indent) if err = encoder.Encode(in); err != nil { return nil, err @@ -433,6 +434,30 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) { return b.Bytes(), nil } +func guessIndent(root *yaml.Node) int { + node := root + if root.Kind == yaml.DocumentNode { + if len(node.Content) == 0 { + return 2 + } + node = root.Content[0] + } + // anything other than a map at the root makes guessing difficult + if node.Kind != yaml.MappingNode || len(node.Content) == 0 { + return 2 + } + // first level map entries that are themselves mappings or sequences, + // in block style, and are indented, allow guessing the preferred indent. + for i, child := range node.Content { + if i%2 == 1 && child.Column > 1 && child.Column < 10 && child.Style != yaml.FlowStyle { + if child.Kind == yaml.MappingNode || child.Kind == yaml.SequenceNode { + return child.Column - 1 + } + } + } + return 2 +} + // marshalParamsOverride marshals the parameter overrides of a given application // into YAML bytes func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { @@ -479,6 +504,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by if err != nil { return nil, err } + indent := guessIndent(&helmNewValues) for _, c := range images { if c.ImageAlias == "" { @@ -520,7 +546,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } } - override, err = marshalWithIndent(&helmNewValues, 2) + override, err = marshalWithIndent(&helmNewValues, indent) } else { var params helmOverride newParams := helmOverride{ diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 4c921ab3..3dca1869 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -2439,6 +2439,68 @@ image: assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) + t.Run("Indentation is guessed from nested mappings", func(t *testing.T) { + expected := ` +unguessable: + - unguessable +image: + attributes: + tag: v2.0.0 +` + + inputData := []byte(` +unguessable: +- unguessable +image: + attributes: + tag: v1.0.0 +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) + indent := guessIndent(&input) + + key := "image.attributes.tag" + value := "v2.0.0" + + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := marshalWithIndent(&input, indent) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + }) + + t.Run("Indentation is guessed from indented lists", func(t *testing.T) { + expected := ` +unguessable: [unguessable] +guessable: + - guessable +image: + attributes: + tag: v2.0.0 +` + + inputData := []byte(` +unguessable: [unguessable] +guessable: + - guessable +`) + input := yaml.Node{} + err := yaml.Unmarshal(inputData, &input) + require.NoError(t, err) + indent := guessIndent(&input) + + key := "image.attributes.tag" + value := "v2.0.0" + + err = setHelmValue(&input, key, value) + require.NoError(t, err) + + output, err := marshalWithIndent(&input, indent) + require.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + }) } func Test_GetWriteBackConfig(t *testing.T) {