From 50a5549f8a59f61f68a4ce2cd302458d78560a98 Mon Sep 17 00:00:00 2001 From: Derek Menteer <105233703+hashi-derek@users.noreply.github.com> Date: Tue, 13 Dec 2022 09:16:31 -0600 Subject: [PATCH] Fix DialedDirectly configuration for Consul dataplane. (#15760) Fix DialedDirectly configuration for Consul dataplane. --- .changelog/15760.txt | 3 + agent/consul/discoverychain/compile.go | 10 +- agent/consul/discoverychain/compile_test.go | 118 ++++++++++++++++++++ agent/proxycfg/state_test.go | 15 ++- agent/proxycfg/upstreams.go | 21 +++- agent/structs/discovery_chain.go | 5 +- 6 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 .changelog/15760.txt diff --git a/.changelog/15760.txt b/.changelog/15760.txt new file mode 100644 index 000000000..83816802b --- /dev/null +++ b/.changelog/15760.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: Fix issue where DialedDirectly configuration was not used by Consul Dataplane. +``` diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index 3a9a1f0ed..da8754cb0 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -1007,10 +1007,16 @@ RESOLVE_AGAIN: // Default mesh gateway settings if serviceDefault := c.entries.GetService(targetID); serviceDefault != nil { target.MeshGateway = serviceDefault.MeshGateway + target.TransparentProxy.DialedDirectly = serviceDefault.TransparentProxy.DialedDirectly } proxyDefault := c.entries.GetProxyDefaults(targetID.PartitionOrDefault()) - if proxyDefault != nil && target.MeshGateway.Mode == structs.MeshGatewayModeDefault { - target.MeshGateway.Mode = proxyDefault.MeshGateway.Mode + if proxyDefault != nil { + if target.MeshGateway.Mode == structs.MeshGatewayModeDefault { + target.MeshGateway.Mode = proxyDefault.MeshGateway.Mode + } + if !target.TransparentProxy.DialedDirectly { + target.TransparentProxy.DialedDirectly = proxyDefault.TransparentProxy.DialedDirectly + } } if c.overrideMeshGateway.Mode != structs.MeshGatewayModeDefault { diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go index a4c9c65ed..590e9f87c 100644 --- a/agent/consul/discoverychain/compile_test.go +++ b/agent/consul/discoverychain/compile_test.go @@ -82,6 +82,11 @@ func TestCompile(t *testing.T) { // circular references "circular resolver redirect": testcase_Resolver_CircularRedirect(), "circular split": testcase_CircularSplit(), + + // tproxy + "tproxy service defaults only": testcase_ServiceDefaultsTProxy(), + "tproxy proxy defaults only": testcase_ProxyDefaultsTProxy(), + "tproxy service defaults override": testcase_ServiceDefaultsOverrideTProxy(), } for name, tc := range cases { @@ -2942,6 +2947,119 @@ func testcase_LBResolver() compileTestCase { return compileTestCase{entries: entries, expect: expect} } +func testcase_ServiceDefaultsTProxy() compileTestCase { + entries := newEntries() + entries.AddServices( + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "main", + TransparentProxy: structs.TransparentProxyConfig{ + DialedDirectly: true, + }, + }, + ) + + expect := &structs.CompiledDiscoveryChain{ + Protocol: "tcp", + Default: true, + StartNode: "resolver:main.default.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:main.default.default.dc1": { + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "main.default.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "main.default.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { + t.TransparentProxy.DialedDirectly = true + }), + }, + } + return compileTestCase{entries: entries, expect: expect} +} + +func testcase_ProxyDefaultsTProxy() compileTestCase { + entries := newEntries() + entries.AddProxyDefaults(&structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + TransparentProxy: structs.TransparentProxyConfig{ + DialedDirectly: true, + }, + }) + + expect := &structs.CompiledDiscoveryChain{ + Protocol: "tcp", + Default: true, + StartNode: "resolver:main.default.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:main.default.default.dc1": { + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "main.default.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "main.default.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { + t.TransparentProxy.DialedDirectly = true + }), + }, + } + return compileTestCase{entries: entries, expect: expect} +} + +func testcase_ServiceDefaultsOverrideTProxy() compileTestCase { + entries := newEntries() + entries.AddProxyDefaults(&structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + TransparentProxy: structs.TransparentProxyConfig{ + DialedDirectly: false, + }, + }) + entries.AddServices( + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "main", + TransparentProxy: structs.TransparentProxyConfig{ + DialedDirectly: true, + }, + }, + ) + + expect := &structs.CompiledDiscoveryChain{ + Protocol: "tcp", + Default: true, + StartNode: "resolver:main.default.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:main.default.default.dc1": { + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "main.default.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "main.default.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { + t.TransparentProxy.DialedDirectly = true + }), + }, + } + return compileTestCase{entries: entries, expect: expect} +} + func newSimpleRoute(name string, muts ...func(*structs.ServiceRoute)) structs.ServiceRoute { r := structs.ServiceRoute{ Match: &structs.ServiceRouteMatch{ diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 85cf0228e..894f2bd47 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -2359,7 +2359,16 @@ func TestState_WatchesAndUpdates(t *testing.T) { { CorrelationID: "discovery-chain:" + dbUID.String(), Result: &structs.DiscoveryChainResponse{ - Chain: discoverychain.TestCompileConfigEntries(t, "db", "default", "default", "dc1", "trustdomain.consul", nil), + Chain: discoverychain.TestCompileConfigEntries( + t, "db", "default", "default", "dc1", "trustdomain.consul", nil, + &structs.ServiceConfigEntry{ + Kind: structs.ServiceDefaults, + Name: "db", + TransparentProxy: structs.TransparentProxyConfig{ + DialedDirectly: true, + }, + }, + ), }, Err: nil, }, @@ -2396,7 +2405,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { Proxy: structs.ConnectProxyConfig{ DestinationServiceName: "db", TransparentProxy: structs.TransparentProxyConfig{ - DialedDirectly: true, + DialedDirectly: false, // This is set true by the service-defaults entry above. }, }, RaftIndex: structs.RaftIndex{ @@ -2455,7 +2464,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { Proxy: structs.ConnectProxyConfig{ DestinationServiceName: "db", TransparentProxy: structs.TransparentProxyConfig{ - DialedDirectly: true, + DialedDirectly: false, }, }, RaftIndex: structs.RaftIndex{ diff --git a/agent/proxycfg/upstreams.go b/agent/proxycfg/upstreams.go index 4ebbabb65..31a51c661 100644 --- a/agent/proxycfg/upstreams.go +++ b/agent/proxycfg/upstreams.go @@ -131,6 +131,7 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u UpdateEv } upstreamsSnapshot.WatchedUpstreamEndpoints[uid][targetID] = resp.Nodes + // Skip adding passthroughs unless it's a connect sidecar in tproxy mode. if s.kind != structs.ServiceKindConnectProxy || s.proxyCfg.Mode != structs.ProxyModeTransparent { return nil } @@ -148,7 +149,17 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u UpdateEv passthroughs := make(map[string]struct{}) for _, node := range resp.Nodes { - if !node.Service.Proxy.TransparentProxy.DialedDirectly { + dialedDirectly := node.Service.Proxy.TransparentProxy.DialedDirectly + // We must do a manual merge here on the DialedDirectly field, because the service-defaults + // and proxy-defaults are not automatically merged into the CheckServiceNodes when in + // agentless mode (because the streaming backend doesn't yet support the MergeCentralConfig field). + if chain := snap.ConnectProxy.DiscoveryChain[uid]; chain != nil { + if target := chain.Targets[targetID]; target != nil { + dialedDirectly = dialedDirectly || target.TransparentProxy.DialedDirectly + } + } + // Skip adding a passthrough for the upstream node if not DialedDirectly. + if !dialedDirectly { continue } @@ -179,10 +190,12 @@ func (s *handlerUpstreams) handleUpdateUpstreams(ctx context.Context, u UpdateEv upstreamsSnapshot.PassthroughIndices[addr] = indexedTarget{idx: csnIdx, upstreamID: uid, targetID: targetID} passthroughs[addr] = struct{}{} } + // Always clear out the existing target passthroughs list so that clusters are cleaned up + // correctly if no entries are populated. + upstreamsSnapshot.PassthroughUpstreams[uid] = make(map[string]map[string]struct{}) if len(passthroughs) > 0 { - upstreamsSnapshot.PassthroughUpstreams[uid] = map[string]map[string]struct{}{ - targetID: passthroughs, - } + // Add the passthroughs to the target if any were found. + upstreamsSnapshot.PassthroughUpstreams[uid][targetID] = passthroughs } case strings.HasPrefix(u.CorrelationID, "mesh-gateway:"): diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index ca64d070d..6d1beb1e7 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -192,8 +192,9 @@ type DiscoveryTarget struct { Datacenter string `json:",omitempty"` Peer string `json:",omitempty"` - MeshGateway MeshGatewayConfig `json:",omitempty"` - Subset ServiceResolverSubset `json:",omitempty"` + MeshGateway MeshGatewayConfig `json:",omitempty"` + Subset ServiceResolverSubset `json:",omitempty"` + TransparentProxy TransparentProxyConfig `json:",omitempty"` ConnectTimeout time.Duration `json:",omitempty"`