Skip to content

Commit

Permalink
feat: Respect original parameter overrides with git write-back (argop…
Browse files Browse the repository at this point in the history 
…roj-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]>
  • Loading branch information
@ks-yim @sribiere-jellysmack
2 people authored and sribiere-jellysmack committed Aug 13, 2024
1 parent 17e7e45 commit 0fb863f
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 20 deletions.
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -77,7 +78,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
Expand Down Expand Up @@ -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-20210901193431-a062eea981d2 // 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
Expand Down
6 changes: 4 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,9 @@ 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=
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=
Expand Down Expand Up @@ -964,8 +965,9 @@ 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=
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=
Expand Down
20 changes: 13 additions & 7 deletions pkg/argocd/git.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
55 changes: 52 additions & 3 deletions pkg/argocd/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -375,7 +377,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

Expand All @@ -385,21 +387,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")
Expand All @@ -411,6 +438,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
Expand Down
77 changes: 71 additions & 6 deletions pkg/argocd/update_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1066,6 +1066,7 @@ func Test_MarshalParamsOverride(t *testing.T) {
expected := `
kustomize:
images:
- baz
- foo
- bar
`
Expand Down Expand Up @@ -1093,8 +1094,12 @@ kustomize:
SourceType: v1alpha1.ApplicationSourceTypeKustomize,
},
}

yaml, err := marshalParamsOverride(&app)
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)))
Expand All @@ -1120,7 +1125,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)))
Expand All @@ -1130,6 +1135,9 @@ kustomize:
expected := `
helm:
parameters:
- name: baz
value: baz
forcestring: false
- name: foo
value: bar
forcestring: true
Expand Down Expand Up @@ -1170,7 +1178,14 @@ helm:
},
}

yaml, err := marshalParamsOverride(&app)
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)))
Expand All @@ -1196,7 +1211,7 @@ helm:
},
}

yaml, err := marshalParamsOverride(&app)
yaml, err := marshalParamsOverride(&app, nil)
require.NoError(t, err)
assert.Empty(t, yaml)
})
Expand Down Expand Up @@ -1227,7 +1242,7 @@ helm:
},
}

_, err := marshalParamsOverride(&app)
_, err := marshalParamsOverride(&app, nil)
assert.Error(t, err)
})
}
Expand Down Expand Up @@ -1808,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"
Expand Down

0 comments on commit 0fb863f

Please sign in to comment.