Enable tproxy for the Consul service mesh #481

ishustava · 2021-04-09T16:46:19Z

Changes proposed in this PR:

add new -enable-transparent-proxy flag to inject-connect command,
which will enable all transparent proxy mode for all applications running
on the Consul service mesh.
add a new pod annotation consul.hashicorp.com/transparent-proxy to allow
enabling or disabling transparent proxy mode.
endpoints-controller will configure proxy service registrations to enable
transparent proxy when the above flag is passed or when tproxy annotation is set.
It will also add service's cluster IP to both the service and proxy registrations
as a tagged address so that Consul can configure Envoy filter traffic based on that IP.
consul-connect-inject-init container will apply traffic redirection rules
via consul connect redirect-traffic command which runs iptables under the hood.
To run the command, the init container needs to both be a root user and have
NET_ADMIN capability.

How I've tested this PR:
Ran acceptance tests in hashicorp/consul-helm#905. Note that we can't yet run enterprise acceptance tests as we're still waiting for some fixes to make it to the enterprise image

How I expect reviewers to test this PR:

Code review
If you'd like you can try it out with consul-k8s image ishustava/consul-k8s-dev:04-13-2021-65883b7 and consul image: ishustava/consul-dev:tproxy-test

Checklist:

Tests added
CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

whiskeysierra · 2021-04-12T17:05:35Z

Is there some document or discussion somewhere about how the transparent proxy is going to work? E.g. which parts of Kubernetes DNS will it understand and support?

ishustava · 2021-04-12T17:24:31Z

Hey @whiskeysierra

We're going to release the docs together with the next Helm, consul-k8s and consul beta release (probably this week).

build-support/docker/Dev.dockerfile

* add new -enable-transparent-proxy flag to inject-connect command, which will enable all transparent proxy mode for all applications running on the Consul service mesh. * add a new pod annotation consul.hashicorp.com/transparent-proxy to allow enabling or disabling transparent proxy mode. * endpoints-controller will configure proxy service registrations to enable transparent proxy when the above flag is passed or when tproxy annotation is set. It will also add service's cluster IP to both the service and proxy registrations as a tagged address so that Consul can configure Envoy filter traffic based on that IP. * consul-connect-inject-init container will apply traffic redirection rules via consul connect redirect-traffic command which runs iptables under the hood. To run the command, the init container needs to both be a root user and have NET_ADMIN capability.

connect-inject/container_init_test.go

connect-inject/container_init.go

connect-inject/container_init_test.go

kschoche · 2021-04-13T22:35:32Z

connect-inject/container_init_test.go

+export CONSUL_HTTP_ADDR="${HOST_IP}:8500"
+export CONSUL_GRPC_ADDR="${HOST_IP}:8502"
+consul-k8s connect-init -pod-name=${POD_NAME} -pod-namespace=${POD_NAMESPACE} \
+  -consul-service-namespace="default" \


no changes needed here, just a general comment that its kinda reading weird now that we have consul-service-namespace and namespace used across the various connect commands now, I think they all mean the same thing but obv we cannot change them on the consul side.

Yeah, the problem is that some of these are consul cli flags and some are flags for consul-k8s commands. For consul, there is only one namespace (the consul one), whereas in consul-k8s, it could be either consul or k8s namespace and so we need to disambiguate. Not sure if there is a way around it.

connect-inject/endpoints_controller_test.go

kschoche · 2021-04-13T22:53:26Z

subcommand/inject-connect/command.go

@@ -150,6 +153,8 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagCrossNamespaceACLPolicy, "consul-cross-namespace-acl-policy", "",
 		"[Enterprise Only] Name of the ACL policy to attach to all created Consul namespaces to allow service "+
 			"discovery across Consul namespaces. Only necessary if ACLs are enabled.")
+	c.flagSet.BoolVar(&c.flagEnableTransparentProxy, "enable-transparent-proxy", false,


Is the plan to keep this disabled by default in master?

It'll default to true in the helm chart. I suppose we can default it true here as well for consistency.

kschoche

This looks fantastic! I had a couple non-blocking questions but overall I really like the approach and the testing! great work.

lkysow

Looks great. I haven't had a chance to try it out yet but won't block.

lkysow · 2021-04-13T23:20:43Z

connect-inject/container_init.go

+  -bootstrap > /consul/connect-inject/envoy-bootstrap.yaml
+
+{{- if .EnableTransparentProxy }}
+


Suggested change

This was here intentionally to allow an extra newline between this and the previous command. I didn't want the extra newline after envoy bootstrap command when tproxy was disabled. I didn't find a way to do it with go template. I know it's kind of silly, but it really bothered me 😄 😄

I'll add a comment saying that.

lkysow · 2021-04-13T23:21:44Z

connect-inject/container_init.go

+  -namespace="{{ .ConsulNamespace }}" \
+  {{- end }}
+  -proxy-id="$(cat /consul/connect-inject/proxyid)" \
+  -proxy-uid=0


If someone changes rootUserAndGroupID will this also need to be updated? If so, maybe this should be added as a templated field.

That one is only used by the init container, where is -proxy-uid is the user id of the Envoy proxy container. Once we don't run envoy as root, this will need to be changed. I think we can add it to the template once we support running envoy as non-root.

Note that to support tproxy and be able to run iptables we need that init container to be root, unfortunately.

connect-inject/container_init_test.go

lkysow · 2021-04-13T23:30:16Z

connect-inject/endpoints_controller.go

@@ -32,6 +33,7 @@ const (
 	kubernetesSuccessReasonMsg = "Kubernetes health checks passing"
 	podPendingReasonMsg        = "Pod is pending"


my ide says this isn't used anymore

your IDE is 100% correct! great catch!

lkysow · 2021-04-13T23:32:26Z

connect-inject/endpoints_controller.go

+
+			proxyService.Proxy.Mode = api.ProxyModeTransparent
+		} else {
+			r.Log.Info("skipping syncing service cluster IP to Consul", "name", k8sService.Name, "ns", k8sService.Namespace, "ip", k8sService.Spec.ClusterIP)


should this be debug level? Wondering if there's tons of services the you don't want this. Although I guess this only applies to headless services?

Hmm, yeah this applies only to headless services. But maybe still this should be debug because it's not really useful to you since everything will still work just using pod IPs. I'll change it to debug.

or I won't 😄 It looks like zap logger doesn't have debug log level?

subcommand/inject-connect/command.go

Co-authored-by: Kyle Schochenmaier <[email protected]> Co-authored-by: Luke Kysow <[email protected]>

Enable tproxy for the Consul service mesh. * add new -enable-transparent-proxy flag to inject-connect command, which will enable all transparent proxy mode for all applications running on the Consul service mesh. * add a new pod annotation consul.hashicorp.com/transparent-proxy to allow enabling or disabling transparent proxy mode. * endpoints-controller will configure proxy service registrations to enable transparent proxy when the above flag is passed or when tproxy annotation is set. It will also add service's cluster IP to both the service and proxy registrations as a tagged address so that Consul can configure Envoy filter traffic based on that IP. * consul-connect-inject-init container will apply traffic redirection rules via consul connect redirect-traffic command which runs iptables under the hood. To run the command, the init container needs to both be a root user and have NET_ADMIN capability.

Support custom CA cert for snapshot agent For users running an S3 compatible storage for the snapshot agent with their own CA cert they need a way to include that in the system certs so that the snapshot agent can connect with their URL.

ishustava changed the base branch from master to feature-tproxy April 9, 2021 16:46

ishustava force-pushed the enable-tproxy branch 5 times, most recently from 65883b7 to ea74b72 Compare April 13, 2021 21:40

ishustava changed the title ~~Enable tproxy~~ Enable tproxy for the Consul service mesh Apr 13, 2021

ishustava marked this pull request as ready for review April 13, 2021 22:05

ishustava requested review from a team, lkysow and kschoche and removed request for a team April 13, 2021 22:05

ishustava commented Apr 13, 2021

View reviewed changes

build-support/docker/Dev.dockerfile Show resolved Hide resolved

ishustava force-pushed the enable-tproxy branch from ea74b72 to 9d345f0 Compare April 13, 2021 22:08

ishustava commented Apr 13, 2021

View reviewed changes

connect-inject/container_init_test.go Show resolved Hide resolved

ishustava commented Apr 13, 2021

View reviewed changes

connect-inject/container_init.go Show resolved Hide resolved

go mod tidy

8f91d97

kschoche reviewed Apr 13, 2021

View reviewed changes

connect-inject/container_init.go Outdated Show resolved Hide resolved

kschoche reviewed Apr 13, 2021

View reviewed changes

connect-inject/container_init_test.go Outdated Show resolved Hide resolved

kschoche reviewed Apr 13, 2021

View reviewed changes

connect-inject/endpoints_controller_test.go Show resolved Hide resolved

kschoche reviewed Apr 13, 2021

View reviewed changes

kschoche approved these changes Apr 13, 2021

View reviewed changes

lkysow approved these changes Apr 13, 2021

View reviewed changes

ishustava and others added 2 commits April 13, 2021 17:30

Apply suggestions from code review

18c3637

Co-authored-by: Kyle Schochenmaier <[email protected]> Co-authored-by: Luke Kysow <[email protected]>

More code review improvements

a523db7

ishustava force-pushed the enable-tproxy branch from 209249c to a523db7 Compare April 14, 2021 00:39

ishustava merged commit fd90051 into feature-tproxy Apr 14, 2021

ishustava deleted the enable-tproxy branch April 14, 2021 02:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tproxy for the Consul service mesh #481

Enable tproxy for the Consul service mesh #481

ishustava commented Apr 9, 2021 •

edited

Loading

whiskeysierra commented Apr 12, 2021

ishustava commented Apr 12, 2021

kschoche Apr 13, 2021

ishustava Apr 14, 2021

kschoche Apr 13, 2021

ishustava Apr 14, 2021

kschoche left a comment

lkysow left a comment

lkysow Apr 13, 2021

ishustava Apr 14, 2021 •

edited

Loading

lkysow Apr 13, 2021

ishustava Apr 14, 2021 •

edited

Loading

lkysow Apr 13, 2021

ishustava Apr 14, 2021

lkysow Apr 13, 2021

ishustava Apr 14, 2021

ishustava Apr 14, 2021

		-bootstrap > /consul/connect-inject/envoy-bootstrap.yaml

		{{- if .EnableTransparentProxy }}

		@@ -32,6 +33,7 @@ const (
		kubernetesSuccessReasonMsg = "Kubernetes health checks passing"
		podPendingReasonMsg = "Pod is pending"

Enable tproxy for the Consul service mesh #481

Enable tproxy for the Consul service mesh #481

Conversation

ishustava commented Apr 9, 2021 • edited Loading

whiskeysierra commented Apr 12, 2021

ishustava commented Apr 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kschoche left a comment

Choose a reason for hiding this comment

lkysow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishustava Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishustava Apr 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ishustava commented Apr 9, 2021 •

edited

Loading

ishustava Apr 14, 2021 •

edited

Loading

ishustava Apr 14, 2021 •

edited

Loading