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>

Author: bewinsnw

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

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
 		}
 	}