argocd-image-updater icon indicating copy to clipboard operation
argocd-image-updater copied to clipboard

Multiple alias updates doesn't work (Write back method: git, Write back method: helmvalues)

Open dshmelev opened this issue 1 year ago • 2 comments

Describe the bug ArgoCD Image Updater does not update the helm values file when multiple aliases of the same image are in application annotation.

To Reproduce

Create an application with annotations:

    argocd-image-updater.argoproj.io/write-back-method: git:secret:kube-gitops/argocd-repo-creds
    argocd-image-updater.argoproj.io/write-back-target: "helmvalues:/gitops/values/dev.yaml"
    argocd-image-updater.argoproj.io/git-repository: https://github.com/example/example.git
    argocd-image-updater.argoproj.io/git-branch: main
    argocd-image-updater.argoproj.io/image-list: foo=123456789.dkr.ecr.eu-west-1.amazonaws.com/application,bar=123456789.dkr.ecr.eu-west-1.amazonaws.com/application
    argocd-image-updater.argoproj.io/foo.helm.image-name: foo.spec.image.name
    argocd-image-updater.argoproj.io/foo.helm.image-tag: foo.spec.image.tag
    argocd-image-updater.argoproj.io/bar.helm.image-name: bar.spec.image.name
    argocd-image-updater.argoproj.io/bar.helm.image-tag: bar.spec.image.tag

Update 123456789.dkr.ecr.eu-west-1.amazonaws.com/application image and check the latest git commit.

Git commit contains updates only for one application.

bar.spec.image.name
bar.spec.image.tag

I wrote a short test that could help you to investigate an issue

	t.Run("Valid Helm source with Helm values file with multiple aliases", func(t *testing.T) {
		expected := `
foo.image.name: nginx
foo.image.tag: v1.0.0
bar.image.name: nginx
bar.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":          "foo=nginx, bar=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/foo.helm.image-name": "foo.image.name",
					"argocd-image-updater.argoproj.io/foo.helm.image-tag":  "foo.image.tag",
					"argocd-image-updater.argoproj.io/bar.helm.image-name": "bar.image.name",
					"argocd-image-updater.argoproj.io/bar.helm.image-tag":  "bar.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:        "foo.image.name",
									Value:       "nginx",
									ForceString: true,
								},
								{
									Name:        "foo.image.tag",
									Value:       "v1.0.0",
									ForceString: true,
								},
								{
									Name:        "bar.image.name",
									Value:       "nginx",
									ForceString: true,
								},
								{
									Name:        "bar.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",
					},
				},
			},
		}

		originalData := []byte(`
foo.image.name: nginx
foo.image.tag: v0.0.0
bar.image.name: nginx
bar.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)))
	})

Expected behavior Update all values.

Version quay.io/argoprojlabs/argocd-image-updater:latest argocd-image-updater v99.9.9+f9b5d95

Logs

ArgoCD Image Updater logs:

time="2024-08-05T11:38:36Z" level=info msg="Successfully updated the live application spec" application=stack-main-dev
time="2024-08-05T11:38:36Z" level=info msg="Processing results: applications=1 images_considered=2 images_skipped=0 images_updated=2 errors=0"

dshmelev avatar Aug 05 '24 11:08 dshmelev

If I add force-update=true, the above test passed:

"argocd-image-updater.argoproj.io/foo.force-update":    "true",
"argocd-image-updater.argoproj.io/bar.force-update":    "true",

But I think this case should still work regardless of the force-update value. The problem is from the following line: https://github.com/argoproj-labs/argocd-image-updater/blob/0ad5b94ff483bd2045e7adb3f54b1ca4c700d3da/pkg/argocd/argocd.go#L466-L474

where the image updater got the active images from the application status, and then loop thru the image-list annotation to match to an alias. The 2nd alias bar overwrote the 1st alias foo, and the result is bar is updated while foo is not.

chengfang avatar Aug 29 '24 18:08 chengfang

my draft fix: https://github.com/chengfang/argocd-image-updater/commit/5edd5cadd2e5f8be5db837fa5d74f5b49a15aecf The test passed with this fix. The test has 3 aliases all pointing to the same image.

chengfang avatar Aug 29 '24 20:08 chengfang