Skip to content

Commit 988467b

Browse files
committed
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: kubernetes-sigs/kustomize#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 <bewins@nerdwallet.com>
1 parent 3448312 commit 988467b

File tree

3 files changed

+55
-140
lines changed

3 files changed

+55
-140
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ require (
2424
golang.org/x/oauth2 v0.27.0
2525
golang.org/x/sync v0.11.0
2626
google.golang.org/grpc v1.70.0
27-
gopkg.in/yaml.v3 v3.0.1
2827
k8s.io/api v0.31.0
2928
k8s.io/apimachinery v0.31.0
3029
k8s.io/client-go v0.31.0
3130
sigs.k8s.io/kustomize/api v0.17.2
3231
sigs.k8s.io/kustomize/kyaml v0.17.1
32+
sigs.k8s.io/yaml v1.4.0
3333
)
3434

3535
require (
@@ -153,6 +153,7 @@ require (
153153
gopkg.in/inf.v0 v0.9.1 // indirect
154154
gopkg.in/warnings.v0 v0.1.2 // indirect
155155
gopkg.in/yaml.v2 v2.4.0 // indirect
156+
gopkg.in/yaml.v3 v3.0.1 // indirect
156157
k8s.io/apiextensions-apiserver v0.31.2 // indirect
157158
k8s.io/apiserver v0.31.0 // indirect
158159
k8s.io/cli-runtime v0.31.0 // indirect
@@ -167,7 +168,6 @@ require (
167168
oras.land/oras-go/v2 v2.5.0 // indirect
168169
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
169170
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect
170-
sigs.k8s.io/yaml v1.4.0 // indirect
171171
)
172172

173173
replace (

pkg/argocd/update.go

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222

2323
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
2424
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
25-
"gopkg.in/yaml.v3"
25+
yaml "sigs.k8s.io/yaml/goyaml.v3"
2626
)
2727

2828
// Stores some statistics about the results of a run
@@ -61,6 +61,8 @@ const (
6161
WriteBackGit WriteBackMethod = 1
6262
)
6363

64+
const defaultIndent = 2
65+
6466
// WriteBackConfig holds information on how to write back the changes to an Application
6567
type WriteBackConfig struct {
6668
Method WriteBackMethod
@@ -425,6 +427,7 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) {
425427
defer encoder.Close()
426428
// note: yaml.v3 will only respect indents from 1 to 9 inclusive.
427429
encoder.SetIndent(indent)
430+
encoder.CompactSeqIndent()
428431
if err = encoder.Encode(in); err != nil {
429432
return nil, err
430433
}
@@ -434,30 +437,6 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) {
434437
return b.Bytes(), nil
435438
}
436439

437-
func guessIndent(root *yaml.Node) int {
438-
node := root
439-
if root.Kind == yaml.DocumentNode {
440-
if len(node.Content) == 0 {
441-
return 2
442-
}
443-
node = root.Content[0]
444-
}
445-
// anything other than a map at the root makes guessing difficult
446-
if node.Kind != yaml.MappingNode || len(node.Content) == 0 {
447-
return 2
448-
}
449-
// first level map entries that are themselves mappings or sequences,
450-
// in block style, and are indented, allow guessing the preferred indent.
451-
for i, child := range node.Content {
452-
if i%2 == 1 && child.Column > 1 && child.Column < 10 && child.Style != yaml.FlowStyle {
453-
if child.Kind == yaml.MappingNode || child.Kind == yaml.SequenceNode {
454-
return child.Column - 1
455-
}
456-
}
457-
}
458-
return 2
459-
}
460-
461440
// marshalParamsOverride marshals the parameter overrides of a given application
462441
// into YAML bytes
463442
func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) {
@@ -481,16 +460,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
481460
}
482461

483462
if len(originalData) == 0 {
484-
override, err = marshalWithIndent(newParams, 2)
463+
override, err = marshalWithIndent(newParams, defaultIndent)
485464
break
486465
}
487466
err = yaml.Unmarshal(originalData, &params)
488467
if err != nil {
489-
override, err = marshalWithIndent(newParams, 2)
468+
override, err = marshalWithIndent(newParams, defaultIndent)
490469
break
491470
}
492471
mergeKustomizeOverride(&params, &newParams)
493-
override, err = marshalWithIndent(params, 2)
472+
override, err = marshalWithIndent(params, defaultIndent)
494473
case ApplicationTypeHelm:
495474
if appSource.Helm == nil {
496475
return []byte{}, nil
@@ -504,7 +483,6 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
504483
if err != nil {
505484
return nil, err
506485
}
507-
indent := guessIndent(&helmNewValues)
508486

509487
for _, c := range images {
510488
if c.ImageAlias == "" {
@@ -546,7 +524,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
546524
}
547525
}
548526

549-
override, err = marshalWithIndent(&helmNewValues, indent)
527+
override, err = marshalWithIndent(&helmNewValues, defaultIndent)
550528
} else {
551529
var params helmOverride
552530
newParams := helmOverride{
@@ -559,16 +537,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
559537
log.WithContext().AddField("application", app).Debugf("values: '%s'", outputParams)
560538

561539
if len(originalData) == 0 {
562-
override, err = marshalWithIndent(newParams, 2)
540+
override, err = marshalWithIndent(newParams, defaultIndent)
563541
break
564542
}
565543
err = yaml.Unmarshal(originalData, &params)
566544
if err != nil {
567-
override, err = marshalWithIndent(newParams, 2)
545+
override, err = marshalWithIndent(newParams, defaultIndent)
568546
break
569547
}
570548
mergeHelmOverride(&params, &newParams)
571-
override, err = marshalWithIndent(params, 2)
549+
override, err = marshalWithIndent(params, defaultIndent)
572550
}
573551
default:
574552
err = fmt.Errorf("unsupported application type")

0 commit comments

Comments
 (0)