Make -consul-image and -envoy-image required

These flags have been set in Helm for at least 9 months. If they aren't set then our default versions were too old and wouldn't work. Making them required means we won't need to update them here in addition to in consul-helm.
hashicorp · Dec 7, 2020 · e3bed07 · e3bed07
1 parent 1fb37fa
commit e3bed07
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 ## UNRELEASED
 
+BREAKING CHANGES:
+* Connect: the flags `-envoy-image` and `-consul-image` for command `inject-connect` are now required. [[GH-405](https://github.com/hashicorp/consul-k8s/pull/405)]
+
 ## 0.21.0 (November 25, 2020)
 
 IMPROVEMENTS:

diff --git a/connect-inject/handler.go b/connect-inject/handler.go
@@ -21,11 +21,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 )
 
-const (
-	DefaultConsulImage = "consul:1.7.1"
-	DefaultEnvoyImage  = "envoyproxy/envoy-alpine:v1.13.0"
-)
-
 const (
 	// annotationStatus is the key of the annotation that is added to
 	// a pod after an injection is done.

diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go
@@ -106,10 +106,10 @@ func (c *Command) init() {
 		"PEM-encoded TLS certificate to serve. If blank, will generate random cert.")
 	c.flagSet.StringVar(&c.flagKeyFile, "tls-key-file", "",
 		"PEM-encoded TLS private key to serve. If blank, will generate random cert.")
-	c.flagSet.StringVar(&c.flagConsulImage, "consul-image", connectinject.DefaultConsulImage,
-		"Docker image for Consul. Defaults to consul:1.7.1.")
-	c.flagSet.StringVar(&c.flagEnvoyImage, "envoy-image", connectinject.DefaultEnvoyImage,
-		"Docker image for Envoy. Defaults to envoyproxy/envoy-alpine:v1.13.0.")
+	c.flagSet.StringVar(&c.flagConsulImage, "consul-image", "",
+		"Docker image for Consul.")
+	c.flagSet.StringVar(&c.flagEnvoyImage, "envoy-image", "",
+		"Docker image for Envoy.")
 	c.flagSet.StringVar(&c.flagConsulK8sImage, "consul-k8s-image", "",
 		"Docker image for consul-k8s. Used for the connect sidecar.")
 	c.flagSet.StringVar(&c.flagEnvoyExtraArgs, "envoy-extra-args", "",
@@ -187,6 +187,14 @@ func (c *Command) Run(args []string) int {
 		c.UI.Error("-consul-k8s-image must be set")
 		return 1
 	}
+	if c.flagConsulImage == "" {
+		c.UI.Error("-consul-image must be set")
+		return 1
+	}
+	if c.flagEnvoyImage == "" {
+		c.UI.Error("-envoy-image must be set")
+		return 1
+	}
 
 	logger, err := common.Logger(c.flagLogLevel)
 	if err != nil {

diff --git a/subcommand/inject-connect/command_test.go b/subcommand/inject-connect/command_test.go
@@ -24,105 +24,127 @@ func TestRun_FlagValidation(t *testing.T) {
 			expErr: "-consul-k8s-image must be set",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-log-level", "invalid"},
+			flags:  []string{"-consul-k8s-image", "foo"},
+			expErr: "-consul-image must be set",
+		},
+		{
+			flags:  []string{"-consul-k8s-image", "foo", "-consul-image", "foo"},
+			expErr: "-envoy-image must be set",
+		},
+		{
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-log-level", "invalid"},
 			expErr: "unknown log level: invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-ca-file", "bar"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-ca-file", "bar"},
 			expErr: "Error reading Consul's CA cert file \"bar\"",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-default-sidecar-proxy-cpu-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-default-sidecar-proxy-cpu-limit=unparseable"},
 			expErr: "-default-sidecar-proxy-cpu-limit is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-default-sidecar-proxy-cpu-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-default-sidecar-proxy-cpu-request=unparseable"},
 			expErr: "-default-sidecar-proxy-cpu-request is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-default-sidecar-proxy-memory-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-default-sidecar-proxy-memory-limit=unparseable"},
 			expErr: "-default-sidecar-proxy-memory-limit is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-default-sidecar-proxy-memory-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-default-sidecar-proxy-memory-request=unparseable"},
 			expErr: "-default-sidecar-proxy-memory-request is invalid",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-default-sidecar-proxy-memory-request=50Mi",
 				"-default-sidecar-proxy-memory-limit=25Mi",
 			},
 			expErr: "request must be <= limit: -default-sidecar-proxy-memory-request value of \"50Mi\" is greater than the -default-sidecar-proxy-memory-limit value of \"25Mi\"",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-default-sidecar-proxy-cpu-request=50m",
 				"-default-sidecar-proxy-cpu-limit=25m",
 			},
 			expErr: "request must be <= limit: -default-sidecar-proxy-cpu-request value of \"50m\" is greater than the -default-sidecar-proxy-cpu-limit value of \"25m\"",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-init-container-cpu-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-init-container-cpu-limit=unparseable"},
 			expErr: "-init-container-cpu-limit 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-init-container-cpu-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-init-container-cpu-request=unparseable"},
 			expErr: "-init-container-cpu-request 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-init-container-memory-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-init-container-memory-limit=unparseable"},
 			expErr: "-init-container-memory-limit 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-init-container-memory-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-init-container-memory-request=unparseable"},
 			expErr: "-init-container-memory-request 'unparseable' is invalid",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-init-container-memory-request=50Mi",
 				"-init-container-memory-limit=25Mi",
 			},
 			expErr: "request must be <= limit: -init-container-memory-request value of \"50Mi\" is greater than the -init-container-memory-limit value of \"25Mi\"",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-init-container-cpu-request=50m",
 				"-init-container-cpu-limit=25m",
 			},
 			expErr: "request must be <= limit: -init-container-cpu-request value of \"50m\" is greater than the -init-container-cpu-limit value of \"25m\"",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-lifecycle-sidecar-cpu-limit=unparseable"},
 			expErr: "-lifecycle-sidecar-cpu-limit 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-cpu-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-lifecycle-sidecar-cpu-request=unparseable"},
 			expErr: "-lifecycle-sidecar-cpu-request 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-limit=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-lifecycle-sidecar-memory-limit=unparseable"},
 			expErr: "-lifecycle-sidecar-memory-limit 'unparseable' is invalid",
 		},
 		{
-			flags:  []string{"-consul-k8s-image", "foo", "-lifecycle-sidecar-memory-request=unparseable"},
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
+				"-lifecycle-sidecar-memory-request=unparseable"},
 			expErr: "-lifecycle-sidecar-memory-request 'unparseable' is invalid",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-lifecycle-sidecar-memory-request=50Mi",
 				"-lifecycle-sidecar-memory-limit=25Mi",
 			},
 			expErr: "request must be <= limit: -lifecycle-sidecar-memory-request value of \"50Mi\" is greater than the -lifecycle-sidecar-memory-limit value of \"25Mi\"",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "foo",
+			flags: []string{"-consul-k8s-image", "foo", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-lifecycle-sidecar-cpu-request=50m",
 				"-lifecycle-sidecar-cpu-limit=25m",
 			},
 			expErr: "request must be <= limit: -lifecycle-sidecar-cpu-request value of \"50m\" is greater than the -lifecycle-sidecar-cpu-limit value of \"25m\"",
 		},
 		{
-			flags: []string{"-consul-k8s-image", "hashicorpdev/consul-k8s:latest",
+			flags: []string{"-consul-k8s-image", "hashicorpdev/consul-k8s:latest", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-enable-health-checks-controller=true"},
 			expErr: "CONSUL_HTTP_ADDR is not specified",
 		},
@@ -169,7 +191,7 @@ func TestRun_ValidationHealthCheckEnv(t *testing.T) {
 	}{
 		{
 			envVars: []string{api.HTTPAddrEnvName, "0.0.0.0:999999"},
-			flags: []string{"-consul-k8s-image", "hashicorp/consul-k8s",
+			flags: []string{"-consul-k8s-image", "hashicorp/consul-k8s", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 				"-enable-health-checks-controller=true"},
 			expErr: "Error parsing CONSUL_HTTP_ADDR: parse \"0.0.0.0:999999\": first path segment in URL cannot contain colon",
 		},
@@ -202,7 +224,7 @@ func TestRun_CommandFailsWithInvalidListener(t *testing.T) {
 	}
 	os.Setenv(api.HTTPAddrEnvName, "http://0.0.0.0:9999")
 	code := cmd.Run([]string{
-		"-consul-k8s-image", "hashicorp/consul-k8s",
+		"-consul-k8s-image", "hashicorp/consul-k8s", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 		"-enable-health-checks-controller=true",
 		"-listen", "999999",
 	})
@@ -235,7 +257,7 @@ func testSignalHandling(sig os.Signal) func(*testing.T) {
 
 		// Start the command asynchronously and then we'll send an interrupt.
 		exitChan := runCommandAsynchronously(&cmd, []string{
-			"-consul-k8s-image", "hashicorp/consul-k8s",
+			"-consul-k8s-image", "hashicorp/consul-k8s", "-consul-image", "foo", "-envoy-image", "envoy:1.16.0",
 			"-enable-health-checks-controller=true",
 			"-listen", fmt.Sprintf(":%d", ports[0]),
 		})