Git write back to helm values is incorrect during the first pass and corrupts existing data #884

kirgene · 2024-10-08T03:05:55Z

Describe the bug
Using git write back with helmvalues and existing values.yaml file with user data leads to incorrect data writes and corruption of user data.

To Reproduce

Having this application:

apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
  name: test-app-custom
  namespace: argo-cd
  annotations:
    argocd-image-updater.argoproj.io/write-back-method: git:secret:argo-cd/repo
    argocd-image-updater.argoproj.io/test-app.secret: argo-cd/docker#auth
    argocd-image-updater.argoproj.io/image-list: 'image1=XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app,image2=XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2'
    argocd-image-updater.argoproj.io/image1.helm.image-name: image.name
    argocd-image-updater.argoproj.io/image1.helm.image-tag: image.tag
    argocd-image-updater.argoproj.io/image2.helm.image-name: image2.name
    argocd-image-updater.argoproj.io/image2.helm.image-tag: image2.tag
    argocd-image-updater.argoproj.io/image1.update-strategy: semver
    argocd-image-updater.argoproj.io/image2.update-strategy: semver
    argocd-image-updater.argoproj.io/write-back-target: "helmvalues:/apps/test-app/values.yaml"
    argocd-image-updater.argoproj.io/git-repository: "https://github.com/XXXX/test-gitops-test.git"
    argocd-image-updater.argoproj.io/git-branch: "dev"

spec:
  destination:
    namespace: test-app
    server: https://kubernetes.default.svc
  project: default
  sources:
  - chart: test-app
    helm:
      releaseName: test-app-custom
      valueFiles:
      - $vars/apps/test-app/values.yaml
    repoURL: XXXX.dkr.ecr.us-west-2.amazonaws.com/charts
    targetRevision: 0.4.0
  - ref: vars
    repoURL: https://github.com/XXXX/test-gitops-test

Content of the /apps/test-app/values.yaml in git repo:

# comment
test-value1: one
test-value2: two

After the first pass of argocd image updater the the values.yaml file is modified like this:

test-value1:
  tag: 1.1.0
test-value2: two
image2:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app2
image:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app

After the second pass, the file looks like this:

test-value1:
  tag: 1.1.0
test-value2: two
image2:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app2
  tag: 5.0.2
image:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app
  tag: 1.1.0

Expected behavior
The expected values.yaml file:

# comment
test-value1: one
test-value2: two
image2:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app2
  tag: 5.0.2
image:
  name: 549231858676.dkr.ecr.us-west-2.amazonaws.com/images/test-app
  tag: 1.1.0

Additional context
Add any other context about the problem here.

Version
Using the latest image tag of argocd image updater.

Logs

time="2024-10-08T02:59:03Z" level=info msg="Starting image update cycle, considering 1 annotated application(s) for update"
time="2024-10-08T02:59:03Z" level=info msg="Setting new image to XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:1.1.0" alias=image1 application=test-app-custom image_name=images/test-app image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T02:59:03Z" level=info msg="Successfully updated image 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:0.1.0' to 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:1.1.0', but pending spec update (dry run=false)" alias=image1 application=test-app-custom image_name=images/test-app image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T02:59:03Z" level=info msg="Setting new image to XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:5.0.2" alias=image2 application=test-app-custom image_name=images/test-app2 image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T02:59:03Z" level=info msg="Successfully updated image 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:0.1.0' to 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:5.0.2', but pending spec update (dry run=false)" alias=image2 application=test-app-custom image_name=images/test-app2 image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T02:59:03Z" level=info msg="Committing 2 parameter update(s) for application test-app-custom" application=test-app-custom
time="2024-10-08T02:59:03Z" level=info msg="Initializing https://github.com/XXXX/test-gitops-test.git to /tmp/git-test-app-custom4263206627"
time="2024-10-08T02:59:03Z" level=info msg="git config user.name argocd-image-updater" dir=/tmp/git-test-app-custom4263206627 execID=9e7de
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git config user.name argocd-image-updater]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=4.663557
time="2024-10-08T02:59:04Z" level=info msg="git config user.email noreply@argoproj.io" dir=/tmp/git-test-app-custom4263206627 execID=f00ff
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git config user.email noreply@argoproj.io]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=5.005586
time="2024-10-08T02:59:04Z" level=info msg="git fetch origin dev --force --prune --depth 1" dir=/tmp/git-test-app-custom4263206627 execID=e5016
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git fetch origin dev --force --prune --depth 1]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=652.223059
time="2024-10-08T02:59:04Z" level=info msg="git checkout --force dev" dir=/tmp/git-test-app-custom4263206627 execID=32038
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git checkout --force dev]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=14.699959
time="2024-10-08T02:59:04Z" level=info msg="git clean -ffdx" dir=/tmp/git-test-app-custom4263206627 execID=fa12d
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git clean -ffdx]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=2.478948
time="2024-10-08T02:59:04Z" level=info msg="git -c gpg.format=openpgp commit -a -F /tmp/image-updater-commit-msg971746535" dir=/tmp/git-test-app-custom4263206627 execID=26b6c
time="2024-10-08T02:59:04Z" level=info msg=Trace args="[git -c gpg.format=openpgp commit -a -F /tmp/image-updater-commit-msg971746535]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=9.392866
time="2024-10-08T02:59:04Z" level=info msg="git push origin dev" dir=/tmp/git-test-app-custom4263206627 execID=759ae
time="2024-10-08T02:59:05Z" level=info msg=Trace args="[git push origin dev]" dir=/tmp/git-test-app-custom4263206627 operation_name="exec git" time_ms=828.473824
time="2024-10-08T02:59:05Z" level=info msg="Successfully updated the live application spec" application=test-app-custom
time="2024-10-08T02:59:05Z" level=info msg="Processing results: applications=1 images_considered=2 images_skipped=0 images_updated=2 errors=0"




time="2024-10-08T03:01:05Z" level=info msg="Starting image update cycle, considering 1 annotated application(s) for update"
time="2024-10-08T03:01:05Z" level=info msg="Setting new image to XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:1.1.0" alias=image1 application=test-app-custom image_name=images/test-app image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T03:01:05Z" level=info msg="Successfully updated image 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:0.1.0' to 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app:1.1.0', but pending spec update (dry run=false)" alias=image1 application=test-app-custom image_name=images/test-app image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T03:01:05Z" level=info msg="Setting new image to XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:5.0.2" alias=image2 application=test-app-custom image_name=images/test-app2 image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T03:01:05Z" level=info msg="Successfully updated image 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:0.1.0' to 'XXXX.dkr.ecr.us-west-2.amazonaws.com/images/test-app2:5.0.2', but pending spec update (dry run=false)" alias=image2 application=test-app-custom image_name=images/test-app2 image_tag=0.1.0 registry=XXXX.dkr.ecr.us-west-2.amazonaws.com
time="2024-10-08T03:01:05Z" level=info msg="Committing 2 parameter update(s) for application test-app-custom" application=test-app-custom
time="2024-10-08T03:01:05Z" level=info msg="Initializing https://github.com/XXXX/test-gitops-test.git to /tmp/git-test-app-custom385552754"
time="2024-10-08T03:01:05Z" level=info msg="git config user.name argocd-image-updater" dir=/tmp/git-test-app-custom385552754 execID=4c0e5
time="2024-10-08T03:01:05Z" level=info msg=Trace args="[git config user.name argocd-image-updater]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=2.747734
time="2024-10-08T03:01:05Z" level=info msg="git config user.email noreply@argoproj.io" dir=/tmp/git-test-app-custom385552754 execID=47baa
time="2024-10-08T03:01:05Z" level=info msg=Trace args="[git config user.email noreply@argoproj.io]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=2.757565
time="2024-10-08T03:01:05Z" level=info msg="git fetch origin dev --force --prune --depth 1" dir=/tmp/git-test-app-custom385552754 execID=d6017
time="2024-10-08T03:01:06Z" level=info msg=Trace args="[git fetch origin dev --force --prune --depth 1]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=680.141973
time="2024-10-08T03:01:06Z" level=info msg="git checkout --force dev" dir=/tmp/git-test-app-custom385552754 execID=311fd
time="2024-10-08T03:01:06Z" level=info msg=Trace args="[git checkout --force dev]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=9.136949
time="2024-10-08T03:01:06Z" level=info msg="git clean -ffdx" dir=/tmp/git-test-app-custom385552754 execID=7d735
time="2024-10-08T03:01:06Z" level=info msg=Trace args="[git clean -ffdx]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=2.172142
time="2024-10-08T03:01:06Z" level=info msg="git -c gpg.format=openpgp commit -a -F /tmp/image-updater-commit-msg1207982649" dir=/tmp/git-test-app-custom385552754 execID=d1783
time="2024-10-08T03:01:06Z" level=info msg=Trace args="[git -c gpg.format=openpgp commit -a -F /tmp/image-updater-commit-msg1207982649]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=8.748512999999999
time="2024-10-08T03:01:06Z" level=info msg="git push origin dev" dir=/tmp/git-test-app-custom385552754 execID=12ca9
time="2024-10-08T03:01:07Z" level=info msg=Trace args="[git push origin dev]" dir=/tmp/git-test-app-custom385552754 operation_name="exec git" time_ms=971.21598
time="2024-10-08T03:01:07Z" level=info msg="Successfully updated the live application spec" application=test-app-custom
time="2024-10-08T03:01:07Z" level=info msg="Processing results: applications=1 images_considered=2 images_skipped=0 images_updated=2 errors=0"

The text was updated successfully, but these errors were encountered:

chengfang · 2024-10-10T03:29:27Z

There is this test that verifies a very similar setup to yours:

argocd-image-updater/pkg/argocd/update_test.go

Line 1505 in 60b89f2

 t.Run("Valid Helm source with Helm values file with multiple images", func(t *testing.T) { 

The above test has 2 images in image-list, each having distinct image name and tag fields. This test runs fine.

Note that the # commet line will be stripped in the update.

kirgene · 2024-10-10T22:12:52Z

@chengfang I've removed the following line to mimic the values file I posted above (using the chart's default image spec)

argocd-image-updater/pkg/argocd/update_test.go

Line 1496 in 60b89f2

image.spec.foo: nginx:v0.0.0

then run make test
and got similar results to what I posted:

--- FAIL: Test_MarshalParamsOverride (0.00s)
    --- FAIL: Test_MarshalParamsOverride/Valid_Helm_source_with_Helm_values_file_and_image-spec (0.00s)
        update_test.go:1501: 
            	Error Trace:	/home/yevhen/portx/argocd-image-updater-src/pkg/argocd/update_test.go:1501
            	Error:      	Not equal: 
            	            	expected: "image.spec.foo: nginx:v1.0.0\nreplicas: 1"
            	            	actual  : "replicas:\n  spec:\n    foo: nginx:v1.0.0\nimage: {}"
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1,2 +1,4 @@
            	            	-image.spec.foo: nginx:v1.0.0
            	            	-replicas: 1
            	            	+replicas:
            	            	+  spec:
            	            	+    foo: nginx:v1.0.0
            	            	+image: {}
            	Test:       	Test_MarshalParamsOverride/Valid_Helm_source_with_Helm_values_file_and_image-spec

You can see the replicas value gets corrupted.

chengfang · 2024-10-11T15:35:53Z

Note that when the image.name or image.tag keys do not exist in the target value file, the fields to be added can be either of the 2 formats for specified key like my.image.name. The 2nd one is probably more common.

my.image.name: foo

or

my:
  image:
    name: foo

chengfang · 2024-10-11T15:51:21Z

In this line, the parentIdx should be updated to point to the newly added element:

argocd-image-updater/pkg/argocd/update.go

Line 593 in 60b89f2

(*parent)[parentIdx].Value = newParent

kirgene added the bug Something isn't working label Oct 8, 2024

chengfang added the helm label Oct 9, 2024

chengfang mentioned this issue Oct 11, 2024

fix: Git write back to helm values is incorrect during the first pass and corrupts existing data #885

Merged

pasha-codefresh closed this as completed in #885 Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Git write back to helm values is incorrect during the first pass and corrupts existing data #884

Git write back to helm values is incorrect during the first pass and corrupts existing data #884

kirgene commented Oct 8, 2024

chengfang commented Oct 10, 2024

kirgene commented Oct 10, 2024

chengfang commented Oct 11, 2024

chengfang commented Oct 11, 2024

Git write back to helm values is incorrect during the first pass and corrupts existing data #884

Git write back to helm values is incorrect during the first pass and corrupts existing data #884

Comments

kirgene commented Oct 8, 2024

chengfang commented Oct 10, 2024

kirgene commented Oct 10, 2024

chengfang commented Oct 11, 2024

chengfang commented Oct 11, 2024