Merge pull request #725 from askhari/fix/set-default-image-alias-with-helmvalues

fix: set default ImageAlias for Helm app type using helmvalues git write-back method

3 files changed, 108 insertions, 16 deletions

diff --git a/pkg/argocd/argocd.go b/pkg/argocd/argocd.go
index 4584dbe..2603903 100644
--- a/pkg/argocd/argocd.go
+++ b/pkg/argocd/argocd.go
@@ -309,29 +309,26 @@ func (client *argoCD) UpdateSpec(ctx context.Context, in *application.Applicatio
 // getHelmParamNamesFromAnnotation inspects the given annotations for whether
 // the annotations for specifying Helm parameter names are being set and
 // returns their values.
-func getHelmParamNamesFromAnnotation(annotations map[string]string, symbolicName string) (string, string) {
+func getHelmParamNamesFromAnnotation(annotations map[string]string, img *image.ContainerImage) (string, string) {
 	// Return default values without symbolic name given
-	if symbolicName == "" {
+	if img.ImageAlias == "" {
 		return "image.name", "image.tag"
 	}
 
 	var annotationName, helmParamName, helmParamVersion string
 
 	// Image spec is a full-qualified specifier, if we have it, we return early
-	annotationName = fmt.Sprintf(common.HelmParamImageSpecAnnotation, symbolicName)
-	if param, ok := annotations[annotationName]; ok {
+	if param := img.GetParameterHelmImageSpec(annotations); param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		return strings.TrimSpace(param), ""
 	}
 
-	annotationName = fmt.Sprintf(common.HelmParamImageNameAnnotation, symbolicName)
-	if param, ok := annotations[annotationName]; ok {
+	if param := img.GetParameterHelmImageName(annotations); param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		helmParamName = param
 	}
 
-	annotationName = fmt.Sprintf(common.HelmParamImageTagAnnotation, symbolicName)
-	if param, ok := annotations[annotationName]; ok {
+	if param := img.GetParameterHelmImageTag(annotations); param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		helmParamVersion = param
 	}
@@ -503,6 +500,24 @@ func GetImagesFromApplication(app *v1alpha1.Application) image.ContainerImageLis
 	return images
 }
 
+// GetImagesFromApplicationImagesAnnotation returns the list of known images for the given application from the images annotation
+func GetImagesAndAliasesFromApplication(app *v1alpha1.Application) image.ContainerImageList {
+	images := GetImagesFromApplication(app)
+
+	// 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
+			} else {
+				image.ImageAlias = img.ImageAlias
+			}
+		}
+	}
+
+	return images
+}
+
 // GetApplicationTypeByName first retrieves application with given appName and
 // returns its application type
 func GetApplicationTypeByName(client ArgoCD, appName string) (ApplicationType, error) {
diff --git a/pkg/argocd/argocd_test.go b/pkg/argocd/argocd_test.go
index 9555357..cbe5b07 100644
--- a/pkg/argocd/argocd_test.go
+++ b/pkg/argocd/argocd_test.go
@@ -79,6 +79,65 @@ func Test_GetImagesFromApplication(t *testing.T) {
 	})
 }
 
+func Test_GetImagesAndAliasesFromApplication(t *testing.T) {
+	t.Run("Get list of images from application", func(t *testing.T) {
+		application := &v1alpha1.Application{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "test-app",
+				Namespace: "argocd",
+			},
+			Spec: v1alpha1.ApplicationSpec{},
+			Status: v1alpha1.ApplicationStatus{
+				Summary: v1alpha1.ApplicationSummary{
+					Images: []string{"nginx:1.12.2", "that/image", "quay.io/dexidp/dex:v1.23.0"},
+				},
+			},
+		}
+		imageList := GetImagesAndAliasesFromApplication(application)
+		require.Len(t, imageList, 3)
+		assert.Equal(t, "nginx", imageList[0].ImageName)
+		assert.Equal(t, "that/image", imageList[1].ImageName)
+		assert.Equal(t, "dexidp/dex", imageList[2].ImageName)
+	})
+
+	t.Run("Get list of images and image aliases from application that has no images", func(t *testing.T) {
+		application := &v1alpha1.Application{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "test-app",
+				Namespace: "argocd",
+			},
+			Spec: v1alpha1.ApplicationSpec{},
+			Status: v1alpha1.ApplicationStatus{
+				Summary: v1alpha1.ApplicationSummary{},
+			},
+		}
+		imageList := GetImagesAndAliasesFromApplication(application)
+		assert.Empty(t, imageList)
+	})
+
+	t.Run("Get list of images and aliases from application annotations", func(t *testing.T) {
+		application := &v1alpha1.Application{
+			ObjectMeta: v1.ObjectMeta{
+				Name:      "test-app",
+				Namespace: "argocd",
+				Annotations: map[string]string{
+					common.ImageUpdaterAnnotation: "webserver=nginx",
+				},
+			},
+			Spec: v1alpha1.ApplicationSpec{},
+			Status: v1alpha1.ApplicationStatus{
+				Summary: v1alpha1.ApplicationSummary{
+					Images: []string{"nginx:1.12.2"},
+				},
+			},
+		}
+		imageList := GetImagesAndAliasesFromApplication(application)
+		require.Len(t, imageList, 1)
+		assert.Equal(t, "nginx", imageList[0].ImageName)
+		assert.Equal(t, "webserver", imageList[0].ImageAlias)
+	})
+}
+
 func Test_GetApplicationType(t *testing.T) {
 	t.Run("Get application of type Helm", func(t *testing.T) {
 		application := &v1alpha1.Application{
@@ -541,7 +600,9 @@ func Test_GetHelmParamAnnotations(t *testing.T) {
 			fmt.Sprintf(common.HelmParamImageSpecAnnotation, "myimg"): "image.blub",
 			fmt.Sprintf(common.HelmParamImageTagAnnotation, "myimg"):  "image.blab",
 		}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "",
+		})
 		assert.Equal(t, "image.name", name)
 		assert.Equal(t, "image.tag", tag)
 	})
@@ -551,7 +612,9 @@ func Test_GetHelmParamAnnotations(t *testing.T) {
 			fmt.Sprintf(common.HelmParamImageSpecAnnotation, "myimg"): "image.path",
 			fmt.Sprintf(common.HelmParamImageTagAnnotation, "myimg"):  "image.tag",
 		}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "myimg")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "myimg",
+		})
 		assert.Equal(t, "image.path", name)
 		assert.Empty(t, tag)
 	})
@@ -561,7 +624,9 @@ func Test_GetHelmParamAnnotations(t *testing.T) {
 			fmt.Sprintf(common.HelmParamImageNameAnnotation, "myimg"): "image.name",
 			fmt.Sprintf(common.HelmParamImageTagAnnotation, "myimg"):  "image.tag",
 		}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "myimg")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "myimg",
+		})
 		assert.Equal(t, "image.name", name)
 		assert.Equal(t, "image.tag", tag)
 	})
@@ -571,7 +636,9 @@ func Test_GetHelmParamAnnotations(t *testing.T) {
 			fmt.Sprintf(common.HelmParamImageNameAnnotation, "otherimg"): "image.name",
 			fmt.Sprintf(common.HelmParamImageTagAnnotation, "otherimg"):  "image.tag",
 		}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "myimg")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "myimg",
+		})
 		assert.Empty(t, name)
 		assert.Empty(t, tag)
 	})
@@ -580,14 +647,18 @@ func Test_GetHelmParamAnnotations(t *testing.T) {
 		annotations := map[string]string{
 			fmt.Sprintf(common.HelmParamImageTagAnnotation, "myimg"): "image.tag",
 		}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "myimg")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "myimg",
+		})
 		assert.Empty(t, name)
 		assert.Equal(t, "image.tag", tag)
 	})
 
 	t.Run("No suitable annotations found", func(t *testing.T) {
 		annotations := map[string]string{}
-		name, tag := getHelmParamNamesFromAnnotation(annotations, "myimg")
+		name, tag := getHelmParamNamesFromAnnotation(annotations, &image.ContainerImage{
+			ImageAlias: "myimg",
+		})
 		assert.Empty(t, name)
 		assert.Empty(t, tag)
 	})
diff --git a/pkg/argocd/update.go b/pkg/argocd/update.go
index fd1463e..0f1cab2 100644
--- a/pkg/argocd/update.go
+++ b/pkg/argocd/update.go
@@ -416,10 +416,16 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 		}
 
 		if strings.HasPrefix(app.Annotations[common.WriteBackTargetAnnotation], common.HelmPrefix) {
-			images := GetImagesFromApplication(app)
+			images := GetImagesAndAliasesFromApplication(app)
 
 			for _, c := range images {
-				helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c.ImageName)
+
+				if c.ImageAlias == "" {
+					continue
+				}
+
+				helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c)
+
 				if helmAnnotationParamName == "" {
 					return nil, fmt.Errorf("could not find an image-name annotation for image %s", c.ImageName)
 				}