Merge branch 'argoproj-labs:master' into master

ext/git/writer.go

@@ -61,7 +61,7 @@ func (m *nativeGitClient) Commit(pathSpec string, opts *CommitOptions) error {
 
 	out, err := m.runCmd(args...)
 	if err != nil {
-		log.Errorf(out)
+		log.Errorf("%s %v", out, err)
 		return err
 	}
 

pkg/argocd/argocd.go

@@ -465,10 +465,22 @@ func GetImagesAndAliasesFromApplication(app *v1alpha1.Application) image.Contain
 	// We update the ImageAlias field of the Images found in the app.Status.Summary.Images list.
 	for _, img := range *parseImageList(app.Annotations) {
 		if image := images.ContainsImage(img, false); image != nil {
-			if img.ImageAlias == "" {
-				image.ImageAlias = img.ImageName
+			if image.ImageAlias != "" {
+				// this image has already been matched to an alias, so create a copy
+				// and assign this alias to the image copy to avoid overwriting the existing alias association
+				imageCopy := *image
+				if img.ImageAlias == "" {
+					imageCopy.ImageAlias = img.ImageName
+				} else {
+					imageCopy.ImageAlias = img.ImageAlias
+				}
+				images = append(images, &imageCopy)
 			} else {
-				image.ImageAlias = img.ImageAlias
+				if img.ImageAlias == "" {
+					image.ImageAlias = img.ImageName
+				} else {
+					image.ImageAlias = img.ImageAlias
+				}
 			}
 		}
 	}

pkg/argocd/update.go

@@ -465,11 +465,11 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 					return nil, fmt.Errorf("%s parameter not found", helmAnnotationParamVersion)
 				}
 
-				err = setHelmValue(helmNewValues, helmAnnotationParamName, helmParamName.Value)
+				err = setHelmValue(&helmNewValues, helmAnnotationParamName, helmParamName.Value)
 				if err != nil {
 					return nil, fmt.Errorf("failed to set image parameter name value: %v", err)
 				}
-				err = setHelmValue(helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value)
+				err = setHelmValue(&helmNewValues, helmAnnotationParamVersion, helmParamVersion.Value)
 				if err != nil {
 					return nil, fmt.Errorf("failed to set image parameter version value: %v", err)
 				}
@@ -542,34 +542,53 @@ func findHelmValuesKey(m yaml.MapSlice, key string) (int, bool) {
 }
 
 // set value of the parameter passed from the annotations.
-func setHelmValue(m yaml.MapSlice, key string, value interface{}) error {
+func setHelmValue(currentValues *yaml.MapSlice, key string, value interface{}) error {
 	// Check if the full key exists
-	if idx, found := findHelmValuesKey(m, key); found {
-		m[idx].Value = value
+	if idx, found := findHelmValuesKey(*currentValues, key); found {
+		(*currentValues)[idx].Value = value
 		return nil
 	}
 
 	var err error
 	keys := strings.Split(key, ".")
-	current := m
+	current := currentValues
+	var parent *yaml.MapSlice
+	var parentIdx int
 
 	for i, k := range keys {
-		if idx, found := findHelmValuesKey(current, k); found {
+		if idx, found := findHelmValuesKey(*current, k); found {
 			if i == len(keys)-1 {
 				// If we're at the final key, set the value and return
-				current[idx].Value = value
+				(*current)[idx].Value = value
 				return nil
 			} else {
 				// Navigate deeper into the map
-				if nestedMap, ok := current[idx].Value.(yaml.MapSlice); ok {
-					current = nestedMap
+				if nestedMap, ok := (*current)[idx].Value.(yaml.MapSlice); ok {
+					parent = current
+					parentIdx = idx
+					current = &nestedMap
 				} else {
-					return fmt.Errorf("unexpected type %T for key %s", current[idx].Value, k)
+					return fmt.Errorf("unexpected type %T for key %s", (*current)[idx].Value, k)
 				}
 			}
 		} else {
-			err = fmt.Errorf("key %s not found in the map", k)
-			break
+			newCurrent := yaml.MapSlice{}
+			var newParent yaml.MapSlice
+
+			if i == len(keys)-1 {
+				newParent = append(*current, yaml.MapItem{Key: k, Value: value})
+			} else {
+				newParent = append(*current, yaml.MapItem{Key: k, Value: newCurrent})
+			}
+
+			if parent == nil {
+				*currentValues = newParent
+			} else {
+				(*parent)[parentIdx].Value = newParent
+			}
+
+			parent = &newParent
+			current = &newCurrent
 		}
 	}
 

pkg/argocd/update_test.go

@@ -1471,7 +1471,117 @@ 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 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
+bbb.image.name: nginx
+bbb.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, bbb=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",
+					"argocd-image-updater.argoproj.io/bbb.helm.image-name": "bbb.image.name",
+					"argocd-image-updater.argoproj.io/bbb.helm.image-tag":  "bbb.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,
+								},
+								{
+									Name:        "bbb.image.name",
+									Value:       "nginx",
+									ForceString: true,
+								},
+								{
+									Name:        "bbb.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
+bbb.image.name: nginx
+bbb.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) {
+		expected := `
+image:
+  name: nginx
+  tag: v1.0.0
+replicas: 1
+`
+
 		app := v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
 				Name: "testapp",
@@ -1514,16 +1624,24 @@ replicas: 1
 		}
 
 		originalData := []byte(`
-image_name: nginx
-image.tag: v0.0.0
+image:
+  name: nginx
 replicas: 1
 `)
-		_, err := marshalParamsOverride(&app, originalData)
-		assert.Error(t, err)
-		assert.Equal(t, "failed to set image parameter name value: key image not found in the map", err.Error())
+
+		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 version", func(t *testing.T) {
+		expected := `
+image:
+  tag: v1.0.0
+  name: nginx
+replicas: 1
+`
 		app := v1alpha1.Application{
 			ObjectMeta: v1.ObjectMeta{
 				Name: "testapp",
@@ -1566,13 +1684,15 @@ replicas: 1
 		}
 
 		originalData := []byte(`
-image.name: nginx
-image_tag: v0.0.0
+image:
+  tag: v0.0.0
 replicas: 1
 `)
-		_, err := marshalParamsOverride(&app, originalData)
-		assert.Error(t, err)
-		assert.Equal(t, "failed to set image parameter version value: key image not found in the map", err.Error())
+
+		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("Missing annotation image-tag for helmvalues write-back-target", func(t *testing.T) {
@@ -1865,7 +1985,7 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		require.NoError(t, err)
 		assert.Equal(t, expected, input)
 	})
@@ -1881,23 +2001,72 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		require.NoError(t, err)
 		assert.Equal(t, expected, input)
 	})
 
 	t.Run("Key not found", func(t *testing.T) {
+		expected := yaml.MapSlice{
+			{Key: "image", Value: yaml.MapSlice{
+				{Key: "attributes", Value: yaml.MapSlice{
+					{Key: "name", Value: "repo-name"},
+					{Key: "tag", Value: "v2.0.0"},
+				}},
+			}},
+		}
+
 		input := yaml.MapSlice{
 			{Key: "image", Value: yaml.MapSlice{
-				{Key: "tag", Value: "v1.0.0"},
+				{Key: "attributes", Value: yaml.MapSlice{
+					{Key: "name", Value: "repo-name"},
+				}},
 			}},
 		}
+
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
-		assert.Error(t, err)
-		assert.Equal(t, "key attributes not found in the map", err.Error())
+		err := setHelmValue(&input, key, value)
+		require.NoError(t, err)
+		assert.Equal(t, expected, input)
+	})
+
+	t.Run("Root key not found", func(t *testing.T) {
+		expected := yaml.MapSlice{
+			{Key: "name", Value: "repo-name"},
+			{Key: "tag", Value: "v2.0.0"},
+		}
+
+		input := yaml.MapSlice{
+			{Key: "name", Value: "repo-name"},
+		}
+
+		key := "tag"
+		value := "v2.0.0"
+
+		err := setHelmValue(&input, key, value)
+		require.NoError(t, err)
+		assert.Equal(t, expected, input)
+	})
+
+	t.Run("Empty values with deep key", func(t *testing.T) {
+		expected := yaml.MapSlice{
+			{Key: "image", Value: yaml.MapSlice{
+				{Key: "attributes", Value: yaml.MapSlice{
+					{Key: "tag", Value: "v2.0.0"},
+				}},
+			}},
+		}
+
+		input := yaml.MapSlice{}
+
+		key := "image.attributes.tag"
+		value := "v2.0.0"
+
+		err := setHelmValue(&input, key, value)
+		require.NoError(t, err)
+		assert.Equal(t, expected, input)
 	})
 
 	t.Run("Unexpected type for key", func(t *testing.T) {
@@ -1909,7 +2078,7 @@ func Test_SetHelmValue(t *testing.T) {
 		key := "image.attributes.tag"
 		value := "v2.0.0"
 
-		err := setHelmValue(input, key, value)
+		err := setHelmValue(&input, key, value)
 		assert.Error(t, err)
 		assert.Equal(t, "unexpected type string for key attributes", err.Error())
 	})