Skip to content

Commit 7f928e5

Browse files
akhilnittalajgwest
andauthored
fix: Version override in ArgoCD Rollouts causes operator to use upstream images (#126)
* fix: version override image in argocd rollouts manager Signed-off-by: akhil nittala <[email protected]> * fix: Version override in ArgoCD Rollout causes operator to use upstream images Signed-off-by: akhil nittala <[email protected]> * fix: Version override in ArgoCD Rollout causes operator to use upstream images Signed-off-by: akhil nittala <[email protected]> * fix: Version override in ArgoCD Rollout causes operator to use upstream images Signed-off-by: akhil nittala <[email protected]> * fix: Version override in ArgoCD Rollout causes operator to use upstream images Signed-off-by: Jonathan West <[email protected]> --------- Signed-off-by: akhil nittala <[email protected]> Signed-off-by: Jonathan West <[email protected]> Co-authored-by: Jonathan West <[email protected]>
1 parent c01a15f commit 7f928e5

File tree

6 files changed

+149
-30
lines changed

6 files changed

+149
-30
lines changed

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
# Build the manager binary
2-
FROM golang:1.21 AS builder
2+
FROM golang:1.24 AS builder
33
ARG TARGETOS
44
ARG TARGETARCH
55

Makefile

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
# - use environment variables to overwrite this value (e.g export VERSION=0.0.2)
66
VERSION ?= 0.0.1
77

8+
# Try to detect Docker or Podman
9+
CONTAINER_RUNTIME := $(shell command -v docker 2> /dev/null || command -v podman 2> /dev/null)
10+
11+
# If neither Docker nor Podman is found, print an error message and exit
12+
ifeq ($(CONTAINER_RUNTIME),)
13+
$(warning "No container runtime (Docker or Podman) found in PATH. Please install one of them.")
14+
endif
815
NAMESPACE_SCOPED_ARGO_ROLLOUTS ?= false
916

1017
# CHANNELS define the bundle channels used in the bundle.
@@ -165,12 +172,12 @@ run: manifests generate fmt vet ## Run a controller from your host.
165172
# (i.e. docker build --platform linux/arm64 ). However, you must enable docker buildKit for it.
166173
# More info: https://docs.docker.com/develop/develop-images/build_enhancements/
167174
.PHONY: docker-build
168-
docker-build: test ## Build docker image with the manager.
169-
docker build -t ${IMG} .
175+
docker-build: test ## Build container image with the manager.
176+
$(CONTAINER_RUNTIME) build -t ${IMG} .
170177

171178
.PHONY: docker-push
172-
docker-push: ## Push docker image with the manager.
173-
docker push ${IMG}
179+
docker-push: ## Push container image with the manager.
180+
$(CONTAINER_RUNTIME) push ${IMG}
174181

175182
# PLATFORMS defines the target platforms for the manager image be build to provide support to multiple
176183
# architectures. (i.e. make docker-buildx IMG=myregistry/mypoperator:0.0.1). To use this option you need to:
@@ -273,7 +280,7 @@ bundle: operator-sdk manifests kustomize ## Generate bundle manifests and metada
273280

274281
.PHONY: bundle-build
275282
bundle-build: ## Build the bundle image.
276-
docker build -f bundle.Dockerfile -t $(BUNDLE_IMG) .
283+
$(CONTAINER_RUNTIME) build -f bundle.Dockerfile -t $(BUNDLE_IMG) .
277284

278285
.PHONY: bundle-push
279286
bundle-push: ## Push the bundle image.
@@ -313,7 +320,7 @@ endif
313320
# https://github.com/operator-framework/community-operators/blob/7f1438c/docs/packaging-operator.md#updating-your-existing-operator
314321
.PHONY: catalog-build
315322
catalog-build: opm ## Build a catalog image.
316-
$(OPM) index add --container-tool docker --mode semver --tag $(CATALOG_IMG) --bundles $(BUNDLE_IMGS) $(FROM_INDEX_OPT)
323+
$(OPM) index add --container-tool $(CONTAINER_RUNTIME) --mode semver --tag $(CATALOG_IMG) --bundles $(BUNDLE_IMGS) $(FROM_INDEX_OPT)
317324

318325
# Push the catalog image.
319326
.PHONY: catalog-push

controllers/deployment.go

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"reflect"
88

99
rolloutsmanagerv1alpha1 "github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
10+
"github.com/distribution/reference"
1011
appsv1 "k8s.io/api/apps/v1"
1112
corev1 "k8s.io/api/core/v1"
1213
"k8s.io/apimachinery/pkg/api/errors"
@@ -320,10 +321,15 @@ func rolloutsContainer(cr rolloutsmanagerv1alpha1.RolloutManager) (corev1.Contai
320321
return corev1.Container{}, err
321322
}
322323

324+
image, err := getRolloutsContainerImage(cr)
325+
if err != nil {
326+
return corev1.Container{}, err
327+
}
328+
323329
return corev1.Container{
324330
Args: commandArgs,
325331
Env: rolloutsEnv,
326-
Image: getRolloutsContainerImage(cr),
332+
Image: image,
327333
ImagePullPolicy: corev1.PullAlways,
328334
LivenessProbe: &corev1.Probe{
329335
FailureThreshold: 3,
@@ -649,27 +655,62 @@ func boolPtr(val bool) *bool {
649655
}
650656

651657
// Returns the container image for rollouts controller.
652-
func getRolloutsContainerImage(cr rolloutsmanagerv1alpha1.RolloutManager) string {
653-
defaultImg, defaultTag := false, false
658+
func getRolloutsContainerImage(cr rolloutsmanagerv1alpha1.RolloutManager) (string, error) {
659+
660+
// Order of priority:
661+
// 1) cr.spec.image/tag
662+
// 2) env
663+
// 3) default images
654664

655-
img := cr.Spec.Image
665+
image := cr.Spec.Image
656666
tag := cr.Spec.Version
657667

658-
// If spec is empty, use the defaults
659-
if img == "" {
660-
img = DefaultArgoRolloutsImage
661-
defaultImg = true
668+
// Check if environment variable is set
669+
envVal := os.Getenv(ArgoRolloutsImageEnvName)
670+
671+
if envVal != "" {
672+
673+
// If no spec values are provided and env var is set, use env var as-is
674+
if image == "" && tag == "" {
675+
return envVal, nil
676+
}
677+
678+
// If no cr.Spec.Image, use value from env
679+
if image == "" {
680+
// Parse environment variable image if it exists and extract the base image name
681+
baseImageName, err := extractBaseImageName(envVal)
682+
if err != nil {
683+
log.Error(err, "Failed to parse environment variable image", "envVal", envVal)
684+
return "", err
685+
}
686+
image = baseImageName
687+
}
688+
689+
}
690+
691+
if image == "" {
692+
image = DefaultArgoRolloutsImage
662693
}
663694
if tag == "" {
664695
tag = DefaultArgoRolloutsVersion
665-
defaultTag = true
666696
}
667697

668-
// If an env var is specified then use that, but don't override the spec values (if they are present)
669-
if e := os.Getenv(ArgoRolloutsImageEnvName); e != "" && (defaultTag && defaultImg) {
670-
return e
698+
return combineImageTag(image, tag), nil
699+
}
700+
701+
// extractBaseImageName extracts the base image name from a full image reference (removing tag/digest)
702+
func extractBaseImageName(imageRef string) (string, error) {
703+
ref, err := reference.Parse(imageRef)
704+
if err != nil {
705+
return "", err
706+
}
707+
var name string
708+
if named, ok := ref.(reference.Named); ok {
709+
name = named.Name()
710+
} else {
711+
return "", fmt.Errorf("unable to extract base image name: %s", imageRef)
671712
}
672-
return combineImageTag(img, tag)
713+
return name, nil
673714
}
674715

675716
// getRolloutsCommand will return the command for the Rollouts controller component.

controllers/deployment_test.go

Lines changed: 74 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -727,55 +727,120 @@ var _ = Describe("getRolloutsContainerImage tests", func() {
727727
})
728728

729729
AfterEach(func() {
730-
a = *makeTestRolloutManager()
731730
os.Unsetenv("ARGO_ROLLOUTS_IMAGE") // Ensure env variable is not set unless needed
732731
})
733732

734733
When("the spec Image and Version are empty", func() {
735734
It("returns the default image and tag combined", func() {
736735
a.Spec.Image = ""
737736
a.Spec.Version = ""
738-
Expect(getRolloutsContainerImage(a)).To(Equal(DefaultArgoRolloutsImage + ":" + DefaultArgoRolloutsVersion))
737+
img, err := getRolloutsContainerImage(a)
738+
Expect(err).ToNot(HaveOccurred())
739+
Expect(img).To(Equal(DefaultArgoRolloutsImage + ":" + DefaultArgoRolloutsVersion))
739740
})
740741
})
741742

742743
When("the spec Image is set but Version is empty", func() {
743744
It("returns the custom image with the default tag", func() {
744745
a.Spec.Image = "custom-image"
745-
Expect(getRolloutsContainerImage(a)).To(Equal("custom-image:" + DefaultArgoRolloutsVersion))
746+
a.Spec.Version = ""
747+
img, err := getRolloutsContainerImage(a)
748+
Expect(err).ToNot(HaveOccurred())
749+
Expect(img).To(Equal("custom-image:" + DefaultArgoRolloutsVersion))
746750
})
747751
})
748752

749753
When("the spec Image is empty but Version is set", func() {
750754
It("returns the default image with the custom tag", func() {
755+
a.Spec.Image = ""
751756
a.Spec.Version = "custom-tag"
752-
Expect(getRolloutsContainerImage(a)).To(Equal(DefaultArgoRolloutsImage + ":custom-tag"))
757+
img, err := getRolloutsContainerImage(a)
758+
Expect(err).ToNot(HaveOccurred())
759+
Expect(img).To(Equal(DefaultArgoRolloutsImage + ":custom-tag"))
753760
})
754761
})
755762

756763
When("both spec Image and Version are set", func() {
757764
It("returns the custom image and custom tag combined", func() {
758765
a.Spec.Image = "custom-image"
759766
a.Spec.Version = "custom-tag"
760-
Expect(getRolloutsContainerImage(a)).To(Equal("custom-image:custom-tag"))
767+
img, err := getRolloutsContainerImage(a)
768+
Expect(err).ToNot(HaveOccurred())
769+
Expect(img).To(Equal("custom-image:custom-tag"))
761770
})
762771
})
763772

764773
When("the environment variable is set and spec is empty", func() {
765774
It("returns the environment variable image", func() {
766-
os.Setenv("ARGO_ROLLOUTS_IMAGE", "env-image")
767-
Expect(getRolloutsContainerImage(a)).To(Equal("env-image"))
775+
a.Spec.Image = ""
776+
a.Spec.Version = ""
777+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest@sha256:841328df1b9f8c4087adbdcfec6cc99ac8308805dea83f6d415d6fb8d40227c1")
778+
779+
img, err := getRolloutsContainerImage(a)
780+
Expect(err).ToNot(HaveOccurred())
781+
Expect(img).To(Equal("quay.io/argoproj/argo-rollouts-custom:latest@sha256:841328df1b9f8c4087adbdcfec6cc99ac8308805dea83f6d415d6fb8d40227c1"))
782+
})
783+
})
784+
785+
When("the environment variable is set and version is set, but env contains unparseable image", func() {
786+
It("returns an error", func() {
787+
a.Spec.Image = ""
788+
a.Spec.Version = "custom-tag"
789+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest@sha256:invalidsha256")
790+
791+
_, err := getRolloutsContainerImage(a)
792+
Expect(err).To(HaveOccurred())
793+
})
794+
})
795+
796+
When("the environment variable is set and image/version are set, but env contains unparseable image", func() {
797+
It("does not return an error since env var does not need to be parsed", func() {
798+
a.Spec.Image = "custom-image"
799+
a.Spec.Version = "custom-tag"
800+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest@sha256:invalidsha256")
801+
802+
img, err := getRolloutsContainerImage(a)
803+
Expect(err).ToNot(HaveOccurred())
804+
Expect(img).To(Equal("custom-image:custom-tag"))
768805
})
769806
})
770807

771808
When("the environment variable is set but spec is not empty", func() {
772809
It("returns the custom image and tag ignoring the environment variable", func() {
773810
a.Spec.Image = "custom-image"
774811
a.Spec.Version = "custom-tag"
775-
os.Setenv("ARGO_ROLLOUTS_IMAGE", "env-image")
776-
Expect(getRolloutsContainerImage(a)).To(Equal("custom-image:custom-tag"))
812+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest")
813+
img, err := getRolloutsContainerImage(a)
814+
Expect(err).ToNot(HaveOccurred())
815+
816+
Expect(img).To(Equal("custom-image:custom-tag"))
777817
})
778818
})
819+
820+
When("the environment variable is set, and spec version is set", func() {
821+
It("returns the env var image with tag from spec", func() {
822+
a.Spec.Image = ""
823+
a.Spec.Version = "custom-tag"
824+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest")
825+
826+
img, err := getRolloutsContainerImage(a)
827+
Expect(err).ToNot(HaveOccurred())
828+
Expect(img).To(Equal("quay.io/argoproj/argo-rollouts-custom:custom-tag"))
829+
})
830+
})
831+
832+
When("the environment variable is set with a full sha256 value, and spec version is set", func() {
833+
It("returns the env var image with tag from spec", func() {
834+
a.Spec.Image = ""
835+
a.Spec.Version = "custom-tag"
836+
os.Setenv("ARGO_ROLLOUTS_IMAGE", "quay.io/argoproj/argo-rollouts-custom:latest@sha256:841328df1b9f8c4087adbdcfec6cc99ac8308805dea83f6d415d6fb8d40227c1")
837+
838+
img, err := getRolloutsContainerImage(a)
839+
Expect(err).ToNot(HaveOccurred())
840+
Expect(img).To(Equal("quay.io/argoproj/argo-rollouts-custom:custom-tag"))
841+
})
842+
})
843+
779844
})
780845

781846
var _ = Describe("rolloutsContainer tests", func() {

go.mod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
module github.com/argoproj-labs/argo-rollouts-manager
22

3-
go 1.23.0
3+
go 1.24.5
44

55
require (
6+
github.com/distribution/reference v0.6.0
67
github.com/go-logr/logr v1.4.1
78
github.com/onsi/ginkgo/v2 v2.14.0
89
github.com/onsi/gomega v1.30.0
@@ -46,6 +47,7 @@ require (
4647
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
4748
github.com/modern-go/reflect2 v1.0.2 // indirect
4849
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
50+
github.com/opencontainers/go-digest v1.0.0 // indirect
4951
github.com/pkg/errors v0.9.1 // indirect
5052
github.com/prometheus/client_golang v1.18.0 // indirect
5153
github.com/prometheus/client_model v0.5.0 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
1010
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
1111
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
1212
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
13+
github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=
14+
github.com/distribution/reference v0.6.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
1315
github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
1416
github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
1517
github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCvpL6mnFh5mB2/l16U=
@@ -79,6 +81,8 @@ github.com/onsi/ginkgo/v2 v2.14.0 h1:vSmGj2Z5YPb9JwCWT6z6ihcUvDhuXLc3sJiqd3jMKAY
7981
github.com/onsi/ginkgo/v2 v2.14.0/go.mod h1:JkUdW7JkN0V6rFvsHcJ478egV3XH9NxpD27Hal/PhZw=
8082
github.com/onsi/gomega v1.30.0 h1:hvMK7xYz4D3HapigLTeGdId/NcfQx1VHMJc60ew99+8=
8183
github.com/onsi/gomega v1.30.0/go.mod h1:9sxs+SwGrKI0+PWe4Fxa9tFQQBG5xSsSbMXOI8PPpoQ=
84+
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
85+
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
8286
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
8387
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
8488
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

0 commit comments

Comments
 (0)