diff --git a/connect-inject/container_init.go b/connect-inject/container_init.go index 8d66e17a667a..0bcfd22dcd38 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,9 +267,9 @@ services { } EOF -{{- if .CentralConfig }} -# Create the central config's service registration -cat </consul/connect-inject/central-config.hcl +{{- if .WriteServiceDefaults }} +# Create the service-defaults config for the service +cat </consul/connect-inject/service-defaults.hcl kind = "service-defaults" name = "{{ .ServiceName }}" protocol = "{{ .ServiceProtocol }}" @@ -274,12 +281,14 @@ 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" \ {{- end }} - /consul/connect-inject/central-config.hcl || true + /consul/connect-inject/service-defaults.hcl || true {{- end }} /bin/consul services register \ diff --git a/connect-inject/container_init_test.go b/connect-inject/container_init_test.go index cf601ad2821d..902493776ad9 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,14 +519,62 @@ func TestHandlerContainerInit_centralConfig(t *testing.T) { 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 +# Create the service-defaults config for the service +cat </consul/connect-inject/service-defaults.hcl kind = "service-defaults" name = "foo" protocol = "grpc" EOF /bin/consul config write -cas -modify-index 0 \ - /consul/connect-inject/central-config.hcl || true + /consul/connect-inject/service-defaults.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/service-defaults.hcl +kind = "service-defaults" +name = "foo" +protocol = "grpc" +EOF +/bin/consul config write -cas -modify-index 0 \ + /consul/connect-inject/service-defaults.hcl || true /bin/consul services register \ /consul/connect-inject/service.hcl @@ -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,8 +668,8 @@ func TestHandlerContainerInit_authMethodAndCentralConfig(t *testing.T) { 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 +# Create the service-defaults config for the service +cat </consul/connect-inject/service-defaults.hcl kind = "service-defaults" name = "foo" protocol = "grpc" @@ -631,7 +680,7 @@ EOF -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 + /consul/connect-inject/service-defaults.hcl || true /bin/consul services register \ -token-file="/consul/connect-inject/acl-token" \ @@ -644,3 +693,42 @@ 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{ + 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/service-defaults.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/service-defaults.hcl || true`) +} diff --git a/connect-inject/handler.go b/connect-inject/handler.go index 1b8007d83428..69dd6d888ac2 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 dfa1499b4d3a..e76c5a8c69f6 100644 --- a/connect-inject/handler_test.go +++ b/connect-inject/handler_test.go @@ -208,8 +208,8 @@ func TestHandlerHandle(t *testing.T) { }, { - "empty pod basic, protocol in annotation", - Handler{CentralConfig: true, Log: hclog.Default().Named("handler")}, + "empty pod basic, no default protocol", + Handler{WriteServiceDefaults: true, DefaultProtocol: "", Log: hclog.Default().Named("handler")}, v1beta1.AdmissionRequest{ Object: encodeRaw(t, &corev1.Pod{ Spec: basicSpec, @@ -224,8 +224,39 @@ func TestHandlerHandle(t *testing.T) { []jsonpatch.JsonPatchOperation{ { Operation: "add", - Path: "/metadata/annotations/" + escapeJSONPointer(annotationProtocol), + Path: "/spec/volumes", + }, + { + Operation: "add", + Path: "/spec/initContainers", + }, + { + Operation: "add", + Path: "/spec/containers/-", + }, + { + Operation: "add", + Path: "/metadata/annotations/" + escapeJSONPointer(annotationStatus), }, + }, + }, + + { + "empty pod basic, protocol in annotation", + Handler{WriteServiceDefaults: true, Log: hclog.Default().Named("handler")}, + v1beta1.AdmissionRequest{ + Object: encodeRaw(t, &corev1.Pod{ + Spec: basicSpec, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationService: "foo", + annotationProtocol: "grpc", + }, + }, + }), + }, + "", + []jsonpatch.JsonPatchOperation{ { Operation: "add", Path: "/spec/volumes", @@ -247,7 +278,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 07e2a5fc9f01..4896a9ec47ca 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)