fix: getHelmParamNamesFromAnnotation should take info from container image for prevent code duplication

Signed-off-by: Pasha Kostohrys <pavel@codefresh.io>

4 files changed, 39 insertions, 27 deletions

diff --git a/pkg/argocd/argocd.go b/pkg/argocd/argocd.go
index e3f7063..1a389e3 100644
--- a/pkg/argocd/argocd.go
+++ b/pkg/argocd/argocd.go
@@ -309,29 +309,29 @@ 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 {
+	param := img.GetParameterHelmImageSpec(annotations)
+	if param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		return strings.TrimSpace(param), ""
 	}
 
-	annotationName = fmt.Sprintf(common.HelmParamImageNameAnnotation, symbolicName)
-	if param, ok := annotations[annotationName]; ok {
+	param = img.GetParameterHelmImageName(annotations)
+	if param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		helmParamName = param
 	}
 
-	annotationName = fmt.Sprintf(common.HelmParamImageTagAnnotation, symbolicName)
-	if param, ok := annotations[annotationName]; ok {
+	param = img.GetParameterHelmImageTag(annotations)
+	if param != "" {
 		log.Tracef("found annotation %s", annotationName)
 		helmParamVersion = param
 	}
diff --git a/pkg/argocd/argocd_test.go b/pkg/argocd/argocd_test.go
index 84df479..cbe5b07 100644
--- a/pkg/argocd/argocd_test.go
+++ b/pkg/argocd/argocd_test.go
@@ -600,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)
 	})
@@ -610,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)
 	})
@@ -620,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)
 	})
@@ -630,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)
 	})
@@ -639,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 a2c5c25..85dee36 100644
--- a/pkg/argocd/update.go
+++ b/pkg/argocd/update.go
@@ -419,7 +419,7 @@ func marshalParamsOverride(app *v1alpha1.Application, originalData []byte) ([]by
 			images := GetImagesAndAliasesFromApplication(app)
 
 			for _, c := range images {
-				helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c.NormalizedSymbolicName())
+				helmAnnotationParamName, helmAnnotationParamVersion := getHelmParamNamesFromAnnotation(app.Annotations, c)
 
 				if helmAnnotationParamName == "" {
 					return nil, fmt.Errorf("could not find an image-name annotation for image %s", c.ImageAlias)
diff --git a/pkg/image/options.go b/pkg/image/options.go
index 21aae15..af6d12a 100644
--- a/pkg/image/options.go
+++ b/pkg/image/options.go
@@ -13,7 +13,7 @@ import (
 // GetParameterHelmImageName gets the value for image-name option for the image
 // from a set of annotations
 func (img *ContainerImage) GetParameterHelmImageName(annotations map[string]string) string {
-	key := fmt.Sprintf(common.HelmParamImageNameAnnotation, img.NormalizedSymbolicName())
+	key := fmt.Sprintf(common.HelmParamImageNameAnnotation, img.normalizedSymbolicName())
 	val, ok := annotations[key]
 	if !ok {
 		return ""
@@ -24,7 +24,7 @@ func (img *ContainerImage) GetParameterHelmImageName(annotations map[string]stri
 // GetParameterHelmImageTag gets the value for image-tag option for the image
 // from a set of annotations
 func (img *ContainerImage) GetParameterHelmImageTag(annotations map[string]string) string {
-	key := fmt.Sprintf(common.HelmParamImageTagAnnotation, img.NormalizedSymbolicName())
+	key := fmt.Sprintf(common.HelmParamImageTagAnnotation, img.normalizedSymbolicName())
 	val, ok := annotations[key]
 	if !ok {
 		return ""
@@ -35,7 +35,7 @@ func (img *ContainerImage) GetParameterHelmImageTag(annotations map[string]strin
 // GetParameterHelmImageSpec gets the value for image-spec option for the image
 // from a set of annotations
 func (img *ContainerImage) GetParameterHelmImageSpec(annotations map[string]string) string {
-	key := fmt.Sprintf(common.HelmParamImageSpecAnnotation, img.NormalizedSymbolicName())
+	key := fmt.Sprintf(common.HelmParamImageSpecAnnotation, img.normalizedSymbolicName())
 	val, ok := annotations[key]
 	if !ok {
 		return ""
@@ -46,7 +46,7 @@ func (img *ContainerImage) GetParameterHelmImageSpec(annotations map[string]stri
 // GetParameterKustomizeImageName gets the value for image-spec option for the
 // image from a set of annotations
 func (img *ContainerImage) GetParameterKustomizeImageName(annotations map[string]string) string {
-	key := fmt.Sprintf(common.KustomizeApplicationNameAnnotation, img.NormalizedSymbolicName())
+	key := fmt.Sprintf(common.KustomizeApplicationNameAnnotation, img.normalizedSymbolicName())
 	val, ok := annotations[key]
 	if !ok {
 		return ""
@@ -58,7 +58,7 @@ func (img *ContainerImage) GetParameterKustomizeImageName(annotations map[string
 // image from a set of annotations
 func (img *ContainerImage) HasForceUpdateOptionAnnotation(annotations map[string]string) bool {
 	forceUpdateAnnotations := []string{
-		fmt.Sprintf(common.ForceUpdateOptionAnnotation, img.NormalizedSymbolicName()),
+		fmt.Sprintf(common.ForceUpdateOptionAnnotation, img.normalizedSymbolicName()),
 		common.ApplicationWideForceUpdateOptionAnnotation,
 	}
 	var forceUpdateVal = ""
@@ -75,7 +75,7 @@ func (img *ContainerImage) HasForceUpdateOptionAnnotation(annotations map[string
 // image from a set of annotations
 func (img *ContainerImage) GetParameterUpdateStrategy(annotations map[string]string) UpdateStrategy {
 	updateStrategyAnnotations := []string{
-		fmt.Sprintf(common.UpdateStrategyAnnotation, img.NormalizedSymbolicName()),
+		fmt.Sprintf(common.UpdateStrategyAnnotation, img.normalizedSymbolicName()),
 		common.ApplicationWideUpdateStrategyAnnotation,
 	}
 	var updateStrategyVal = ""
@@ -123,7 +123,7 @@ func (img *ContainerImage) ParseUpdateStrategy(val string) UpdateStrategy {
 // default, to prevent accidental matches.
 func (img *ContainerImage) GetParameterMatch(annotations map[string]string) (MatchFuncFn, interface{}) {
 	allowTagsAnnotations := []string{
-		fmt.Sprintf(common.AllowTagsOptionAnnotation, img.NormalizedSymbolicName()),
+		fmt.Sprintf(common.AllowTagsOptionAnnotation, img.normalizedSymbolicName()),
 		common.ApplicationWideAllowTagsOptionAnnotation,
 	}
 	var allowTagsVal = ""
@@ -137,7 +137,7 @@ func (img *ContainerImage) GetParameterMatch(annotations map[string]string) (Mat
 	if allowTagsVal == "" {
 		// The old match-tag annotation is deprecated and will be subject to removal
 		// in a future version.
-		key := fmt.Sprintf(common.OldMatchOptionAnnotation, img.NormalizedSymbolicName())
+		key := fmt.Sprintf(common.OldMatchOptionAnnotation, img.normalizedSymbolicName())
 		val, ok := annotations[key]
 		if ok {
 			logCtx.Warnf("The 'tag-match' annotation is deprecated and subject to removal. Please use 'allow-tags' annotation instead.")
@@ -182,7 +182,7 @@ func (img *ContainerImage) ParseMatchfunc(val string) (MatchFuncFn, interface{})
 // GetParameterPullSecret retrieves an image's pull secret credentials
 func (img *ContainerImage) GetParameterPullSecret(annotations map[string]string) *CredentialSource {
 	pullSecretAnnotations := []string{
-		fmt.Sprintf(common.PullSecretAnnotation, img.NormalizedSymbolicName()),
+		fmt.Sprintf(common.PullSecretAnnotation, img.normalizedSymbolicName()),
 		common.ApplicationWidePullSecretAnnotation,
 	}
 	var pullSecretVal = ""
@@ -208,7 +208,7 @@ func (img *ContainerImage) GetParameterPullSecret(annotations map[string]string)
 // GetParameterIgnoreTags retrieves a list of tags to ignore from a comma-separated string
 func (img *ContainerImage) GetParameterIgnoreTags(annotations map[string]string) []string {
 	ignoreTagsAnnotations := []string{
-		fmt.Sprintf(common.IgnoreTagsOptionAnnotation, img.NormalizedSymbolicName()),
+		fmt.Sprintf(common.IgnoreTagsOptionAnnotation, img.normalizedSymbolicName()),
 		common.ApplicationWideIgnoreTagsOptionAnnotation,
 	}
 	var ignoreTagsVal = ""
@@ -242,7 +242,7 @@ func (img *ContainerImage) GetParameterIgnoreTags(annotations map[string]string)
 func (img *ContainerImage) GetPlatformOptions(annotations map[string]string, unrestricted bool) *options.ManifestOptions {
 	logCtx := img.LogContext()
 	var opts *options.ManifestOptions = options.NewManifestOptions()
-	key := fmt.Sprintf(common.PlatformsAnnotation, img.NormalizedSymbolicName())
+	key := fmt.Sprintf(common.PlatformsAnnotation, img.normalizedSymbolicName())
 	val, ok := annotations[key]
 	if !ok {
 		if !unrestricted {
@@ -291,6 +291,6 @@ func ParsePlatform(platformID string) (string, string, string, error) {
 	return os, arch, variant, nil
 }
 
-func (img *ContainerImage) NormalizedSymbolicName() string {
+func (img *ContainerImage) normalizedSymbolicName() string {
 	return strings.ReplaceAll(img.ImageAlias, "/", "_")
 }