From a4347375f2e8d30df8ff51a2592c94a2efae8e8d Mon Sep 17 00:00:00 2001
From: Harsh Soni <devilincarcerated020@yahoo.com>
Date: Fri, 28 Jul 2023 00:45:19 +0530
Subject: [PATCH] cli: inconsistent precedence for registry flag

Problem:
Commands `jaeger install`, `multicluster link` give precedence to `LINKERD_DOCKER_REGISTRY` env var, whereas commands `install`, `upgrade` and `inject` give preference to `--register` flag.

Solution:
Make the commands consitent by giving precedence to `--register` flag in all commands.

Fixes: #11115

Signed-off-by: Harsh Soni <devilincarcerated020@yahoo.com>
---
 jaeger/cmd/install.go    | 18 +++++++++++++-----
 multicluster/cmd/link.go | 11 +++++++----
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/jaeger/cmd/install.go b/jaeger/cmd/install.go
index 814eb8f19e0fb..d0d1672b891ed 100644
--- a/jaeger/cmd/install.go
+++ b/jaeger/cmd/install.go
@@ -42,6 +42,13 @@ func newCmdInstall() *cobra.Command {
 	var wait time.Duration
 	var options values.Options
 
+	// If LINKERD_DOCKER_REGISTRY is not null, use it as default registry path.
+	// If --registry option is provided, it will override the env variable.
+	defaultDockerRegistry := pkgcmd.DefaultDockerRegistry
+	if regOverride := os.Getenv(flags.EnvOverrideDockerRegistry); regOverride != "" {
+		defaultDockerRegistry = regOverride
+	}
+
 	cmd := &cobra.Command{
 		Use:   "install [flags]",
 		Args:  cobra.NoArgs,
@@ -75,7 +82,7 @@ A full list of configurable values can be found at https://www.github.com/linker
 		},
 	}
 
-	cmd.Flags().StringVar(&registry, "registry", pkgcmd.DefaultDockerRegistry,
+	cmd.Flags().StringVar(&registry, "registry", defaultDockerRegistry,
 		fmt.Sprintf("Docker registry to pull jaeger-webhook image from ($%s)", flags.EnvOverrideDockerRegistry))
 	cmd.Flags().BoolVar(&skipChecks, "skip-checks", false, `Skip checks for linkerd core control-plane existence`)
 	cmd.Flags().BoolVar(&ignoreCluster, "ignore-cluster", false,
@@ -151,13 +158,14 @@ func render(w io.Writer, valuesOverrides map[string]interface{}, registry string
 	}
 
 	regOrig := vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"].(string)
+
+	// registry variable can never be empty. The precedence are as:
+	// 1. --registry
+	// 2. EnvOverrideDockerRegistry
+	// 3. DefaultDockerRegistry
 	if registry != "" {
 		vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"] = pkgcmd.RegistryOverride(regOrig, registry)
 	}
-	// env var overrides CLI flag
-	if override := os.Getenv(flags.EnvOverrideDockerRegistry); override != "" {
-		vals["webhook"].(map[string]interface{})["image"].(map[string]interface{})["name"] = pkgcmd.RegistryOverride(regOrig, override)
-	}
 
 	fullValues := map[string]interface{}{
 		"Values": vals,
diff --git a/multicluster/cmd/link.go b/multicluster/cmd/link.go
index c266b3c077392..f57bc56e55c53 100644
--- a/multicluster/cmd/link.go
+++ b/multicluster/cmd/link.go
@@ -53,6 +53,13 @@ type (
 
 func newLinkCommand() *cobra.Command {
 	opts, err := newLinkOptionsWithDefault()
+
+	// Override the default value with env registry path.
+	// If cli cmd contains --registry flag, it will override env variable.
+	if registry := os.Getenv(flags.EnvOverrideDockerRegistry); registry != "" {
+		opts.dockerRegistry = registry
+	}
+
 	var valuesOptions valuespkg.Options
 
 	if err != nil {
@@ -423,10 +430,6 @@ func buildServiceMirrorValues(opts *linkOptions) (*multicluster.Values, error) {
 		return nil, err
 	}
 
-	if reg := os.Getenv(flags.EnvOverrideDockerRegistry); reg != "" {
-		opts.dockerRegistry = reg
-	}
-
 	defaults.TargetClusterName = opts.clusterName
 	defaults.ServiceMirrorRetryLimit = opts.serviceMirrorRetryLimit
 	defaults.LogLevel = opts.logLevel