luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

server: ensure that service-defaults meta is incorporated into the discovery chain response (#12511) 

Also add a new "Default" field to the discovery chain response to clients
This commit is contained in:
R.B. Boyer 2022-03-30 10:04:18 -05:00 committed by GitHub
parent 0fd6cdc900
commit d4e80b8800
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 214 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
.changelog/12511.txt Normal file
View File

@ -0,0 +1,7 @@
```release-note:feature
server: ensure that service-defaults meta is incorporated into the discovery chain response
```
```release-note:feature
server: discovery chains now include a response field named "Default" to indicate if they were not constructed from any service-resolver, service-splitter, or service-router config entries
```

8
agent/consul/discovery_chain_endpoint_test.go
View File

@ -98,6 +98,7 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) {
Datacenter: "dc1",
Protocol: "tcp",
StartNode: "resolver:web.default.default.dc1",
Default: true,
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:web.default.default.dc1": {
Type: structs.DiscoveryGraphNodeTypeResolver,
@ -286,12 +287,7 @@ func TestDiscoveryChainEndpoint_Get_BlockOnNoChange(t *testing.T) {
args.QueryOptions.MinQueryIndex = minQueryIndex
var out structs.DiscoveryChainResponse
errCh := channelCallRPC(s1, "DiscoveryChain.Get", &args, &out, func() error {
if !out.Chain.IsDefault() {
return fmt.Errorf("expected default chain")
}
return nil
})
errCh := channelCallRPC(s1, "DiscoveryChain.Get", &args, &out, nil)
return &out.QueryMeta, errCh
},
func(i int) <-chan error {

44
agent/consul/discoverychain/compile.go
View File

@ -161,6 +161,12 @@ type compiler struct {
// This is an OUTPUT field.
protocol string
// serviceMeta is the Meta field from the service-defaults entry that
// shares a name with this discovery chain.
//
// This is an OUTPUT field.
serviceMeta map[string]string
// startNode is computed inside of assembleChain()
//
// This is an OUTPUT field.
@ -327,14 +333,47 @@ func (c *compiler) compile() (*structs.CompiledDiscoveryChain, error) {
Namespace: c.evaluateInNamespace,
Partition: c.evaluateInPartition,
Datacenter: c.evaluateInDatacenter,
Default: c.determineIfDefaultChain(),
CustomizationHash: customizationHash,
Protocol: c.protocol,
ServiceMeta: c.serviceMeta,
StartNode: c.startNode,
Nodes: c.nodes,
Targets: c.loadedTargets,
}, nil
}
// determineIfDefaultChain returns true if the compiled chain represents no
// routing, no splitting, and only the default resolution. We have to be
// careful here to avoid returning "yep this is default" when the only
// resolver action being applied is redirection to another resolver that is
// default, so we double check the resolver matches the requested resolver.
//
// NOTE: "default chain" mostly means that this is compatible with how things
// worked (roughly) in consul 1.5 pre-discovery chain, not that there are zero
// config entries in play (like service-defaults).
func (c *compiler) determineIfDefaultChain() bool {
if c.startNode == "" || len(c.nodes) == 0 {
return true
}
node := c.nodes[c.startNode]
if node == nil {
panic("not possible: missing node named '" + c.startNode + "' in chain '" + c.serviceName + "'")
}
if node.Type != structs.DiscoveryGraphNodeTypeResolver {
return false
}
if !node.Resolver.Default {
return false
}
target := c.loadedTargets[node.Resolver.Target]
return target.Service == c.serviceName && target.Namespace == c.evaluateInNamespace && target.Partition == c.evaluateInPartition
}
func (c *compiler) detectCircularReferences() error {
var (
todo stringStack
@ -515,6 +554,11 @@ func (c *compiler) assembleChain() error {
sid := structs.NewServiceID(c.serviceName, c.GetEnterpriseMeta())
// Extract the service meta for the service named by this discovery chain.
if serviceDefault := c.entries.GetService(sid); serviceDefault != nil {
c.serviceMeta = serviceDefault.GetMeta()
}
// Check for short circuit path.
if len(c.resolvers) == 0 && c.entries.IsChainEmpty() {
// Materialize defaults and cache.

125
agent/consul/discoverychain/compile_test.go
View File

@ -15,8 +15,6 @@ type compileTestCase struct {
entries *configentry.DiscoveryChainSet
setup func(req *CompileRequest)
expect *structs.CompiledDiscoveryChain
// expectIsDefault tests behavior of CompiledDiscoveryChain.IsDefault()
expectIsDefault bool
expectCustom bool
expectErr string
expectGraphErr bool
@ -56,6 +54,8 @@ func TestCompile(t *testing.T) {
"loadbalancer splitter and resolver": testcase_LBSplitterAndResolver(),
"loadbalancer resolver": testcase_LBResolver(),
"service redirect to service with default resolver is not a default chain": testcase_RedirectToDefaultResolverIsNotDefaultChain(),
"service meta projection": testcase_ServiceMetaProjection(),
"service meta projection with redirect": testcase_ServiceMetaProjectionWithRedirect(),
"all the bells and whistles": testcase_AllBellsAndWhistles(),
"multi dc canary": testcase_MultiDatacenterCanary(),
@ -141,7 +141,6 @@ func TestCompile(t *testing.T) {
}
require.Equal(t, tc.expect, res)
require.Equal(t, tc.expectIsDefault, res.IsDefault())
}
})
}
@ -1429,6 +1428,7 @@ func testcase_DefaultResolver() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
Default: true,
StartNode: "resolver:main.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
@ -1446,7 +1446,7 @@ func testcase_DefaultResolver() compileTestCase {
"main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_DefaultResolver_WithProxyDefaults() compileTestCase {
@ -1465,6 +1465,7 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "grpc",
Default: true,
StartNode: "resolver:main.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
@ -1485,11 +1486,69 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase {
}),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase {
func testcase_ServiceMetaProjection() compileTestCase {
entries := newEntries()
entries.AddServices(
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
Meta: map[string]string{
"foo": "bar",
"abc": "123",
},
},
)
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
Default: true,
ServiceMeta: map[string]string{
"foo": "bar",
"abc": "123",
},
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("main", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_ServiceMetaProjectionWithRedirect() compileTestCase {
entries := newEntries()
entries.AddServices(
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
Meta: map[string]string{
"foo": "bar",
"abc": "123",
},
},
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "other",
Meta: map[string]string{
"zim": "gir",
"abc": "456",
"xyz": "999",
},
},
)
entries.AddResolvers(
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
@ -1502,6 +1561,11 @@ func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
ServiceMeta: map[string]string{
// Note this is main's meta, not other's.
"foo": "bar",
"abc": "123",
},
StartNode: "resolver:other.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:other.default.default.dc1": {
@ -1519,7 +1583,42 @@ func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase {
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: false /*being explicit here because this is the whole point of this test*/}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase {
entries := newEntries()
entries.AddResolvers(
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "main",
Redirect: &structs.ServiceResolverRedirect{
Service: "other",
},
},
)
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
StartNode: "resolver:other.default.default.dc1",
Default: false, /*being explicit here because this is the whole point of this test*/
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:other.default.default.dc1": {
Type: structs.DiscoveryGraphNodeTypeResolver,
Name: "other.default.default.dc1",
Resolver: &structs.DiscoveryResolver{
Default: true,
ConnectTimeout: 5 * time.Second,
Target: "other.default.default.dc1",
},
},
},
Targets: map[string]*structs.DiscoveryTarget{
"other.default.default.dc1": newTarget("other", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_Resolve_WithDefaultSubset() compileTestCase {
@ -1570,6 +1669,7 @@ func testcase_DefaultResolver_ExternalSNI() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
Default: true,
StartNode: "resolver:main.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
@ -1589,7 +1689,7 @@ func testcase_DefaultResolver_ExternalSNI() compileTestCase {
}),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true}
return compileTestCase{entries: entries, expect: expect}
}
func testcase_Resolver_ExternalSNI_FailoverNotAllowed() compileTestCase {
@ -2249,6 +2349,7 @@ func testcase_ResolverProtocolOverride() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "http2",
Default: true,
StartNode: "resolver:main.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
@ -2266,7 +2367,7 @@ func testcase_ResolverProtocolOverride() compileTestCase {
"main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true,
return compileTestCase{entries: entries, expect: expect,
expectCustom: true,
setup: func(req *CompileRequest) {
req.OverrideProtocol = "http2"
@ -2282,6 +2383,7 @@ func testcase_ResolverProtocolOverrideIgnored() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "http2",
Default: true,
StartNode: "resolver:main.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
@ -2299,7 +2401,7 @@ func testcase_ResolverProtocolOverrideIgnored() compileTestCase {
"main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true,
return compileTestCase{entries: entries, expect: expect,
setup: func(req *CompileRequest) {
req.OverrideProtocol = "http2"
},
@ -2320,6 +2422,7 @@ func testcase_RouterIgnored_ResolverProtocolOverride() compileTestCase {
expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
StartNode: "resolver:main.default.default.dc1",
Default: true,
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.default.dc1": {
Type: structs.DiscoveryGraphNodeTypeResolver,
@ -2336,7 +2439,7 @@ func testcase_RouterIgnored_ResolverProtocolOverride() compileTestCase {
"main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true,
return compileTestCase{entries: entries, expect: expect,
expectCustom: true,
setup: func(req *CompileRequest) {
req.OverrideProtocol = "tcp"

6
agent/discovery_chain_endpoint_test.go
View File

@ -8,11 +8,12 @@ import (
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/sdk/testutil/retry"
"github.com/hashicorp/consul/testrpc"
"github.com/stretchr/testify/require"
)
func TestDiscoveryChainRead(t *testing.T) {
@ -79,6 +80,7 @@ func TestDiscoveryChainRead(t *testing.T) {
Partition: "default",
Datacenter: "dc1",
Protocol: "tcp",
Default: true,
StartNode: "resolver:web.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:web.default.default.dc1": {
@ -122,6 +124,7 @@ func TestDiscoveryChainRead(t *testing.T) {
Namespace: "default",
Partition: "default",
Datacenter: "dc2",
Default: true,
Protocol: "tcp",
StartNode: "resolver:web.default.default.dc2",
Nodes: map[string]*structs.DiscoveryGraphNode{
@ -175,6 +178,7 @@ func TestDiscoveryChainRead(t *testing.T) {
Namespace: "default",
Partition: "default",
Datacenter: "dc1",
Default: true,
Protocol: "tcp",
StartNode: "resolver:web.default.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{

2
agent/proxycfg/testing_connect_proxy.go
View File

@ -16,7 +16,7 @@ func TestConfigSnapshot(t testing.T, nsFn func(ns *structs.NodeService), extraUp
// no entries implies we'll get a default chain
dbChain := discoverychain.TestCompileConfigEntries(t, "db", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
assert.True(t, dbChain.IsDefault())
assert.True(t, dbChain.Default)
var (
upstreams = structs.TestUpstreams(t)

8
agent/proxycfg/testing_ingress_gateway.go
View File

@ -685,10 +685,10 @@ func TestConfigSnapshotIngress_HTTPMultipleServices(t testing.T) *ConfigSnapshot
quxChain = discoverychain.TestCompileConfigEntries(t, "qux", "default", "default", "dc1", connect.TestClusterID+".consul", nil, entries...)
)
require.False(t, fooChain.IsDefault())
require.False(t, barChain.IsDefault())
require.True(t, bazChain.IsDefault())
require.True(t, quxChain.IsDefault())
require.False(t, fooChain.Default)
require.False(t, barChain.Default)
require.True(t, bazChain.Default)
require.True(t, quxChain.Default)
return TestConfigSnapshotIngressGateway(t, false, "http", "default", nil, func(entry *structs.IngressGatewayConfigEntry) {
entry.Listeners = []structs.IngressListener{

35
agent/structs/discovery_chain.go
View File

@ -26,9 +26,17 @@ type CompiledDiscoveryChain struct {
// non-customized versions.
CustomizationHash string `json:",omitempty"`
// Default indicates if this discovery chain is based on no
// service-resolver, service-splitter, or service-router config entries.
Default bool `json:",omitempty"`
// Protocol is the overall protocol shared by everything in the chain.
Protocol string `json:",omitempty"`
// ServiceMeta is the metadata from the underlying service-defaults config
// entry for the service named ServiceName.
ServiceMeta map[string]string `json:",omitempty"`
// StartNode is the first key into the Nodes map that should be followed
// when walking the discovery chain.
StartNode string `json:",omitempty"`
@ -62,33 +70,6 @@ func (c *CompiledDiscoveryChain) WillFailoverThroughMeshGateway(node *DiscoveryG
return false
}
// IsDefault returns true if the compiled chain represents no routing, no
// splitting, and only the default resolution. We have to be careful here to
// avoid returning "yep this is default" when the only resolver action being
// applied is redirection to another resolver that is default, so we double
// check the resolver matches the requested resolver.
func (c *CompiledDiscoveryChain) IsDefault() bool {
if c.StartNode == "" || len(c.Nodes) == 0 {
return true
}
node := c.Nodes[c.StartNode]
if node == nil {
panic("not possible: missing node named '" + c.StartNode + "' in chain '" + c.ServiceName + "'")
}
if node.Type != DiscoveryGraphNodeTypeResolver {
return false
}
if !node.Resolver.Default {
return false
}
target := c.Targets[node.Resolver.Target]
return target.Service == c.ServiceName && target.Namespace == c.Namespace && target.Partition == c.Partition
}
// ID returns an ID that encodes the service, namespace, partition, and datacenter.
// This ID allows us to compare a discovery chain target to the chain upstream itself.
func (c *CompiledDiscoveryChain) ID() string {

2
agent/xds/clusters.go
View File

@ -612,7 +612,7 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain(
var escapeHatchCluster *envoy_cluster_v3.Cluster
if cfg.EnvoyClusterJSON != "" {
if chain.IsDefault() {
if chain.Default {
// If you haven't done anything to setup the discovery chain, then
// you can use the envoy_cluster_json escape hatch.
escapeHatchCluster, err = makeClusterFromUserConfig(cfg.EnvoyClusterJSON)

2
agent/xds/endpoints.go
View File

@ -393,7 +393,7 @@ func (s *ResourceGenerator) endpointsFromDiscoveryChain(
var escapeHatchCluster *envoy_cluster_v3.Cluster
if cfg.EnvoyClusterJSON != "" {
if chain.IsDefault() {
if chain.Default {
// If you haven't done anything to setup the discovery chain, then
// you can use the envoy_cluster_json escape hatch.
escapeHatchCluster, err = makeClusterFromUserConfig(cfg.EnvoyClusterJSON)

4
agent/xds/listeners.go
View File

@ -115,7 +115,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg.
}
// RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain.
useRDS := chain.Protocol != "tcp" && !chain.IsDefault()
useRDS := chain.Protocol != "tcp" && !chain.Default
var clusterName string
if !useRDS {
@ -1303,7 +1303,7 @@ func (s *ResourceGenerator) getAndModifyUpstreamConfigForListener(
if u != nil {
configMap = u.Config
}
if chain == nil || chain.IsDefault() {
if chain == nil || chain.Default {
cfg, err = structs.ParseUpstreamConfigNoDefaults(configMap)
if err != nil {
// Don't hard fail on a config typo, just warn. The parse func returns

2
agent/xds/listeners_ingress.go
View File

@ -48,7 +48,7 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap
// RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain.
// TODO(freddy): Why can the protocol of the listener be overridden here?
useRDS := cfg.Protocol != "tcp" && !chain.IsDefault()
useRDS := cfg.Protocol != "tcp" && !chain.Default
var clusterName string
if !useRDS {

2
agent/xds/routes.go
View File

@ -45,7 +45,7 @@ func (s *ResourceGenerator) routesFromSnapshot(cfgSnap *proxycfg.ConfigSnapshot)
func (s *ResourceGenerator) routesForConnectProxy(cfgSnap *proxycfg.ConfigSnapshot) ([]proto.Message, error) {
var resources []proto.Message
for uid, chain := range cfgSnap.ConnectProxy.DiscoveryChain {
if chain.IsDefault() {
if chain.Default {
continue
}

8
api/discovery_chain.go
View File

@ -109,9 +109,17 @@ type CompiledDiscoveryChain struct {
// non-customized versions.
CustomizationHash string
// Default indicates if this discovery chain is based on no
// service-resolver, service-splitter, or service-router config entries.
Default bool
// Protocol is the overall protocol shared by everything in the chain.
Protocol string
// ServiceMeta is the metadata from the underlying service-defaults config
// entry for the service named ServiceName.
ServiceMeta map[string]string
// StartNode is the first key into the Nodes map that should be followed
// when walking the discovery chain.
StartNode string

2
api/discovery_chain_test.go
View File

@ -32,6 +32,7 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
Namespace: "default",
Datacenter: "dc1",
Protocol: "tcp",
Default: true,
StartNode: "resolver:web.default.default.dc1",
Nodes: map[string]*DiscoveryGraphNode{
"resolver:web.default.default.dc1": {
@ -72,6 +73,7 @@ func TestAPI_DiscoveryChain_Get(t *testing.T) {
Namespace: "default",
Datacenter: "dc2",
Protocol: "tcp",
Default: true,
StartNode: "resolver:web.default.default.dc2",
Nodes: map[string]*DiscoveryGraphNode{
"resolver:web.default.default.dc2": {

6
website/content/docs/connect/l7-traffic/discovery-chain.mdx
View File

@ -121,9 +121,15 @@ resolved by name using the [`Targets`](#targets) field.
balancer data plane objects to avoid sharing customized and non-customized
versions.
- `Default` `(bool: <optional>)` - Indicates if this discovery chain is based on no
`service-resolver`, `service-splitter`, or `service-router` config entries.
- `Protocol` `(string)` - The overall protocol shared by everything in the
chain.
- `ServiceMeta` `(map<string|string>)` - The metadata from the underlying `service-defaults` config
entry for the service named `ServiceName`.
- `StartNode` `(string)` - The first key into the `Nodes` map that should be
followed when traversing the discovery chain.