From 402b0bd8a5264a3f2ca805f3afd9017cc4978f54 Mon Sep 17 00:00:00 2001 From: Brian Ewins Date: Wed, 12 Mar 2025 16:41:59 +0000 Subject: [PATCH] fix: use compact indent for sequences We had some grumbles from developers after the goyaml.v3 upgrade that this changed the sequence indent used by kustomize, kubectl etc. There was a whole thread about this when the goyaml.v3 upgrade happened in kustomize itself: https://github.com/kubernetes-sigs/kustomize/issues/3946 and the result was that kubesigs forked goyaml. In order to more closely match what the kubernetes tools do, this commit switches to the forked version, which has a compact option on the encoder: https://github.com/kubernetes-sigs/yaml/blob/56d672052dcff7362af5b66f6424976539cddd78/goyaml.v3/patch.go#L25-L28 When I first tried this I also changed the code to guess if compact was the preferred style from the yaml. However, it turns out that the yaml formatter misbehaves when told to use "compact" and an indent other than 2; if set to 3 and compact it produces code like this: mapping: - list item Instead of: mapping: - list item This meant I couldn't get the "guess indentation" code to work, so I've removed that and its test to avoid raising a fault expectation. Signed-off-by: Brian Ewins --- go.mod | 4 +- pkg/argocd/update.go | 44 +++--------- pkg/argocd/update_test.go | 147 +++++++++++--------------------------- 3 files changed, 55 insertions(+), 140 deletions(-) diff --git a/go.mod b/go.mod index fa80cf2a..d1bc223b 100644 --- a/go.mod +++ b/go.mod @@ -24,12 +24,12 @@ require ( golang.org/x/oauth2 v0.27.0 golang.org/x/sync v0.11.0 google.golang.org/grpc v1.70.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 sigs.k8s.io/kustomize/api v0.17.2 sigs.k8s.io/kustomize/kyaml v0.17.1 + sigs.k8s.io/yaml v1.4.0 ) require ( @@ -153,6 +153,7 @@ require ( gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/apiextensions-apiserver v0.31.2 // indirect k8s.io/apiserver v0.31.0 // indirect k8s.io/cli-runtime v0.31.0 // indirect @@ -167,7 +168,6 @@ require ( oras.land/oras-go/v2 v2.5.0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect - sigs.k8s.io/yaml v1.4.0 // indirect ) replace ( diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 562fa5a6..64ec08bc 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -22,7 +22,7 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "gopkg.in/yaml.v3" + yaml "sigs.k8s.io/yaml/goyaml.v3" ) // Stores some statistics about the results of a run @@ -61,6 +61,8 @@ const ( WriteBackGit WriteBackMethod = 1 ) +const defaultIndent = 2 + // WriteBackConfig holds information on how to write back the changes to an Application type WriteBackConfig struct { Method WriteBackMethod @@ -425,6 +427,7 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) { defer encoder.Close() // note: yaml.v3 will only respect indents from 1 to 9 inclusive. encoder.SetIndent(indent) + encoder.CompactSeqIndent() if err = encoder.Encode(in); err != nil { return nil, err } @@ -434,30 +437,6 @@ 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) { @@ -481,16 +460,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } if len(originalData) == 0 { - override, err = marshalWithIndent(newParams, 2) + override, err = marshalWithIndent(newParams, defaultIndent) break } err = yaml.Unmarshal(originalData, ¶ms) if err != nil { - override, err = marshalWithIndent(newParams, 2) + override, err = marshalWithIndent(newParams, defaultIndent) break } mergeKustomizeOverride(¶ms, &newParams) - override, err = marshalWithIndent(params, 2) + override, err = marshalWithIndent(params, defaultIndent) case ApplicationTypeHelm: if appSource.Helm == nil { return []byte{}, nil @@ -504,7 +483,6 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by if err != nil { return nil, err } - indent := guessIndent(&helmNewValues) for _, c := range images { if c.ImageAlias == "" { @@ -546,7 +524,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by } } - override, err = marshalWithIndent(&helmNewValues, indent) + override, err = marshalWithIndent(&helmNewValues, defaultIndent) } else { var params helmOverride newParams := helmOverride{ @@ -559,16 +537,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by log.WithContext().AddField("application", app).Debugf("values: '%s'", outputParams) if len(originalData) == 0 { - override, err = marshalWithIndent(newParams, 2) + override, err = marshalWithIndent(newParams, defaultIndent) break } err = yaml.Unmarshal(originalData, ¶ms) if err != nil { - override, err = marshalWithIndent(newParams, 2) + override, err = marshalWithIndent(newParams, defaultIndent) break } mergeHelmOverride(¶ms, &newParams) - override, err = marshalWithIndent(params, 2) + override, err = marshalWithIndent(params, defaultIndent) } default: err = fmt.Errorf("unsupported application type") diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 3dca1869..66bb6477 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "gopkg.in/yaml.v3" + yaml "sigs.k8s.io/yaml/goyaml.v3" "github.com/argoproj-labs/argocd-image-updater/ext/git" gitmock "github.com/argoproj-labs/argocd-image-updater/ext/git/mocks" @@ -1180,9 +1180,9 @@ func Test_MarshalParamsOverride(t *testing.T) { expected := ` kustomize: images: - - baz - - foo - - bar + - baz + - foo + - bar ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1211,7 +1211,7 @@ kustomize: originalData := []byte(` kustomize: images: - - baz + - baz `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1223,9 +1223,9 @@ kustomize: expected := ` kustomize: images: - - existing:latest - - updated:latest - - new + - existing:latest + - updated:latest + - new ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1253,8 +1253,8 @@ kustomize: originalData := []byte(` kustomize: images: - - existing:latest - - updated:old + - existing:latest + - updated:old `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1292,15 +1292,15 @@ kustomize: expected := ` helm: parameters: - - name: baz - value: baz - forcestring: false - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + - name: baz + value: baz + forcestring: false + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1338,9 +1338,9 @@ helm: originalData := []byte(` helm: parameters: - - name: baz - value: baz - forcestring: false + - name: baz + value: baz + forcestring: false `) yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) @@ -1352,12 +1352,12 @@ helm: expected := ` helm: parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -1403,12 +1403,12 @@ helm: expected := ` helm: parameters: - - name: foo - value: bar - forcestring: true - - name: bar - value: foo - forcestring: true + - name: foo + value: bar + forcestring: true + - name: bar + value: foo + forcestring: true ` app := v1alpha1.Application{ ObjectMeta: v1.ObjectMeta{ @@ -2222,7 +2222,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) 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 := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2269,7 +2269,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2291,7 +2291,7 @@ tag: v2.0.0 err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2312,7 +2312,7 @@ tag: v2.0.0 err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2370,7 +2370,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2403,7 +2403,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) }) @@ -2434,70 +2434,7 @@ image: err = setHelmValue(&input, key, value) require.NoError(t, err) - output, err := marshalWithIndent(&input, 2) - require.NoError(t, err) - 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) + output, err := marshalWithIndent(&input, defaultIndent) require.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) })