Skip to content

Commit 19988fe

Browse files
akhilnittalaakhilnittala
authored
fix: Version override in ArgoCD CR causes operator to use upstream images (#1915)
Signed-off-by: akhil nittala <nakhil@redhat.com>
1 parent c28c9ce commit 19988fe

File tree

3 files changed

+172
-62
lines changed

3 files changed

+172
-62
lines changed

‎controllers/argocd/applicationset.go‎

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -855,30 +855,7 @@ func (r *ReconcileArgoCD) reconcileApplicationSetRoleBinding(cr *argoproj.ArgoCD
855855
}
856856

857857
func getApplicationSetContainerImage(cr *argoproj.ArgoCD) string {
858-
859-
defaultImg, defaultTag := false, false
860-
img := cr.Spec.ApplicationSet.Image
861-
if img == "" {
862-
img = cr.Spec.Image
863-
if img == "" {
864-
img = common.ArgoCDDefaultArgoImage
865-
defaultImg = true
866-
}
867-
}
868-
869-
tag := cr.Spec.ApplicationSet.Version
870-
if tag == "" {
871-
tag = cr.Spec.Version
872-
if tag == "" {
873-
tag = common.ArgoCDDefaultArgoVersion
874-
defaultTag = true
875-
}
876-
}
877-
878-
// If an env var is specified then use that, but don't override the spec values (if they are present)
879-
if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
880-
return e
881-
}
858+
img, tag := GetImageAndTag(common.ArgoCDImageEnvName, cr.Spec.ApplicationSet.Image, cr.Spec.ApplicationSet.Version, cr.Spec.Image, cr.Spec.Version)
882859
return argoutil.CombineImageTag(img, tag)
883860
}
884861

‎controllers/argocd/applicationset_test.go‎

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,8 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {
472472
{
473473
name: "verify env var substitution overrides default",
474474
appSetField: &argoproj.ArgoCDApplicationSet{},
475-
envVars: map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
476-
expectedContainerImage: "custom-env-image",
475+
envVars: map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
476+
expectedContainerImage: "docker.io/library/ubuntu:latest",
477477
},
478478

479479
{
@@ -482,16 +482,16 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {
482482
Image: "custom-image",
483483
Version: "custom-version",
484484
},
485-
envVars: map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
485+
envVars: map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
486486
expectedContainerImage: "custom-image:custom-version",
487487
},
488488
{
489489
name: "ensure scm tls cert mount is present",
490490
appSetField: &argoproj.ArgoCDApplicationSet{
491491
SCMRootCAConfigMap: "test-scm-tls-mount",
492492
},
493-
envVars: map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
494-
expectedContainerImage: "custom-env-image",
493+
envVars: map[string]string{common.ArgoCDImageEnvName: "docker.io/library/ubuntu:latest"},
494+
expectedContainerImage: "docker.io/library/ubuntu:latest",
495495
},
496496
}
497497

@@ -1349,3 +1349,105 @@ func TestArgoCDApplicationSet_removeUnmanagedApplicationSetSourceNamespaceResour
13491349
assert.True(t, found)
13501350
assert.Equal(t, a.Namespace, val)
13511351
}
1352+
1353+
func TestGetApplicationSetContainerImage(t *testing.T) {
1354+
logf.SetLogger(ZapLogger(true))
1355+
1356+
// when env var is set and spec fields are not set, env var should be returned
1357+
cr := argoproj.ArgoCD{}
1358+
cr.Spec = argoproj.ArgoCDSpec{}
1359+
cr.Spec.ApplicationSet = &argoproj.ArgoCDApplicationSet{}
1360+
os.Setenv(common.ArgoCDImageEnvName, "testingimage@sha:123456")
1361+
out := getApplicationSetContainerImage(&cr)
1362+
assert.Equal(t, "testingimage@sha:123456", out)
1363+
1364+
// when env var is set and also spec image and version fields are set, spec fields should be returned
1365+
cr.Spec.Image = "customimage"
1366+
cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
1367+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
1368+
out = getApplicationSetContainerImage(&cr)
1369+
assert.Equal(t, "customimage@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
1370+
1371+
// when spec.image and spec.applicationset.image is passed and also env is passed, container level image should take priority
1372+
cr.Spec.Image = "customimage"
1373+
cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
1374+
cr.Spec.ApplicationSet.Image = "containerImage"
1375+
cr.Spec.ApplicationSet.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b"
1376+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2c")
1377+
out = getApplicationSetContainerImage(&cr)
1378+
assert.Equal(t, "containerImage@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b", out)
1379+
1380+
// 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
1381+
cr.Spec.Image = ""
1382+
cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
1383+
cr.Spec.ApplicationSet.Image = ""
1384+
cr.Spec.ApplicationSet.Version = ""
1385+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2b")
1386+
out = getApplicationSetContainerImage(&cr)
1387+
assert.Equal(t, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
1388+
1389+
// when env var in wrong format is set and also spec version field is set but image field is not set
1390+
cr.Spec.Image = ""
1391+
cr.Spec.Version = "sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a"
1392+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest")
1393+
out = getApplicationSetContainerImage(&cr)
1394+
assert.Equal(t, "quay.io/project/registry@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
1395+
1396+
cr.Spec.Image = ""
1397+
cr.Spec.Version = ""
1398+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
1399+
out = getApplicationSetContainerImage(&cr)
1400+
assert.Equal(t, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a", out)
1401+
1402+
cr.Spec.Image = ""
1403+
cr.Spec.Version = ""
1404+
os.Setenv(common.ArgoCDImageEnvName, "docker.io/library/ubuntu")
1405+
out = getApplicationSetContainerImage(&cr)
1406+
assert.Equal(t, "docker.io/library/ubuntu", out)
1407+
1408+
cr.Spec.Image = ""
1409+
cr.Spec.Version = "v0.0.1"
1410+
os.Setenv(common.ArgoCDImageEnvName, "quay.io/project/registry:latest@sha256:7e0aa2f42232f6b2f0a9d5f98b2e3a9a6b8c9b7f3a4c1d2e5f6a7b8c9d0e1f2a")
1411+
out = getApplicationSetContainerImage(&cr)
1412+
assert.Equal(t, "quay.io/project/registry:v0.0.1", out)
1413+
1414+
cr.Spec.Image = ""
1415+
cr.Spec.Version = "v0.0.1"
1416+
os.Setenv(common.ArgoCDImageEnvName, "docker.io/library/ubuntu")
1417+
out = getApplicationSetContainerImage(&cr)
1418+
assert.Equal(t, "docker.io/library/ubuntu:v0.0.1", out)
1419+
1420+
cr.Spec.Image = ""
1421+
cr.Spec.Version = "v0.0.1"
1422+
os.Setenv(common.ArgoCDImageEnvName, "ubuntu")
1423+
out = getApplicationSetContainerImage(&cr)
1424+
assert.Equal(t, "ubuntu:v0.0.1", out)
1425+
1426+
// when env var is not set and spec image and version fields are not set, default image should be returned
1427+
os.Setenv(common.ArgoCDImageEnvName, "")
1428+
cr.Spec.Image = ""
1429+
cr.Spec.Version = ""
1430+
out = getApplicationSetContainerImage(&cr)
1431+
assert.Equal(t, "quay.io/argoproj/argocd@sha256:2815b045273dc14b271c40d01a75ae2efa88613c90300e778d5ad5171e2b0f7f", out)
1432+
1433+
// when env var is not set and spec image and version fields are set, spec fields should be returned
1434+
cr.Spec.Image = "customimage"
1435+
cr.Spec.Version = "sha256:1234567890abcdef"
1436+
os.Setenv(common.ArgoCDImageEnvName, "")
1437+
out = getApplicationSetContainerImage(&cr)
1438+
assert.Equal(t, "customimage@sha256:1234567890abcdef", out)
1439+
1440+
// 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
1441+
cr.Spec.Image = ""
1442+
cr.Spec.Version = "customversion"
1443+
os.Setenv(common.ArgoCDImageEnvName, "")
1444+
out = getApplicationSetContainerImage(&cr)
1445+
assert.Equal(t, "quay.io/argoproj/argocd:customversion", out)
1446+
1447+
// 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
1448+
cr.Spec.Image = "customimage"
1449+
cr.Spec.Version = ""
1450+
os.Setenv(common.ArgoCDImageEnvName, "")
1451+
out = getApplicationSetContainerImage(&cr)
1452+
assert.Equal(t, "customimage@sha256:2815b045273dc14b271c40d01a75ae2efa88613c90300e778d5ad5171e2b0f7f", out)
1453+
}

‎controllers/argocd/util.go‎

Lines changed: 64 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,12 @@ package argocd
1717
import (
1818
"bytes"
1919
"context"
20+
"crypto"
2021
"crypto/rand"
22+
"crypto/sha256"
2123
"encoding/base64"
2224
"fmt"
25+
"hash"
2326
"os"
2427
"reflect"
2528
"sort"
@@ -30,6 +33,7 @@ import (
3033
"sigs.k8s.io/controller-runtime/pkg/builder"
3134

3235
"github.com/argoproj/argo-cd/v3/util/glob"
36+
"github.com/distribution/reference"
3337
"github.com/go-logr/logr"
3438

3539
"github.com/argoproj-labs/argocd-operator/api/v1alpha1"
@@ -192,22 +196,7 @@ func getArgoApplicationControllerCommand(cr *argoproj.ArgoCD, useTLSForRedis boo
192196

193197
// getArgoContainerImage will return the container image for ArgoCD.
194198
func getArgoContainerImage(cr *argoproj.ArgoCD) string {
195-
defaultTag, defaultImg := false, false
196-
img := cr.Spec.Image
197-
if img == "" {
198-
img = common.ArgoCDDefaultArgoImage
199-
defaultImg = true
200-
}
201-
202-
tag := cr.Spec.Version
203-
if tag == "" {
204-
tag = common.ArgoCDDefaultArgoVersion
205-
defaultTag = true
206-
}
207-
if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
208-
return e
209-
}
210-
199+
img, tag := GetImageAndTag(common.ArgoCDImageEnvName, "", "", cr.Spec.Image, cr.Spec.Version)
211200
return argoutil.CombineImageTag(img, tag)
212201
}
213202

@@ -225,28 +214,70 @@ func getArgoContainerImage(cr *argoproj.ArgoCD) string {
225214
// 4. the default is configured in common.ArgoCDDefaultArgoVersion and
226215
// common.ArgoCDDefaultArgoImage.
227216
func getRepoServerContainerImage(cr *argoproj.ArgoCD) string {
228-
defaultImg, defaultTag := false, false
229-
img := cr.Spec.Repo.Image
230-
if img == "" {
231-
img = cr.Spec.Image
232-
if img == "" {
233-
img = common.ArgoCDDefaultArgoImage
234-
defaultImg = true
235-
}
217+
img, tag := GetImageAndTag(common.ArgoCDImageEnvName, cr.Spec.Repo.Image, cr.Spec.Repo.Version, cr.Spec.Image, cr.Spec.Version)
218+
return argoutil.CombineImageTag(img, tag)
219+
}
220+
221+
// GetImageAndTag determines the image and tag for an ArgoCD component, considering container-level and common-level overrides.
222+
// Priority order: containerSpec > commonSpec > environment variable > defaults
223+
// Returns: image, tag
224+
func GetImageAndTag(envVar, containerSpecImage, containerSpecVersion, commonSpecImage, commonSpecVersion string) (string, string) {
225+
// Start with defaults
226+
image := common.ArgoCDDefaultArgoImage
227+
tag := common.ArgoCDDefaultArgoVersion
228+
229+
// Check if environment variable is set
230+
envVal := os.Getenv(envVar)
231+
232+
// If no spec values are provided and env var is set, use env var as-is
233+
if envVal != "" && containerSpecImage == "" && commonSpecImage == "" &&
234+
containerSpecVersion == "" && commonSpecVersion == "" {
235+
return envVal, ""
236236
}
237237

238-
tag := cr.Spec.Repo.Version
239-
if tag == "" {
240-
tag = cr.Spec.Version
241-
if tag == "" {
242-
tag = common.ArgoCDDefaultArgoVersion
243-
defaultTag = true
238+
// Parse environment variable image if it exists and we need to extract the base image name
239+
if envVal != "" {
240+
baseImageName, err := extractBaseImageName(envVal)
241+
if err != nil {
242+
log.Error(err, "Failed to parse environment variable image", "envVal", envVal)
243+
return "", ""
244+
}
245+
if baseImageName != "" {
246+
image = baseImageName
244247
}
245248
}
246-
if e := os.Getenv(common.ArgoCDImageEnvName); e != "" && (defaultTag && defaultImg) {
247-
return e
249+
250+
// Apply spec overrides with container spec taking precedence over common spec
251+
image = getPriorityValue(containerSpecImage, commonSpecImage, image)
252+
tag = getPriorityValue(containerSpecVersion, commonSpecVersion, tag)
253+
254+
return image, tag
255+
}
256+
257+
// extractBaseImageName extracts the base image name from a full image reference (removing tag/digest)
258+
func extractBaseImageName(imageRef string) (string, error) {
259+
// Handle digest format (image@sha256:...)
260+
crypto.RegisterHash(crypto.SHA256, func() hash.Hash { return sha256.New() })
261+
ref, err := reference.Parse(imageRef)
262+
if err != nil {
263+
return "", err
248264
}
249-
return argoutil.CombineImageTag(img, tag)
265+
var name string
266+
if named, ok := ref.(reference.Named); ok {
267+
name = named.Name()
268+
}
269+
return name, nil
270+
}
271+
272+
// getPriorityValue returns the highest priority value from container spec, common spec, or fallback
273+
func getPriorityValue(containerLevelSpec, commonLevelSpec, fallback string) string {
274+
if containerLevelSpec != "" {
275+
return containerLevelSpec
276+
}
277+
if commonLevelSpec != "" {
278+
return commonLevelSpec
279+
}
280+
return fallback
250281
}
251282

252283
// getArgoRepoResources will return the ResourceRequirements for the Argo CD Repo server container.

0 commit comments

Comments
 (0)