Skip to content

Commit 01ef91c

Browse files
committed
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 <bewins@nerdwallet.com>
1 parent 1027f80 commit 01ef91c

File tree

3 files changed

+382
-149
lines changed

3 files changed

+382
-149
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ require (
2626
golang.org/x/oauth2 v0.25.0
2727
golang.org/x/sync v0.10.0
2828
google.golang.org/grpc v1.70.0
29-
gopkg.in/yaml.v2 v2.4.0
29+
gopkg.in/yaml.v3 v3.0.1
3030
k8s.io/api v0.31.0
3131
k8s.io/apimachinery v0.31.0
3232
k8s.io/client-go v0.31.0
@@ -154,7 +154,7 @@ require (
154154
gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
155155
gopkg.in/inf.v0 v0.9.1 // indirect
156156
gopkg.in/warnings.v0 v0.1.2 // indirect
157-
gopkg.in/yaml.v3 v3.0.1 // indirect
157+
gopkg.in/yaml.v2 v2.4.0 // indirect
158158
k8s.io/apiextensions-apiserver v0.31.2 // indirect
159159
k8s.io/apiserver v0.31.0 // indirect
160160
k8s.io/cli-runtime v0.31.0 // indirect

pkg/argocd/update.go

Lines changed: 68 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121

2222
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
2323
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
24-
"gopkg.in/yaml.v2"
24+
"gopkg.in/yaml.v3"
2525
)
2626

2727
// Stores some statistics about the results of a run
@@ -459,7 +459,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
459459
if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) {
460460
images := GetImagesAndAliasesFromApplication(app)
461461

462-
helmNewValues := yaml.MapSlice{}
462+
helmNewValues := yaml.Node{}
463463
err = yaml.Unmarshal(originalData, &helmNewValues)
464464
if err != nil {
465465
return nil, err
@@ -505,7 +505,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
505505
}
506506
}
507507

508-
override, err = yaml.Marshal(helmNewValues)
508+
override, err = yaml.Marshal(&helmNewValues)
509509
} else {
510510
var params helmOverride
511511
newParams := helmOverride{
@@ -572,72 +572,94 @@ func mergeKustomizeOverride(t *kustomizeOverride, o *kustomizeOverride) {
572572
}
573573
}
574574

575-
// Check if a key exists in a MapSlice and return its index and value
576-
func findHelmValuesKey(m yaml.MapSlice, key string) (int, bool) {
577-
for i, item := range m {
578-
if item.Key == key {
579-
return i, true
575+
// Check if a key exists in a MappingNode and return the index of its value
576+
func findHelmValuesKey(m *yaml.Node, key string) (int, bool) {
577+
for i, item := range m.Content {
578+
if i%2 == 0 && item.Value == key {
579+
return i + 1, true
580580
}
581581
}
582582
return -1, false
583583
}
584584

585+
func nodeKindString(k yaml.Kind) string {
586+
return map[yaml.Kind]string{
587+
yaml.DocumentNode: "DocumentNode",
588+
yaml.SequenceNode: "SequenceNode",
589+
yaml.MappingNode: "MappingNode",
590+
yaml.ScalarNode: "ScalarNode",
591+
yaml.AliasNode: "AliasNode",
592+
}[k]
593+
}
594+
585595
// set value of the parameter passed from the annotations.
586-
func setHelmValue(currentValues *yaml.MapSlice, key string, value interface{}) error {
596+
func setHelmValue(currentValues *yaml.Node, key string, value interface{}) error {
597+
current := currentValues
598+
599+
// an unmarshalled document has a DocumentNode at the root, but
600+
// we navigate from a MappingNode.
601+
if current.Kind == yaml.DocumentNode {
602+
current = current.Content[0]
603+
}
604+
605+
if current.Kind != yaml.MappingNode {
606+
return fmt.Errorf("unexpected type %s for root", nodeKindString(current.Kind))
607+
}
608+
587609
// Check if the full key exists
588-
if idx, found := findHelmValuesKey(*currentValues, key); found {
589-
(*currentValues)[idx].Value = value
610+
if idx, found := findHelmValuesKey(current, key); found {
611+
(*current).Content[idx].Value = value.(string)
590612
return nil
591613
}
592614

593615
var err error
594616
keys := strings.Split(key, ".")
595-
current := currentValues
596-
var parent *yaml.MapSlice
597-
parentIdx := -1
598617

599618
for i, k := range keys {
600-
if idx, found := findHelmValuesKey(*current, k); found {
619+
if idx, found := findHelmValuesKey(current, k); found {
620+
// Navigate deeper into the map
621+
current = (*current).Content[idx]
622+
// unpack one level of alias; an alias of an alias is not supported
623+
if current.Kind == yaml.AliasNode {
624+
current = current.Alias
625+
}
601626
if i == len(keys)-1 {
602627
// If we're at the final key, set the value and return
603-
(*current)[idx].Value = value
604-
return nil
605-
} else {
606-
// Navigate deeper into the map
607-
if nestedMap, ok := (*current)[idx].Value.(yaml.MapSlice); ok {
608-
parent = current
609-
parentIdx = idx
610-
current = &nestedMap
628+
if current.Kind == yaml.ScalarNode {
629+
current.Value = value.(string)
611630
} else {
612-
return fmt.Errorf("unexpected type %T for key %s", (*current)[idx].Value, k)
631+
return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k)
613632
}
633+
return nil
634+
} else if current.Kind != yaml.MappingNode {
635+
return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k)
614636
}
615637
} else {
616-
newCurrent := yaml.MapSlice{}
617-
var newParent yaml.MapSlice
618-
619638
if i == len(keys)-1 {
620-
newParent = append(*current, yaml.MapItem{Key: k, Value: value})
621-
} else {
622-
newParent = append(*current, yaml.MapItem{Key: k, Value: newCurrent})
623-
}
624-
625-
if parent == nil {
626-
*currentValues = newParent
639+
current.Content = append(current.Content,
640+
&yaml.Node{
641+
Kind: yaml.ScalarNode,
642+
Value: k,
643+
},
644+
&yaml.Node{
645+
Kind: yaml.ScalarNode,
646+
Value: value.(string),
647+
},
648+
)
649+
return nil
627650
} else {
628-
// if parentIdx has not been set (parent element is also new), set it to the last element
629-
if parentIdx == -1 {
630-
parentIdx = len(*parent) - 1
631-
if parentIdx < 0 {
632-
parentIdx = 0
633-
}
634-
}
635-
(*parent)[parentIdx].Value = newParent
651+
current.Content = append(current.Content,
652+
&yaml.Node{
653+
Kind: yaml.ScalarNode,
654+
Value: k,
655+
},
656+
&yaml.Node{
657+
Kind: yaml.MappingNode,
658+
Content: []*yaml.Node{},
659+
},
660+
)
661+
current = current.Content[len(current.Content)-1]
636662
}
637-
638-
parent = &newParent
639-
current = &newCurrent
640-
parentIdx = -1
641663
}
642664
}
643665

0 commit comments

Comments
 (0)