Fix original override not respected

Signed-off-by: KS. Yim <[email protected]>

Author: ks-yim

go.mod

@@ -77,7 +77,7 @@ require (
 	github.com/golang-jwt/jwt/v4 v4.2.0 // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
 	github.com/google/btree v1.0.1 // indirect
-	github.com/google/go-cmp v0.5.6 // indirect
+	github.com/google/go-cmp v0.5.8 // indirect
 	github.com/google/go-github/v29 v29.0.2 // indirect
 	github.com/google/go-github/v38 v38.0.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
@@ -128,7 +128,7 @@ require (
 	github.com/xanzy/ssh-agent v0.2.1 // indirect
 	github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca // indirect
 	go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
-	golang.org/x/exp v0.0.0-20210901193431-a062eea981d2 // indirect
+	golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 // indirect
 	golang.org/x/net v0.7.0 // indirect
 	golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
 	golang.org/x/sys v0.5.0 // indirect

go.sum

@@ -417,6 +417,8 @@ github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
 github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
+github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-github/v29 v29.0.2 h1:opYN6Wc7DOz7Ku3Oh4l7prmkOMwEcQxpFtxdU8N8Pts=
 github.com/google/go-github/v29 v29.0.2/go.mod h1:CHKiKKPHJ0REzfwc14QMklvtHwCveD0PxlMjLlzAM5E=
 github.com/google/go-github/v38 v38.0.0 h1:l/BalRp6dmFh/SFbl32RrlaVvbByhxpy+/LY0sv9isM=
@@ -966,6 +968,8 @@ golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMk
 golang.org/x/exp v0.0.0-20210220032938-85be41e4509f/go.mod h1:I6l2HNBLBZEcrOoCpyKLdY2lHoRZ8lI4x60KMCQDft4=
 golang.org/x/exp v0.0.0-20210901193431-a062eea981d2 h1:Or4Ra3AAlhUlNn8WmIzw2Yq2vUHSkrP6E2e/FIESpF8=
 golang.org/x/exp v0.0.0-20210901193431-a062eea981d2/go.mod h1:a3o/VtDNHN+dCVLEpzjjUHOzR+Ln3DHX056ZPzoZGGA=
+golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
+golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
 golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=

pkg/argocd/git.go

@@ -259,23 +259,29 @@ func writeOverrides(app *v1alpha1.Application, wbc *WriteBackConfig, gitC git.Cl
 		}
 	}
 
-	override, err := marshalParamsOverride(app)
-	if err != nil {
-		return
-	}
-
 	// If the target file already exist in the repository, we will check whether
 	// our generated new file is the same as the existing one, and if yes, we
 	// don't proceed further for commit.
+	var override []byte
+	var originalData []byte
 	if targetExists {
-		data, err := os.ReadFile(targetFile)
+		originalData, err = os.ReadFile(targetFile)
 		if err != nil {
 			return err, false
 		}
-		if string(data) == string(override) {
+		override, err = marshalParamsOverride(app, originalData)
+		if err != nil {
+			return
+		}
+		if string(originalData) == string(override) {
 			logCtx.Debugf("target parameter file and marshaled data are the same, skipping commit.")
 			return nil, true
 		}
+	} else {
+		override, err = marshalParamsOverride(app, nil)
+		if err != nil {
+			return
+		}
 	}
 
 	err = os.WriteFile(targetFile, override, 0600)

pkg/argocd/update.go

@@ -3,6 +3,7 @@ package argocd
 import (
 	"context"
 	"fmt"
+	"golang.org/x/exp/slices"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -375,7 +376,7 @@ func setAppImage(app *v1alpha1.Application, img *image.ContainerImage) error {
 
 // marshalParamsOverride marshals the parameter overrides of a given application
 // into YAML bytes
-func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
+func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) {
 	var override []byte
 	var err error
 
@@ -385,21 +386,46 @@ func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
 		if app.Spec.Source.Kustomize == nil {
 			return []byte{}, nil
 		}
-		params := kustomizeOverride{
+
+		var params kustomizeOverride
+		newParams := kustomizeOverride{
 			Kustomize: kustomizeImages{
 				Images: &app.Spec.Source.Kustomize.Images,
 			},
 		}
+
+		if len(originalData) == 0 {
+			override, err = yaml.Marshal(newParams)
+			break
+		}
+		err := yaml.Unmarshal(originalData, &params)
+		if err != nil {
+			override, err = yaml.Marshal(newParams)
+			break
+		}
+		mergeKustomizeOverride(&params, &newParams)
 		override, err = yaml.Marshal(params)
 	case ApplicationTypeHelm:
 		if app.Spec.Source.Helm == nil {
 			return []byte{}, nil
 		}
-		params := helmOverride{
+		var params helmOverride
+		newParams := helmOverride{
 			Helm: helmParameters{
 				Parameters: app.Spec.Source.Helm.Parameters,
 			},
 		}
+
+		if len(originalData) == 0 {
+			override, err = yaml.Marshal(newParams)
+			break
+		}
+		err := yaml.Unmarshal(originalData, &params)
+		if err != nil {
+			override, err = yaml.Marshal(newParams)
+			break
+		}
+		mergeHelmOverride(&params, &newParams)
 		override, err = yaml.Marshal(params)
 	default:
 		err = fmt.Errorf("unsupported application type")
@@ -411,6 +437,28 @@ func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
 	return override, nil
 }
 
+func mergeHelmOverride(t *helmOverride, o *helmOverride) {
+	for _, param := range o.Helm.Parameters {
+		idx := slices.IndexFunc(t.Helm.Parameters, func(tp v1alpha1.HelmParameter) bool { return tp.Name == param.Name })
+		if idx != -1 {
+			t.Helm.Parameters[idx] = param
+			continue
+		}
+		t.Helm.Parameters = append(t.Helm.Parameters, param)
+	}
+}
+
+func mergeKustomizeOverride(t *kustomizeOverride, o *kustomizeOverride) {
+	for _, image := range *o.Kustomize.Images {
+		idx := t.Kustomize.Images.Find(image)
+		if idx != -1 {
+			(*t.Kustomize.Images)[idx] = image
+			continue
+		}
+		*t.Kustomize.Images = append(*t.Kustomize.Images, image)
+	}
+}
+
 func getWriteBackConfig(app *v1alpha1.Application, kubeClient *kube.KubernetesClient, argoClient ArgoCD) (*WriteBackConfig, error) {
 	wbc := &WriteBackConfig{}
 	// Default write-back is to use Argo CD API

pkg/argocd/update_test.go

@@ -1094,7 +1094,7 @@ kustomize:
 			},
 		}
 
-		yaml, err := marshalParamsOverride(&app)
+		yaml, err := marshalParamsOverride(&app, nil)
 		require.NoError(t, err)
 		assert.NotEmpty(t, yaml)
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(yaml)))
@@ -1120,7 +1120,7 @@ kustomize:
 			},
 		}
 
-		yaml, err := marshalParamsOverride(&app)
+		yaml, err := marshalParamsOverride(&app, nil)
 		require.NoError(t, err)
 		assert.Empty(t, yaml)
 		assert.Equal(t, "", strings.TrimSpace(string(yaml)))
@@ -1170,7 +1170,7 @@ helm:
 			},
 		}
 
-		yaml, err := marshalParamsOverride(&app)
+		yaml, err := marshalParamsOverride(&app, nil)
 		require.NoError(t, err)
 		assert.NotEmpty(t, yaml)
 		assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", "  ")), strings.TrimSpace(string(yaml)))
@@ -1196,7 +1196,7 @@ helm:
 			},
 		}
 
-		yaml, err := marshalParamsOverride(&app)
+		yaml, err := marshalParamsOverride(&app, nil)
 		require.NoError(t, err)
 		assert.Empty(t, yaml)
 	})
@@ -1227,7 +1227,7 @@ helm:
 			},
 		}
 
-		_, err := marshalParamsOverride(&app)
+		_, err := marshalParamsOverride(&app, nil)
 		assert.Error(t, err)
 	})
 }