From e3bed07ca17467f6aca4d899bf8891022e9e1f0d Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Wed, 25 Nov 2020 16:44:46 -0800 Subject: [PATCH] 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. --- CHANGELOG.md | 3 + connect-inject/handler.go | 5 -- subcommand/inject-connect/command.go | 16 ++++-- subcommand/inject-connect/command_test.go | 70 +++++++++++++++-------- 4 files changed, 61 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acc33df589..2dd05bbade 100644 --- 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 index 319dd51444..91bac78182 100644 --- 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 index ed0639fb93..3aaf2ccdb2 100644 --- 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 index eaab19bcd1..7340e74811 100644 --- 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]), })