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>
linkerd · Jul 27, 2023 · 7c8b201 · 7c8b201
1 parent bc484be
commit 7c8b201
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 9 deletions.
diff --git 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/metadata-controller.json b/metadata-controller.json
@@ -0,0 +1,33 @@
+{
+  "containerimage.buildinfo": {
+    "frontend": "dockerfile.v0",
+    "attrs": {
+      "build-arg:LINKERD_VERSION": "git-09626867",
+      "filename": "Dockerfile",
+      "vcs:revision": "096268674dfecc5cb9a5a78e35c2fb6111bbc40c",
+      "vcs:source": "https://github.com/harsh020/linkerd2.git"
+    },
+    "sources": [
+      {
+        "type": "docker-image",
+        "ref": "docker.io/library/golang:1.19-alpine",
+        "pin": "sha256:bb72ffd13e9c75ccb65487e89845ecc572d749a07635c9b7fb4df2047c1bef68"
+      }
+    ]
+  },
+  "containerimage.config.digest": "sha256:26597fb8f812b6ce1800c85d874f06abce277896a6b595ca0b02fd6c61df686e",
+  "containerimage.descriptor": {
+    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+    "digest": "sha256:447a83bb76601c1cd9912d6ca9adb6943eab6cd42bdae1e8b2d301126feda96c",
+    "size": 896,
+    "annotations": {
+      "org.opencontainers.image.created": "2023-07-26T10:28:59Z"
+    },
+    "platform": {
+      "architecture": "arm64",
+      "os": "linux"
+    }
+  },
+  "containerimage.digest": "sha256:447a83bb76601c1cd9912d6ca9adb6943eab6cd42bdae1e8b2d301126feda96c",
+  "image.name": "cr.l5d.io/linkerd/controller:git-09626867"
+}
diff --git a/metadata-proxy.json b/metadata-proxy.json
@@ -0,0 +1,48 @@
+{
+  "containerimage.buildinfo": {
+    "frontend": "dockerfile.v0",
+    "attrs": {
+      "build-arg:LINKERD_VERSION": "git-09626867",
+      "filename": "Dockerfile-proxy",
+      "vcs:revision": "096268674dfecc5cb9a5a78e35c2fb6111bbc40c",
+      "vcs:source": "https://github.com/harsh020/linkerd2.git"
+    },
+    "sources": [
+      {
+        "type": "docker-image",
+        "ref": "docker.io/curlimages/curl:7.86.0",
+        "pin": "sha256:cfdeba7f88bb85f6c87f2ec9135115b523a1c24943976a61fbf59c4f2eafd78e"
+      },
+      {
+        "type": "docker-image",
+        "ref": "docker.io/library/debian:bullseye-slim",
+        "pin": "sha256:3460d74bec6b88496cd183d7731930be55234c094f581f7dbdd96f56c1fc34d8"
+      },
+      {
+        "type": "docker-image",
+        "ref": "docker.io/library/golang:1.19-alpine",
+        "pin": "sha256:bb72ffd13e9c75ccb65487e89845ecc572d749a07635c9b7fb4df2047c1bef68"
+      },
+      {
+        "type": "docker-image",
+        "ref": "gcr.io/distroless/cc:latest",
+        "pin": "sha256:b53fbf5f81f4a120a489fedff2092e6fcbeacf7863fce3e45d99cc58dc230ccc"
+      }
+    ]
+  },
+  "containerimage.config.digest": "sha256:3a9ed5c34fa5a153e8ad4c2ab343747f7bfa6d8dd5bd2b63f6070aec70a0255e",
+  "containerimage.descriptor": {
+    "mediaType": "application/vnd.docker.distribution.manifest.v2+json",
+    "digest": "sha256:21d905b5241aa8f8f1b4c1009e1404db6c3fe3f2b4ae172a87d4fbe97e3e769e",
+    "size": 4620,
+    "annotations": {
+      "org.opencontainers.image.created": "2023-07-26T10:25:20Z"
+    },
+    "platform": {
+      "architecture": "arm64",
+      "os": "linux"
+    }
+  },
+  "containerimage.digest": "sha256:21d905b5241aa8f8f1b4c1009e1404db6c3fe3f2b4ae172a87d4fbe97e3e769e",
+  "image.name": "cr.l5d.io/linkerd/proxy:git-09626867"
+}
diff --git 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