From d1087c97e7362b05582e8a72d02df345f3029177 Mon Sep 17 00:00:00 2001 From: "KS. Yim" Date: Fri, 9 Jun 2023 17:27:41 +0900 Subject: [PATCH 1/5] Fix original override not respected Signed-off-by: KS. Yim --- go.mod | 4 +-- go.sum | 4 +++ pkg/argocd/git.go | 20 ++++++++++----- pkg/argocd/update.go | 54 ++++++++++++++++++++++++++++++++++++--- pkg/argocd/update_test.go | 10 ++++---- 5 files changed, 75 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index 34f6ed18..3710b071 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 9e799ffe..9cbfb27a 100644 --- a/go.sum +++ b/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= diff --git a/pkg/argocd/git.go b/pkg/argocd/git.go index 6aa4b421..529787bc 100644 --- a/pkg/argocd/git.go +++ b/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) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index c92730dc..b9220298 100644 --- a/pkg/argocd/update.go +++ b/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, ¶ms) + if err != nil { + override, err = yaml.Marshal(newParams) + break + } + mergeKustomizeOverride(¶ms, &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, ¶ms) + if err != nil { + override, err = yaml.Marshal(newParams) + break + } + mergeHelmOverride(¶ms, &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 diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index ea5cdccf..907e6d8a 100644 --- a/pkg/argocd/update_test.go +++ b/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) }) } From 1cd5b0b340377a59ea9ffa9578140a67202aaafc Mon Sep 17 00:00:00 2001 From: "KS. Yim" Date: Fri, 9 Jun 2023 17:46:01 +0900 Subject: [PATCH 2/5] Add writeOverrides unittest Signed-off-by: KS. Yim --- pkg/argocd/update_test.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index 907e6d8a..d3cb4fab 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -1066,6 +1066,7 @@ func Test_MarshalParamsOverride(t *testing.T) { expected := ` kustomize: images: + - baz - foo - bar ` @@ -1093,8 +1094,12 @@ kustomize: SourceType: v1alpha1.ApplicationSourceTypeKustomize, }, } - - yaml, err := marshalParamsOverride(&app, nil) + originalData := []byte(` +kustomize: + images: + - baz +`) + yaml, err := marshalParamsOverride(&app, originalData) require.NoError(t, err) assert.NotEmpty(t, yaml) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(yaml))) @@ -1130,6 +1135,9 @@ kustomize: expected := ` helm: parameters: + - name: baz + value: baz + forcestring: false - name: foo value: bar forcestring: true @@ -1170,7 +1178,14 @@ helm: }, } - yaml, err := marshalParamsOverride(&app, nil) + originalData := []byte(` +helm: + parameters: + - name: baz + value: baz + forcestring: false +`) + 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))) From fc6183d4d819d7341616100047fcee42b7c85dba Mon Sep 17 00:00:00 2001 From: "KS. Yim" Date: Fri, 9 Jun 2023 18:21:14 +0900 Subject: [PATCH 3/5] Add helm override commit test Signed-off-by: KS. Yim --- pkg/argocd/update_test.go | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go index d3cb4fab..c7a4b30d 100644 --- a/pkg/argocd/update_test.go +++ b/pkg/argocd/update_test.go @@ -1823,6 +1823,56 @@ func Test_CommitUpdates(t *testing.T) { assert.NoError(t, err) }) + t.Run("Good commit to helm override", func(t *testing.T) { + app := app.DeepCopy() + app.Status.SourceType = "Helm" + app.Spec.Source.Helm = &v1alpha1.ApplicationSourceHelm{Parameters: []v1alpha1.HelmParameter{ + {Name: "bar", Value: "bar", ForceString: true}, + {Name: "baz", Value: "baz", ForceString: true}, + }} + gitMock, dir, cleanup := mockGit(t) + defer cleanup() + of := filepath.Join(dir, ".argocd-source-testapp.yaml") + assert.NoError(t, os.WriteFile(of, []byte(` +helm: + parameters: + - name: foo + value: foo + forcestring: true +`), os.ModePerm)) + + gitMock.On("Checkout", mock.Anything).Run(func(args mock.Arguments) { + args.Assert(t, "mydefaultbranch") + }).Return(nil) + gitMock.On("Add", mock.Anything).Return(nil) + gitMock.On("Commit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + gitMock.On("Push", mock.Anything, mock.Anything, mock.Anything).Return(nil) + gitMock.On("SymRefToBranch", mock.Anything).Return("mydefaultbranch", nil) + wbc, err := getWriteBackConfig(app, &kubeClient, &argoClient) + require.NoError(t, err) + wbc.GitClient = gitMock + app.Spec.Source.TargetRevision = "HEAD" + wbc.GitBranch = "" + + err = commitChanges(app, wbc, nil) + assert.NoError(t, err) + override, err := os.ReadFile(of) + assert.NoError(t, err) + assert.YAMLEq(t, ` +helm: + parameters: + - name: foo + value: foo + forcestring: true + - name: bar + value: bar + forcestring: true + - name: baz + value: baz + forcestring: true +`, string(override)) + }) + t.Run("Good commit to kustomization", func(t *testing.T) { app := app.DeepCopy() app.Annotations[common.WriteBackTargetAnnotation] = "kustomization" From 27b216ace99a6c1ea176d87a61e1b55aab65bc9d Mon Sep 17 00:00:00 2001 From: "KS. Yim" Date: Sat, 10 Jun 2023 11:22:08 +0900 Subject: [PATCH 4/5] lint Signed-off-by: KS. Yim --- go.mod | 2 +- go.sum | 2 -- pkg/argocd/update.go | 4 ++-- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 3710b071..8717b732 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/stretchr/testify v1.7.0 go.uber.org/ratelimit v0.1.1-0.20201110185707-e86515f0dda9 golang.org/x/crypto v0.1.0 + golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 gopkg.in/yaml.v2 v2.4.0 k8s.io/api v1.22.4 @@ -128,7 +129,6 @@ 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-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 diff --git a/go.sum b/go.sum index 9cbfb27a..f0bee111 100644 --- a/go.sum +++ b/go.sum @@ -415,7 +415,6 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= 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= @@ -966,7 +965,6 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= 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= diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index b9220298..6a0e1266 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -3,7 +3,6 @@ package argocd import ( "context" "fmt" - "golang.org/x/exp/slices" "path/filepath" "strings" "sync" @@ -17,6 +16,7 @@ import ( "github.com/argoproj-labs/argocd-image-updater/pkg/log" "github.com/argoproj-labs/argocd-image-updater/pkg/registry" "github.com/argoproj-labs/argocd-image-updater/pkg/tag" + "golang.org/x/exp/slices" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" @@ -398,7 +398,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by override, err = yaml.Marshal(newParams) break } - err := yaml.Unmarshal(originalData, ¶ms) + err = yaml.Unmarshal(originalData, ¶ms) if err != nil { override, err = yaml.Marshal(newParams) break From 4a9f69d16a1b3ab9d2c22cf64771a0f04d1a420d Mon Sep 17 00:00:00 2001 From: "KS. Yim" Date: Sat, 8 Jul 2023 12:26:59 +0900 Subject: [PATCH 5/5] fix shadowed err Signed-off-by: KS. Yim --- pkg/argocd/update.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go index 6a0e1266..2773a5ce 100644 --- a/pkg/argocd/update.go +++ b/pkg/argocd/update.go @@ -9,6 +9,8 @@ import ( "text/template" "time" + "golang.org/x/exp/slices" + "github.com/argoproj-labs/argocd-image-updater/ext/git" "github.com/argoproj-labs/argocd-image-updater/pkg/common" "github.com/argoproj-labs/argocd-image-updater/pkg/image" @@ -16,7 +18,6 @@ import ( "github.com/argoproj-labs/argocd-image-updater/pkg/log" "github.com/argoproj-labs/argocd-image-updater/pkg/registry" "github.com/argoproj-labs/argocd-image-updater/pkg/tag" - "golang.org/x/exp/slices" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" @@ -420,7 +421,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by override, err = yaml.Marshal(newParams) break } - err := yaml.Unmarshal(originalData, ¶ms) + err = yaml.Unmarshal(originalData, ¶ms) if err != nil { override, err = yaml.Marshal(newParams) break