From c921ac390760473d6804e738cddb374964181427 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Thu, 26 Sep 2019 16:03:58 -0700 Subject: [PATCH] Refactor tag/annotation fixes, add tests - add back support for the old tags annotation but mark as deprecated - add tests that check more of the template rendering - fix bug where the -default-protocol flag wasn't being used - clean up whitespace in rendered init container script --- connect-inject/container_init.go | 58 +-- connect-inject/container_init_test.go | 541 ++++++++++++++++++++++---- connect-inject/handler.go | 17 +- 3 files changed, 517 insertions(+), 99 deletions(-) diff --git a/connect-inject/container_init.go b/connect-inject/container_init.go index 37e9f5e94e..8d66e17a66 100644 --- a/connect-inject/container_init.go +++ b/connect-inject/container_init.go @@ -10,8 +10,11 @@ import ( ) type initContainerCommandData struct { - ServiceName string - ServicePort int32 + ServiceName string + ServicePort int32 + // ServiceProtocol is the protocol for the service-defaults config + // that will be written if CentralConfig is true. If empty, Consul + // will default to "tcp". ServiceProtocol string AuthMethod string CentralConfig bool @@ -30,9 +33,13 @@ type initContainerCommandUpstreamData struct { // containerInit returns the init container spec for registering the Consul // service, setting up the Envoy bootstrap, etc. func (h *Handler) containerInit(pod *corev1.Pod) (corev1.Container, error) { + protocol := h.DefaultProtocol + if annoProtocol, ok := pod.Annotations[annotationProtocol]; ok { + protocol = annoProtocol + } data := initContainerCommandData{ ServiceName: pod.Annotations[annotationService], - ServiceProtocol: pod.Annotations[annotationProtocol], + ServiceProtocol: protocol, AuthMethod: h.AuthMethod, CentralConfig: h.CentralConfig, } @@ -50,26 +57,31 @@ func (h *Handler) containerInit(pod *corev1.Pod) (corev1.Container, error) { } } - // If tags are specified split the string into an array and create - // the tags string + var tags []string if raw, ok := pod.Annotations[annotationTags]; ok && raw != "" { - tags := strings.Split(raw, ",") + tags = strings.Split(raw, ",") + } + // Get the tags from the deprecated tags annotation and combine. + if raw, ok := pod.Annotations[annotationConnectTags]; ok && raw != "" { + tags = append(tags, strings.Split(raw, ",")...) + } - // Create json array from the annotations + if len(tags) > 0 { + // Create json array from the annotations since we're going to output + // this in an HCL config file and HCL arrays are json formatted. jsonTags, err := json.Marshal(tags) if err != nil { h.Log.Error("Error json marshaling tags", "Error", err, "Tags", tags) + } else { + data.Tags = string(jsonTags) } - - data.Tags = string(jsonTags) } - // If there is metadata specified split into a map and create + // If there is metadata specified split into a map and create. data.Meta = make(map[string]string) for k, v := range pod.Annotations { - if strings.HasPrefix(k, annotationMeta) && v != "" { - md := strings.Split(k, annotationMeta) - data.Meta[md[1]] = v + if strings.HasPrefix(k, annotationMeta) && strings.TrimPrefix(k, annotationMeta) != "" { + data.Meta[strings.TrimPrefix(k, annotationMeta)] = v } } @@ -194,14 +206,12 @@ services { proxy { destination_service_name = "{{ .ServiceName }}" - destination_service_id = "{{ .ServiceName}}" - {{ if (gt .ServicePort 0) -}} + destination_service_id = "{{ .ServiceName }}" + {{- if (gt .ServicePort 0) }} local_service_address = "127.0.0.1" local_service_port = {{ .ServicePort }} - {{ end -}} - - - {{ range .Upstreams -}} + {{- end }} + {{- range .Upstreams }} upstreams { {{- if .Name }} destination_type = "service" @@ -216,7 +226,7 @@ services { datacenter = "{{ .Datacenter }}" {{- end}} } - {{ end }} + {{- end }} } checks { @@ -250,7 +260,7 @@ services { } EOF -{{ if .CentralConfig -}} +{{- if .CentralConfig }} # Create the central config's service registration cat </consul/connect-inject/central-config.hcl kind = "service-defaults" @@ -258,15 +268,13 @@ name = "{{ .ServiceName }}" protocol = "{{ .ServiceProtocol }}" EOF {{- end }} - -{{ if .AuthMethod -}} +{{- if .AuthMethod }} /bin/consul login -method="{{ .AuthMethod }}" \ -bearer-token-file="/var/run/secrets/kubernetes.io/serviceaccount/token" \ -token-sink-file="/consul/connect-inject/acl-token" \ -meta="pod=${POD_NAMESPACE}/${POD_NAME}" {{- end }} - -{{ if .CentralConfig -}} +{{- if .CentralConfig }} /bin/consul config write -cas -modify-index 0 \ {{- if .AuthMethod }} -token-file="/consul/connect-inject/acl-token" \ diff --git a/connect-inject/container_init_test.go b/connect-inject/container_init_test.go index 427ee08ae9..cf601ad282 100644 --- a/connect-inject/container_init_test.go +++ b/connect-inject/container_init_test.go @@ -21,11 +21,10 @@ func TestHandlerContainerInit(t *testing.T) { Spec: corev1.PodSpec{ Containers: []corev1.Container{ - corev1.Container{ + { Name: "web", }, - - corev1.Container{ + { Name: "web-side", }, }, @@ -39,14 +38,64 @@ func TestHandlerContainerInit(t *testing.T) { Cmd string // Strings.Contains test CmdNot string // Not contains }{ + // The first test checks the whole template. Subsequent tests check + // the parts that change. { - "Only service", + "Only service, whole template", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" return pod }, - `alias_service = "web"`, - `upstreams`, + `/bin/sh -ec export CONSUL_HTTP_ADDR="${HOST_IP}:8500" +export CONSUL_GRPC_ADDR="${HOST_IP}:8502" + +# Register the service. The HCL is stored in the volume so that +# the preStop hook can access it to deregister the service. +cat </consul/connect-inject/service.hcl +services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + + proxy { + destination_service_name = "web" + destination_service_id = "web" + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 0 +} +EOF + +/bin/consul services register \ + /consul/connect-inject/service.hcl + +# Generate the envoy bootstrap code +/bin/consul connect envoy \ + -proxy-id="${POD_NAME}-web-sidecar-proxy" \ + -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml + +# Copy the Consul binary +cp /bin/consul /consul/connect-inject/consul`, + "", }, { @@ -56,7 +105,39 @@ func TestHandlerContainerInit(t *testing.T) { pod.Annotations[annotationPort] = "1234" return pod }, - "local_service_port = 1234", + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 +}`, "", }, @@ -67,7 +148,15 @@ func TestHandlerContainerInit(t *testing.T) { pod.Annotations[annotationUpstreams] = "db:1234" return pod }, - `destination_name = "db"`, + `proxy { + destination_service_name = "web" + destination_service_id = "web" + upstreams { + destination_type = "service" + destination_name = "db" + local_bind_port = 1234 + } + }`, "", }, @@ -78,7 +167,16 @@ func TestHandlerContainerInit(t *testing.T) { pod.Annotations[annotationUpstreams] = "db:1234:dc1" return pod }, - `datacenter = "dc1"`, + `proxy { + destination_service_name = "web" + destination_service_id = "web" + upstreams { + destination_type = "service" + destination_name = "db" + local_bind_port = 1234 + datacenter = "dc1" + } + }`, "", }, @@ -93,70 +191,206 @@ func TestHandlerContainerInit(t *testing.T) { `datacenter`, }, { - "Check Destination Type Query Annotation", + "Upstream prepared query", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" pod.Annotations[annotationUpstreams] = "prepared_query:handle:1234" return pod }, - `destination_type = "prepared_query"`, - `destination_type = "service"`, - }, - - { - "Check Destination Name Query Annotation", - func(pod *corev1.Pod) *corev1.Pod { - pod.Annotations[annotationService] = "web" - pod.Annotations[annotationUpstreams] = "prepared_query:handle:1234" - return pod - }, - `destination_name = "handle"`, + `proxy { + destination_service_name = "web" + destination_service_id = "web" + upstreams { + destination_type = "prepared_query" + destination_name = "handle" + local_bind_port = 1234 + } + }`, "", }, { - "Service ID set to POD_NAME env var", + "Single Tag specified", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" - pod.Annotations[annotationUpstreams] = "db:1234" + pod.Annotations[annotationPort] = "1234" + pod.Annotations[annotationTags] = "abc" return pod }, - `id = "${POD_NAME}-web-sidecar-proxy"`, + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + tags = ["abc"] + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 + tags = ["abc"] +}`, "", }, { - "Proxy ID set to POD_NAME env var", + "Multiple Tags specified", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" - pod.Annotations[annotationUpstreams] = "db:1234" + pod.Annotations[annotationPort] = "1234" + pod.Annotations[annotationTags] = "abc,123" return pod }, - `-proxy-id="${POD_NAME}-web-sidecar-proxy"`, + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + tags = ["abc","123"] + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 + tags = ["abc","123"] +}`, "", }, { - "Single Tag specified", + "Tags using old annotation", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" - pod.Annotations[annotationUpstreams] = "db:1234:dc1" - pod.Annotations[annotationTags] = "abc" + pod.Annotations[annotationPort] = "1234" + pod.Annotations[annotationConnectTags] = "abc,123" return pod }, - `tags = ["abc"]`, + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + tags = ["abc","123"] + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 + tags = ["abc","123"] +}`, "", }, { - "Multiple Tags specified", + "Tags using old and new annotations", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" - pod.Annotations[annotationUpstreams] = "db:1234:dc1" + pod.Annotations[annotationPort] = "1234" pod.Annotations[annotationTags] = "abc,123" + pod.Annotations[annotationConnectTags] = "abc,123,def,456" return pod }, - `tags = ["abc","123"]`, + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + tags = ["abc","123","abc","123","def","456"] + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 + tags = ["abc","123","abc","123","def","456"] +}`, "", }, @@ -169,51 +403,56 @@ func TestHandlerContainerInit(t *testing.T) { "", `tags`, }, - - /* - { - "services": [{ - "name": "api", - "ID": "api-{{ env "NOMAD_ALLOC_ID" }}", - "port": {{ env "NOMAD_PORT_postie_http" }}, - "meta": { - "version": "2" - }, - "tags":["v2"], - "connect": { - "sidecar_service": { - "port": {{ env "NOMAD_PORT_sidecar_ingress" }}, - "proxy": { - "local_service_address": "127.0.0.1", - "config": { - "protocol": "http", - "envoy_prometheus_bind_addr": "0.0.0.0:{{ env "NOMAD_PORT_sidecar_metrics" }}" - } - } - } - } - }, - { - "name": "metrics", - "ID": "metrics-{{ env "NOMAD_ALLOC_ID" }}", - "port": {{ env "NOMAD_PORT_sidecar_metrics" }}, - "tags":["v2"] - }] - } - */ - { "Metadata specified", func(pod *corev1.Pod) *corev1.Pod { pod.Annotations[annotationService] = "web" + pod.Annotations[annotationPort] = "1234" pod.Annotations[fmt.Sprintf("%sname", annotationMeta)] = "abc" pod.Annotations[fmt.Sprintf("%sversion", annotationMeta)] = "2" return pod }, - `meta = { + `services { + id = "${POD_NAME}-web-sidecar-proxy" + name = "web-sidecar-proxy" + kind = "connect-proxy" + address = "${POD_IP}" + port = 20000 + meta = { name = "abc" version = "2" - }`, + } + + proxy { + destination_service_name = "web" + destination_service_id = "web" + local_service_address = "127.0.0.1" + local_service_port = 1234 + } + + checks { + name = "Proxy Public Listener" + tcp = "${POD_IP}:20000" + interval = "10s" + deregister_critical_service_after = "10m" + } + + checks { + name = "Destination Alias" + alias_service = "web" + } +} + +services { + id = "${POD_NAME}-web" + name = "web" + address = "${POD_IP}" + port = 1234 + meta = { + name = "abc" + version = "2" + } +}`, "", }, @@ -226,6 +465,16 @@ func TestHandlerContainerInit(t *testing.T) { "", `meta`, }, + + { + "Central config", + func(pod *corev1.Pod) *corev1.Pod { + pod.Annotations[annotationService] = "web" + return pod + }, + "", + `meta`, + }, } for _, tt := range cases { @@ -243,3 +492,155 @@ func TestHandlerContainerInit(t *testing.T) { }) } } + +func TestHandlerContainerInit_centralConfig(t *testing.T) { + require := require.New(t) + h := Handler{ + CentralConfig: true, + DefaultProtocol: "grpc", + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationService: "foo", + }, + }, + + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + }, + }, + }, + } + container, err := h.containerInit(pod) + require.NoError(err) + actual := strings.Join(container.Command, " ") + require.Contains(actual, ` +# Create the central config's service registration +cat </consul/connect-inject/central-config.hcl +kind = "service-defaults" +name = "foo" +protocol = "grpc" +EOF +/bin/consul config write -cas -modify-index 0 \ + /consul/connect-inject/central-config.hcl || true + +/bin/consul services register \ + /consul/connect-inject/service.hcl + +# Generate the envoy bootstrap code +/bin/consul connect envoy \ + -proxy-id="${POD_NAME}-foo-sidecar-proxy" \ + -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml + +# Copy the Consul binary +cp /bin/consul /consul/connect-inject/consul`) +} + +func TestHandlerContainerInit_authMethod(t *testing.T) { + require := require.New(t) + h := Handler{ + AuthMethod: "release-name-consul-k8s-auth-method", + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationService: "foo", + }, + }, + + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "default-token-podid", + ReadOnly: true, + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + }, + }, + }, + }, + }, + } + container, err := h.containerInit(pod) + require.NoError(err) + actual := strings.Join(container.Command, " ") + require.Contains(actual, ` +/bin/consul login -method="release-name-consul-k8s-auth-method" \ + -bearer-token-file="/var/run/secrets/kubernetes.io/serviceaccount/token" \ + -token-sink-file="/consul/connect-inject/acl-token" \ + -meta="pod=${POD_NAMESPACE}/${POD_NAME}" + +/bin/consul services register \ + -token-file="/consul/connect-inject/acl-token" \ + /consul/connect-inject/service.hcl + +# Generate the envoy bootstrap code +/bin/consul connect envoy \ + -proxy-id="${POD_NAME}-foo-sidecar-proxy" \ + -token-file="/consul/connect-inject/acl-token" \ + -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml`) +} + +func TestHandlerContainerInit_authMethodAndCentralConfig(t *testing.T) { + require := require.New(t) + h := Handler{ + AuthMethod: "release-name-consul-k8s-auth-method", + CentralConfig: true, + DefaultProtocol: "grpc", + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationService: "foo", + }, + }, + + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "default-token-podid", + ReadOnly: true, + MountPath: "/var/run/secrets/kubernetes.io/serviceaccount", + }, + }, + }, + }, + }, + } + container, err := h.containerInit(pod) + require.NoError(err) + actual := strings.Join(container.Command, " ") + require.Contains(actual, ` +# Create the central config's service registration +cat </consul/connect-inject/central-config.hcl +kind = "service-defaults" +name = "foo" +protocol = "grpc" +EOF +/bin/consul login -method="release-name-consul-k8s-auth-method" \ + -bearer-token-file="/var/run/secrets/kubernetes.io/serviceaccount/token" \ + -token-sink-file="/consul/connect-inject/acl-token" \ + -meta="pod=${POD_NAMESPACE}/${POD_NAME}" +/bin/consul config write -cas -modify-index 0 \ + -token-file="/consul/connect-inject/acl-token" \ + /consul/connect-inject/central-config.hcl || true + +/bin/consul services register \ + -token-file="/consul/connect-inject/acl-token" \ + /consul/connect-inject/service.hcl + +# Generate the envoy bootstrap code +/bin/consul connect envoy \ + -proxy-id="${POD_NAME}-foo-sidecar-proxy" \ + -token-file="/consul/connect-inject/acl-token" \ + -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml +`) +} diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 79720ba46e..1b8007d834 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -53,14 +53,23 @@ const ( // be a named port. annotationUpstreams = "consul.hashicorp.com/connect-service-upstreams" + // annotationTags is a list of tags to register with the service + // this is specified as a comma separated list e.g. abc,123 + annotationTags = "consul.hashicorp.com/service-tags" + + // annotationConnectTags is a list of tags to register with the service + // this is specified as a comma separated list e.g. abc,123 + // + // Deprecated: 'consul.hashicorp.com/service-tags' is the new annotation + // and should be used instead. We made this change because the tagging is + // not specific to connect as both the connect proxy *and* the Consul + // service that gets registered is tagged. + annotationConnectTags = "consul.hashicorp.com/connect-service-tags" + // annotationMeta is a list of metadata key/value pairs to add to the service // registration. This is specified in the format `:` // e.g. consul.hashicorp.com/service-meta-foo:bar annotationMeta = "consul.hashicorp.com/service-meta-" - - // annotationTags is a list of tags to register with the service - // this is specified as a comma separated list e.g. abc,123 - annotationTags = "consul.hashicorp.com/service-tags" ) var (