From 491b9fa21d96f777810432653d3dda039619c385 Mon Sep 17 00:00:00 2001
From: Michael Sauter <michael.sauter@boehringer-ingelheim.com>
Date: Mon, 10 Oct 2022 15:12:02 +0200
Subject: [PATCH] Apply correct tag when pushing to external registry

Fixes #606.
---
 CHANGELOG.md                      |  1 +
 cmd/deploy-with-helm/main.go      | 19 +++++++------
 cmd/deploy-with-helm/main_test.go | 47 +++++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd64ef62..51275af2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@ listed in the changelog.
 ### Fixed
 
 - Aqua and helm-diff log output is incomplete ([#593](https://github.com/opendevstack/ods-pipeline/issues/593))
+- Image is tagged with `latest` instead of correct tag when pushed to external registry ([#606](https://github.com/opendevstack/ods-pipeline/issues/606))
 
 ## [0.6.0] - 2022-07-20
 
diff --git a/cmd/deploy-with-helm/main.go b/cmd/deploy-with-helm/main.go
index db3f8397..88777329 100644
--- a/cmd/deploy-with-helm/main.go
+++ b/cmd/deploy-with-helm/main.go
@@ -181,7 +181,6 @@ func main() {
 			imageStream := imageArtifact.Name
 			fmt.Printf("Copying image %s ...\n", imageStream)
 			srcImageURL := imageArtifact.Image
-			var destImageURL string
 			// If the source registry should be TLS verified, the destination
 			// should be verified by default as well.
 			destRegistryTLSVerify := opts.srcRegistryTLSVerify
@@ -191,14 +190,10 @@ func main() {
 			if strings.HasPrefix(imageArtifact.Registry, "kind-registry.kind") {
 				srcRegistryTLSVerify = false
 			}
-			if len(targetConfig.RegistryHost) > 0 {
-				destImageURL = fmt.Sprintf("%s/%s/%s", targetConfig.RegistryHost, releaseNamespace, imageStream)
-				if targetConfig.RegistryTLSVerify != nil {
-					destRegistryTLSVerify = *targetConfig.RegistryTLSVerify
-				}
-			} else {
-				destImageURL = strings.Replace(imageArtifact.Image, "/"+imageArtifact.Repository+"/", "/"+releaseNamespace+"/", -1)
+			if len(targetConfig.RegistryHost) > 0 && targetConfig.RegistryTLSVerify != nil {
+				destRegistryTLSVerify = *targetConfig.RegistryTLSVerify
 			}
+			destImageURL := getImageDestURL(targetConfig.RegistryHost, releaseNamespace, imageArtifact)
 			fmt.Printf("src=%s\n", srcImageURL)
 			fmt.Printf("dest=%s\n", destImageURL)
 			// TODO: for QA and PROD we want to ensure that the SHA recorded in Nexus
@@ -465,3 +460,11 @@ func collectImageDigests(imageDigestsDir string) ([]string, error) {
 	}
 	return files, nil
 }
+
+func getImageDestURL(registryHost, releaseNamespace string, imageArtifact artifact.Image) string {
+	if registryHost != "" {
+		return fmt.Sprintf("%s/%s/%s:%s", registryHost, releaseNamespace, imageArtifact.Name, imageArtifact.Tag)
+	} else {
+		return strings.Replace(imageArtifact.Image, "/"+imageArtifact.Repository+"/", "/"+releaseNamespace+"/", -1)
+	}
+}
diff --git a/cmd/deploy-with-helm/main_test.go b/cmd/deploy-with-helm/main_test.go
index 54a81194..fef0c494 100644
--- a/cmd/deploy-with-helm/main_test.go
+++ b/cmd/deploy-with-helm/main_test.go
@@ -1,7 +1,10 @@
 package main
 
 import (
+	"fmt"
 	"testing"
+
+	"github.com/opendevstack/pipeline/pkg/artifact"
 )
 
 func TestArtifactFilename(t *testing.T) {
@@ -52,3 +55,47 @@ func TestArtifactFilename(t *testing.T) {
 		})
 	}
 }
+
+func TestGetImageURLs(t *testing.T) {
+	srcHost := "image-registry.openshift-image-registry.svc:5000"
+	destHost := "default-route-openshift-image-registry.apps.example.com"
+	imgArtifact := artifact.Image{
+		Image:      fmt.Sprintf("%s/foo/bar:baz", srcHost),
+		Repository: "foo", Name: "bar", Tag: "baz",
+	}
+	tests := map[string]struct {
+		registryHost     string
+		releaseNamespace string
+		want             string
+	}{
+		"same cluster, same namespace": {
+			registryHost:     "",
+			releaseNamespace: "foo",
+			want:             fmt.Sprintf("%s/foo/bar:baz", srcHost),
+		},
+		"same cluster, different namespace": {
+			registryHost:     "",
+			releaseNamespace: "other",
+			want:             fmt.Sprintf("%s/other/bar:baz", srcHost),
+		},
+		"different cluster, same namespace": {
+			registryHost:     destHost,
+			releaseNamespace: "foo",
+			want:             fmt.Sprintf("%s/foo/bar:baz", destHost),
+		},
+		"different cluster, different namespace": {
+			registryHost:     destHost,
+			releaseNamespace: "other",
+			want:             fmt.Sprintf("%s/other/bar:baz", destHost),
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			got := getImageDestURL(tc.registryHost, tc.releaseNamespace, imgArtifact)
+			if got != tc.want {
+				t.Fatalf("want: %s, got: %s", tc.want, got)
+			}
+		})
+	}
+}