Skip to content

Commit f5419f6

Browse files
ks-yimks-yim
authored
feat: Respect original parameter overrides with git write-back (argoproj-labs#573)
* Fix original override not respected Signed-off-by: KS. Yim <[email protected]> * Add writeOverrides unittest Signed-off-by: KS. Yim <[email protected]> * Add helm override commit test Signed-off-by: KS. Yim <[email protected]> * lint Signed-off-by: KS. Yim <[email protected]> * fix shadowed err Signed-off-by: KS. Yim <[email protected]> --------- Signed-off-by: KS. Yim <[email protected]> Co-authored-by: KS. Yim <[email protected]>
1 parent 8dd44c8 commit f5419f6

File tree

5 files changed

+142
-20
lines changed

5 files changed

+142
-20
lines changed

‎go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ require (
2020
github.com/stretchr/testify v1.7.0
2121
go.uber.org/ratelimit v0.1.1-0.20201110185707-e86515f0dda9
2222
golang.org/x/crypto v0.1.0
23+
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1
2324
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
2425
gopkg.in/yaml.v2 v2.4.0
2526
k8s.io/api v1.22.4
@@ -77,7 +78,7 @@ require (
7778
github.com/golang-jwt/jwt/v4 v4.2.0 // indirect
7879
github.com/golang/protobuf v1.5.2 // indirect
7980
github.com/google/btree v1.0.1 // indirect
80-
github.com/google/go-cmp v0.5.6 // indirect
81+
github.com/google/go-cmp v0.5.8 // indirect
8182
github.com/google/go-github/v29 v29.0.2 // indirect
8283
github.com/google/go-github/v38 v38.0.0 // indirect
8384
github.com/google/go-querystring v1.1.0 // indirect
@@ -128,7 +129,6 @@ require (
128129
github.com/xanzy/ssh-agent v0.2.1 // indirect
129130
github.com/xlab/treeprint v0.0.0-20181112141820-a009c3971eca // indirect
130131
go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5 // indirect
131-
golang.org/x/exp v0.0.0-20210901193431-a062eea981d2 // indirect
132132
golang.org/x/net v0.7.0 // indirect
133133
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f // indirect
134134
golang.org/x/sys v0.5.0 // indirect

‎go.sum

+4-2
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,9 @@ github.com/google/go-cmp v0.5.1/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
415415
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
416416
github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
417417
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
418-
github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ=
419418
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
419+
github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
420+
github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
420421
github.com/google/go-github/v29 v29.0.2 h1:opYN6Wc7DOz7Ku3Oh4l7prmkOMwEcQxpFtxdU8N8Pts=
421422
github.com/google/go-github/v29 v29.0.2/go.mod h1:CHKiKKPHJ0REzfwc14QMklvtHwCveD0PxlMjLlzAM5E=
422423
github.com/google/go-github/v38 v38.0.0 h1:l/BalRp6dmFh/SFbl32RrlaVvbByhxpy+/LY0sv9isM=
@@ -964,8 +965,9 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0
964965
golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
965966
golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
966967
golang.org/x/exp v0.0.0-20210220032938-85be41e4509f/go.mod h1:I6l2HNBLBZEcrOoCpyKLdY2lHoRZ8lI4x60KMCQDft4=
967-
golang.org/x/exp v0.0.0-20210901193431-a062eea981d2 h1:Or4Ra3AAlhUlNn8WmIzw2Yq2vUHSkrP6E2e/FIESpF8=
968968
golang.org/x/exp v0.0.0-20210901193431-a062eea981d2/go.mod h1:a3o/VtDNHN+dCVLEpzjjUHOzR+Ln3DHX056ZPzoZGGA=
969+
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1 h1:k/i9J1pBpvlfR+9QsetwPyERsqu1GIbi967PQMq3Ivc=
970+
golang.org/x/exp v0.0.0-20230522175609-2e198f4a06a1/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w=
969971
golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
970972
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
971973
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=

‎pkg/argocd/git.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -259,23 +259,29 @@ func writeOverrides(app *v1alpha1.Application, wbc *WriteBackConfig, gitC git.Cl
259259
}
260260
}
261261

262-
override, err := marshalParamsOverride(app)
263-
if err != nil {
264-
return
265-
}
266-
267262
// If the target file already exist in the repository, we will check whether
268263
// our generated new file is the same as the existing one, and if yes, we
269264
// don't proceed further for commit.
265+
var override []byte
266+
var originalData []byte
270267
if targetExists {
271-
data, err := os.ReadFile(targetFile)
268+
originalData, err = os.ReadFile(targetFile)
272269
if err != nil {
273270
return err, false
274271
}
275-
if string(data) == string(override) {
272+
override, err = marshalParamsOverride(app, originalData)
273+
if err != nil {
274+
return
275+
}
276+
if string(originalData) == string(override) {
276277
logCtx.Debugf("target parameter file and marshaled data are the same, skipping commit.")
277278
return nil, true
278279
}
280+
} else {
281+
override, err = marshalParamsOverride(app, nil)
282+
if err != nil {
283+
return
284+
}
279285
}
280286

281287
err = os.WriteFile(targetFile, override, 0600)

‎pkg/argocd/update.go

+52-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"text/template"
1010
"time"
1111

12+
"golang.org/x/exp/slices"
13+
1214
"github.com/argoproj-labs/argocd-image-updater/ext/git"
1315
"github.com/argoproj-labs/argocd-image-updater/pkg/common"
1416
"github.com/argoproj-labs/argocd-image-updater/pkg/image"
@@ -375,7 +377,7 @@ func setAppImage(app *v1alpha1.Application, img *image.ContainerImage) error {
375377

376378
// marshalParamsOverride marshals the parameter overrides of a given application
377379
// into YAML bytes
378-
func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
380+
func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]byte, error) {
379381
var override []byte
380382
var err error
381383

@@ -385,21 +387,46 @@ func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
385387
if app.Spec.Source.Kustomize == nil {
386388
return []byte{}, nil
387389
}
388-
params := kustomizeOverride{
390+
391+
var params kustomizeOverride
392+
newParams := kustomizeOverride{
389393
Kustomize: kustomizeImages{
390394
Images: &app.Spec.Source.Kustomize.Images,
391395
},
392396
}
397+
398+
if len(originalData) == 0 {
399+
override, err = yaml.Marshal(newParams)
400+
break
401+
}
402+
err = yaml.Unmarshal(originalData, &params)
403+
if err != nil {
404+
override, err = yaml.Marshal(newParams)
405+
break
406+
}
407+
mergeKustomizeOverride(&params, &newParams)
393408
override, err = yaml.Marshal(params)
394409
case ApplicationTypeHelm:
395410
if app.Spec.Source.Helm == nil {
396411
return []byte{}, nil
397412
}
398-
params := helmOverride{
413+
var params helmOverride
414+
newParams := helmOverride{
399415
Helm: helmParameters{
400416
Parameters: app.Spec.Source.Helm.Parameters,
401417
},
402418
}
419+
420+
if len(originalData) == 0 {
421+
override, err = yaml.Marshal(newParams)
422+
break
423+
}
424+
err = yaml.Unmarshal(originalData, &params)
425+
if err != nil {
426+
override, err = yaml.Marshal(newParams)
427+
break
428+
}
429+
mergeHelmOverride(&params, &newParams)
403430
override, err = yaml.Marshal(params)
404431
default:
405432
err = fmt.Errorf("unsupported application type")
@@ -411,6 +438,28 @@ func marshalParamsOverride(app *v1alpha1.Application) ([]byte, error) {
411438
return override, nil
412439
}
413440

441+
func mergeHelmOverride(t *helmOverride, o *helmOverride) {
442+
for _, param := range o.Helm.Parameters {
443+
idx := slices.IndexFunc(t.Helm.Parameters, func(tp v1alpha1.HelmParameter) bool { return tp.Name == param.Name })
444+
if idx != -1 {
445+
t.Helm.Parameters[idx] = param
446+
continue
447+
}
448+
t.Helm.Parameters = append(t.Helm.Parameters, param)
449+
}
450+
}
451+
452+
func mergeKustomizeOverride(t *kustomizeOverride, o *kustomizeOverride) {
453+
for _, image := range *o.Kustomize.Images {
454+
idx := t.Kustomize.Images.Find(image)
455+
if idx != -1 {
456+
(*t.Kustomize.Images)[idx] = image
457+
continue
458+
}
459+
*t.Kustomize.Images = append(*t.Kustomize.Images, image)
460+
}
461+
}
462+
414463
func getWriteBackConfig(app *v1alpha1.Application, kubeClient *kube.KubernetesClient, argoClient ArgoCD) (*WriteBackConfig, error) {
415464
wbc := &WriteBackConfig{}
416465
// Default write-back is to use Argo CD API

‎pkg/argocd/update_test.go

+71-6
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,7 @@ func Test_MarshalParamsOverride(t *testing.T) {
10661066
expected := `
10671067
kustomize:
10681068
images:
1069+
- baz
10691070
- foo
10701071
- bar
10711072
`
@@ -1093,8 +1094,12 @@ kustomize:
10931094
SourceType: v1alpha1.ApplicationSourceTypeKustomize,
10941095
},
10951096
}
1096-
1097-
yaml, err := marshalParamsOverride(&app)
1097+
originalData := []byte(`
1098+
kustomize:
1099+
images:
1100+
- baz
1101+
`)
1102+
yaml, err := marshalParamsOverride(&app, originalData)
10981103
require.NoError(t, err)
10991104
assert.NotEmpty(t, yaml)
11001105
assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(yaml)))
@@ -1120,7 +1125,7 @@ kustomize:
11201125
},
11211126
}
11221127

1123-
yaml, err := marshalParamsOverride(&app)
1128+
yaml, err := marshalParamsOverride(&app, nil)
11241129
require.NoError(t, err)
11251130
assert.Empty(t, yaml)
11261131
assert.Equal(t, "", strings.TrimSpace(string(yaml)))
@@ -1130,6 +1135,9 @@ kustomize:
11301135
expected := `
11311136
helm:
11321137
parameters:
1138+
- name: baz
1139+
value: baz
1140+
forcestring: false
11331141
- name: foo
11341142
value: bar
11351143
forcestring: true
@@ -1170,7 +1178,14 @@ helm:
11701178
},
11711179
}
11721180

1173-
yaml, err := marshalParamsOverride(&app)
1181+
originalData := []byte(`
1182+
helm:
1183+
parameters:
1184+
- name: baz
1185+
value: baz
1186+
forcestring: false
1187+
`)
1188+
yaml, err := marshalParamsOverride(&app, originalData)
11741189
require.NoError(t, err)
11751190
assert.NotEmpty(t, yaml)
11761191
assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", " ")), strings.TrimSpace(string(yaml)))
@@ -1196,7 +1211,7 @@ helm:
11961211
},
11971212
}
11981213

1199-
yaml, err := marshalParamsOverride(&app)
1214+
yaml, err := marshalParamsOverride(&app, nil)
12001215
require.NoError(t, err)
12011216
assert.Empty(t, yaml)
12021217
})
@@ -1227,7 +1242,7 @@ helm:
12271242
},
12281243
}
12291244

1230-
_, err := marshalParamsOverride(&app)
1245+
_, err := marshalParamsOverride(&app, nil)
12311246
assert.Error(t, err)
12321247
})
12331248
}
@@ -1808,6 +1823,56 @@ func Test_CommitUpdates(t *testing.T) {
18081823
assert.NoError(t, err)
18091824
})
18101825

1826+
t.Run("Good commit to helm override", func(t *testing.T) {
1827+
app := app.DeepCopy()
1828+
app.Status.SourceType = "Helm"
1829+
app.Spec.Source.Helm = &v1alpha1.ApplicationSourceHelm{Parameters: []v1alpha1.HelmParameter{
1830+
{Name: "bar", Value: "bar", ForceString: true},
1831+
{Name: "baz", Value: "baz", ForceString: true},
1832+
}}
1833+
gitMock, dir, cleanup := mockGit(t)
1834+
defer cleanup()
1835+
of := filepath.Join(dir, ".argocd-source-testapp.yaml")
1836+
assert.NoError(t, os.WriteFile(of, []byte(`
1837+
helm:
1838+
parameters:
1839+
- name: foo
1840+
value: foo
1841+
forcestring: true
1842+
`), os.ModePerm))
1843+
1844+
gitMock.On("Checkout", mock.Anything).Run(func(args mock.Arguments) {
1845+
args.Assert(t, "mydefaultbranch")
1846+
}).Return(nil)
1847+
gitMock.On("Add", mock.Anything).Return(nil)
1848+
gitMock.On("Commit", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
1849+
gitMock.On("Push", mock.Anything, mock.Anything, mock.Anything).Return(nil)
1850+
gitMock.On("SymRefToBranch", mock.Anything).Return("mydefaultbranch", nil)
1851+
wbc, err := getWriteBackConfig(app, &kubeClient, &argoClient)
1852+
require.NoError(t, err)
1853+
wbc.GitClient = gitMock
1854+
app.Spec.Source.TargetRevision = "HEAD"
1855+
wbc.GitBranch = ""
1856+
1857+
err = commitChanges(app, wbc, nil)
1858+
assert.NoError(t, err)
1859+
override, err := os.ReadFile(of)
1860+
assert.NoError(t, err)
1861+
assert.YAMLEq(t, `
1862+
helm:
1863+
parameters:
1864+
- name: foo
1865+
value: foo
1866+
forcestring: true
1867+
- name: bar
1868+
value: bar
1869+
forcestring: true
1870+
- name: baz
1871+
value: baz
1872+
forcestring: true
1873+
`, string(override))
1874+
})
1875+
18111876
t.Run("Good commit to kustomization", func(t *testing.T) {
18121877
app := app.DeepCopy()
18131878
app.Annotations[common.WriteBackTargetAnnotation] = "kustomization"

0 commit comments

Comments
 (0)