diff --git a/connect-inject/container_init.go b/connect-inject/container_init.go index 8d66e17a66..fc646b8bb7 100644 --- a/connect-inject/container_init.go +++ b/connect-inject/container_init.go @@ -13,14 +13,15 @@ type initContainerCommandData struct { 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". + // that will be written if WriteServiceDefaults is true. ServiceProtocol string AuthMethod string - CentralConfig bool - Upstreams []initContainerCommandUpstreamData - Tags string - Meta map[string]string + // WriteServiceDefaults controls whether a service-defaults config is + // written for this service. + WriteServiceDefaults bool + Upstreams []initContainerCommandUpstreamData + Tags string + Meta map[string]string } type initContainerCommandUpstreamData struct { @@ -37,11 +38,17 @@ func (h *Handler) containerInit(pod *corev1.Pod) (corev1.Container, error) { if annoProtocol, ok := pod.Annotations[annotationProtocol]; ok { protocol = annoProtocol } + // We only write a service-defaults config if central config is enabled + // and a protocol is specified. Previously, we would write a config when + // the protocol was empty. This is the same as setting it to tcp. This + // would then override any global proxy-defaults config. Now, we only + // write the config if a protocol is explicitly set. + writeServiceDefaults := h.WriteServiceDefaults && protocol != "" data := initContainerCommandData{ - ServiceName: pod.Annotations[annotationService], - ServiceProtocol: protocol, - AuthMethod: h.AuthMethod, - CentralConfig: h.CentralConfig, + ServiceName: pod.Annotations[annotationService], + ServiceProtocol: protocol, + AuthMethod: h.AuthMethod, + WriteServiceDefaults: writeServiceDefaults, } if data.ServiceName == "" { // Assertion, since we call defaultAnnotations above and do @@ -260,8 +267,8 @@ services { } EOF -{{- if .CentralConfig }} -# Create the central config's service registration +{{- if .WriteServiceDefaults }} +# Create the service-defaults config for the service cat </consul/connect-inject/central-config.hcl kind = "service-defaults" name = "{{ .ServiceName }}" @@ -274,7 +281,9 @@ EOF -token-sink-file="/consul/connect-inject/acl-token" \ -meta="pod=${POD_NAMESPACE}/${POD_NAME}" {{- end }} -{{- if .CentralConfig }} +{{- if .WriteServiceDefaults }} +{{- /* We use -cas and -modify-index 0 so that if a service-defaults config + already exists for this service, we don't override it*/}} /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 cf601ad282..c3c681e5c9 100644 --- a/connect-inject/container_init_test.go +++ b/connect-inject/container_init_test.go @@ -493,11 +493,12 @@ services { } } -func TestHandlerContainerInit_centralConfig(t *testing.T) { +// Test that we write service-defaults config and use the default protocol. +func TestHandlerContainerInit_writeServiceDefaultsDefaultProtocol(t *testing.T) { require := require.New(t) h := Handler{ - CentralConfig: true, - DefaultProtocol: "grpc", + WriteServiceDefaults: true, + DefaultProtocol: "grpc", } pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -518,7 +519,55 @@ func TestHandlerContainerInit_centralConfig(t *testing.T) { require.NoError(err) actual := strings.Join(container.Command, " ") require.Contains(actual, ` -# Create the central config's service registration +# Create the service-defaults config for the service +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`) +} + +// Test that we write service-defaults config and use the protocol from the Pod. +func TestHandlerContainerInit_writeServiceDefaultsPodProtocol(t *testing.T) { + require := require.New(t) + h := Handler{ + WriteServiceDefaults: true, + DefaultProtocol: "http", + } + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationService: "foo", + annotationProtocol: "grpc", + }, + }, + + 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 service-defaults config for the service cat </consul/connect-inject/central-config.hcl kind = "service-defaults" name = "foo" @@ -589,9 +638,9 @@ func TestHandlerContainerInit_authMethod(t *testing.T) { func TestHandlerContainerInit_authMethodAndCentralConfig(t *testing.T) { require := require.New(t) h := Handler{ - AuthMethod: "release-name-consul-k8s-auth-method", - CentralConfig: true, - DefaultProtocol: "grpc", + AuthMethod: "release-name-consul-k8s-auth-method", + WriteServiceDefaults: true, + DefaultProtocol: "grpc", } pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -619,7 +668,7 @@ func TestHandlerContainerInit_authMethodAndCentralConfig(t *testing.T) { require.NoError(err) actual := strings.Join(container.Command, " ") require.Contains(actual, ` -# Create the central config's service registration +# Create the service-defaults config for the service cat </consul/connect-inject/central-config.hcl kind = "service-defaults" name = "foo" @@ -644,3 +693,43 @@ EOF -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml `) } + +// If the default protocol is empty and no protocol is set on the Pod, +// we expect no service-defaults config to be written. +func TestHandlerContainerInit_noDefaultProtocol(t *testing.T) { + require := require.New(t) + h := Handler{ + AuthMethod: "release-name-consul-k8s-auth-method", + WriteServiceDefaults: true, + DefaultProtocol: "", + } + 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.NotContains(actual, ` +# Create the service-defaults config for the service +cat </consul/connect-inject/central-config.hcl +kind = "service-defaults" +name = "foo" +protocol = "" +EOF`) + require.NotContains(actual, ` +/bin/consul config write -cas -modify-index 0 \ + -token-file="/consul/connect-inject/acl-token" \ + /consul/connect-inject/central-config.hcl || true`) +} diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 1b8007d834..69dd6d888a 100644 --- a/connect-inject/handler.go +++ b/connect-inject/handler.go @@ -101,10 +101,10 @@ type Handler struct { // use for identity with connectInjection if ACLs are enabled AuthMethod string - // CentralConfig tracks whether injection should register services - // to central config as well as normal service registration. + // WriteServiceDefaults controls whether injection should write a + // service-defaults config entry for each service. // Requires an additional `protocol` parameter. - CentralConfig bool + WriteServiceDefaults bool // DefaultProtocol is the default protocol to use for central config // registrations. It will be overridden by a specific annotation. @@ -364,9 +364,9 @@ func (h *Handler) defaultAnnotations(pod *corev1.Pod, patches *[]jsonpatch.JsonP } } - if h.CentralConfig { + if h.WriteServiceDefaults { // Default protocol is specified by a flag if not explicitly annotated - if _, ok := pod.ObjectMeta.Annotations[annotationProtocol]; !ok { + if _, ok := pod.ObjectMeta.Annotations[annotationProtocol]; !ok && h.DefaultProtocol != "" { if cs := pod.Spec.Containers; len(cs) > 0 { // Create the patch for this first, so that the Annotation // object will be created if necessary diff --git a/connect-inject/handler_test.go b/connect-inject/handler_test.go index dfa1499b4d..3164632750 100644 --- a/connect-inject/handler_test.go +++ b/connect-inject/handler_test.go @@ -209,7 +209,7 @@ func TestHandlerHandle(t *testing.T) { { "empty pod basic, protocol in annotation", - Handler{CentralConfig: true, Log: hclog.Default().Named("handler")}, + Handler{WriteServiceDefaults: true, Log: hclog.Default().Named("handler")}, v1beta1.AdmissionRequest{ Object: encodeRaw(t, &corev1.Pod{ Spec: basicSpec, @@ -247,7 +247,7 @@ func TestHandlerHandle(t *testing.T) { { "empty pod basic, default protocol specified", - Handler{CentralConfig: true, DefaultProtocol: "http", Log: hclog.Default().Named("handler")}, + Handler{WriteServiceDefaults: true, DefaultProtocol: "http", Log: hclog.Default().Named("handler")}, v1beta1.AdmissionRequest{ Object: encodeRaw(t, &corev1.Pod{ Spec: basicSpec, diff --git a/subcommand/inject-connect/command.go b/subcommand/inject-connect/command.go index 07e2a5fc9f..4896a9ec47 100644 --- a/subcommand/inject-connect/command.go +++ b/subcommand/inject-connect/command.go @@ -61,7 +61,8 @@ func (c *Command) init() { "Docker image for Envoy. Defaults to Envoy 1.8.0.") 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, "Enable central config.") + c.flagSet.BoolVar(&c.flagCentralConfig, "enable-central-config", false, + "Write a service-defaults config for every Connect service using protocol from -default-protocol or Pod annotation.") c.flagSet.StringVar(&c.flagDefaultProtocol, "default-protocol", "", "The default protocol to use in central config registrations.") c.help = flags.Usage(help, c.flagSet) @@ -109,13 +110,13 @@ func (c *Command) Run(args []string) int { // Build the HTTP handler and server injector := connectinject.Handler{ - ImageConsul: c.flagConsulImage, - ImageEnvoy: c.flagEnvoyImage, - RequireAnnotation: !c.flagDefaultInject, - AuthMethod: c.flagACLAuthMethod, - CentralConfig: c.flagCentralConfig, - DefaultProtocol: c.flagDefaultProtocol, - Log: hclog.Default().Named("handler"), + ImageConsul: c.flagConsulImage, + ImageEnvoy: c.flagEnvoyImage, + RequireAnnotation: !c.flagDefaultInject, + AuthMethod: c.flagACLAuthMethod, + WriteServiceDefaults: c.flagCentralConfig, + DefaultProtocol: c.flagDefaultProtocol, + Log: hclog.Default().Named("handler"), } mux := http.NewServeMux() mux.HandleFunc("/mutate", injector.Handle)