Fix updates to multiple images when using helm values file

When updating multiple images in a Helm values file, ArgoCD would only ever update a single image. This is because the code only takes into account the new helm values from the last image that it processed ignoring the changes for the previous images. This commit fixes that behavior. The `helmNewValues` var is moved outside of the for-loop that iterates over the app images. For each image, the tag is updated in the `helmNewValues` var. Signed-off-by: Lyupcho Kotev <[email protected]>
argoproj-labs · Jul 18, 2024 · fca9928 · fca9928
1 parent 9611c99
commit fca9928
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 13 deletions.
diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go
@@ -435,8 +435,13 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 		if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) {
 			images := GetImagesAndAliasesFromApplication(app)
 
-			for _, c := range images {
+			helmNewValues := yaml.MapSlice{}
+			err = yaml.Unmarshal(originalData, &helmNewValues)
+			if err != nil {
+				return nil, err
+			}
 
+			for _, c := range images {
 				if c.ImageAlias == "" {
 					continue
 				}
@@ -460,14 +465,6 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 					return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion)
 				}
 
-				// Create new values structure
-				//var helmNewValues map[string]interface{}
-				helmNewValues := yaml.MapSlice{}
-				err = yaml.Unmarshal(originalData, &helmNewValues)
-				if err != nil {
-					return nil, err
-				}
-
 				err = setHelmValue(helmNewValues, helmAnnotationParamName, helmParamName.Value)
 				if err != nil {
 					return nil, fmt.Errorf("failed to set image parameter name value: %v", err)

diff --git a/pkg/argocd/update_test.go b/pkg/argocd/update_test.go
@@ -1383,6 +1383,94 @@ replicas: 1
 		assert.Equal(t, strings.TrimSpace(strings.ReplaceAll(expected, "\t", "  ")), strings.TrimSpace(string(yaml)))
 	})
 
+	t.Run("Valid Helm source with Helm values file with multiple images", func(t *testing.T) {
+		expected := `
+nginx.image.name: nginx
+nginx.image.tag: v1.0.0
+redis.image.name: redis
+redis.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":            "nginx=nginx, redis=redis",
+					"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/nginx.helm.image-name": "nginx.image.name",
+					"argocd-image-updater.argoproj.io/nginx.helm.image-tag":  "nginx.image.tag",
+					"argocd-image-updater.argoproj.io/redis.helm.image-name": "redis.image.name",
+					"argocd-image-updater.argoproj.io/redis.helm.image-tag":  "redis.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:        "nginx.image.name",
+									Value:       "nginx",
+									ForceString: true,
+								},
+								{
+									Name:        "nginx.image.tag",
+									Value:       "v1.0.0",
+									ForceString: true,
+								},
+								{
+									Name:        "redis.image.name",
+									Value:       "redis",
+									ForceString: true,
+								},
+								{
+									Name:        "redis.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",
+						"redis:v0.0.0",
+					},
+				},
+			},
+		}
+
+		originalData := []byte(`
+nginx.image.name: nginx
+nginx.image.tag: v0.0.0
+redis.image.name: redis
+redis.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)))
+	})
+
 	t.Run("Failed to setValue image parameter name", func(t *testing.T) {
 		app := v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
@@ -1528,7 +1616,7 @@ replicas: 1
 			},
 		}
 
-		originalData := []byte(`random content`)
+		originalData := []byte(`random: yaml`)
 		_, err := marshalParamsOverride(&app, originalData)
 		assert.Error(t, err)
 		assert.Equal(t, "could not find an image-tag annotation for image nginx", err.Error())
@@ -1575,7 +1663,7 @@ replicas: 1
 			},
 		}
 
-		originalData := []byte(`random content`)
+		originalData := []byte(`random: yaml`)
 		_, err := marshalParamsOverride(&app, originalData)
 		assert.Error(t, err)
 		assert.Equal(t, "could not find an image-name annotation for image nginx", err.Error())
@@ -1623,7 +1711,7 @@ replicas: 1
 			},
 		}
 
-		originalData := []byte(`random content`)
+		originalData := []byte(`random: yaml`)
 		_, err := marshalParamsOverride(&app, originalData)
 		assert.Error(t, err)
 		assert.Equal(t, "wrongimage.name parameter not found", err.Error())
@@ -1671,7 +1759,7 @@ replicas: 1
 			},
 		}
 
-		originalData := []byte(`random content`)
+		originalData := []byte(`random: yaml`)
 		_, err := marshalParamsOverride(&app, originalData)
 		assert.Error(t, err)
 		assert.Equal(t, "wrongimage.tag parameter not found", err.Error())