From eacb73cb78da7d92326a3766adb1e1e8bb1d2366 Mon Sep 17 00:00:00 2001 From: freddygv Date: Fri, 22 Oct 2021 15:13:48 -0600 Subject: [PATCH] Remove useInDatacenter from disco chain requests useInDatacenter was used to determine whether the mesh gateway mode of the upstream should be returned in the discovery chain target. This commit makes it so that the mesh gateway mode is returned every time, and it is up to the caller to decide whether mesh gateways should be watched or used. --- agent/consul/discovery_chain_endpoint.go | 1 - agent/consul/discoverychain/compile.go | 15 ++++----------- agent/consul/discoverychain/compile_test.go | 13 ++++++++++--- agent/consul/discoverychain/testing.go | 3 +-- agent/consul/state/config_entry.go | 4 ---- agent/discovery_chain_endpoint_test.go | 9 +++++++-- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/agent/consul/discovery_chain_endpoint.go b/agent/consul/discovery_chain_endpoint.go index 7cd8e39425..3e91dd9572 100644 --- a/agent/consul/discovery_chain_endpoint.go +++ b/agent/consul/discovery_chain_endpoint.go @@ -57,7 +57,6 @@ func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs EvaluateInNamespace: entMeta.NamespaceOrDefault(), EvaluateInPartition: entMeta.PartitionOrDefault(), EvaluateInDatacenter: evalDC, - UseInDatacenter: c.srv.config.Datacenter, OverrideMeshGateway: args.OverrideMeshGateway, OverrideProtocol: args.OverrideProtocol, OverrideConnectTimeout: args.OverrideConnectTimeout, diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index 758af0da20..99f4c357d0 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -18,7 +18,6 @@ type CompileRequest struct { EvaluateInPartition string EvaluateInDatacenter string EvaluateInTrustDomain string - UseInDatacenter string // where the results will be used from // OverrideMeshGateway allows for the setting to be overridden for any // resolver in the compiled chain. @@ -62,7 +61,6 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) { evaluateInPartition = req.EvaluateInPartition evaluateInDatacenter = req.EvaluateInDatacenter evaluateInTrustDomain = req.EvaluateInTrustDomain - useInDatacenter = req.UseInDatacenter entries = req.Entries ) if serviceName == "" { @@ -80,9 +78,6 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) { if evaluateInTrustDomain == "" { return nil, fmt.Errorf("evaluateInTrustDomain is required") } - if useInDatacenter == "" { - return nil, fmt.Errorf("useInDatacenter is required") - } if entries == nil { return nil, fmt.Errorf("entries is required") } @@ -93,7 +88,6 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) { evaluateInNamespace: evaluateInNamespace, evaluateInDatacenter: evaluateInDatacenter, evaluateInTrustDomain: evaluateInTrustDomain, - useInDatacenter: useInDatacenter, overrideMeshGateway: req.OverrideMeshGateway, overrideProtocol: req.OverrideProtocol, overrideConnectTimeout: req.OverrideConnectTimeout, @@ -130,7 +124,6 @@ type compiler struct { evaluateInPartition string evaluateInDatacenter string evaluateInTrustDomain string - useInDatacenter string overrideMeshGateway structs.MeshGatewayConfig overrideProtocol string overrideConnectTimeout time.Duration @@ -936,10 +929,10 @@ RESOLVE_AGAIN: } } - // TODO (mesh-gateway)- maybe allow using a gateway within a datacenter at some point - if target.Datacenter == c.useInDatacenter { - target.MeshGateway.Mode = structs.MeshGatewayModeDefault - } else if target.External { + // TODO(partitions): Document this change in behavior. Discovery chain targets will now return a mesh gateway + // mode as long as they are not external. Regardless of the datacenter/partition where + // the chain will be used. + if target.External { // Bypass mesh gateways if it is an external service. target.MeshGateway.Mode = structs.MeshGatewayModeDefault } else { diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go index 54c91e2eaf..360b40e48e 100644 --- a/agent/consul/discoverychain/compile_test.go +++ b/agent/consul/discoverychain/compile_test.go @@ -105,7 +105,6 @@ func TestCompile(t *testing.T) { EvaluateInPartition: "default", EvaluateInDatacenter: "dc1", EvaluateInTrustDomain: "trustdomain.consul", - UseInDatacenter: "dc1", Entries: tc.entries, } if tc.setup != nil { @@ -1338,7 +1337,11 @@ func testcase_DatacenterFailover_WithMeshGateways() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), "main.default.default.dc2": newTarget("main", "", "default", "default", "dc2", func(t *structs.DiscoveryTarget) { t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, @@ -1469,7 +1472,11 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true} diff --git a/agent/consul/discoverychain/testing.go b/agent/consul/discoverychain/testing.go index c13801dd7c..c3ab7ba7c4 100644 --- a/agent/consul/discoverychain/testing.go +++ b/agent/consul/discoverychain/testing.go @@ -12,7 +12,7 @@ func TestCompileConfigEntries(t testing.T, evaluateInPartition string, evaluateInDatacenter string, evaluateInTrustDomain string, - useInDatacenter string, + _ string, setup func(req *CompileRequest), entries ...structs.ConfigEntry) *structs.CompiledDiscoveryChain { set := structs.NewDiscoveryChainConfigEntries() @@ -24,7 +24,6 @@ func TestCompileConfigEntries(t testing.T, EvaluateInPartition: evaluateInPartition, EvaluateInDatacenter: evaluateInDatacenter, EvaluateInTrustDomain: evaluateInTrustDomain, - UseInDatacenter: useInDatacenter, Entries: set, } if setup != nil { diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go index adc6d4e354..0ec343e6b4 100644 --- a/agent/consul/state/config_entry.go +++ b/agent/consul/state/config_entry.go @@ -394,7 +394,6 @@ func (s *Store) discoveryChainTargetsTxn(tx ReadTxn, ws memdb.WatchSet, dc, serv EvaluateInNamespace: source.NamespaceOrDefault(), EvaluateInPartition: source.PartitionOrDefault(), EvaluateInDatacenter: dc, - UseInDatacenter: dc, } idx, chain, err := s.serviceDiscoveryChainTxn(tx, ws, source.Name, entMeta, req) if err != nil { @@ -452,7 +451,6 @@ func (s *Store) discoveryChainSourcesTxn(tx ReadTxn, ws memdb.WatchSet, dc strin EvaluateInNamespace: sn.NamespaceOrDefault(), EvaluateInPartition: sn.PartitionOrDefault(), EvaluateInDatacenter: dc, - UseInDatacenter: dc, } idx, chain, err := s.serviceDiscoveryChainTxn(tx, ws, sn.Name, &sn.EnterpriseMeta, req) if err != nil { @@ -723,7 +721,6 @@ func testCompileDiscoveryChain( EvaluateInPartition: entMeta.PartitionOrDefault(), EvaluateInDatacenter: "dc1", EvaluateInTrustDomain: "b6fc9da3-03d4-4b5a-9134-c045e9b20152.consul", - UseInDatacenter: "dc1", Entries: speculativeEntries, } chain, err := discoverychain.Compile(req) @@ -1208,7 +1205,6 @@ func protocolForService( EvaluateInDatacenter: "dc1", // Use a dummy trust domain since that won't affect the protocol here. EvaluateInTrustDomain: "b6fc9da3-03d4-4b5a-9134-c045e9b20152.consul", - UseInDatacenter: "dc1", Entries: entries, } chain, err := discoverychain.Compile(req) diff --git a/agent/discovery_chain_endpoint_test.go b/agent/discovery_chain_endpoint_test.go index 177f9e5daa..78fcfe303f 100644 --- a/agent/discovery_chain_endpoint_test.go +++ b/agent/discovery_chain_endpoint_test.go @@ -264,6 +264,11 @@ func TestDiscoveryChainRead(t *testing.T) { }) })) + expectTarget_DC1 := newTarget("web", "", "default", "default", "dc1") + expectTarget_DC1.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeLocal, + } + expectTarget_DC2 := newTarget("web", "", "default", "default", "dc2") expectTarget_DC2.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeLocal, @@ -291,8 +296,8 @@ func TestDiscoveryChainRead(t *testing.T) { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "web.default.default.dc1": newTarget("web", "", "default", "default", "dc1"), - expectTarget_DC2.ID: expectTarget_DC2, + expectTarget_DC1.ID: expectTarget_DC1, + expectTarget_DC2.ID: expectTarget_DC2, }, }