Implement traffic redirection exclusion based on proxy config and user-provided values #10134
Conversation
…r-provided values. * If -proxy-id is provided to the redirect-traffic command, exclude any listener ports from inbound traffic redirection. This includes envoy_prometheus_bind_addr, envoy_stats_bind_addr, and the ListenerPort from the Expose configuration. * Allow users to provide additional inbound and outbound ports, outbound CIDRs and additional user IDs to be excluded from traffic redirection. This affects both the traffic-redirect command and the iptables SDK package.
github-actions
bot
added
the
theme/cli
ishustava
added
backport/1.10
theme/connect
| "Outbound port to exclude from traffic redirection. May be provided multiple times.") | ||
| c.flags.Var((*flags.AppendSliceValue)(&c.excludeOutboundCIDRs), "exclude-outbound-cidr", | ||
| "Outbound CIDR to exclude from traffic redirection. May be provided multiple times.") | ||
| c.flags.Var((*flags.AppendSliceValue)(&c.excludeUIDs), "exclude-uid", |
There was a problem hiding this comment.
Would it be valuable to also be able to exclude GIDs?
There was a problem hiding this comment.
I think it could be. The main reason we're adding this is to better support the VM use case when you might want to exclude other processes/uids/gids from traffic redirection. I'm inclined to wait for users' feedback on this one because I'm not sure what would be valuable there.
| } | ||
|
|
||
| // Exclude the ListenerPort from Expose configs from inbound traffic redirection. | ||
| for _, exposePath := range svc.Proxy.Expose.Paths { |
There was a problem hiding this comment.
One thing I'm realizing is missing is this bit of magic:
https://www.consul.io/docs/connect/registration/service-registration#checks
If someone enables this, Consul will dynamically expose checks on a listener for each check:
Line 3839 in c343544
We don't expose what port is allocated in any of our APIs, similarly to how we don't expose check definitions via any of our APIs, only check statuses. See this comment for more information on why: #7330 (comment)
There are a few potential solutions to discover those ports off the top of my head:
- Expand the
structs.HealthChecktype to include the exposed path. - Add a Meta map to checks, like we have for services, and include the exposed path there.
- Include all check definition details in
structs.HealthCheck. That comes with the security risk mentioned above.
There was a problem hiding this comment.
So based on the discussion today we should go with #1.
The exposed ports are generated in a few places.
One is where we add checks:
Line 2610 in 0c9555e
Line 2541 in 0c9555e
In addCheck it's easy to expand the HealthCheck type, since we already have the check pointer to it.
And the other is where we add services with checks:
Line 3791 in 0c9555e
Line 3818 in 0c9555e
In these two other methods we'll need to query a.State.Check, I think, and modify that. (Assuming the state lock is held)
There was a problem hiding this comment.
That's a great find! Let me address this in a separate PR so that this one can be included in beta2.
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/359312. |
|
🍒✅ Cherry pick of commit 8dffb89 onto |
…er-provided values (#10134) * Use proxy outbound port from TransparentProxyConfig if provided * If -proxy-id is provided to the redirect-traffic command, exclude any listener ports from inbound traffic redirection. This includes envoy_prometheus_bind_addr, envoy_stats_bind_addr, and the ListenerPort from the Expose configuration. * Allow users to provide additional inbound and outbound ports, outbound CIDRs and additional user IDs to be excluded from traffic redirection. This affects both the traffic-redirect command and the iptables SDK package.
-proxy-idis provided to the redirect-traffic command, exclude any listener ports from inbound traffic redirection. This includesenvoy_prometheus_bind_addr,envoy_stats_bind_addr, and theListenerPortfrom theExposeconfiguration.