fix: Version override in ArgoCD CR causes operator to use upstream images

controllers/argocd/applicationset.go

@@ -855,30 +855,7 @@ func (r *ReconcileArgoCD) reconcileApplicationSetRoleBinding(cr *argoproj.ArgoCD
 }
 
 func getApplicationSetContainerImage(cr *argoproj.ArgoCD) string {
-
-	defaultImg, defaultTag := false, false
-	img := cr.Spec.ApplicationSet.Image
-	if img == "" {
-		img = cr.Spec.Image
-		if img == "" {
-			img = common.ArgoCDDefaultArgoImage
-			defaultImg = true
-		}
-	}
-
-	tag := cr.Spec.ApplicationSet.Version
-	if tag == "" {
-		tag = cr.Spec.Version
-		if tag == "" {
-			tag = common.ArgoCDDefaultArgoVersion
-			defaultTag = true
-		}
-	}
-
-	// If an env var is specified then use that, but don't override the spec values (if they are present)
-	if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
-		return e
-	}
+	img, tag := GetImageAndTag(common.ArgoCDImageEnvName, cr.Spec.ApplicationSet.Image, cr.Spec.ApplicationSet.Version, cr.Spec.Image, cr.Spec.Version)
 	return argoutil.CombineImageTag(img, tag)
 }
 

controllers/argocd/applicationset_test.go

@@ -472,8 +472,8 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {
 		{
 			name:                   "verify env var substitution overrides default",
 			appSetField:            &argoproj.ArgoCDApplicationSet{},
-			envVars:                map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
-			expectedContainerImage: "custom-env-image",
+			envVars:                map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
+			expectedContainerImage: "docker.io/library/ubuntu:latest",
 		},
 
 		{
@@ -482,16 +482,16 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {
 				Image:   "custom-image",
 				Version: "custom-version",
 			},
-			envVars:                map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
+			envVars:                map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
 			expectedContainerImage: "custom-image:custom-version",
 		},
 		{
 			name: "ensure scm tls cert mount is present",
 			appSetField: &argoproj.ArgoCDApplicationSet{
 				SCMRootCAConfigMap: "test-scm-tls-mount",
 			},
-			envVars:                map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
-			expectedContainerImage: "custom-env-image",
+			envVars:                map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
+			expectedContainerImage: "docker.io/library/ubuntu:latest",
 		},
 	}
 
@@ -1349,3 +1349,105 @@ func TestArgoCDApplicationSet_removeUnmanagedApplicationSetSourceNamespaceResour
 	assert.True(t, found)
 	assert.Equal(t, a.Namespace, val)
 }
+
+func TestGetApplicationSetContainerImage(t *testing.T) {
+	logf.SetLogger(ZapLogger(true))
+
+	// when env var is set and spec fields are not set, env var should be returned
+	cr := argoproj.ArgoCD{}
+	cr.Spec = argoproj.ArgoCDSpec{}
+	cr.Spec.ApplicationSet = &argoproj.ArgoCDApplicationSet{}
+	os.Setenv(common.ArgoCDImageEnvName, "testingimage@sha:123456")
+	out := getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "testingimage@sha:123456", out)
+
+	// when env var is set and also spec image and version fields are set, spec fields should be returned
+	cr.Spec.Image = "customimage"
+	cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "customimage@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
+
+	// when spec.image and spec.applicationset.image is passed and also env is passed, container level image should take priority
+	cr.Spec.Image = "customimage"
+	cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
+	cr.Spec.ApplicationSet.Image = "containerImage"
+	cr.Spec.ApplicationSet.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b"
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2c")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "containerImage@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b", out)
+
+	// when env var is set and also spec version field is set but image field is not set, should return env var image with spec version
+	cr.Spec.Image = ""
+	cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
+	cr.Spec.ApplicationSet.Image = ""
+	cr.Spec.ApplicationSet.Version = ""
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
+
+	// when env var in wrong format is set and also spec version field is set but image field is not set
+	cr.Spec.Image = ""
+	cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
+
+	cr.Spec.Image = ""
+	cr.Spec.Version = ""
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
+
+	cr.Spec.Image = ""
+	cr.Spec.Version = ""
+	os.Setenv(common.ArgoCDImageEnvName, "docker.io/library/ubuntu")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "docker.io/library/ubuntu", out)
+
+	cr.Spec.Image = ""
+	cr.Spec.Version = "v0.0.1"
+	os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/project/registry:v0.0.1", out)
+
+	cr.Spec.Image = ""
+	cr.Spec.Version = "v0.0.1"
+	os.Setenv(common.ArgoCDImageEnvName, "docker.io/library/ubuntu")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "docker.io/library/ubuntu:v0.0.1", out)
+
+	cr.Spec.Image = ""
+	cr.Spec.Version = "v0.0.1"
+	os.Setenv(common.ArgoCDImageEnvName, "ubuntu")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "ubuntu:v0.0.1", out)
+
+	// when env var is not set and spec image and version fields are not set, default image should be returned
+	os.Setenv(common.ArgoCDImageEnvName, "")
+	cr.Spec.Image = ""
+	cr.Spec.Version = ""
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/argoproj/argocd@sha256:2815b045273dc14b271c40d01a75ae2efa88613c90300e778d5ad5171e2b0f7f", out)
+
+	// when env var is not set and spec image and version fields are set, spec fields should be returned
+	cr.Spec.Image = "customimage"
+	cr.Spec.Version = "sha256:1234567890abcdef"
+	os.Setenv(common.ArgoCDImageEnvName, "")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "customimage@sha256:1234567890abcdef", out)
+
+	// when env var is not set and spec version field is set but image field is not set, should return default image with spec version tag
+	cr.Spec.Image = ""
+	cr.Spec.Version = "customversion"
+	os.Setenv(common.ArgoCDImageEnvName, "")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "quay.io/argoproj/argocd:customversion", out)
+
+	// when env var is not set and spec image field is set but version field is not set, should return spec image with default tag
+	cr.Spec.Image = "customimage"
+	cr.Spec.Version = ""
+	os.Setenv(common.ArgoCDImageEnvName, "")
+	out = getApplicationSetContainerImage(&cr)
+	assert.Equal(t, "customimage@sha256:2815b045273dc14b271c40d01a75ae2efa88613c90300e778d5ad5171e2b0f7f", out)
+}

controllers/argocd/util.go

@@ -17,9 +17,12 @@ package argocd
 import (
 	"bytes"
 	"context"
+	"crypto"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/base64"
 	"fmt"
+	"hash"
 	"os"
 	"reflect"
 	"sort"
@@ -30,6 +33,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 
 	"github.com/argoproj/argo-cd/v3/util/glob"
+	"github.com/distribution/reference"
 	"github.com/go-logr/logr"
 
 	"github.com/argoproj-labs/argocd-operator/api/v1alpha1"
@@ -192,22 +196,7 @@ func getArgoApplicationControllerCommand(cr *argoproj.ArgoCD, useTLSForRedis boo
 
 // getArgoContainerImage will return the container image for ArgoCD.
 func getArgoContainerImage(cr *argoproj.ArgoCD) string {
-	defaultTag, defaultImg := false, false
-	img := cr.Spec.Image
-	if img == "" {
-		img = common.ArgoCDDefaultArgoImage
-		defaultImg = true
-	}
-
-	tag := cr.Spec.Version
-	if tag == "" {
-		tag = common.ArgoCDDefaultArgoVersion
-		defaultTag = true
-	}
-	if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
-		return e
-	}
-
+	img, tag := GetImageAndTag(common.ArgoCDImageEnvName, "", "", cr.Spec.Image, cr.Spec.Version)
 	return argoutil.CombineImageTag(img, tag)
 }
 
@@ -225,28 +214,70 @@ func getArgoContainerImage(cr *argoproj.ArgoCD) string {
 // 4. the default is configured in common.ArgoCDDefaultArgoVersion and
 // common.ArgoCDDefaultArgoImage.
 func getRepoServerContainerImage(cr *argoproj.ArgoCD) string {
-	defaultImg, defaultTag := false, false
-	img := cr.Spec.Repo.Image
-	if img == "" {
-		img = cr.Spec.Image
-		if img == "" {
-			img = common.ArgoCDDefaultArgoImage
-			defaultImg = true
-		}
+	img, tag := GetImageAndTag(common.ArgoCDImageEnvName, cr.Spec.Repo.Image, cr.Spec.Repo.Version, cr.Spec.Image, cr.Spec.Version)
+	return argoutil.CombineImageTag(img, tag)
+}
+
+// GetImageAndTag determines the image and tag for an ArgoCD component, considering container-level and common-level overrides.
+// Priority order: containerSpec > commonSpec > environment variable > defaults
+// Returns: image, tag
+func GetImageAndTag(envVar, containerSpecImage, containerSpecVersion, commonSpecImage, commonSpecVersion string) (string, string) {
+	// Start with defaults
+	image := common.ArgoCDDefaultArgoImage
+	tag := common.ArgoCDDefaultArgoVersion
+
+	// Check if environment variable is set
+	envVal := os.Getenv(envVar)
+
+	// If no spec values are provided and env var is set, use env var as-is
+	if envVal != "" && containerSpecImage == "" && commonSpecImage == "" &&
+		containerSpecVersion == "" && commonSpecVersion == "" {
+		return envVal, ""
 	}
 
-	tag := cr.Spec.Repo.Version
-	if tag == "" {
-		tag = cr.Spec.Version
-		if tag == "" {
-			tag = common.ArgoCDDefaultArgoVersion
-			defaultTag = true
+	// Parse environment variable image if it exists and we need to extract the base image name
+	if envVal != "" {
+		baseImageName, err := extractBaseImageName(envVal)
+		if err != nil {
+			log.Error(err, "Failed to parse environment variable image", "envVal", envVal)
+			return "", ""
+		}
+		if baseImageName != "" {
+			image = baseImageName
 		}
 	}
-	if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
-		return e
+
+	// Apply spec overrides with container spec taking precedence over common spec
+	image = getPriorityValue(containerSpecImage, commonSpecImage, image)
+	tag = getPriorityValue(containerSpecVersion, commonSpecVersion, tag)
+
+	return image, tag
+}
+
+// extractBaseImageName extracts the base image name from a full image reference (removing tag/digest)
+func extractBaseImageName(imageRef string) (string, error) {
+	// Handle digest format (image@sha256:...)
+	crypto.RegisterHash(crypto.SHA256, func() hash.Hash { return sha256.New() })
+	ref, err := reference.Parse(imageRef)
+	if err != nil {
+		return "", err
 	}
-	return argoutil.CombineImageTag(img, tag)
+	var name string
+	if named, ok := ref.(reference.Named); ok {
+		name = named.Name()
+	}
+	return name, nil
+}
+
+// getPriorityValue returns the highest priority value from container spec, common spec, or fallback
+func getPriorityValue(containerLevelSpec, commonLevelSpec, fallback string) string {
+	if containerLevelSpec != "" {
+		return containerLevelSpec
+	}
+	if commonLevelSpec != "" {
+		return commonLevelSpec
+	}
+	return fallback
 }
 
 // getArgoRepoResources will return the ResourceRequirements for the Argo CD Repo server container.