From a67cf26637d236d38f67d9cf206d557848cdf8e0 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Thu, 5 Jun 2025 14:44:28 -0400 Subject: [PATCH 1/9] Remove appType variable Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 64ec08bc..5ce9760b 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -443,10 +443,9 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by var override []byte var err error - appType := GetApplicationType(app) appSource := getApplicationSource(app) - switch appType { + switch GetApplicationType(app) { case ApplicationTypeKustomize: if appSource.Kustomize == nil { return []byte{}, nil From e5c9439cd9f8e7ca5763e7c719c7bdae411adfd4 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Thu, 5 Jun 2025 15:31:52 -0400 Subject: [PATCH 2/9] extract ApplicationTypeKustomize case to helper function Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 46 +++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 5ce9760b..71212479 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -437,6 +437,29 @@ func marshalWithIndent(in interface{}, indent int) (out []byte, err error) { return b.Bytes(), nil } +func marshalKustomizeOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { + src := getApplicationSource(app) + if src.Kustomize == nil { + return []byte{}, nil + } + + overrides := kustomizeOverride{ + Kustomize: kustomizeImages{ + Images: &src.Kustomize.Images, + }, + } + + if len(originalData) > 0 { + var existing kustomizeOverride + if err := yaml.Unmarshal(originalData, &existing); err == nil { + mergeKustomizeOverride(&existing, &overrides) + overrides = existing + } + } + + return marshalWithIndent(overrides, defaultIndent) +} + // marshalParamsOverride marshals the parameter overrides of a given application // into YAML bytes func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { @@ -447,28 +470,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by switch GetApplicationType(app) { case ApplicationTypeKustomize: - if appSource.Kustomize == nil { - return []byte{}, nil - } - - var params kustomizeOverride - newParams := kustomizeOverride{ - Kustomize: kustomizeImages{ - Images: &appSource.Kustomize.Images, - }, - } - - if len(originalData) == 0 { - override, err = marshalWithIndent(newParams, defaultIndent) - break - } - err = yaml.Unmarshal(originalData, ¶ms) - if err != nil { - override, err = marshalWithIndent(newParams, defaultIndent) - break - } - mergeKustomizeOverride(¶ms, &newParams) - override, err = marshalWithIndent(params, defaultIndent) + override, err = marshalKustomizeOverride(app, originalData) case ApplicationTypeHelm: if appSource.Helm == nil { return []byte{}, nil From 8395c2e15a2355459cc5f2379c41fd7190601117 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Thu, 5 Jun 2025 15:37:23 -0400 Subject: [PATCH 3/9] Extract ApplicationTypeHelm case into helper function Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 144 ++++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 71 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 71212479..46f2e98c 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -460,95 +460,97 @@ func marshalKustomizeOverride(app *v1alpha1.Application, originalData []byte) ([ return marshalWithIndent(overrides, defaultIndent) } -// marshalParamsOverride marshals the parameter overrides of a given application -// into YAML bytes -func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { - var override []byte - var err error - +func marshalHelmOverride(app *v1alpha1.Application, originalData []byte) (override []byte, err error) { appSource := getApplicationSource(app) + if appSource.Helm == nil { + return []byte{}, nil + } - switch GetApplicationType(app) { - case ApplicationTypeKustomize: - override, err = marshalKustomizeOverride(app, originalData) - case ApplicationTypeHelm: - if appSource.Helm == nil { - return []byte{}, nil - } + if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) { + images := GetImagesAndAliasesFromApplication(app) - if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) { - images := GetImagesAndAliasesFromApplication(app) + helmNewValues := yaml.Node{} + if unmarshalErr := yaml.Unmarshal(originalData, &helmNewValues); unmarshalErr != nil { + return nil, unmarshalErr + } - helmNewValues := yaml.Node{} - err = yaml.Unmarshal(originalData, &helmNewValues) - if err != nil { - return nil, err + for _, c := range images { + if c.ImageAlias == "" { + continue } - for _, c := range images { - if c.ImageAlias == "" { - continue - } - - helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c) + helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c) - if helmAnnotationParamName == "" { - return nil, fmt.Errorf("could not find an image-name annotation for image %s", c.ImageName) - } - // for image-spec annotation, helmAnnotationParamName holds image-spec annotation value, - // and helmAnnotationParamVersion is empty - if helmAnnotationParamVersion == "" { - if c.GetParameterHelmImageSpec(app.Annotations, common.ImageUpdaterAnnotationPrefix) == "" { - // not a full image-spec, so image-tag is required - return nil, fmt.Errorf("could not find an image-tag annotation for image %s", c.ImageName) - } - } else { - // image-tag annotation is present, so continue to process image-tag - helmParamVersion := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamVersion) - if helmParamVersion == nil { - return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion) - } - err = setHelmValue(&helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value) - if err != nil { - return nil, fmt.Errorf("failed to set image parameter version value: %v", err) - } + if helmAnnotationParamName == "" { + return nil, fmt.Errorf("could not find an image-name annotation for image %s", c.ImageName) + } + // for image-spec annotation, helmAnnotationParamName holds image-spec annotation value, + // and helmAnnotationParamVersion is empty + if helmAnnotationParamVersion == "" { + if c.GetParameterHelmImageSpec(app.Annotations, common.ImageUpdaterAnnotationPrefix) == "" { + // not a full image-spec, so image-tag is required + return nil, fmt.Errorf("could not find an image-tag annotation for image %s", c.ImageName) } - - helmParamName := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamName) - if helmParamName == nil { - return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamName) + } else { + // image-tag annotation is present, so continue to process image-tag + helmParamVersion := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamVersion) + if helmParamVersion == nil { + return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion) } - - err = setHelmValue(&helmNewValues, helmAnnotationParamName, helmParamName.Value) + err = setHelmValue(&helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value) if err != nil { - return nil, fmt.Errorf("failed to set image parameter name value: %v", err) + return nil, fmt.Errorf("failed to set image parameter version value: %v", err) } } - override, err = marshalWithIndent(&helmNewValues, defaultIndent) - } else { - var params helmOverride - newParams := helmOverride{ - Helm: helmParameters{ - Parameters: appSource.Helm.Parameters, - }, + helmParamName := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamName) + if helmParamName == nil { + return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamName) } - outputParams := appSource.Helm.ValuesYAML() - log.WithContext().AddField("application", app).Debugf("values: '%s'", outputParams) - - if len(originalData) == 0 { - override, err = marshalWithIndent(newParams, defaultIndent) - break - } - err = yaml.Unmarshal(originalData, ¶ms) + err = setHelmValue(&helmNewValues, helmAnnotationParamName, helmParamName.Value) if err != nil { - override, err = marshalWithIndent(newParams, defaultIndent) - break + return nil, fmt.Errorf("failed to set image parameter name value: %v", err) } - mergeHelmOverride(¶ms, &newParams) - override, err = marshalWithIndent(params, defaultIndent) } + return marshalWithIndent(&helmNewValues, defaultIndent) + } + + var params helmOverride + newParams := helmOverride{ + Helm: helmParameters{ + Parameters: appSource.Helm.Parameters, + }, + } + + outputParams := appSource.Helm.ValuesYAML() + log.WithContext().AddField("application", app).Debugf("values: '%s'", outputParams) + + if len(originalData) == 0 { + override, err = marshalWithIndent(newParams, defaultIndent) + return override, err + } + err = yaml.Unmarshal(originalData, ¶ms) + if err != nil { + // TODO: if err is not nill, why do we try to do marshalWithIndent and not return nil? + override, err = marshalWithIndent(newParams, defaultIndent) + return override, err + } + mergeHelmOverride(¶ms, &newParams) + return marshalWithIndent(params, defaultIndent) +} + +// marshalParamsOverride marshals the parameter overrides of a given application +// into YAML bytes +func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { + var override []byte + var err error + + switch GetApplicationType(app) { + case ApplicationTypeKustomize: + override, err = marshalKustomizeOverride(app, originalData) + case ApplicationTypeHelm: + override, err = marshalHelmOverride(app, originalData) default: err = fmt.Errorf("unsupported application type") } From 629165c2a2f601544ed23bce3c2da69075c40cff Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Thu, 5 Jun 2025 15:38:09 -0400 Subject: [PATCH 4/9] Use early returns for marshalParamsOverride Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 46f2e98c..a55042be 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -543,22 +543,14 @@ func marshalHelmOverride(app *v1alpha1.Application, originalData []byte) (overri // marshalParamsOverride marshals the parameter overrides of a given application // into YAML bytes func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) { - var override []byte - var err error - switch GetApplicationType(app) { case ApplicationTypeKustomize: - override, err = marshalKustomizeOverride(app, originalData) + return marshalKustomizeOverride(app, originalData) case ApplicationTypeHelm: - override, err = marshalHelmOverride(app, originalData) + return marshalHelmOverride(app, originalData) default: - err = fmt.Errorf("unsupported application type") - } - if err != nil { - return nil, err + return nil, fmt.Errorf("unsupported application type") } - - return override, nil } func mergeHelmOverride(t *helmOverride, o *helmOverride) { From 1a223712522f380236241e943b7e5de6eb170f01 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Fri, 6 Jun 2025 06:46:11 -0400 Subject: [PATCH 5/9] Use a better name for looping variable Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index a55042be..769360b4 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -474,22 +474,22 @@ func marshalHelmOverride(app *v1alpha1.Application, originalData []byte) (overri return nil, unmarshalErr } - for _, c := range images { - if c.ImageAlias == "" { + for _, img := range images { + if img.ImageAlias == "" { continue } - helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c) + helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, img) if helmAnnotationParamName == "" { - return nil, fmt.Errorf("could not find an image-name annotation for image %s", c.ImageName) + return nil, fmt.Errorf("could not find an image-name annotation for image %s", img.ImageName) } // for image-spec annotation, helmAnnotationParamName holds image-spec annotation value, // and helmAnnotationParamVersion is empty if helmAnnotationParamVersion == "" { - if c.GetParameterHelmImageSpec(app.Annotations, common.ImageUpdaterAnnotationPrefix) == "" { + if img.GetParameterHelmImageSpec(app.Annotations, common.ImageUpdaterAnnotationPrefix) == "" { // not a full image-spec, so image-tag is required - return nil, fmt.Errorf("could not find an image-tag annotation for image %s", c.ImageName) + return nil, fmt.Errorf("could not find an image-tag annotation for image %s", img.ImageName) } } else { // image-tag annotation is present, so continue to process image-tag From fe91e4bdec0a20b64fe16cdd769abc42f716e634 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Fri, 13 Jun 2025 20:28:16 -0400 Subject: [PATCH 6/9] Add github.com/goccy/go-yaml Signed-off-by: Graham Beckley --- go.mod | 1 + go.sum | 2 ++ 2 files changed, 3 insertions(+) diff --git a/go.mod b/go.mod index 463ea72a..43865d0f 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/bradleyfalzon/ghinstallation/v2 v2.16.0 github.com/distribution/distribution/v3 v3.0.0-20230722181636-7b502560cad4 github.com/go-git/go-git/v5 v5.16.0 + github.com/goccy/go-yaml v1.18.0 github.com/google/uuid v1.6.0 github.com/patrickmn/go-cache v2.1.0+incompatible github.com/prometheus/client_golang v1.22.0 diff --git a/go.sum b/go.sum index 034215c2..11a1242f 100644 --- a/go.sum +++ b/go.sum @@ -190,6 +190,8 @@ github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJA github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u12GKvMCM= github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw= github.com/gobwas/ws v1.2.1/go.mod h1:hRKAFb8wOxFROYNsT1bqfWnhX+b5MFeJM9r2ZSwg/KY= +github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw= +github.com/goccy/go-yaml v1.18.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA= github.com/gogits/go-gogs-client v0.0.0-20200905025246-8bb8a50cb355 h1:HTVNOdTWO/gHYeFnr/HwpYwY6tgMcYd+Rgf1XrHnORY= github.com/gogits/go-gogs-client v0.0.0-20200905025246-8bb8a50cb355/go.mod h1:cY2AIrMgHm6oOHmR7jY+9TtjzSjQ3iG7tURJG3Y6XH0= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= From d67d9211ad2df1f150d4400064f10d58abb2b4e8 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Fri, 13 Jun 2025 22:19:28 -0400 Subject: [PATCH 7/9] Use goccy/go-yaml to update Helm values Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 217 +++++++++++++----------- pkg/argocd/update_test.go | 349 ++++++++++++++++++++++++++------------ 2 files changed, 350 insertions(+), 216 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 769360b4..eb812079 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -22,6 +22,10 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + gyaml "github.com/goccy/go-yaml" + "github.com/goccy/go-yaml/ast" + "github.com/goccy/go-yaml/parser" + "github.com/goccy/go-yaml/token" yaml "sigs.k8s.io/yaml/goyaml.v3" ) @@ -465,55 +469,52 @@ func marshalHelmOverride(app *v1alpha1.Application, originalData []byte) (overri if appSource.Helm == nil { return []byte{}, nil } - - if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) { + target := app.Annotations[common.WriteBackTargetAnnotation] + if strings.HasPrefix(target, common.HelmPrefix) { images := GetImagesAndAliasesFromApplication(app) - helmNewValues := yaml.Node{} - if unmarshalErr := yaml.Unmarshal(originalData, &helmNewValues); unmarshalErr != nil { - return nil, unmarshalErr + root, err := parser.ParseBytes([]byte(originalData), parser.ParseComments) + if err != nil { + return nil, fmt.Errorf("failed to parse original helm values: %w", err) } for _, img := range images { if img.ImageAlias == "" { continue } - helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, img) - - if helmAnnotationParamName == "" { - return nil, fmt.Errorf("could not find an image-name annotation for image %s", img.ImageName) - } // for image-spec annotation, helmAnnotationParamName holds image-spec annotation value, - // and helmAnnotationParamVersion is empty + // and version is empty if helmAnnotationParamVersion == "" { if img.GetParameterHelmImageSpec(app.Annotations, common.ImageUpdaterAnnotationPrefix) == "" { // not a full image-spec, so image-tag is required return nil, fmt.Errorf("could not find an image-tag annotation for image %s", img.ImageName) } } else { - // image-tag annotation is present, so continue to process image-tag helmParamVersion := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamVersion) if helmParamVersion == nil { return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion) } - err = setHelmValue(&helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value) + err = applyHelmParam(root, helmAnnotationParamVersion, helmParamVersion.Value) if err != nil { - return nil, fmt.Errorf("failed to set image parameter version value: %v", err) + return nil, err } } - + if helmAnnotationParamName == "" { + return nil, fmt.Errorf("could not find an image-name annotation for image %s", img.ImageName) + } helmParamName := getHelmParam(appSource.Helm.Parameters, helmAnnotationParamName) if helmParamName == nil { return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamName) } - - err = setHelmValue(&helmNewValues, helmAnnotationParamName, helmParamName.Value) + err = applyHelmParam(root, helmAnnotationParamName, helmParamName.Value) if err != nil { - return nil, fmt.Errorf("failed to set image parameter name value: %v", err) + return nil, err } } - return marshalWithIndent(&helmNewValues, defaultIndent) + + out := root.String() + return []byte(out), nil } var params helmOverride @@ -586,102 +587,112 @@ func mergeKustomizeOverride(t *kustomizeOverride, o *kustomizeOverride) { } } -// 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 +func findAnchorByName(root ast.Node, name string) *ast.AnchorNode { + for _, n := range ast.Filter(ast.AnchorType, root) { + anchor := n.(*ast.AnchorNode) + nameNode := anchor.Name.(*ast.StringNode) + if nameNode.Value == name { + return anchor } } - 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] + return nil } -// set value of the parameter passed from the annotations. -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(current, key); found { - (*current).Content[idx].Value = value.(string) +func createOrUpdateNode(node ast.Node, path []string, value string, root ...ast.Node) error { + // Keep track of the root in case we need to find an anchor for an alias + rootNode := node + if len(root) > 0 { + rootNode = root[0] + } + // Base case. We've recursed all the way down the path and found a node + if len(path) == 0 { + switch currentNode := node.(type) { + case *ast.StringNode: + currentNode.Value = value + case *ast.AnchorNode: + currentNode.Value = ast.String(&token.Token{Value: value}) + case *ast.AliasNode: + anchorName := currentNode.Value.(*ast.StringNode).Value + anchor := findAnchorByName(rootNode.(*ast.MappingNode), anchorName) + if anchor == nil { + return fmt.Errorf("alias %q not found", anchorName) + } + anchor.Value = ast.String(&token.Token{Value: value}) + default: + return fmt.Errorf("unexpected leaf node type %T", node) + } return nil } - - var err error - keys := strings.Split(key, ".") - - for i, k := range keys { - 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 + key, rest := path[0], path[1:] + switch currentNode := node.(type) { + case *ast.DocumentNode: + // Create a base mapping node if the incoming document is empty + if currentNode.Body == nil { + newNode, err := gyaml.ValueToNode(map[string]any{}) + if err != nil { + return err } - if i == len(keys)-1 { - // 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) - } - return nil - } else if current.Kind != yaml.MappingNode { - return fmt.Errorf("unexpected type %s for key %s", nodeKindString(current.Kind), k) + mn, ok := newNode.(*ast.MappingNode) + if !ok { + return fmt.Errorf("expected a MappingNode but got %T", newNode) } - } else { - if i == len(keys)-1 { - current.Content = append(current.Content, - &yaml.Node{ - Kind: yaml.ScalarNode, - Value: k, - Tag: "!!str", - }, - &yaml.Node{ - Kind: yaml.ScalarNode, - Value: value.(string), - Tag: "!!str", - }, - ) - return nil - } else { - current.Content = append(current.Content, - &yaml.Node{ - Kind: yaml.ScalarNode, - Value: k, - Tag: "!!str", - }, - &yaml.Node{ - Kind: yaml.MappingNode, - Content: []*yaml.Node{}, - }, - ) - current = current.Content[len(current.Content)-1] + currentNode.Body = mn + } + return createOrUpdateNode(currentNode.Body, path, value, currentNode.Body) + case *ast.AnchorNode: + return createOrUpdateNode(currentNode.Value, path, value, rootNode) + case *ast.AliasNode: + aliasName := currentNode.Value.(*ast.StringNode).Value + anchor := findAnchorByName(rootNode.(*ast.MappingNode), aliasName) + if anchor == nil { + return fmt.Errorf("alias %q not found", aliasName) + } + return createOrUpdateNode(anchor.Value, path, value, rootNode) + case *ast.MappingNode: + for _, mappingValueNode := range currentNode.Values { + nodeKey := mappingValueNode.Key.String() + if nodeKey == key { + return createOrUpdateNode(mappingValueNode.Value, rest, value, rootNode) } } + var newNodeData map[string]any + if len(rest) == 0 { + newNodeData = map[string]any{key: value} + } else { + newNodeData = map[string]any{key: map[string]any{}} + } + newNode, err := gyaml.ValueToNode(newNodeData) + if err != nil { + return err + } + if err := ast.Merge(currentNode, newNode); err != nil { + return err + } + if mappingValue, ok := newNode.(*ast.MappingNode); ok { + return createOrUpdateNode(mappingValue.Values[0].Value, rest, value, rootNode) + } } - return err + return fmt.Errorf("unexpected type %T for key attributes", node) +} + +func applyHelmParam(root *ast.File, attrPath string, value string) error { + // check if literal path exists, and if it does, replace it + path, _ := gyaml.PathString(fmt.Sprintf("$.'%s'", attrPath)) + if _, err := path.FilterFile(root); err == nil { + stringNode, err := gyaml.ValueToNode(value) + if err != nil { + return err + } + if err := path.ReplaceWithNode(root, stringNode); err != nil { + return err + } + return nil + } + if err := createOrUpdateNode(root.Docs[0], strings.Split(attrPath, "."), value); err != nil { + return err + } + return nil } func getWriteBackConfig(app *v1alpha1.Application, kubeClient *kube.ImageUpdaterKubernetesClient, argoClient ArgoCD) (*WriteBackConfig, error) { diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 66bb6477..68743aac 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -9,8 +9,6 @@ import ( "testing" "time" - 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" argomock "github.com/argoproj-labs/argocd-image-updater/pkg/argocd/mocks" @@ -24,10 +22,10 @@ import ( "github.com/argoproj-labs/argocd-image-updater/registry-scanner/pkg/tag" "github.com/argoproj-labs/argocd-image-updater/test/fake" "github.com/argoproj-labs/argocd-image-updater/test/fixture" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck + "github.com/distribution/distribution/v3/manifest/schema1" + "github.com/goccy/go-yaml/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1704,6 +1702,104 @@ redis: assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", " ")), strings.TrimSpace(string(yaml))) }) + t.Run("Valid Helm source with Helm values file with comments and newlines", func(t *testing.T) { + expected := ` +nginx.image.name: nginx +nginx.image.tag: v1.0.0 +replicas: 1 +` + app := v1alpha1.Application{ + ObjectMeta: v1.ObjectMeta{ + Name: "testapp", + Annotations: map[string]string{ + "argocd-image-updater.argoproj.io/image-list": "nginx=nginx", + "argocd-image-updater.argoproj.io/write-back-method": "git", + "argocd-image-updater.argoproj.io/write-back-target": "helmvalues:./test-values.yaml", + "argocd-image-updater.argoproj.io/nginx.helm.image-name": "nginx.image.name", + "argocd-image-updater.argoproj.io/nginx.helm.image-tag": "nginx.image.tag", + }, + }, + Spec: v1alpha1.ApplicationSpec{ + Sources: []v1alpha1.ApplicationSource{ + { + Chart: "my-app", + Helm: &v1alpha1.ApplicationSourceHelm{ + ReleaseName: "my-app", + ValueFiles: []string{"$values/some/dir/values.yaml"}, + Parameters: []v1alpha1.HelmParameter{ + { + Name: "nginx.image.name", + Value: "nginx", + ForceString: true, + }, + { + Name: "nginx.image.tag", + Value: "v1.0.0", + ForceString: true, + }, + }, + }, + RepoURL: "https://example.com/example", + TargetRevision: "main", + }, + { + Ref: "values", + RepoURL: "https://example.com/example2", + TargetRevision: "main", + }, + }, + }, + Status: v1alpha1.ApplicationStatus{ + SourceTypes: []v1alpha1.ApplicationSourceType{ + v1alpha1.ApplicationSourceTypeHelm, + "", + }, + Summary: v1alpha1.ApplicationSummary{ + Images: []string{ + "nginx:v0.0.0", + "redis:v0.0.0", + }, + }, + }, + } + + originalData := []byte(` +nginx.image.name: nginx +nginx.image.tag: v0.0.0 +replicas: 1 +`) + yaml, err := marshalParamsOverride(&app, originalData) + require.NoError(t, err) + assert.NotEmpty(t, yaml) + assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", " ")), strings.TrimSpace(string(yaml))) + + // comments and newlines + originalData = []byte(` +test-value1: one + +#comment +test-value2: two + +test-value3: three #inline comment +`) + expected = ` +test-value1: one + +#comment +test-value2: two + +test-value3: three #inline comment +nginx: + image: + tag: v1.0.0 + name: nginx +` + yaml, err = marshalParamsOverride(&app, originalData) + require.NoError(t, err) + assert.NotEmpty(t, yaml) + assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", " ")), strings.TrimSpace(string(yaml))) + }) + t.Run("Valid Helm source with Helm values file with multiple aliases", func(t *testing.T) { expected := ` foo.image.name: nginx @@ -2197,7 +2293,7 @@ replicas: 1 }) } -func Test_SetHelmValue(t *testing.T) { +func Test_createOrUpdateNode(t *testing.T) { t.Run("Update existing Key", func(t *testing.T) { expected := ` image: @@ -2212,38 +2308,31 @@ image: 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) + root, err := parser.ParseBytes(inputData, parser.ParseComments) require.NoError(t, err) - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + require.NoError(t, err) + } + + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Update Key with dots", func(t *testing.T) { expected := `image.attributes.tag: v2.0.0` - inputData := []byte(`image.attributes.tag: v1.0.0`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) + input := []byte(`image.attributes.tag: v1.0.0`) + root, err := parser.ParseBytes(input, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" - - err = setHelmValue(&input, key, value) - require.NoError(t, err) + if err := createOrUpdateNode(root.Docs[0], []string{"image.attributes.tag"}, "v2.0.0"); err != nil { + require.NoError(t, err) + } - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + actual := strings.TrimSpace(root.String()) + assert.Equal(t, expected, actual) }) t.Run("Key not found", func(t *testing.T) { @@ -2254,24 +2343,21 @@ image: tag: v2.0.0 ` - inputData := []byte(` + input := []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) + root, err := parser.ParseBytes(input, parser.ParseComments) require.NoError(t, err) - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } + + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Root key not found", func(t *testing.T) { @@ -2280,66 +2366,58 @@ name: repo-name tag: v2.0.0 ` - inputData := []byte(`name: repo-name`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) + input := []byte(`name: repo-name`) + root, err := parser.ParseBytes(input, parser.ParseComments) require.NoError(t, err) - key := "tag" - value := "v2.0.0" - - err = setHelmValue(&input, key, value) - require.NoError(t, err) + if err := createOrUpdateNode(root.Docs[0], []string{"tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Empty values with deep key", func(t *testing.T) { - // 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}}}` + expected := ` +image: + attributes: + tag: v2.0.0 +` - inputData := []byte(`{}`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) + input := []byte(``) + root, err := parser.ParseBytes(input, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" - - err = setHelmValue(&input, key, value) - require.NoError(t, err) + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Unexpected type for key", func(t *testing.T) { - inputData := []byte(` + inputData := ` image: attributes: v1.0.0 -`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) +` + input := []byte(inputData) + root, err := parser.ParseBytes(input, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" + expectedErr := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0") + assert.Error(t, expectedErr) + assert.Equal(t, "unexpected type *ast.StringNode for key attributes", expectedErr.Error()) - err = setHelmValue(&input, key, value) - 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 := ` + input := ` image: attributes: name: &repo repo-name - tag: v2.0.0 + tag: v1.0.0 # this is a comment multiline: | one @@ -2347,68 +2425,67 @@ image: three alias: *repo ` - - inputData := []byte(` + expected := ` image: attributes: name: &repo repo-name - tag: v1.0.0 + tag: v2.0.0 # this is a comment multiline: | one two three alias: *repo -`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) +` + src := []byte(input) + root, err := parser.ParseBytes(src, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" - - err = setHelmValue(&input, key, value) - require.NoError(t, err) + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Aliases to mappings are followed", func(t *testing.T) { - expected := ` + input := ` global: attributes: &attrs name: &repo repo-name - tag: v2.0.0 + tag: v1.0.0 image: attributes: *attrs ` - - inputData := []byte(` + expected := ` global: attributes: &attrs name: &repo repo-name - tag: v1.0.0 + tag: v2.0.0 image: attributes: *attrs -`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) +` + src := []byte(input) + root, err := parser.ParseBytes(src, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" - - err = setHelmValue(&input, key, value) - require.NoError(t, err) + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } - output, err := marshalWithIndent(&input, defaultIndent) - require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) t.Run("Aliases to scalars are followed", func(t *testing.T) { + input := ` +image: + attributes: + name: repo-name + version: &ver v1.0.0 + tag: *ver +` expected := ` image: attributes: @@ -2416,28 +2493,74 @@ image: version: &ver v2.0.0 tag: *ver ` + src := []byte(input) + root, err := parser.ParseBytes(src, parser.ParseComments) + require.NoError(t, err) - inputData := []byte(` + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } + + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) + }) + + t.Run("Anchors for scalars are updated appropriately", func(t *testing.T) { + input := ` image: attributes: name: repo-name version: &ver v1.0.0 tag: *ver -`) - input := yaml.Node{} - err := yaml.Unmarshal(inputData, &input) +` + expected := ` +image: + attributes: + name: repo-name + version: &ver v2.0.0 + tag: *ver +` + src := []byte(input) + root, err := parser.ParseBytes(src, parser.ParseComments) require.NoError(t, err) - key := "image.attributes.tag" - value := "v2.0.0" + if err := createOrUpdateNode(root.Docs[0], []string{"image", "attributes", "version"}, "v2.0.0"); err != nil { + t.Fatal(err) + } - err = setHelmValue(&input, key, value) - require.NoError(t, err) + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) + }) - output, err := marshalWithIndent(&input, defaultIndent) + t.Run("Anchors for maps are updated appropriately", func(t *testing.T) { + input := ` +global: + attributes: &attrs + name: repo-name + tag: v1.0.0 +image: + attributes: *attrs +` + expected := ` +global: + attributes: &attrs + name: repo-name + tag: v2.0.0 +image: + attributes: *attrs +` + src := []byte(input) + root, err := parser.ParseBytes(src, parser.ParseComments) require.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(output))) + + if err := createOrUpdateNode(root.Docs[0], []string{"global", "attributes", "tag"}, "v2.0.0"); err != nil { + t.Fatal(err) + } + + actual := root.String() + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(actual)) }) + } func Test_GetWriteBackConfig(t *testing.T) { From 181d425722701183d56f6ada058fa3d31b831ce2 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Fri, 13 Jun 2025 23:08:08 -0400 Subject: [PATCH 8/9] Fix linting errors Signed-off-by: Graham Beckley --- pkg/argocd/update.go | 2 +- pkg/argocd/update_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index eb812079..42c6edca 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -473,7 +473,7 @@ func marshalHelmOverride(app *v1alpha1.Application, originalData []byte) (overri if strings.HasPrefix(target, common.HelmPrefix) { images := GetImagesAndAliasesFromApplication(app) - root, err := parser.ParseBytes([]byte(originalData), parser.ParseComments) + root, err := parser.ParseBytes(originalData, parser.ParseComments) if err != nil { return nil, fmt.Errorf("failed to parse original helm values: %w", err) } diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 68743aac..85b604bf 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -24,7 +24,7 @@ import ( "github.com/argoproj-labs/argocd-image-updater/test/fixture" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/distribution/distribution/v3/manifest/schema1" + "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck "github.com/goccy/go-yaml/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" From 022839c5c25f0e045426d8a729cebadd2de5926f Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Mon, 16 Jun 2025 09:55:55 -0400 Subject: [PATCH 9/9] Fix update_test imports Signed-off-by: Graham Beckley --- pkg/argocd/update_test.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 85b604bf..10633da8 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -9,6 +9,15 @@ import ( "testing" "time" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck + "github.com/goccy/go-yaml/parser" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/argoproj-labs/argocd-image-updater/ext/git" gitmock "github.com/argoproj-labs/argocd-image-updater/ext/git/mocks" argomock "github.com/argoproj-labs/argocd-image-updater/pkg/argocd/mocks" @@ -22,14 +31,6 @@ import ( "github.com/argoproj-labs/argocd-image-updater/registry-scanner/pkg/tag" "github.com/argoproj-labs/argocd-image-updater/test/fake" "github.com/argoproj-labs/argocd-image-updater/test/fixture" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" - "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/distribution/distribution/v3/manifest/schema1" //nolint:staticcheck - "github.com/goccy/go-yaml/parser" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func Test_UpdateApplication(t *testing.T) {