Add flag for transparent proxies to dial individual instances #10329
Conversation
github-actions
bot
added
theme/api
freddygv
force-pushed
the
tproxy/support-dialing-instances
branch
from
2477b73
to
0f87c47
Compare
freddygv
force-pushed
the
tproxy/support-dialing-instances
branch
from
0f87c47
to
dfcd192
Compare
| @@ -352,40 +352,41 @@ func resourceTagSpecifiers(omitDeprecatedTags bool) ([]string, error) { | |||
| // - cluster.f8f8f8f8~pong.default.dc2.internal.e5b08d03-bfc3-c870-1833-baddb116e648.consul.bind_errors: 0 | |||
| // - cluster.v2.pong.default.dc2.internal.e5b08d03-bfc3-c870-1833-baddb116e648.consul.bind_errors: 0 | |||
| // - cluster.f8f8f8f8~v2.pong.default.dc2.internal.e5b08d03-bfc3-c870-1833-baddb116e648.consul.bind_errors: 0 | |||
| // - cluster.passthrough.pong.default.dc2.internal.e5b08d03-bfc3-c870-1833-baddb116e648.consul.bind_errors: 0 | |||
There was a problem hiding this comment.
This was updated so we can continue to correctly label metrics for this upstream, in the presence of two distinct cluster names.
| Note that when dialing individual instances HTTP routing rules configured with config entries | ||
| will **not** be considered. The transparent proxy acts as a TCP proxy to the original | ||
| destination IP address. |
There was a problem hiding this comment.
This is the main bit that gives me pause about the current implementation.
Configuration like retrying requests is applied on HTTP routes, and currently the network filter for passthrough is a TCP proxy. If we did attach an HTTP connection manager, what route config would it use? If it used the one for the upstream name, we would no longer route to the passthrough cluster and would instead load balance across the route destination endpoints.
I'm not sure that we can support L7 rules for headless services without creating a separate service registration for each upstream address. This seems really limiting.
freddygv
force-pushed
the
tproxy/support-dialing-instances
branch
from
f86f841
to
97559dc
Compare
| } | ||
|
|
||
| // Only create the outbound listener when there are upstreams and filter chains are present | ||
| if outboundListener != nil && hasFilterChains { |
There was a problem hiding this comment.
If no chains with matches on virtual IPs were created above then we would skip the block below, which was unintended.
… with virtual ips
We don't name filter chains, so the existing sort was a no-op. Sorting on the match addresses is reasonable because each address is guaranteed to be unique. If not unique, Envoy will reject the config.
freddygv
force-pushed
the
tproxy/support-dialing-instances
branch
from
fb157e8
to
ef2f6f7
Compare
| @@ -197,7 +197,8 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. | |||
|
|
|||
| // Filter chains are stable sorted to avoid draining if the list is provided out of order | |||
| sort.SliceStable(outboundListener.FilterChains, func(i, j int) bool { | |||
| return outboundListener.FilterChains[i].Name < outboundListener.FilterChains[j].Name | |||
| return outboundListener.FilterChains[i].FilterChainMatch.PrefixRanges[0].AddressPrefix < | |||
There was a problem hiding this comment.
And this makes sense because the PrefixRanges slice is already sorted stably by AddressPrefix, so taking the zeroth element to compare on is fine.
| // the destination service, so that remains intact. | ||
| if node.Service.Kind == structs.ServiceKindConnectProxy { | ||
| dst := node.Service.Proxy.DestinationServiceName | ||
| if dst == "" { |
There was a problem hiding this comment.
isn't this backwards? I thought IDs were more specific than Names
There was a problem hiding this comment.
I think we want to prefer the name for the SNI, and only fall back if it doesn't exist. (which shouldn't happen ™️ )
Follows this: https://github.com/hashicorp/consul/blob/master/agent/structs/structs.go#L952
|
🍒 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/383864. |
|
🍒✅ Cherry pick of commit 429f9d8 onto |
This PR adds a new flag to proxy definitions:
Proxy.TransparentProxy.DialedDirectly.When enabled, services can opt into allowing transparent proxies to dial individual instances.
Typically transparent proxies only dial virtual IPs representing a logical service name. Dialing individual instances can be helpful in cases like stateful services, like a database cluster with a leader.
xDS configuration:
There will be a passthrough original destination cluster for each upstream service name. The cluster name is prefixed with passthrough to distinguish from the non-passthrough cluster for the upstream.
For listeners we will add a filter chain for each logical upstream that routes to the passthrough clusters mentioned above.
Limitations:
Main Commits:
TODO: