From d89917e535ed82484fca2d266ef0bab10fe7fa6c Mon Sep 17 00:00:00 2001 From: Kevin Jing Qiu Date: Wed, 11 Dec 2019 18:04:00 -0500 Subject: [PATCH 1/5] Add flagEnvoyExtraArgs to the list of acceptable CLI flags --- connect-inject/container_sidecar.go | 1 + subcommand/inject-connect/command.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/connect-inject/container_sidecar.go b/connect-inject/container_sidecar.go index dbef30c456..6de59ce852 100644 --- a/connect-inject/container_sidecar.go +++ b/connect-inject/container_sidecar.go @@ -51,6 +51,7 @@ func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { "envoy", "--max-obj-name-len", "256", "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", + "--log-level", "debug", }, }, nil } diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index 4896a9ec47..65c65338c5 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -33,6 +33,7 @@ type Command struct { flagDefaultInject bool // True to inject by default flagConsulImage string // Docker image for Consul flagEnvoyImage string // Docker image for Envoy + flagEnvoyExtraArgs string // Extra envoy args when starting envoy flagACLAuthMethod string // Auth Method to use for ACLs, if enabled flagCentralConfig bool // True to enable central config injection flagDefaultProtocol string // Default protocol for use with central config @@ -59,6 +60,7 @@ func (c *Command) init() { "Docker image for Consul. Defaults to an Consul 1.3.0.") c.flagSet.StringVar(&c.flagEnvoyImage, "envoy-image", connectinject.DefaultEnvoyImage, "Docker image for Envoy. Defaults to Envoy 1.8.0.") + c.flagSet.StringVar(&c.flagEnvoyExtraArgs, "envoy-extra-args", "", "Extra envoy command line args to be set when starting envoy") c.flagSet.StringVar(&c.flagACLAuthMethod, "acl-auth-method", "", "The name of the Kubernetes Auth Method to use for connectInjection if ACLs are enabled.") c.flagSet.BoolVar(&c.flagCentralConfig, "enable-central-config", false, From 678e6e8f37d44877e6b27e90f5fc2a1cc967396a Mon Sep 17 00:00:00 2001 From: Kevin Jing Qiu Date: Wed, 11 Dec 2019 18:18:33 -0500 Subject: [PATCH 2/5] Write failing tests --- connect-inject/container_sidecar.go | 2 -- connect-inject/container_sidecar_test.go | 46 ++++++++++++++++++++++++ connect-inject/handler.go | 4 +++ subcommand/inject-connect/command.go | 1 + 4 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 connect-inject/container_sidecar_test.go diff --git a/connect-inject/container_sidecar.go b/connect-inject/container_sidecar.go index 6de59ce852..976ccccd5f 100644 --- a/connect-inject/container_sidecar.go +++ b/connect-inject/container_sidecar.go @@ -9,7 +9,6 @@ import ( ) func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { - // Render the command var buf bytes.Buffer tpl := template.Must(template.New("root").Parse(strings.TrimSpace( @@ -51,7 +50,6 @@ func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { "envoy", "--max-obj-name-len", "256", "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", - "--log-level", "debug", }, }, nil } diff --git a/connect-inject/container_sidecar_test.go b/connect-inject/container_sidecar_test.go new file mode 100644 index 0000000000..549011816a --- /dev/null +++ b/connect-inject/container_sidecar_test.go @@ -0,0 +1,46 @@ +package connectinject + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func TestContainerSidecarCommand(t *testing.T) { + cases := []struct { + name string + extraEnvoyArgs string + expectedContainerCommand []string + }{ + { + name: "no extra args provided", + extraEnvoyArgs: "", + expectedContainerCommand: []string{ + "envoy", "--max-obj-name-len", "256", + "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", + }, + }, + { + name: "extra loglevel args", + extraEnvoyArgs: "--log-level debug", + expectedContainerCommand: []string{ + "envoy", "--max-obj-name-len", "256", + "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", + "--log-level", "debug", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + h := Handler{ + ImageConsul: "hashicorp/consul:latest", + ImageEnvoy: "hashicorp/consul-k8s:latest", + ExtraEnvoyArgs: tc.extraEnvoyArgs, + } + + c, err := h.containerSidecar(nil) + require.NoError(t, err) + require.Equal(t, tc.expectedContainerCommand, c.Command) + }) + } +} diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 69dd6d888a..69cf8081b8 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -93,6 +93,10 @@ type Handler struct { ImageConsul string ImageEnvoy string + // Optional: set when you need extra args to be set when running envoy + // See a list of args here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli + ExtraEnvoyArgs string + // RequireAnnotation means that the annotation must be given to inject. // If this is false, injection is default. RequireAnnotation bool diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index 65c65338c5..efb7ac37d4 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -114,6 +114,7 @@ func (c *Command) Run(args []string) int { injector := connectinject.Handler{ ImageConsul: c.flagConsulImage, ImageEnvoy: c.flagEnvoyImage, + ExtraEnvoyArgs: c.flagEnvoyExtraArgs, RequireAnnotation: !c.flagDefaultInject, AuthMethod: c.flagACLAuthMethod, WriteServiceDefaults: c.flagCentralConfig, From 228ca24ad9a5d36285bbeb2e4fd51d02b0267df6 Mon Sep 17 00:00:00 2001 From: Kevin Jing Qiu Date: Wed, 11 Dec 2019 18:25:42 -0500 Subject: [PATCH 3/5] Add a more advanced test --- connect-inject/container_sidecar.go | 19 ++++++++++++++----- connect-inject/container_sidecar_test.go | 10 ++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/connect-inject/container_sidecar.go b/connect-inject/container_sidecar.go index 976ccccd5f..b5a4a03fdd 100644 --- a/connect-inject/container_sidecar.go +++ b/connect-inject/container_sidecar.go @@ -8,6 +8,19 @@ import ( corev1 "k8s.io/api/core/v1" ) +func (h *Handler) getContainerSidecarCommand() []string { + cmd := []string{ + "envoy", + "--max-obj-name-len", "256", + "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", + } + + if h.ExtraEnvoyArgs != "" { + + } + return cmd +} + func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { // Render the command var buf bytes.Buffer @@ -46,11 +59,7 @@ func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { }, }, }, - Command: []string{ - "envoy", - "--max-obj-name-len", "256", - "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", - }, + Command: h.getContainerSidecarCommand(), }, nil } diff --git a/connect-inject/container_sidecar_test.go b/connect-inject/container_sidecar_test.go index 549011816a..69b1c10674 100644 --- a/connect-inject/container_sidecar_test.go +++ b/connect-inject/container_sidecar_test.go @@ -28,6 +28,16 @@ func TestContainerSidecarCommand(t *testing.T) { "--log-level", "debug", }, }, + { + name: "extraEnvoyArgs with quotes inside", + extraEnvoyArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", + expectedContainerCommand: []string{ + "envoy", "--max-obj-name-len", "256", + "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", + "--log-level", "debug", + "--admin-address-path", "\"/tmp/consul/foo bar\"", + }, + }, } for _, tc := range cases { From 5ed8aefef4883fafd2a5010d1cc5fd1ad91fbabf Mon Sep 17 00:00:00 2001 From: Kevin Jing Qiu Date: Wed, 11 Dec 2019 18:43:38 -0500 Subject: [PATCH 4/5] Append extra envoy args to the cmd --- connect-inject/container_sidecar.go | 24 ++++++++++++++++++++---- go.mod | 1 + go.sum | 2 ++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/connect-inject/container_sidecar.go b/connect-inject/container_sidecar.go index b5a4a03fdd..5e60811ec0 100644 --- a/connect-inject/container_sidecar.go +++ b/connect-inject/container_sidecar.go @@ -2,13 +2,15 @@ package connectinject import ( "bytes" + "strconv" "strings" "text/template" + "github.com/google/shlex" corev1 "k8s.io/api/core/v1" ) -func (h *Handler) getContainerSidecarCommand() []string { +func (h *Handler) getContainerSidecarCommand() ([]string, error) { cmd := []string{ "envoy", "--max-obj-name-len", "256", @@ -16,9 +18,18 @@ func (h *Handler) getContainerSidecarCommand() []string { } if h.ExtraEnvoyArgs != "" { - + tokens, err := shlex.Split(h.ExtraEnvoyArgs) + if err != nil { + return []string{}, err + } + for _, t := range tokens { + if strings.Contains(t, " ") { + t = strconv.Quote(t) + } + cmd = append(cmd, t) + } } - return cmd + return cmd, nil } func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { @@ -31,6 +42,11 @@ func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { return corev1.Container{}, err } + cmd, err := h.getContainerSidecarCommand() + if err != nil { + return corev1.Container{}, err + } + return corev1.Container{ Name: "consul-connect-envoy-sidecar", Image: h.ImageEnvoy, @@ -59,7 +75,7 @@ func (h *Handler) containerSidecar(pod *corev1.Pod) (corev1.Container, error) { }, }, }, - Command: h.getContainerSidecarCommand(), + Command: cmd, }, nil } diff --git a/go.mod b/go.mod index 9cb2f6dd2e..16a64f4704 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/go-sql-driver/mysql v1.4.0 // indirect github.com/gocql/gocql v0.0.0-20180828192252-db20ccb04312 // indirect github.com/golang/groupcache v0.0.0-20180513044358-24b0969c4cb7 // indirect + github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/hashicorp/consul v1.5.0 github.com/hashicorp/consul/api v1.1.0 github.com/hashicorp/consul/sdk v0.1.1 diff --git a/go.sum b/go.sum index 6f42f7a07c..94460f7bef 100644 --- a/go.sum +++ b/go.sum @@ -141,6 +141,8 @@ github.com/google/go-querystring v0.0.0-20170111101155-53e6ce116135 h1:zLTLjkaOF github.com/google/go-querystring v0.0.0-20170111101155-53e6ce116135/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf h1:+RRA9JqSOZFfKrOeqr2z77+8R2RKyh8PG66dcu1V0ck= github.com/google/gofuzz v0.0.0-20170612174753-24818f796faf/go.mod h1:HP5RmnzzSNb993RKQDq4+1A4ia9nllfqcQFTQJedwGI= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4= +github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ= github.com/googleapis/gnostic v0.2.0 h1:l6N3VoaVzTncYYW+9yOz2LJJammFZGBO13sqgEhpy9g= github.com/googleapis/gnostic v0.2.0/go.mod h1:sJBsCZ4ayReDTBIg8b9dl28c5xFWyhBTVRp3pOg5EKY= github.com/gophercloud/gophercloud v0.0.0-20180828235145-f29afc2cceca h1:wobTb8SE189AuxzEKClyYxiI4nUGWlpVtl13eLiFlOE= From 493d9df6579161aa13745cc49ea31e740f6a23e7 Mon Sep 17 00:00:00 2001 From: Kevin Jing Qiu Date: Wed, 11 Dec 2019 22:47:28 -0500 Subject: [PATCH 5/5] Change `ExtraEnvoyArgs` to `ExtraEnvoyOpts` --- connect-inject/container_sidecar.go | 4 ++-- connect-inject/container_sidecar_test.go | 16 ++++++++-------- connect-inject/handler.go | 4 ++-- subcommand/inject-connect/command.go | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/connect-inject/container_sidecar.go b/connect-inject/container_sidecar.go index 5e60811ec0..72b718990d 100644 --- a/connect-inject/container_sidecar.go +++ b/connect-inject/container_sidecar.go @@ -17,8 +17,8 @@ func (h *Handler) getContainerSidecarCommand() ([]string, error) { "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", } - if h.ExtraEnvoyArgs != "" { - tokens, err := shlex.Split(h.ExtraEnvoyArgs) + if h.ExtraEnvoyOpts != "" { + tokens, err := shlex.Split(h.ExtraEnvoyOpts) if err != nil { return []string{}, err } diff --git a/connect-inject/container_sidecar_test.go b/connect-inject/container_sidecar_test.go index 69b1c10674..ecfafef53c 100644 --- a/connect-inject/container_sidecar_test.go +++ b/connect-inject/container_sidecar_test.go @@ -8,20 +8,20 @@ import ( func TestContainerSidecarCommand(t *testing.T) { cases := []struct { name string - extraEnvoyArgs string + extraEnvoyOpts string expectedContainerCommand []string }{ { - name: "no extra args provided", - extraEnvoyArgs: "", + name: "no extra options provided", + extraEnvoyOpts: "", expectedContainerCommand: []string{ "envoy", "--max-obj-name-len", "256", "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", }, }, { - name: "extra loglevel args", - extraEnvoyArgs: "--log-level debug", + name: "extra log-level option", + extraEnvoyOpts: "--log-level debug", expectedContainerCommand: []string{ "envoy", "--max-obj-name-len", "256", "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", @@ -29,8 +29,8 @@ func TestContainerSidecarCommand(t *testing.T) { }, }, { - name: "extraEnvoyArgs with quotes inside", - extraEnvoyArgs: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", + name: "extraEnvoyOpts with quotes inside", + extraEnvoyOpts: "--log-level debug --admin-address-path \"/tmp/consul/foo bar\"", expectedContainerCommand: []string{ "envoy", "--max-obj-name-len", "256", "--config-path", "/consul/connect-inject/envoy-bootstrap.yaml", @@ -45,7 +45,7 @@ func TestContainerSidecarCommand(t *testing.T) { h := Handler{ ImageConsul: "hashicorp/consul:latest", ImageEnvoy: "hashicorp/consul-k8s:latest", - ExtraEnvoyArgs: tc.extraEnvoyArgs, + ExtraEnvoyOpts: tc.extraEnvoyOpts, } c, err := h.containerSidecar(nil) diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 69cf8081b8..1bee694793 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -93,9 +93,9 @@ type Handler struct { ImageConsul string ImageEnvoy string - // Optional: set when you need extra args to be set when running envoy + // Optional: set when you need extra options to be set when running envoy // See a list of args here: https://www.envoyproxy.io/docs/envoy/latest/operations/cli - ExtraEnvoyArgs string + ExtraEnvoyOpts string // RequireAnnotation means that the annotation must be given to inject. // If this is false, injection is default. diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index efb7ac37d4..07c9c61d4a 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -114,7 +114,7 @@ func (c *Command) Run(args []string) int { injector := connectinject.Handler{ ImageConsul: c.flagConsulImage, ImageEnvoy: c.flagEnvoyImage, - ExtraEnvoyArgs: c.flagEnvoyExtraArgs, + ExtraEnvoyOpts: c.flagEnvoyExtraArgs, RequireAnnotation: !c.flagDefaultInject, AuthMethod: c.flagACLAuthMethod, WriteServiceDefaults: c.flagCentralConfig,