diff --git a/agent/cache-types/discovery_chain.go b/agent/cache-types/discovery_chain.go index 41d5633a3..62e4d04f5 100644 --- a/agent/cache-types/discovery_chain.go +++ b/agent/cache-types/discovery_chain.go @@ -38,7 +38,7 @@ func (c *CompiledDiscoveryChain) Fetch(opts cache.FetchOptions, req cache.Reques // Fetch var reply structs.DiscoveryChainResponse - if err := c.RPC.RPC("ConfigEntry.ReadDiscoveryChain", reqReal, &reply); err != nil { + if err := c.RPC.RPC("DiscoveryChain.Get", reqReal, &reply); err != nil { return result, err } diff --git a/agent/cache-types/discovery_chain_test.go b/agent/cache-types/discovery_chain_test.go index 444d38256..a5c5d465d 100644 --- a/agent/cache-types/discovery_chain_test.go +++ b/agent/cache-types/discovery_chain_test.go @@ -16,13 +16,12 @@ func TestCompiledDiscoveryChain(t *testing.T) { typ := &CompiledDiscoveryChain{RPC: rpc} // just do the default chain - entries := structs.NewDiscoveryChainConfigEntries() chain := discoverychain.TestCompileConfigEntries(t, "web", "default", "dc1", nil) // Expect the proper RPC call. This also sets the expected value // since that is return-by-pointer in the arguments. var resp *structs.DiscoveryChainResponse - rpc.On("RPC", "ConfigEntry.ReadDiscoveryChain", mock.Anything, mock.Anything).Return(nil). + rpc.On("RPC", "DiscoveryChain.Get", mock.Anything, mock.Anything).Return(nil). Run(func(args mock.Arguments) { req := args.Get(1).(*structs.DiscoveryChainRequest) require.Equal(t, uint64(24), req.QueryOptions.MinQueryIndex) @@ -30,7 +29,6 @@ func TestCompiledDiscoveryChain(t *testing.T) { require.True(t, req.AllowStale) reply := args.Get(2).(*structs.DiscoveryChainResponse) - reply.ConfigEntries = entries reply.Chain = chain reply.QueryMeta.Index = 48 resp = reply diff --git a/agent/consul/acl_endpoint_test.go b/agent/consul/acl_endpoint_test.go index 4acc1f8bb..daeb27c88 100644 --- a/agent/consul/acl_endpoint_test.go +++ b/agent/consul/acl_endpoint_test.go @@ -5255,6 +5255,22 @@ func upsertTestToken(codec rpc.ClientCodec, masterToken string, datacenter strin return &out, nil } +func upsertTestTokenWithPolicyRules(codec rpc.ClientCodec, masterToken string, datacenter string, rules string) (*structs.ACLToken, error) { + policy, err := upsertTestPolicyWithRules(codec, masterToken, datacenter, rules) + if err != nil { + return nil, err + } + + token, err := upsertTestToken(codec, masterToken, datacenter, func(token *structs.ACLToken) { + token.Policies = []structs.ACLTokenPolicyLink{{ID: policy.ID}} + }) + if err != nil { + return nil, err + } + + return token, nil +} + func retrieveTestTokenAccessorForSecret(codec rpc.ClientCodec, masterToken string, datacenter string, id string) (string, error) { arg := structs.ACLTokenGetRequest{ TokenID: "root", @@ -5312,6 +5328,10 @@ func deleteTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter stri // upsertTestPolicy creates a policy for testing purposes func upsertTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter string) (*structs.ACLPolicy, error) { + return upsertTestPolicyWithRules(codec, masterToken, datacenter, "") +} + +func upsertTestPolicyWithRules(codec rpc.ClientCodec, masterToken string, datacenter string, rules string) (*structs.ACLPolicy, error) { // Make sure test policies can't collide policyUnq, err := uuid.GenerateUUID() if err != nil { @@ -5321,7 +5341,8 @@ func upsertTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter stri arg := structs.ACLPolicySetRequest{ Datacenter: datacenter, Policy: structs.ACLPolicy{ - Name: fmt.Sprintf("test-policy-%s", policyUnq), + Name: fmt.Sprintf("test-policy-%s", policyUnq), + Rules: rules, }, WriteRequest: structs.WriteRequest{Token: masterToken}, } diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 8571be6c9..d3ab49d67 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -6,7 +6,6 @@ import ( metrics "github.com/armon/go-metrics" "github.com/hashicorp/consul/acl" - "github.com/hashicorp/consul/agent/consul/discoverychain" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" memdb "github.com/hashicorp/go-memdb" @@ -313,64 +312,3 @@ func (c *ConfigEntry) ResolveServiceConfig(args *structs.ServiceConfigRequest, r return nil }) } - -func (c *ConfigEntry) ReadDiscoveryChain(args *structs.DiscoveryChainRequest, reply *structs.DiscoveryChainResponse) error { - if done, err := c.srv.forward("ConfigEntry.ReadDiscoveryChain", args, args, reply); done { - return err - } - defer metrics.MeasureSince([]string{"config_entry", "read_discovery_chain"}, time.Now()) - - // Fetch the ACL token, if any. - rule, err := c.srv.ResolveToken(args.Token) - if err != nil { - return err - } - if rule != nil && !rule.ServiceRead(args.Name) { - return acl.ErrPermissionDenied - } - - if args.Name == "" { - return fmt.Errorf("Must provide service name") - } - - evalDC := args.EvaluateInDatacenter - if evalDC == "" { - evalDC = c.srv.config.Datacenter - } - - evalNS := args.EvaluateInNamespace - if evalNS == "" { - // TODO(namespaces) pull from something else? - evalNS = "default" - } - - return c.srv.blockingQuery( - &args.QueryOptions, - &reply.QueryMeta, - func(ws memdb.WatchSet, state *state.Store) error { - index, entries, err := state.ReadDiscoveryChainConfigEntries(ws, args.Name) - if err != nil { - return err - } - - // Then we compile it into something useful. - chain, err := discoverychain.Compile(discoverychain.CompileRequest{ - ServiceName: args.Name, - CurrentNamespace: evalNS, - CurrentDatacenter: evalDC, - OverrideMeshGateway: args.OverrideMeshGateway, - OverrideProtocol: args.OverrideProtocol, - OverrideConnectTimeout: args.OverrideConnectTimeout, - Entries: entries, - }) - if err != nil { - return err - } - - reply.Index = index - reply.ConfigEntries = entries - reply.Chain = chain - - return nil - }) -} diff --git a/agent/consul/discovery_chain_endpoint.go b/agent/consul/discovery_chain_endpoint.go new file mode 100644 index 000000000..e79a5039a --- /dev/null +++ b/agent/consul/discovery_chain_endpoint.go @@ -0,0 +1,77 @@ +package consul + +import ( + "fmt" + "time" + + metrics "github.com/armon/go-metrics" + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/consul/discoverychain" + "github.com/hashicorp/consul/agent/consul/state" + "github.com/hashicorp/consul/agent/structs" + memdb "github.com/hashicorp/go-memdb" +) + +type DiscoveryChain struct { + srv *Server +} + +func (c *DiscoveryChain) Get(args *structs.DiscoveryChainRequest, reply *structs.DiscoveryChainResponse) error { + if done, err := c.srv.forward("DiscoveryChain.Get", args, args, reply); done { + return err + } + defer metrics.MeasureSince([]string{"discovery_chain", "get"}, time.Now()) + + // Fetch the ACL token, if any. + rule, err := c.srv.ResolveToken(args.Token) + if err != nil { + return err + } + if rule != nil && !rule.ServiceRead(args.Name) { + return acl.ErrPermissionDenied + } + + if args.Name == "" { + return fmt.Errorf("Must provide service name") + } + + evalDC := args.EvaluateInDatacenter + if evalDC == "" { + evalDC = c.srv.config.Datacenter + } + + evalNS := args.EvaluateInNamespace + if evalNS == "" { + // TODO(namespaces) pull from something else? + evalNS = "default" + } + + return c.srv.blockingQuery( + &args.QueryOptions, + &reply.QueryMeta, + func(ws memdb.WatchSet, state *state.Store) error { + index, entries, err := state.ReadDiscoveryChainConfigEntries(ws, args.Name) + if err != nil { + return err + } + + // Then we compile it into something useful. + chain, err := discoverychain.Compile(discoverychain.CompileRequest{ + ServiceName: args.Name, + CurrentNamespace: evalNS, + CurrentDatacenter: evalDC, + OverrideMeshGateway: args.OverrideMeshGateway, + OverrideProtocol: args.OverrideProtocol, + OverrideConnectTimeout: args.OverrideConnectTimeout, + Entries: entries, + }) + if err != nil { + return err + } + + reply.Index = index + reply.Chain = chain + + return nil + }) +} diff --git a/agent/consul/discovery_chain_endpoint_test.go b/agent/consul/discovery_chain_endpoint_test.go new file mode 100644 index 000000000..a69ce2972 --- /dev/null +++ b/agent/consul/discovery_chain_endpoint_test.go @@ -0,0 +1,196 @@ +package consul + +import ( + "fmt" + "os" + "testing" + "time" + + "github.com/hashicorp/consul/acl" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/testrpc" + msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/stretchr/testify/require" +) + +func TestDiscoveryChainEndpoint_Get(t *testing.T) { + t.Parallel() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.PrimaryDatacenter = "dc1" + c.ACLDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLMasterToken = "root" + c.ACLDefaultPolicy = "deny" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForTestAgent(t, s1.RPC, "dc1") + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + denyToken, err := upsertTestTokenWithPolicyRules(codec, "root", "dc1", "") + require.NoError(t, err) + + allowToken, err := upsertTestTokenWithPolicyRules(codec, "root", "dc1", `service "web" { policy = "read" }`) + require.NoError(t, err) + + getChain := func(args *structs.DiscoveryChainRequest) (*structs.DiscoveryChainResponse, error) { + resp := structs.DiscoveryChainResponse{} + err := msgpackrpc.CallWithCodec(codec, "DiscoveryChain.Get", &args, &resp) + if err != nil { + return nil, err + } + // clear fields that we don't care about + resp.QueryMeta = structs.QueryMeta{} + return &resp, nil + } + + // ==== compiling the default chain (no config entries) + + { // no token + _, err := getChain(&structs.DiscoveryChainRequest{ + Name: "web", + EvaluateInDatacenter: "dc1", + EvaluateInNamespace: "default", + Datacenter: "dc1", + }) + if !acl.IsErrPermissionDenied(err) { + t.Fatalf("err: %v", err) + } + } + + { // wrong token + _, err := getChain(&structs.DiscoveryChainRequest{ + Name: "web", + EvaluateInDatacenter: "dc1", + EvaluateInNamespace: "default", + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: denyToken.SecretID}, + }) + if !acl.IsErrPermissionDenied(err) { + t.Fatalf("err: %v", err) + } + } + + expectDefaultResponse_DC1_Default := &structs.DiscoveryChainResponse{ + Chain: &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc1": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"), + }, + }, + } + + // various ways with good token + for _, tc := range []struct { + evalDC string + evalNS string + expect *structs.DiscoveryChainResponse + }{ + { + evalDC: "dc1", + evalNS: "default", + expect: expectDefaultResponse_DC1_Default, + }, + { + evalDC: "", + evalNS: "default", + expect: expectDefaultResponse_DC1_Default, + }, + { + evalDC: "dc1", + evalNS: "", + expect: expectDefaultResponse_DC1_Default, + }, + { + evalDC: "", + evalNS: "", + expect: expectDefaultResponse_DC1_Default, + }, + } { + tc := tc + name := fmt.Sprintf("dc=%q ns=%q", tc.evalDC, tc.evalNS) + require.True(t, t.Run(name, func(t *testing.T) { + resp, err := getChain(&structs.DiscoveryChainRequest{ + Name: "web", + EvaluateInDatacenter: tc.evalDC, + EvaluateInNamespace: tc.evalNS, + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: allowToken.SecretID}, + }) + require.NoError(t, err) + + require.Equal(t, tc.expect, resp) + })) + } + + { // Now create one config entry. + out := false + require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConfigEntry.Apply", + &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "web", + ConnectTimeout: 33 * time.Second, + }, + WriteRequest: structs.WriteRequest{Token: "root"}, + }, &out)) + require.True(t, out) + } + + // ==== compiling a chain with config entries + + { // good token + resp, err := getChain(&structs.DiscoveryChainRequest{ + Name: "web", + EvaluateInDatacenter: "dc1", + EvaluateInNamespace: "default", + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: allowToken.SecretID}, + }) + require.NoError(t, err) + + expect := &structs.DiscoveryChainResponse{ + Chain: &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc1": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &structs.DiscoveryResolver{ + ConnectTimeout: 33 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"), + }, + }, + } + require.Equal(t, expect, resp) + } +} diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index b1bdd229b..7fc311563 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -81,11 +81,11 @@ func Compile(req CompileRequest) (*structs.CompiledDiscoveryChain, error) { resolvers: make(map[string]*structs.ServiceResolverConfigEntry), splitterNodes: make(map[string]*structs.DiscoveryGraphNode), - resolveNodes: make(map[structs.DiscoveryTarget]*structs.DiscoveryGraphNode), + resolveNodes: make(map[string]*structs.DiscoveryGraphNode), - nodes: make(map[string]*structs.DiscoveryGraphNode), - - targets: make(map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig), + nodes: make(map[string]*structs.DiscoveryGraphNode), + loadedTargets: make(map[string]*structs.DiscoveryTarget), + retainedTargets: make(map[string]struct{}), } if req.OverrideProtocol != "" { @@ -123,7 +123,7 @@ type compiler struct { // cached nodes splitterNodes map[string]*structs.DiscoveryGraphNode - resolveNodes map[structs.DiscoveryTarget]*structs.DiscoveryGraphNode + resolveNodes map[string]*structs.DiscoveryGraphNode // usesAdvancedRoutingFeatures is set to true if config entries for routing // or splitting appear in the compiled chain @@ -155,7 +155,8 @@ type compiler struct { nodes map[string]*structs.DiscoveryGraphNode // This is an OUTPUT field. - targets map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig + loadedTargets map[string]*structs.DiscoveryTarget + retainedTargets map[string]struct{} } type customizationMarkers struct { @@ -175,7 +176,7 @@ func (c *compiler) recordNode(node *structs.DiscoveryGraphNode) { case structs.DiscoveryGraphNodeTypeRouter: // no special storage case structs.DiscoveryGraphNodeTypeSplitter: - c.splitterNodes[node.ServiceName()] = node + c.splitterNodes[node.Name] = node case structs.DiscoveryGraphNodeTypeResolver: c.resolveNodes[node.Resolver.Target] = node default: @@ -249,6 +250,12 @@ func (c *compiler) compile() (*structs.CompiledDiscoveryChain, error) { return nil, err } + for targetID, _ := range c.loadedTargets { + if _, ok := c.retainedTargets[targetID]; !ok { + delete(c.loadedTargets, targetID) + } + } + if !enableAdvancedRoutingForProtocol(c.protocol) && c.usesAdvancedRoutingFeatures { return nil, &structs.ConfigEntryGraphError{ Message: fmt.Sprintf( @@ -297,7 +304,7 @@ func (c *compiler) compile() (*structs.CompiledDiscoveryChain, error) { Protocol: c.protocol, StartNode: c.startNode, Nodes: c.nodes, - Targets: c.targets, + Targets: c.loadedTargets, }, nil } @@ -575,19 +582,53 @@ func newDefaultServiceRoute(serviceName string) *structs.ServiceRoute { } } -func (c *compiler) newTarget(service, serviceSubset, namespace, datacenter string) structs.DiscoveryTarget { +func (c *compiler) newTarget(service, serviceSubset, namespace, datacenter string) *structs.DiscoveryTarget { if service == "" { panic("newTarget called with empty service which makes no sense") } - return structs.DiscoveryTarget{ - Service: service, - ServiceSubset: serviceSubset, - Namespace: defaultIfEmpty(namespace, c.currentNamespace), - Datacenter: defaultIfEmpty(datacenter, c.currentDatacenter), + + t := structs.NewDiscoveryTarget( + service, + serviceSubset, + defaultIfEmpty(namespace, c.currentNamespace), + defaultIfEmpty(datacenter, c.currentDatacenter), + ) + + prev, ok := c.loadedTargets[t.ID] + if ok { + return prev } + c.loadedTargets[t.ID] = t + return t } -func (c *compiler) getSplitterOrResolverNode(target structs.DiscoveryTarget) (*structs.DiscoveryGraphNode, error) { +func (c *compiler) rewriteTarget(t *structs.DiscoveryTarget, service, serviceSubset, namespace, datacenter string) *structs.DiscoveryTarget { + var ( + service2 = t.Service + serviceSubset2 = t.ServiceSubset + namespace2 = t.Namespace + datacenter2 = t.Datacenter + ) + + if service != "" && service != service2 { + service2 = service + // Reset the chosen subset if we reference a service other than our own. + serviceSubset2 = "" + } + if serviceSubset != "" { + serviceSubset2 = serviceSubset + } + if namespace != "" { + namespace2 = namespace + } + if datacenter != "" { + datacenter2 = datacenter + } + + return c.newTarget(service2, serviceSubset2, namespace2, datacenter2) +} + +func (c *compiler) getSplitterOrResolverNode(target *structs.DiscoveryTarget) (*structs.DiscoveryGraphNode, error) { nextNode, err := c.getSplitterNode(target.Service) if err != nil { return nil, err @@ -660,17 +701,17 @@ func (c *compiler) getSplitterNode(name string) (*structs.DiscoveryGraphNode, er // getResolverNode handles most of the code to handle redirection/rewriting // capabilities from a resolver config entry. It recurses into itself to // _generate_ targets used for failover out of convenience. -func (c *compiler) getResolverNode(target structs.DiscoveryTarget, recursedForFailover bool) (*structs.DiscoveryGraphNode, error) { +func (c *compiler) getResolverNode(target *structs.DiscoveryTarget, recursedForFailover bool) (*structs.DiscoveryGraphNode, error) { var ( // State to help detect redirect cycles and print helpful error // messages. - redirectHistory = make(map[structs.DiscoveryTarget]struct{}) - redirectOrder []structs.DiscoveryTarget + redirectHistory = make(map[string]struct{}) + redirectOrder []string ) RESOLVE_AGAIN: // Do we already have the node? - if prev, ok := c.resolveNodes[target]; ok { + if prev, ok := c.resolveNodes[target.ID]; ok { return prev, nil } @@ -686,23 +727,18 @@ RESOLVE_AGAIN: c.resolvers[target.Service] = resolver } - if _, ok := redirectHistory[target]; ok { - redirectOrder = append(redirectOrder, target) - - pretty := make([]string, len(redirectOrder)) - for i, target := range redirectOrder { - pretty[i] = target.String() - } + if _, ok := redirectHistory[target.ID]; ok { + redirectOrder = append(redirectOrder, target.ID) return nil, &structs.ConfigEntryGraphError{ Message: fmt.Sprintf( "detected circular resolver redirect: [%s]", - strings.Join(pretty, " -> "), + strings.Join(redirectOrder, " -> "), ), } } - redirectHistory[target] = struct{}{} - redirectOrder = append(redirectOrder, target) + redirectHistory[target.ID] = struct{}{} + redirectOrder = append(redirectOrder, target.ID) // Handle redirects right up front. // @@ -710,13 +746,14 @@ RESOLVE_AGAIN: if resolver.Redirect != nil { redirect := resolver.Redirect - redirectedTarget := target.CopyAndModify( + redirectedTarget := c.rewriteTarget( + target, redirect.Service, redirect.ServiceSubset, redirect.Namespace, redirect.Datacenter, ) - if redirectedTarget != target { + if redirectedTarget.ID != target.ID { target = redirectedTarget goto RESOLVE_AGAIN } @@ -724,7 +761,13 @@ RESOLVE_AGAIN: // Handle default subset. if target.ServiceSubset == "" && resolver.DefaultSubset != "" { - target.ServiceSubset = resolver.DefaultSubset + target = c.rewriteTarget( + target, + "", + resolver.DefaultSubset, + "", + "", + ) goto RESOLVE_AGAIN } @@ -753,37 +796,34 @@ RESOLVE_AGAIN: // Build node. node := &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: target.Identifier(), + Name: target.ID, Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: resolver.IsDefault(), - Target: target, + Target: target.ID, ConnectTimeout: connectTimeout, }, } - targetConfig := structs.DiscoveryTargetConfig{ - Subset: resolver.Subsets[target.ServiceSubset], - } + target.Subset = resolver.Subsets[target.ServiceSubset] // Default mesh gateway settings if serviceDefault := c.entries.GetService(target.Service); serviceDefault != nil { - targetConfig.MeshGateway = serviceDefault.MeshGateway + target.MeshGateway = serviceDefault.MeshGateway } - if c.entries.GlobalProxy != nil && targetConfig.MeshGateway.Mode == structs.MeshGatewayModeDefault { - targetConfig.MeshGateway.Mode = c.entries.GlobalProxy.MeshGateway.Mode + if c.entries.GlobalProxy != nil && target.MeshGateway.Mode == structs.MeshGatewayModeDefault { + target.MeshGateway.Mode = c.entries.GlobalProxy.MeshGateway.Mode } if c.overrideMeshGateway.Mode != structs.MeshGatewayModeDefault { - if targetConfig.MeshGateway.Mode != c.overrideMeshGateway.Mode { - targetConfig.MeshGateway.Mode = c.overrideMeshGateway.Mode + if target.MeshGateway.Mode != c.overrideMeshGateway.Mode { + target.MeshGateway.Mode = c.overrideMeshGateway.Mode c.customizedBy.MeshGateway = true } } - // Retain this target even if we may not retain the group resolver. - c.targets[target] = targetConfig + // Retain this target in the final results. + c.retainedTargets[target.ID] = struct{}{} if recursedForFailover { // If we recursed here from ourselves in a failover context, just emit @@ -808,42 +848,43 @@ RESOLVE_AGAIN: if ok { // Determine which failover definitions apply. - var failoverTargets []structs.DiscoveryTarget + var failoverTargets []*structs.DiscoveryTarget if len(failover.Datacenters) > 0 { for _, dc := range failover.Datacenters { // Rewrite the target as per the failover policy. - failoverTarget := target.CopyAndModify( + failoverTarget := c.rewriteTarget( + target, failover.Service, failover.ServiceSubset, failover.Namespace, dc, ) - if failoverTarget != target { // don't failover to yourself + if failoverTarget.ID != target.ID { // don't failover to yourself failoverTargets = append(failoverTargets, failoverTarget) } } } else { // Rewrite the target as per the failover policy. - failoverTarget := target.CopyAndModify( + failoverTarget := c.rewriteTarget( + target, failover.Service, failover.ServiceSubset, failover.Namespace, "", ) - if failoverTarget != target { // don't failover to yourself + if failoverTarget.ID != target.ID { // don't failover to yourself failoverTargets = append(failoverTargets, failoverTarget) } } // If we filtered everything out then no point in having a failover. if len(failoverTargets) > 0 { - df := &structs.DiscoveryFailover{ - Definition: &failover, - } + df := &structs.DiscoveryFailover{} node.Resolver.Failover = df - // Convert the targets into targets by cheating a bit and - // recursing into ourselves. + // Take care of doing any redirects or configuration loading + // related to targets by cheating a bit and recursing into + // ourselves. for _, target := range failoverTargets { failoverResolveNode, err := c.getResolverNode(target, true) if err != nil { diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go index 9b464d520..261c32f90 100644 --- a/agent/consul/discoverychain/compile_test.go +++ b/agent/consul/discoverychain/compile_test.go @@ -144,8 +144,6 @@ func testcase_JustRouterWithDefaults() compileTestCase { }, ) - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -156,23 +154,22 @@ func testcase_JustRouterWithDefaults() compileTestCase { Routes: []*structs.DiscoveryRoute{ { Definition: newDefaultServiceRoute("main"), - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -197,8 +194,6 @@ func testcase_RouterWithDefaults_NoSplit_WithResolver() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -209,22 +204,21 @@ func testcase_RouterWithDefaults_NoSplit_WithResolver() compileTestCase { Routes: []*structs.DiscoveryRoute{ { Definition: newDefaultServiceRoute("main"), - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 33 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -251,8 +245,6 @@ func testcase_RouterWithDefaults_WithNoopSplit_DefaultResolver() compileTestCase }, ) - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -273,23 +265,22 @@ func testcase_RouterWithDefaults_WithNoopSplit_DefaultResolver() compileTestCase Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -316,8 +307,6 @@ func testcase_NoopSplit_DefaultResolver_ProtocolFromProxyDefaults() compileTestC }, ) - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -338,23 +327,22 @@ func testcase_NoopSplit_DefaultResolver_ProtocolFromProxyDefaults() compileTestC Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -388,8 +376,6 @@ func testcase_RouterWithDefaults_WithNoopSplit_WithResolver() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -410,22 +396,21 @@ func testcase_RouterWithDefaults_WithNoopSplit_WithResolver() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 33 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -472,9 +457,6 @@ func testcase_RouteBypassesSplit() compileTestCase { router := entries.GetRouter("main") - resolverOther := entries.GetResolver("other") - resolverMain := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "router:main", @@ -485,41 +467,39 @@ func testcase_RouteBypassesSplit() compileTestCase { Routes: []*structs.DiscoveryRoute{ { Definition: &router.Routes[0], - NextNode: "resolver:other,bypass,,dc1", + NextNode: "resolver:bypass.other.default.dc1", }, { Definition: newDefaultServiceRoute("main"), - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, - "resolver:other,bypass,,dc1": &structs.DiscoveryGraphNode{ + "resolver:bypass.other.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "other,bypass,,dc1", + Name: "bypass.other.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverOther, ConnectTimeout: 5 * time.Second, - Target: newTarget("other", "bypass", "default", "dc1"), + Target: "bypass.other.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("other", "bypass", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "bypass.other.default.dc1": newTarget("other", "bypass", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == bypass", - }, - }, + } + }), }, } @@ -540,8 +520,6 @@ func testcase_NoopSplit_DefaultResolver() compileTestCase { }, ) - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -552,23 +530,22 @@ func testcase_NoopSplit_DefaultResolver() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -596,8 +573,6 @@ func testcase_NoopSplit_WithResolver() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -608,22 +583,21 @@ func testcase_NoopSplit_WithResolver() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,,,dc1", + NextNode: "resolver:main.default.dc1", }, }, }, - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 33 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } @@ -659,8 +633,6 @@ func testcase_SubsetSplit() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -671,44 +643,42 @@ func testcase_SubsetSplit() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 60, - NextNode: "resolver:main,v2,,dc1", + NextNode: "resolver:v2.main.default.dc1", }, { Weight: 40, - NextNode: "resolver:main,v1,,dc1", + NextNode: "resolver:v1.main.default.dc1", }, }, }, - "resolver:main,v2,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v2.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v2,,dc1", + Name: "v2.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v2", "default", "dc1"), + Target: "v2.main.default.dc1", }, }, - "resolver:main,v1,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v1.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v1,,dc1", + Name: "v1.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v1", "default", "dc1"), + Target: "v1.main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "v1", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ - Filter: "Service.Meta.version == 1", - }, - }, - newTarget("main", "v2", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "v2.main.default.dc1": newTarget("main", "v2", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", - }, - }, + } + }), + "v1.main.default.dc1": newTarget("main", "v1", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == 1", + } + }), }, } @@ -732,9 +702,6 @@ func testcase_ServiceSplit() compileTestCase { }, ) - resolverFoo := newDefaultServiceResolver("foo") - resolverBar := newDefaultServiceResolver("bar") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -745,38 +712,36 @@ func testcase_ServiceSplit() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 60, - NextNode: "resolver:foo,,,dc1", + NextNode: "resolver:foo.default.dc1", }, { Weight: 40, - NextNode: "resolver:bar,,,dc1", + NextNode: "resolver:bar.default.dc1", }, }, }, - "resolver:foo,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:foo.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "foo,,,dc1", + Name: "foo.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverFoo, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("foo", "", "default", "dc1"), + Target: "foo.default.dc1", }, }, - "resolver:bar,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:bar.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "bar,,,dc1", + Name: "bar.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverBar, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("bar", "", "default", "dc1"), + Target: "bar.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("bar", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("foo", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "foo.default.dc1": newTarget("foo", "", "default", "dc1", nil), + "bar.default.dc1": newTarget("bar", "", "default", "dc1", nil), }, } @@ -826,8 +791,6 @@ func testcase_SplitBypassesSplit() compileTestCase { }, ) - resolverNext := entries.GetResolver("next") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -838,26 +801,25 @@ func testcase_SplitBypassesSplit() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:next,bypassed,,dc1", + NextNode: "resolver:bypassed.next.default.dc1", }, }, }, - "resolver:next,bypassed,,dc1": &structs.DiscoveryGraphNode{ + "resolver:bypassed.next.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "next,bypassed,,dc1", + Name: "bypassed.next.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverNext, ConnectTimeout: 5 * time.Second, - Target: newTarget("next", "bypassed", "default", "dc1"), + Target: "bypassed.next.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("next", "bypassed", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "bypassed.next.default.dc1": newTarget("next", "bypassed", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == bypass", - }, - }, + } + }), }, } @@ -876,25 +838,22 @@ func testcase_ServiceRedirect() compileTestCase { }, ) - resolverOther := newDefaultServiceResolver("other") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:other,,,dc1", + StartNode: "resolver:other.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:other,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:other.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "other,,,dc1", + Name: "other.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverOther, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("other", "", "default", "dc1"), + Target: "other.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("other", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "other.default.dc1": newTarget("other", "", "default", "dc1", nil), }, } @@ -926,28 +885,25 @@ func testcase_ServiceAndSubsetRedirect() compileTestCase { }, ) - resolver := entries.GetResolver("other") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:other,v2,,dc1", + StartNode: "resolver:v2.other.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:other,v2,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v2.other.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "other,v2,,dc1", + Name: "v2.other.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("other", "v2", "default", "dc1"), + Target: "v2.other.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("other", "v2", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "v2.other.default.dc1": newTarget("other", "v2", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -965,24 +921,21 @@ func testcase_DatacenterRedirect() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc9", + StartNode: "resolver:main.default.dc9", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc9": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc9": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc9", + Name: "main.default.dc9", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc9"), + Target: "main.default.dc9", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc9"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc9": newTarget("main", "", "default", "dc9", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1000,33 +953,25 @@ func testcase_ServiceFailover() compileTestCase { }, ) - resolverMain := entries.GetResolver("main") - - wildFail := resolverMain.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("backup", "", "default", "dc1"), - }, + Targets: []string{"backup.default.dc1"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("backup", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "backup.default.dc1": newTarget("backup", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1051,33 +996,25 @@ func testcase_ServiceFailoverThroughRedirect() compileTestCase { }, ) - resolverMain := entries.GetResolver("main") - - wildFail := resolverMain.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("actual", "", "default", "dc1"), - }, + Targets: []string{"actual.default.dc1"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("actual", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "actual.default.dc1": newTarget("actual", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1102,33 +1039,25 @@ func testcase_Resolver_CircularFailover() compileTestCase { }, ) - resolverMain := entries.GetResolver("main") - - wildFail := resolverMain.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("backup", "", "default", "dc1"), - }, + Targets: []string{"backup.default.dc1"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("backup", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "backup.default.dc1": newTarget("backup", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1151,36 +1080,29 @@ func testcase_ServiceAndSubsetFailover() compileTestCase { }, ) - resolver := entries.GetResolver("main") - wildFail := resolver.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("main", "backup", "default", "dc1"), - }, + Targets: []string{"backup.main.default.dc1"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("main", "backup", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "backup.main.default.dc1": newTarget("main", "backup", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == backup", - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -1198,34 +1120,26 @@ func testcase_DatacenterFailover() compileTestCase { }, ) - resolver := entries.GetResolver("main") - wildFail := resolver.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("main", "", "default", "dc2"), - newTarget("main", "", "default", "dc4"), - }, + Targets: []string{"main.default.dc2", "main.default.dc4"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc2"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc4"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), + "main.default.dc2": newTarget("main", "", "default", "dc2", nil), + "main.default.dc4": newTarget("main", "", "default", "dc4", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1250,41 +1164,33 @@ func testcase_ServiceFailover_WithMeshGateways() compileTestCase { }, ) - resolverMain := entries.GetResolver("main") - - wildFail := resolverMain.Failover["*"] - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", Failover: &structs.DiscoveryFailover{ - Definition: &wildFail, - Targets: []structs.DiscoveryTarget{ - newTarget("backup", "", "default", "dc1"), - }, + Targets: []string{"backup.default.dc1"}, }, }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("backup", "", "default", "dc1"): structs.DiscoveryTargetConfig{ - MeshGateway: structs.MeshGatewayConfig{ + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, - }, - }, - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{ - MeshGateway: structs.MeshGatewayConfig{ + } + }), + "backup.default.dc1": newTarget("backup", "", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -1315,8 +1221,6 @@ func testcase_NoopSplit_WithDefaultSubset() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -1327,26 +1231,25 @@ func testcase_NoopSplit_WithDefaultSubset() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 100, - NextNode: "resolver:main,v2,,dc1", + NextNode: "resolver:v2.main.default.dc1", }, }, }, - "resolver:main,v2,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v2.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v2,,dc1", + Name: "v2.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v2", "default", "dc1"), + Target: "v2.main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "v2", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "v2.main.default.dc1": newTarget("main", "v2", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -1355,25 +1258,23 @@ func testcase_NoopSplit_WithDefaultSubset() compileTestCase { func testcase_DefaultResolver() compileTestCase { entries := newEntries() - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + // TODO-TARGET + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true} @@ -1392,29 +1293,26 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase { }, } - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "grpc", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{ - MeshGateway: structs.MeshGatewayConfig{ + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc1": newTarget("main", "", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true} @@ -1432,25 +1330,22 @@ func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase { }, ) - resolver := newDefaultServiceResolver("other") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:other,,,dc1", + StartNode: "resolver:other.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:other,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:other.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "other,,,dc1", + Name: "other.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("other", "", "default", "dc1"), + Target: "other.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("other", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "other.default.dc1": newTarget("other", "", "default", "dc1", nil), }, } @@ -1471,28 +1366,25 @@ func testcase_Resolve_WithDefaultSubset() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,v2,,dc1", + StartNode: "resolver:v2.main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,v2,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v2.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v2,,dc1", + Name: "v2.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v2", "default", "dc1"), + Target: "v2.main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "v2", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "v2.main.default.dc1": newTarget("main", "v2", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", - }, - }, + } + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -1538,8 +1430,6 @@ func testcase_MultiDatacenterCanary() compileTestCase { }, ) - resolver := entries.GetResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http", StartNode: "splitter:main", @@ -1550,36 +1440,34 @@ func testcase_MultiDatacenterCanary() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 60, - NextNode: "resolver:main,,,dc2", + NextNode: "resolver:main.default.dc2", }, { Weight: 40, - NextNode: "resolver:main,,,dc3", + NextNode: "resolver:main.default.dc3", }, }, }, - "resolver:main,,,dc2": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc2": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc2", + Name: "main.default.dc2", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 33 * time.Second, - Target: newTarget("main", "", "default", "dc2"), + Target: "main.default.dc2", }, }, - "resolver:main,,,dc3": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc3": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc3", + Name: "main.default.dc3", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, ConnectTimeout: 33 * time.Second, - Target: newTarget("main", "", "default", "dc3"), + Target: "main.default.dc3", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc2"): structs.DiscoveryTargetConfig{}, - newTarget("main", "", "default", "dc3"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.dc2": newTarget("main", "", "default", "dc2", nil), + "main.default.dc3": newTarget("main", "", "default", "dc3", nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1670,9 +1558,7 @@ func testcase_AllBellsAndWhistles() compileTestCase { ) var ( - router = entries.GetRouter("main") - resolverMain = entries.GetResolver("main") - resolverRedirected = entries.GetResolver("redirected") + router = entries.GetRouter("main") ) expect := &structs.CompiledDiscoveryChain{ @@ -1685,7 +1571,7 @@ func testcase_AllBellsAndWhistles() compileTestCase { Routes: []*structs.DiscoveryRoute{ { Definition: &router.Routes[0], - NextNode: "resolver:redirected,prod,,dc1", + NextNode: "resolver:prod.redirected.default.dc1", }, { Definition: &router.Routes[1], @@ -1693,7 +1579,7 @@ func testcase_AllBellsAndWhistles() compileTestCase { }, { Definition: newDefaultServiceRoute("main"), - NextNode: "resolver:main,default-subset,,dc1", + NextNode: "resolver:default-subset.main.default.dc1", }, }, }, @@ -1703,92 +1589,87 @@ func testcase_AllBellsAndWhistles() compileTestCase { Splits: []*structs.DiscoverySplit{ { Weight: 60, - NextNode: "resolver:redirected,prod,,dc1", + NextNode: "resolver:prod.redirected.default.dc1", }, { Weight: 30, - NextNode: "resolver:main,v1,,dc1", + NextNode: "resolver:v1.main.default.dc1", }, { Weight: 8, - NextNode: "resolver:main,v2,,dc1", + NextNode: "resolver:v2.main.default.dc1", }, { Weight: 2, - NextNode: "resolver:main,v3,,dc1", + NextNode: "resolver:v3.main.default.dc1", }, }, }, - "resolver:redirected,prod,,dc1": &structs.DiscoveryGraphNode{ + "resolver:prod.redirected.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "redirected,prod,,dc1", + Name: "prod.redirected.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverRedirected, ConnectTimeout: 5 * time.Second, - Target: newTarget("redirected", "prod", "default", "dc1"), + Target: "prod.redirected.default.dc1", }, }, - "resolver:main,v1,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v1.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v1,,dc1", + Name: "v1.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v1", "default", "dc1"), + Target: "v1.main.default.dc1", }, }, - "resolver:main,v2,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v2.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v2,,dc1", + Name: "v2.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v2", "default", "dc1"), + Target: "v2.main.default.dc1", }, }, - "resolver:main,v3,,dc1": &structs.DiscoveryGraphNode{ + "resolver:v3.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,v3,,dc1", + Name: "v3.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "v3", "default", "dc1"), + Target: "v3.main.default.dc1", }, }, - "resolver:main,default-subset,,dc1": &structs.DiscoveryGraphNode{ + "resolver:default-subset.main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,default-subset,,dc1", + Name: "default-subset.main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolverMain, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "default-subset", "default", "dc1"), + Target: "default-subset.main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "default-subset", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{OnlyPassing: true}, - }, - newTarget("main", "v1", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ - Filter: "Service.Meta.version == 1", - }, - }, - newTarget("main", "v2", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ - Filter: "Service.Meta.version == 2", - }, - }, - newTarget("main", "v3", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ - Filter: "Service.Meta.version == 3", - }, - }, - newTarget("redirected", "prod", "default", "dc1"): structs.DiscoveryTargetConfig{ - Subset: structs.ServiceResolverSubset{ + Targets: map[string]*structs.DiscoveryTarget{ + "prod.redirected.default.dc1": newTarget("redirected", "prod", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ Filter: "ServiceMeta.env == prod", - }, - }, + } + }), + "v1.main.default.dc1": newTarget("main", "v1", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == 1", + } + }), + "v2.main.default.dc1": newTarget("main", "v2", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == 2", + } + }), + "v3.main.default.dc1": newTarget("main", "v3", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{ + Filter: "Service.Meta.version == 3", + } + }), + "default-subset.main.default.dc1": newTarget("main", "default-subset", "default", "dc1", func(t *structs.DiscoveryTarget) { + t.Subset = structs.ServiceResolverSubset{OnlyPassing: true} + }), }, } return compileTestCase{entries: entries, expect: expect} @@ -1973,25 +1854,23 @@ func testcase_ResolverProtocolOverride() compileTestCase { entries := newEntries() setServiceProtocol(entries, "main", "grpc") - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http2", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + // TODO-TARGET + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true, @@ -2008,25 +1887,23 @@ func testcase_ResolverProtocolOverrideIgnored() compileTestCase { entries := newEntries() setServiceProtocol(entries, "main", "http2") - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "http2", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + // TODO-TARGET + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true, @@ -2047,25 +1924,23 @@ func testcase_RouterIgnored_ResolverProtocolOverride() compileTestCase { }, ) - resolver := newDefaultServiceResolver("main") - expect := &structs.CompiledDiscoveryChain{ Protocol: "tcp", - StartNode: "resolver:main,,,dc1", + StartNode: "resolver:main.default.dc1", Nodes: map[string]*structs.DiscoveryGraphNode{ - "resolver:main,,,dc1": &structs.DiscoveryGraphNode{ + "resolver:main.default.dc1": &structs.DiscoveryGraphNode{ Type: structs.DiscoveryGraphNodeTypeResolver, - Name: "main,,,dc1", + Name: "main.default.dc1", Resolver: &structs.DiscoveryResolver{ - Definition: resolver, Default: true, ConnectTimeout: 5 * time.Second, - Target: newTarget("main", "", "default", "dc1"), + Target: "main.default.dc1", }, }, }, - Targets: map[structs.DiscoveryTarget]structs.DiscoveryTargetConfig{ - newTarget("main", "", "default", "dc1"): structs.DiscoveryTargetConfig{}, + Targets: map[string]*structs.DiscoveryTarget{ + // TODO-TARGET + "main.default.dc1": newTarget("main", "", "default", "dc1", nil), }, } return compileTestCase{entries: entries, expect: expect, expectIsDefault: true, @@ -2168,11 +2043,10 @@ func newEntries() *structs.DiscoveryChainConfigEntries { } } -func newTarget(service, serviceSubset, namespace, datacenter string) structs.DiscoveryTarget { - return structs.DiscoveryTarget{ - Service: service, - ServiceSubset: serviceSubset, - Namespace: namespace, - Datacenter: datacenter, +func newTarget(service, serviceSubset, namespace, datacenter string, modFn func(t *structs.DiscoveryTarget)) *structs.DiscoveryTarget { + t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, datacenter) + if modFn != nil { + modFn(t) } + return t } diff --git a/agent/consul/server_oss.go b/agent/consul/server_oss.go index 1627d4059..25630bd35 100644 --- a/agent/consul/server_oss.go +++ b/agent/consul/server_oss.go @@ -6,6 +6,7 @@ func init() { registerEndpoint(func(s *Server) interface{} { return NewCoordinate(s) }) registerEndpoint(func(s *Server) interface{} { return &ConfigEntry{s} }) registerEndpoint(func(s *Server) interface{} { return &ConnectCA{srv: s} }) + registerEndpoint(func(s *Server) interface{} { return &DiscoveryChain{s} }) registerEndpoint(func(s *Server) interface{} { return &Health{s} }) registerEndpoint(func(s *Server) interface{} { return &Intention{s} }) registerEndpoint(func(s *Server) interface{} { return &Internal{s} }) diff --git a/agent/discovery_chain_endpoint.go b/agent/discovery_chain_endpoint.go new file mode 100644 index 000000000..790ff245f --- /dev/null +++ b/agent/discovery_chain_endpoint.go @@ -0,0 +1,127 @@ +package agent + +import ( + "fmt" + "net/http" + "strings" + "time" + + cachetype "github.com/hashicorp/consul/agent/cache-types" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/lib" + "github.com/mitchellh/mapstructure" +) + +func (s *HTTPServer) DiscoveryChainRead(resp http.ResponseWriter, req *http.Request) (interface{}, error) { + var args structs.DiscoveryChainRequest + if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { + return nil, nil + } + + args.Name = strings.TrimPrefix(req.URL.Path, "/v1/discovery-chain/") + if args.Name == "" { + return nil, BadRequestError{Reason: "Missing chain name"} + } + + args.EvaluateInDatacenter = req.URL.Query().Get("compile-dc") + // TODO(namespaces): args.EvaluateInNamespace = req.URL.Query().Get("compile-namespace") + + if req.Method == "POST" { + var raw map[string]interface{} + if err := decodeBody(req, &raw, nil); err != nil { + return nil, BadRequestError{Reason: fmt.Sprintf("Request decoding failed: %v", err)} + } + + apiReq, err := decodeDiscoveryChainReadRequest(raw) + if err != nil { + return nil, BadRequestError{Reason: fmt.Sprintf("Request decoding failed: %v", err)} + } + + args.OverrideProtocol = apiReq.OverrideProtocol + args.OverrideConnectTimeout = apiReq.OverrideConnectTimeout + + if apiReq.OverrideMeshGateway.Mode != "" { + _, err := structs.ValidateMeshGatewayMode(string(apiReq.OverrideMeshGateway.Mode)) + if err != nil { + resp.WriteHeader(http.StatusBadRequest) + fmt.Fprint(resp, "Invalid OverrideMeshGateway.Mode parameter") + return nil, nil + } + args.OverrideMeshGateway = apiReq.OverrideMeshGateway + } + } + + // Make the RPC request + var out structs.DiscoveryChainResponse + defer setMeta(resp, &out.QueryMeta) + + if args.QueryOptions.UseCache { + raw, m, err := s.agent.cache.Get(cachetype.CompiledDiscoveryChainName, &args) + if err != nil { + return nil, err + } + defer setCacheMeta(resp, &m) + + reply, ok := raw.(*structs.DiscoveryChainResponse) + if !ok { + // This should never happen, but we want to protect against panics + return nil, fmt.Errorf("internal error: response type not correct") + } + out = *reply + } else { + RETRY_ONCE: + if err := s.agent.RPC("DiscoveryChain.Get", &args, &out); err != nil { + return nil, err + } + if args.QueryOptions.AllowStale && args.MaxStaleDuration > 0 && args.MaxStaleDuration < out.LastContact { + args.AllowStale = false + args.MaxStaleDuration = 0 + goto RETRY_ONCE + } + } + out.ConsistencyLevel = args.QueryOptions.ConsistencyLevel() + + return discoveryChainReadResponse{Chain: out.Chain}, nil +} + +// discoveryChainReadRequest is the API variation of structs.DiscoveryChainRequest +type discoveryChainReadRequest struct { + OverrideMeshGateway structs.MeshGatewayConfig + OverrideProtocol string + OverrideConnectTimeout time.Duration +} + +// discoveryChainReadResponse is the API variation of structs.DiscoveryChainResponse +type discoveryChainReadResponse struct { + Chain *structs.CompiledDiscoveryChain +} + +func decodeDiscoveryChainReadRequest(raw map[string]interface{}) (*discoveryChainReadRequest, error) { + // lib.TranslateKeys doesn't understand []map[string]interface{} so we have + // to do this part first. + raw = lib.PatchSliceOfMaps(raw, nil, nil) + + lib.TranslateKeys(raw, map[string]string{ + "override_mesh_gateway": "overridemeshgateway", + "override_protocol": "overrideprotocol", + "override_connect_timeout": "overrideconnecttimeout", + }) + + var apiReq discoveryChainReadRequest + decodeConf := &mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + Result: &apiReq, + WeaklyTypedInput: true, + } + + decoder, err := mapstructure.NewDecoder(decodeConf) + if err != nil { + return nil, err + } + + if err := decoder.Decode(raw); err != nil { + return nil, err + } + + return &apiReq, nil +} diff --git a/agent/discovery_chain_endpoint_test.go b/agent/discovery_chain_endpoint_test.go new file mode 100644 index 000000000..c05bc15da --- /dev/null +++ b/agent/discovery_chain_endpoint_test.go @@ -0,0 +1,295 @@ +package agent + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "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) { + t.Parallel() + + a := NewTestAgent(t, t.Name(), "") + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + for _, method := range []string{"GET", "POST"} { + require.True(t, t.Run(method+": error on no service name", func(t *testing.T) { + var ( + req *http.Request + err error + ) + if method == "GET" { + req, err = http.NewRequest("GET", "/v1/discovery-chain/", nil) + } else { + apiReq := &discoveryChainReadRequest{} + req, err = http.NewRequest("POST", "/v1/discovery-chain/", jsonReader(apiReq)) + } + require.NoError(t, err) + + resp := httptest.NewRecorder() + _, err = a.srv.DiscoveryChainRead(resp, req) + require.Error(t, err) + _, ok := err.(BadRequestError) + require.True(t, ok) + })) + + require.True(t, t.Run(method+": read default chain", func(t *testing.T) { + var ( + req *http.Request + err error + ) + if method == "GET" { + req, err = http.NewRequest("GET", "/v1/discovery-chain/web", nil) + } else { + apiReq := &discoveryChainReadRequest{} + req, err = http.NewRequest("POST", "/v1/discovery-chain/web", jsonReader(apiReq)) + } + require.NoError(t, err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + require.NoError(t, err) + + value := obj.(discoveryChainReadResponse) + + expect := &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc1": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"), + }, + } + require.Equal(t, expect, value.Chain) + })) + + require.True(t, t.Run(method+": read default chain; evaluate in dc2", func(t *testing.T) { + var ( + req *http.Request + err error + ) + if method == "GET" { + req, err = http.NewRequest("GET", "/v1/discovery-chain/web?compile-dc=dc2", nil) + } else { + apiReq := &discoveryChainReadRequest{} + req, err = http.NewRequest("POST", "/v1/discovery-chain/web?compile-dc=dc2", jsonReader(apiReq)) + } + require.NoError(t, err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + require.NoError(t, err) + + value := obj.(discoveryChainReadResponse) + + expect := &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc2", + Protocol: "tcp", + StartNode: "resolver:web.default.dc2", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc2": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc2", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc2", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "web.default.dc2": structs.NewDiscoveryTarget("web", "", "default", "dc2"), + }, + } + require.Equal(t, expect, value.Chain) + })) + + require.True(t, t.Run(method+": read default chain with cache", func(t *testing.T) { + var ( + req *http.Request + err error + ) + if method == "GET" { + req, err = http.NewRequest("GET", "/v1/discovery-chain/web?cached", nil) + } else { + apiReq := &discoveryChainReadRequest{} + req, err = http.NewRequest("POST", "/v1/discovery-chain/web?cached", jsonReader(apiReq)) + } + require.NoError(t, err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + require.NoError(t, err) + + // The GET request primes the cache so the POST is a hit. + if method == "GET" { + // Should be a cache miss + require.Equal(t, "MISS", resp.Header().Get("X-Cache")) + } else { + // Should be a cache HIT now! + require.Equal(t, "HIT", resp.Header().Get("X-Cache")) + } + + value := obj.(discoveryChainReadResponse) + + expect := &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc1": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &structs.DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "web.default.dc1": structs.NewDiscoveryTarget("web", "", "default", "dc1"), + }, + } + require.Equal(t, expect, value.Chain) + })) + } + + { // Now create one config entry. + out := false + require.NoError(t, a.RPC("ConfigEntry.Apply", &structs.ConfigEntryRequest{ + Datacenter: "dc1", + Entry: &structs.ServiceResolverConfigEntry{ + Kind: structs.ServiceResolver, + Name: "web", + ConnectTimeout: 33 * time.Second, + }, + }, &out)) + require.True(t, out) + } + + // Ensure background refresh works + require.True(t, t.Run("GET: read modified chain", func(t *testing.T) { + retry.Run(t, func(r *retry.R) { + req, err := http.NewRequest("GET", "/v1/discovery-chain/web?cached", nil) + r.Check(err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + r.Check(err) + + value := obj.(discoveryChainReadResponse) + chain := value.Chain + + // light comparison + node := chain.Nodes["resolver:web.default.dc1"] + if node == nil { + r.Fatalf("missing node") + } + if node.Resolver.Default { + r.Fatalf("not refreshed yet") + } + + // Should be a cache hit! The data should've updated in the cache + // in the background so this should've been fetched directly from + // the cache. + if resp.Header().Get("X-Cache") != "HIT" { + r.Fatalf("should be a cache hit") + } + }) + })) + + // TODO(namespaces): add a test + + expectTarget := structs.NewDiscoveryTarget("web", "", "default", "dc1") + expectTarget.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeLocal, + } + + expectModifiedWithOverrides := &structs.CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "grpc", + CustomizationHash: "98809527", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:web.default.dc1": &structs.DiscoveryGraphNode{ + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &structs.DiscoveryResolver{ + ConnectTimeout: 22 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + expectTarget.ID: expectTarget, + }, + } + + require.True(t, t.Run("POST: read modified chain with overrides (camel case)", func(t *testing.T) { + body := ` { + "OverrideMeshGateway": { + "Mode": "local" + }, + "OverrideProtocol": "grpc", + "OverrideConnectTimeout": "22s" + } ` + req, err := http.NewRequest("POST", "/v1/discovery-chain/web", strings.NewReader(body)) + require.NoError(t, err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + require.NoError(t, err) + + value := obj.(discoveryChainReadResponse) + + require.Equal(t, expectModifiedWithOverrides, value.Chain) + })) + + require.True(t, t.Run("POST: read modified chain with overrides (snake case)", func(t *testing.T) { + body := ` { + "override_mesh_gateway": { + "mode": "local" + }, + "override_protocol": "grpc", + "override_connect_timeout": "22s" + } ` + req, err := http.NewRequest("POST", "/v1/discovery-chain/web", strings.NewReader(body)) + require.NoError(t, err) + + resp := httptest.NewRecorder() + obj, err := a.srv.DiscoveryChainRead(resp, req) + require.NoError(t, err) + + value := obj.(discoveryChainReadResponse) + + require.Equal(t, expectModifiedWithOverrides, value.Chain) + })) +} diff --git a/agent/http_oss.go b/agent/http_oss.go index fb4e10fda..69d4f278b 100644 --- a/agent/http_oss.go +++ b/agent/http_oss.go @@ -81,6 +81,7 @@ func init() { registerEndpoint("/v1/coordinate/nodes", []string{"GET"}, (*HTTPServer).CoordinateNodes) registerEndpoint("/v1/coordinate/node/", []string{"GET"}, (*HTTPServer).CoordinateNode) registerEndpoint("/v1/coordinate/update", []string{"PUT"}, (*HTTPServer).CoordinateUpdate) + registerEndpoint("/v1/discovery-chain/", []string{"GET", "POST"}, (*HTTPServer).DiscoveryChainRead) registerEndpoint("/v1/event/fire/", []string{"PUT"}, (*HTTPServer).EventFire) registerEndpoint("/v1/event/list", []string{"GET"}, (*HTTPServer).EventList) registerEndpoint("/v1/health/node/", []string{"GET"}, (*HTTPServer).HealthNodeChecks) @@ -88,7 +89,6 @@ func init() { registerEndpoint("/v1/health/state/", []string{"GET"}, (*HTTPServer).HealthChecksInState) registerEndpoint("/v1/health/service/", []string{"GET"}, (*HTTPServer).HealthServiceNodes) registerEndpoint("/v1/health/connect/", []string{"GET"}, (*HTTPServer).HealthConnectServiceNodes) - registerEndpoint("/v1/internal/discovery-chain/", []string{"GET"}, (*HTTPServer).InternalDiscoveryChain) registerEndpoint("/v1/internal/ui/nodes", []string{"GET"}, (*HTTPServer).UINodes) registerEndpoint("/v1/internal/ui/node/", []string{"GET"}, (*HTTPServer).UINodeInfo) registerEndpoint("/v1/internal/ui/services", []string{"GET"}, (*HTTPServer).UIServices) diff --git a/agent/internal_endpoint.go b/agent/internal_endpoint.go deleted file mode 100644 index da7d84526..000000000 --- a/agent/internal_endpoint.go +++ /dev/null @@ -1,40 +0,0 @@ -package agent - -import ( - "fmt" - "net/http" - "strings" - - "github.com/hashicorp/consul/agent/structs" -) - -// InternalDiscoveryChain is helpful for debugging. Eventually we should expose -// this data officially somehow. -func (s *HTTPServer) InternalDiscoveryChain(resp http.ResponseWriter, req *http.Request) (interface{}, error) { - var args structs.DiscoveryChainRequest - if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done { - return nil, nil - } - - args.Name = strings.TrimPrefix(req.URL.Path, "/v1/internal/discovery-chain/") - if args.Name == "" { - resp.WriteHeader(http.StatusBadRequest) - fmt.Fprint(resp, "Missing chain name") - return nil, nil - } - - // Make the RPC request - var out structs.DiscoveryChainResponse - defer setMeta(resp, &out.QueryMeta) - - if err := s.agent.RPC("ConfigEntry.ReadDiscoveryChain", &args, &out); err != nil { - return nil, err - } - - if out.Chain == nil { - resp.WriteHeader(http.StatusNotFound) - return nil, nil - } - - return out.Chain, nil -} diff --git a/agent/proxycfg/manager_test.go b/agent/proxycfg/manager_test.go index 225a3e04a..013eb7e4f 100644 --- a/agent/proxycfg/manager_test.go +++ b/agent/proxycfg/manager_test.go @@ -45,23 +45,6 @@ func TestManager_BasicLifecycle(t *testing.T) { // Create a bunch of common data for the various test cases. roots, leaf := TestCerts(t) - dbTarget := structs.DiscoveryTarget{ - Service: "db", - Namespace: "default", - Datacenter: "dc1", - } - dbTarget_v1 := structs.DiscoveryTarget{ - Service: "db", - ServiceSubset: "v1", - Namespace: "default", - Datacenter: "dc1", - } - dbTarget_v2 := structs.DiscoveryTarget{ - Service: "db", - ServiceSubset: "v2", - Namespace: "default", - Datacenter: "dc1", - } dbDefaultChain := func() *structs.CompiledDiscoveryChain { return discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", func(req *discoverychain.CompileRequest) { @@ -213,9 +196,9 @@ func TestManager_BasicLifecycle(t *testing.T) { "db": dbDefaultChain(), }, WatchedUpstreams: nil, // Clone() clears this out - WatchedUpstreamEndpoints: map[string]map[structs.DiscoveryTarget]structs.CheckServiceNodes{ + WatchedUpstreamEndpoints: map[string]map[string]structs.CheckServiceNodes{ "db": { - dbTarget: TestUpstreamNodes(t), + "db.default.dc1": TestUpstreamNodes(t), }, }, UpstreamEndpoints: map[string]structs.CheckServiceNodes{}, @@ -252,10 +235,10 @@ func TestManager_BasicLifecycle(t *testing.T) { "db": dbSplitChain(), }, WatchedUpstreams: nil, // Clone() clears this out - WatchedUpstreamEndpoints: map[string]map[structs.DiscoveryTarget]structs.CheckServiceNodes{ + WatchedUpstreamEndpoints: map[string]map[string]structs.CheckServiceNodes{ "db": { - dbTarget_v1: TestUpstreamNodes(t), - dbTarget_v2: TestUpstreamNodesAlternate(t), + "v1.db.default.dc1": TestUpstreamNodes(t), + "v2.db.default.dc1": TestUpstreamNodesAlternate(t), }, }, UpstreamEndpoints: map[string]structs.CheckServiceNodes{}, diff --git a/agent/proxycfg/snapshot.go b/agent/proxycfg/snapshot.go index 22e94bcaa..1a99c1283 100644 --- a/agent/proxycfg/snapshot.go +++ b/agent/proxycfg/snapshot.go @@ -10,8 +10,8 @@ import ( type configSnapshotConnectProxy struct { Leaf *structs.IssuedCert DiscoveryChain map[string]*structs.CompiledDiscoveryChain // this is keyed by the Upstream.Identifier(), not the chain name - WatchedUpstreams map[string]map[structs.DiscoveryTarget]context.CancelFunc - WatchedUpstreamEndpoints map[string]map[structs.DiscoveryTarget]structs.CheckServiceNodes + WatchedUpstreams map[string]map[string]context.CancelFunc + WatchedUpstreamEndpoints map[string]map[string]structs.CheckServiceNodes UpstreamEndpoints map[string]structs.CheckServiceNodes // DEPRECATED:see:WatchedUpstreamEndpoints } diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index fa5fd7f04..27f85af4a 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -359,8 +359,8 @@ func (s *state) initialConfigSnapshot() ConfigSnapshot { switch s.kind { case structs.ServiceKindConnectProxy: snap.ConnectProxy.DiscoveryChain = make(map[string]*structs.CompiledDiscoveryChain) - snap.ConnectProxy.WatchedUpstreams = make(map[string]map[structs.DiscoveryTarget]context.CancelFunc) - snap.ConnectProxy.WatchedUpstreamEndpoints = make(map[string]map[structs.DiscoveryTarget]structs.CheckServiceNodes) + snap.ConnectProxy.WatchedUpstreams = make(map[string]map[string]context.CancelFunc) + snap.ConnectProxy.WatchedUpstreamEndpoints = make(map[string]map[string]structs.CheckServiceNodes) snap.ConnectProxy.UpstreamEndpoints = make(map[string]structs.CheckServiceNodes) // TODO(rb): deprecated case structs.ServiceKindMeshGateway: snap.MeshGateway.WatchedServices = make(map[string]context.CancelFunc) @@ -503,22 +503,17 @@ func (s *state) handleUpdateConnectProxy(u cache.UpdateEvent, snap *ConfigSnapsh return fmt.Errorf("invalid type for service response: %T", u.Result) } correlationID := strings.TrimPrefix(u.CorrelationID, "upstream-target:") - encTarget, svc, ok := removeColonPrefix(correlationID) + targetID, svc, ok := removeColonPrefix(correlationID) if !ok { return fmt.Errorf("invalid correlation id %q", u.CorrelationID) } - target := structs.DiscoveryTarget{} - if err := target.UnmarshalText([]byte(encTarget)); err != nil { - return fmt.Errorf("invalid correlation id %q: %v", u.CorrelationID, err) - } - m, ok := snap.ConnectProxy.WatchedUpstreamEndpoints[svc] if !ok { - m = make(map[structs.DiscoveryTarget]structs.CheckServiceNodes) + m = make(map[string]structs.CheckServiceNodes) snap.ConnectProxy.WatchedUpstreamEndpoints[svc] = m } - snap.ConnectProxy.WatchedUpstreamEndpoints[svc][target] = resp.Nodes + snap.ConnectProxy.WatchedUpstreamEndpoints[svc][targetID] = resp.Nodes case strings.HasPrefix(u.CorrelationID, "upstream:"+serviceIDPrefix): resp, ok := u.Result.(*structs.IndexedCheckServiceNodes) @@ -561,11 +556,10 @@ func (s *state) resetWatchesFromChain( // Initialize relevant sub maps. if _, ok := snap.ConnectProxy.WatchedUpstreams[id]; !ok { - snap.ConnectProxy.WatchedUpstreams[id] = make(map[structs.DiscoveryTarget]context.CancelFunc) + snap.ConnectProxy.WatchedUpstreams[id] = make(map[string]context.CancelFunc) } if _, ok := snap.ConnectProxy.WatchedUpstreamEndpoints[id]; !ok { - // TODO(rb): does this belong here? - snap.ConnectProxy.WatchedUpstreamEndpoints[id] = make(map[structs.DiscoveryTarget]structs.CheckServiceNodes) + snap.ConnectProxy.WatchedUpstreamEndpoints[id] = make(map[string]structs.CheckServiceNodes) } // We could invalidate this selectively based on a hash of the relevant @@ -573,24 +567,22 @@ func (s *state) resetWatchesFromChain( // upstream when the chain changes in any way. // // TODO(rb): content hash based add/remove - for target, cancelFn := range snap.ConnectProxy.WatchedUpstreams[id] { - s.logger.Printf("[TRACE] proxycfg: upstream=%q:chain=%q: stopping watch of target %s", id, chain.ServiceName, target) - delete(snap.ConnectProxy.WatchedUpstreams[id], target) - delete(snap.ConnectProxy.WatchedUpstreamEndpoints[id], target) // TODO(rb): safe? + for targetID, cancelFn := range snap.ConnectProxy.WatchedUpstreams[id] { + s.logger.Printf("[TRACE] proxycfg: upstream=%q:chain=%q: stopping watch of target %s", id, chain.ServiceName, targetID) + delete(snap.ConnectProxy.WatchedUpstreams[id], targetID) + delete(snap.ConnectProxy.WatchedUpstreamEndpoints[id], targetID) cancelFn() } - for target, targetConfig := range chain.Targets { - s.logger.Printf("[TRACE] proxycfg: upstream=%q:chain=%q: initializing watch of target %s", id, chain.ServiceName, target) - - encodedTarget := target.Identifier() + for _, target := range chain.Targets { + s.logger.Printf("[TRACE] proxycfg: upstream=%q:chain=%q: initializing watch of target %s", id, chain.ServiceName, target.ID) ctx, cancel := context.WithCancel(s.ctx) // TODO (mesh-gateway)- maybe allow using a gateway within a datacenter at some point meshGateway := structs.MeshGatewayModeDefault if target.Datacenter != s.source.Datacenter { - meshGateway = targetConfig.MeshGateway.Mode + meshGateway = target.MeshGateway.Mode } // if the default mode @@ -600,10 +592,10 @@ func (s *state) resetWatchesFromChain( err := s.watchConnectProxyService( ctx, - "upstream-target:"+encodedTarget+":"+id, + "upstream-target:"+target.ID+":"+id, target.Service, target.Datacenter, - targetConfig.Subset.Filter, + target.Subset.Filter, meshGateway, ) if err != nil { @@ -611,7 +603,7 @@ func (s *state) resetWatchesFromChain( return err } - snap.ConnectProxy.WatchedUpstreams[id][target] = cancel + snap.ConnectProxy.WatchedUpstreams[id][target.ID] = cancel } return nil diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index 92b481a60..66d5f02b2 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -481,17 +481,17 @@ func TestState_WatchesAndUpdates(t *testing.T) { stage1 := verificationStage{ requiredWatches: map[string]verifyWatchRequest{ - "upstream-target:api,,,dc1:api": genVerifyServiceWatch("api", "", "dc1", true), - "upstream-target:api-failover-remote,,,dc2:api-failover-remote?dc=dc2": genVerifyGatewayWatch("dc2"), - "upstream-target:api-failover-local,,,dc2:api-failover-local?dc=dc2": genVerifyGatewayWatch("dc1"), - "upstream-target:api-failover-direct,,,dc2:api-failover-direct?dc=dc2": genVerifyServiceWatch("api-failover-direct", "", "dc2", true), + "upstream-target:api.default.dc1:api": genVerifyServiceWatch("api", "", "dc1", true), + "upstream-target:api-failover-remote.default.dc2:api-failover-remote?dc=dc2": genVerifyGatewayWatch("dc2"), + "upstream-target:api-failover-local.default.dc2:api-failover-local?dc=dc2": genVerifyGatewayWatch("dc1"), + "upstream-target:api-failover-direct.default.dc2:api-failover-direct?dc=dc2": genVerifyServiceWatch("api-failover-direct", "", "dc2", true), }, } if meshGatewayProxyConfigValue == structs.MeshGatewayModeDefault { - stage1.requiredWatches["upstream-target:api,,,dc2:api-dc2"] = genVerifyServiceWatch("api", "", "dc2", true) + stage1.requiredWatches["upstream-target:api.default.dc2:api-dc2"] = genVerifyServiceWatch("api", "", "dc2", true) } else { - stage1.requiredWatches["upstream-target:api,,,dc2:api-dc2"] = genVerifyGatewayWatch("dc1") + stage1.requiredWatches["upstream-target:api.default.dc2:api-dc2"] = genVerifyGatewayWatch("dc1") } return testCase{ diff --git a/agent/proxycfg/testing.go b/agent/proxycfg/testing.go index 82856a108..167123e8d 100644 --- a/agent/proxycfg/testing.go +++ b/agent/proxycfg/testing.go @@ -545,17 +545,6 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE dbChain := discoverychain.TestCompileConfigEntries(t, "db", "default", "dc1", compileSetup, entries...) - dbTarget := structs.DiscoveryTarget{ - Service: "db", - Namespace: "default", - Datacenter: "dc1", - } - failTarget := structs.DiscoveryTarget{ - Service: "fail", - Namespace: "default", - Datacenter: "dc1", - } - snap := &ConfigSnapshot{ Kind: structs.ServiceKindConnectProxy, Service: "web-sidecar-proxy", @@ -578,9 +567,9 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE DiscoveryChain: map[string]*structs.CompiledDiscoveryChain{ "db": dbChain, }, - WatchedUpstreamEndpoints: map[string]map[structs.DiscoveryTarget]structs.CheckServiceNodes{ - "db": map[structs.DiscoveryTarget]structs.CheckServiceNodes{ - dbTarget: TestUpstreamNodes(t), + WatchedUpstreamEndpoints: map[string]map[string]structs.CheckServiceNodes{ + "db": map[string]structs.CheckServiceNodes{ + "db.default.dc1": TestUpstreamNodes(t), }, }, }, @@ -591,24 +580,12 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE case "simple-with-overrides": case "simple": case "failover": - snap.ConnectProxy.WatchedUpstreamEndpoints["db"][failTarget] = + snap.ConnectProxy.WatchedUpstreamEndpoints["db"]["fail.default.dc1"] = TestUpstreamNodesAlternate(t) case "splitter-with-resolver-redirect-multidc": - dbTarget_v1_dc1 := structs.DiscoveryTarget{ - Service: "db", - ServiceSubset: "v1", - Namespace: "default", - Datacenter: "dc1", - } - dbTarget_v2_dc2 := structs.DiscoveryTarget{ - Service: "db", - ServiceSubset: "v2", - Namespace: "default", - Datacenter: "dc2", - } - snap.ConnectProxy.WatchedUpstreamEndpoints["db"] = map[structs.DiscoveryTarget]structs.CheckServiceNodes{ - dbTarget_v1_dc1: TestUpstreamNodes(t), - dbTarget_v2_dc2: TestUpstreamNodesDC2(t), + snap.ConnectProxy.WatchedUpstreamEndpoints["db"] = map[string]structs.CheckServiceNodes{ + "v1.db.default.dc1": TestUpstreamNodes(t), + "v2.db.default.dc2": TestUpstreamNodesDC2(t), } default: t.Fatalf("unexpected variation: %q", variation) diff --git a/agent/structs/config_entry.go b/agent/structs/config_entry.go index 90ecddd9f..58ccd13cf 100644 --- a/agent/structs/config_entry.go +++ b/agent/structs/config_entry.go @@ -336,11 +336,10 @@ func ConfigEntryDecodeRulesForKind(kind string) (skipWhenPatching []string, tran }, nil case ServiceResolver: return nil, map[string]string{ - "connect_timeout": "connecttimeout", - "default_subset": "defaultsubset", - "only_passing": "onlypassing", - "overprovisioning_factor": "overprovisioningfactor", - "service_subset": "servicesubset", + "connect_timeout": "connecttimeout", + "default_subset": "defaultsubset", + "only_passing": "onlypassing", + "service_subset": "servicesubset", }, nil default: return nil, nil, fmt.Errorf("kind %q should be explicitly handled here", kind) diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index 5a6c1c873..70a6e21db 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -677,10 +677,6 @@ func (e *ServiceResolverConfigEntry) Validate() error { } } - if f.OverprovisioningFactor < 0 { - return fmt.Errorf("Bad Failover[%q].OverprovisioningFactor '%d', must be >= 0", subset, f.OverprovisioningFactor) - } - for _, dc := range f.Datacenters { if dc == "" { return fmt.Errorf("Bad Failover[%q].Datacenters: found empty datacenter", subset) @@ -783,11 +779,9 @@ type ServiceResolverRedirect struct { // There are some restrictions on what is allowed in here: // -// - Service, ServiceSubset, Namespace, NearestN, and Datacenters cannot all be +// - Service, ServiceSubset, Namespace, and Datacenters cannot all be // empty at once. // -// - Both 'NearestN' and 'Datacenters' may be specified at once. -// type ServiceResolverFailover struct { // Service is the service to resolve instead of the default as the failover // group of instances (optional). @@ -809,27 +803,12 @@ type ServiceResolverFailover struct { // This is a DESTINATION during failover. Namespace string `json:",omitempty"` - // NearestN is set to the number of remote datacenters to try, based on - // network coordinates. - // - // This is a DESTINATION during failover. - // - // TODO(rb): bring this back after normal DC failover works - // NearestN int `json:",omitempty"` - - // Datacenters is a fixed list of datacenters to try after NearestN. We - // never try a datacenter multiple times, so those are subtracted from this - // list before proceeding. + // Datacenters is a fixed list of datacenters to try. We never try a + // datacenter multiple times, so those are subtracted from this list before + // proceeding. // // This is a DESTINATION during failover. Datacenters []string `json:",omitempty"` - - // OverprovisioningFactor is a pass through for envoy's - // overprovisioning_factor value. - // - // If omitted the overprovisioning factor value will be set so high as to - // imply binary failover (all or nothing). - OverprovisioningFactor int `json:",omitempty"` } type discoveryChainConfigEntry interface { @@ -1051,10 +1030,8 @@ func (r *DiscoveryChainRequest) CacheInfo() cache.RequestInfo { return info } -// TODO(rb): either fix the compiled results, or take the derived data and stash it here in a json/msgpack-friendly way? type DiscoveryChainResponse struct { - ConfigEntries *DiscoveryChainConfigEntries `json:",omitempty"` // TODO(rb): remove these? - Chain *CompiledDiscoveryChain `json:",omitempty"` + Chain *CompiledDiscoveryChain QueryMeta } diff --git a/agent/structs/config_entry_discoverychain_test.go b/agent/structs/config_entry_discoverychain_test.go index 09f7c2c2f..602a9d161 100644 --- a/agent/structs/config_entry_discoverychain_test.go +++ b/agent/structs/config_entry_discoverychain_test.go @@ -456,20 +456,6 @@ func TestServiceResolverConfigEntry(t *testing.T) { }, }, }, - { - name: "failover with invalid overprovisioning factor", - entry: &ServiceResolverConfigEntry{ - Kind: ServiceResolver, - Name: "test", - Failover: map[string]ServiceResolverFailover{ - "*": ServiceResolverFailover{ - Service: "backup", - OverprovisioningFactor: -1, - }, - }, - }, - validateErr: `Bad Failover["*"].OverprovisioningFactor '-1', must be >= 0`, - }, { name: "failover with empty datacenters in list", entry: &ServiceResolverConfigEntry{ diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go index 7576dce8a..0c76f0928 100644 --- a/agent/structs/config_entry_test.go +++ b/agent/structs/config_entry_test.go @@ -424,7 +424,6 @@ func TestDecodeConfigEntry(t *testing.T) { service_subset = "sure" namespace = "neighbor" datacenters = ["dc5", "dc14"] - overprovisioning_factor = 150 }, "*" = { datacenters = ["dc7"] @@ -450,7 +449,6 @@ func TestDecodeConfigEntry(t *testing.T) { ServiceSubset = "sure" Namespace = "neighbor" Datacenters = ["dc5", "dc14"] - OverprovisioningFactor = 150 }, "*" = { Datacenters = ["dc7"] @@ -472,11 +470,10 @@ func TestDecodeConfigEntry(t *testing.T) { }, Failover: map[string]ServiceResolverFailover{ "v2": { - Service: "failcopy", - ServiceSubset: "sure", - Namespace: "neighbor", - Datacenters: []string{"dc5", "dc14"}, - OverprovisioningFactor: 150, + Service: "failcopy", + ServiceSubset: "sure", + Namespace: "neighbor", + Datacenters: []string{"dc5", "dc14"}, }, "*": { Datacenters: []string{"dc7"}, diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index 55ede8e03..80f44bde8 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -1,11 +1,7 @@ package structs import ( - "bytes" - "encoding" "fmt" - "net/url" - "strings" "time" ) @@ -24,10 +20,10 @@ type CompiledDiscoveryChain struct { // If set, this value should be used to prefix/suffix any generated load // balancer data plane objects to avoid sharing customized and // non-customized versions. - CustomizationHash string + CustomizationHash string `json:",omitempty"` // Protocol is the overall protocol shared by everything in the chain. - Protocol string + Protocol string `json:",omitempty"` // StartNode is the first key into the Nodes map that should be followed // when walking the discovery chain. @@ -40,8 +36,8 @@ type CompiledDiscoveryChain struct { // guaranteed to be consistent within a single compilation. Nodes map[string]*DiscoveryGraphNode `json:",omitempty"` - // Targets is a list of all targets and configuration related just to targets. - Targets map[DiscoveryTarget]DiscoveryTargetConfig `json:",omitempty"` + // Targets is a list of all targets used in this chain. + Targets map[string]*DiscoveryTarget `json:",omitempty"` } // IsDefault returns true if the compiled chain represents no routing, no @@ -59,10 +55,16 @@ func (c *CompiledDiscoveryChain) IsDefault() bool { panic("not possible: missing node named '" + c.StartNode + "' in chain '" + c.ServiceName + "'") } - // TODO(rb): include CustomizationHash here? - return node.Type == DiscoveryGraphNodeTypeResolver && - node.Resolver.Default && - node.Resolver.Target.Service == 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 } const ( @@ -98,29 +100,16 @@ func (s *DiscoveryGraphNode) IsResolver() bool { return s.Type == DiscoveryGraphNodeTypeResolver } -func (s *DiscoveryGraphNode) ServiceName() string { - if s.Type == DiscoveryGraphNodeTypeResolver { - return s.Resolver.Target.Service - } - return s.Name -} - func (s *DiscoveryGraphNode) MapKey() string { return fmt.Sprintf("%s:%s", s.Type, s.Name) } // compiled form of ServiceResolverConfigEntry type DiscoveryResolver struct { - Definition *ServiceResolverConfigEntry `json:",omitempty"` - Default bool `json:",omitempty"` - ConnectTimeout time.Duration `json:",omitempty"` - Target DiscoveryTarget `json:",omitempty"` - Failover *DiscoveryFailover `json:",omitempty"` -} - -type DiscoveryTargetConfig struct { - MeshGateway MeshGatewayConfig `json:",omitempty"` - Subset ServiceResolverSubset `json:",omitempty"` + Default bool `json:",omitempty"` + ConnectTimeout time.Duration `json:",omitempty"` + Target string `json:",omitempty"` + Failover *DiscoveryFailover `json:",omitempty"` } // compiled form of ServiceRoute @@ -137,136 +126,44 @@ type DiscoverySplit struct { // compiled form of ServiceResolverFailover type DiscoveryFailover struct { - Definition *ServiceResolverFailover `json:",omitempty"` - Targets []DiscoveryTarget `json:",omitempty"` + Targets []string `json:",omitempty"` } // DiscoveryTarget represents all of the inputs necessary to use a resolver // config entry to execute a catalog query to generate a list of service // instances during discovery. -// -// This is a value type so it can be used as a map key. type DiscoveryTarget struct { + ID string `json:",omitempty"` + Service string `json:",omitempty"` ServiceSubset string `json:",omitempty"` Namespace string `json:",omitempty"` Datacenter string `json:",omitempty"` + + MeshGateway MeshGatewayConfig `json:",omitempty"` + Subset ServiceResolverSubset `json:",omitempty"` } -func (t DiscoveryTarget) IsEmpty() bool { - return t.Service == "" && t.ServiceSubset == "" && t.Namespace == "" && t.Datacenter == "" +func NewDiscoveryTarget(service, serviceSubset, namespace, datacenter string) *DiscoveryTarget { + t := &DiscoveryTarget{ + Service: service, + ServiceSubset: serviceSubset, + Namespace: namespace, + Datacenter: datacenter, + } + t.setID() + return t } -// CopyAndModify will duplicate the target and selectively modify it given the -// requested inputs. -func (t DiscoveryTarget) CopyAndModify( - service, - serviceSubset, - namespace, - datacenter string, -) DiscoveryTarget { - t2 := t // copy - if service != "" && service != t2.Service { - t2.Service = service - // Reset the chosen subset if we reference a service other than our own. - t2.ServiceSubset = "" - } - if serviceSubset != "" && serviceSubset != t2.ServiceSubset { - t2.ServiceSubset = serviceSubset - } - if namespace != "" && namespace != t2.Namespace { - t2.Namespace = namespace - } - if datacenter != "" && datacenter != t2.Datacenter { - t2.Datacenter = datacenter - } - return t2 -} - -var _ encoding.TextMarshaler = DiscoveryTarget{} -var _ encoding.TextUnmarshaler = (*DiscoveryTarget)(nil) - -// MarshalText implements encoding.TextMarshaler. -// -// This should also not include any colons for embedding that happens -// elsewhere. -// -// This should NOT return any errors. -func (t DiscoveryTarget) MarshalText() (text []byte, err error) { - return []byte(t.Identifier()), nil -} - -func (t DiscoveryTarget) Identifier() string { - var buf bytes.Buffer - buf.WriteString(url.QueryEscape(t.Service)) - buf.WriteRune(',') - buf.WriteString(url.QueryEscape(t.ServiceSubset)) // TODO(rb): move this first so the scoping flows from small->large? - buf.WriteRune(',') - if t.Namespace != "default" { - buf.WriteString(url.QueryEscape(t.Namespace)) - } - buf.WriteRune(',') - buf.WriteString(url.QueryEscape(t.Datacenter)) - return buf.String() -} - -// UnmarshalText implements encoding.TextUnmarshaler. -func (t *DiscoveryTarget) UnmarshalText(text []byte) error { - parts := bytes.Split(text, []byte(",")) - bad := false - if len(parts) != 4 { - return fmt.Errorf("invalid target: %q", string(text)) - } - - var err error - t.Service, err = url.QueryUnescape(string(parts[0])) - if err != nil { - bad = true - } - t.ServiceSubset, err = url.QueryUnescape(string(parts[1])) - if err != nil { - bad = true - } - t.Namespace, err = url.QueryUnescape(string(parts[2])) - if err != nil { - bad = true - } - t.Datacenter, err = url.QueryUnescape(string(parts[3])) - if err != nil { - bad = true - } - - if bad { - return fmt.Errorf("invalid target: %q", string(text)) - } - - if t.Namespace == "" { - t.Namespace = "default" - } - return nil -} - -func (t DiscoveryTarget) String() string { - var b strings.Builder - - if t.ServiceSubset != "" { - b.WriteString(t.ServiceSubset) +func (t *DiscoveryTarget) setID() { + // NOTE: this format is similar to the SNI syntax for simplicity + if t.ServiceSubset == "" { + t.ID = fmt.Sprintf("%s.%s.%s", t.Service, t.Namespace, t.Datacenter) } else { - b.WriteString("") + t.ID = fmt.Sprintf("%s.%s.%s.%s", t.ServiceSubset, t.Service, t.Namespace, t.Datacenter) } - b.WriteRune('.') - - b.WriteString(t.Service) - b.WriteRune('.') - - if t.Namespace != "" { - b.WriteString(t.Namespace) - } else { - b.WriteString("") - } - b.WriteRune('.') - - b.WriteString(t.Datacenter) - - return b.String() +} + +func (t *DiscoveryTarget) String() string { + return t.ID } diff --git a/agent/structs/discovery_chain_test.go b/agent/structs/discovery_chain_test.go deleted file mode 100644 index f372fd99d..000000000 --- a/agent/structs/discovery_chain_test.go +++ /dev/null @@ -1,120 +0,0 @@ -package structs - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestDiscoveryTarget_TextMarshal(t *testing.T) { - for _, tc := range []struct { - target DiscoveryTarget - enc string - alt DiscoveryTarget - }{ - { - target: DiscoveryTarget{"", "", "", ""}, - enc: ",,,", - alt: DiscoveryTarget{"", "", "default", ""}, - }, - { - target: DiscoveryTarget{"a:b", "", "", ""}, - enc: "a%3Ab,,,", - alt: DiscoveryTarget{"a:b", "", "default", ""}, - }, - { - target: DiscoveryTarget{"", "a:b", "", ""}, - enc: ",a%3Ab,,", - alt: DiscoveryTarget{"", "a:b", "default", ""}, - }, - { - target: DiscoveryTarget{"", "", "a:b", ""}, - enc: ",,a%3Ab,", - alt: DiscoveryTarget{"", "", "a:b", ""}, - }, - { - target: DiscoveryTarget{"", "", "", "a:b"}, - enc: ",,,a%3Ab", - alt: DiscoveryTarget{"", "", "default", "a:b"}, - }, - { - target: DiscoveryTarget{"one", "two", "three", "four"}, - enc: "one,two,three,four", - }, - } { - tc := tc - t.Run(tc.target.String(), func(t *testing.T) { - out, err := tc.target.MarshalText() - require.NoError(t, err) - require.Equal(t, tc.enc, string(out)) - - var dec DiscoveryTarget - require.NoError(t, dec.UnmarshalText(out)) - if tc.alt.IsEmpty() { - require.Equal(t, tc.target, dec) - } else { - require.Equal(t, tc.alt, dec) - } - }) - } -} - -func TestDiscoveryTarget_CopyAndModify(t *testing.T) { - type fields = DiscoveryTarget // abbreviation - - for _, tc := range []struct { - name string - in fields - mod fields // this is semantically wrong, but the shape of the struct is still what we want - expect fields - }{ - { - name: "service with no subset and no mod", - in: fields{"foo", "", "default", "dc1"}, - mod: fields{}, - expect: fields{"foo", "", "default", "dc1"}, - }, - { - name: "service with subset and no mod", - in: fields{"foo", "v2", "default", "dc1"}, - mod: fields{}, - expect: fields{"foo", "v2", "default", "dc1"}, - }, - { - name: "service with no subset and service mod", - in: fields{"foo", "", "default", "dc1"}, - mod: fields{"bar", "", "", ""}, - expect: fields{"bar", "", "default", "dc1"}, - }, - { - name: "service with subset and service mod", - in: fields{"foo", "v2", "default", "dc1"}, - mod: fields{"bar", "", "", ""}, - expect: fields{"bar", "", "default", "dc1"}, - }, - { - name: "service with subset and noop service mod with dc mod", - in: fields{"foo", "v2", "default", "dc1"}, - mod: fields{"foo", "", "", "dc9"}, - expect: fields{"foo", "v2", "default", "dc9"}, - }, - { - name: "service with subset and namespace mod", - in: fields{"foo", "v2", "default", "dc1"}, - mod: fields{"", "", "fancy", ""}, - expect: fields{"foo", "v2", "fancy", "dc1"}, - }, - } { - tc := tc - t.Run(tc.name, func(t *testing.T) { - out := tc.in.CopyAndModify( - tc.mod.Service, - tc.mod.ServiceSubset, - tc.mod.Namespace, - tc.mod.Datacenter, - ) - require.Equal(t, tc.expect, out) - }) - } - -} diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go index 8bf562300..78009aa25 100644 --- a/agent/xds/clusters.go +++ b/agent/xds/clusters.go @@ -254,7 +254,9 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain( if node.Type != structs.DiscoveryGraphNodeTypeResolver { continue } - target := node.Resolver.Target + targetID := node.Resolver.Target + + target := chain.Targets[targetID] sni := TargetSNI(target, cfgSnap) clusterName := CustomizeClusterName(sni, chain) diff --git a/agent/xds/endpoints.go b/agent/xds/endpoints.go index ff0219d1d..25529d14c 100644 --- a/agent/xds/endpoints.go +++ b/agent/xds/endpoints.go @@ -56,7 +56,6 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps if ok { la := makeLoadAssignment( clusterName, - 0, []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, @@ -79,23 +78,20 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps continue } failover := node.Resolver.Failover - target := node.Resolver.Target + targetID := node.Resolver.Target - endpoints, ok := chainEndpointMap[target] + target := chain.Targets[targetID] + + endpoints, ok := chainEndpointMap[targetID] if !ok { continue // skip the cluster (should not happen) } - targetConfig := chain.Targets[target] - - var ( - endpointGroups []loadAssignmentEndpointGroup - overprovisioningFactor int - ) + var endpointGroups []loadAssignmentEndpointGroup primaryGroup := loadAssignmentEndpointGroup{ Endpoints: endpoints, - OnlyPassing: targetConfig.Subset.OnlyPassing, + OnlyPassing: target.Subset.OnlyPassing, } if failover != nil && len(failover.Targets) > 0 { @@ -103,26 +99,17 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps endpointGroups = append(endpointGroups, primaryGroup) - if failover.Definition.OverprovisioningFactor > 0 { - overprovisioningFactor = failover.Definition.OverprovisioningFactor - } - if overprovisioningFactor <= 0 { - // We choose such a large value here that the failover math should - // in effect not happen until zero instances are healthy. - overprovisioningFactor = 100000 - } - - for _, failTarget := range failover.Targets { - failEndpoints, ok := chainEndpointMap[failTarget] + for _, failTargetID := range failover.Targets { + failEndpoints, ok := chainEndpointMap[failTargetID] if !ok { continue // skip the failover target (should not happen) } - failTargetConfig := chain.Targets[failTarget] + failTarget := chain.Targets[failTargetID] endpointGroups = append(endpointGroups, loadAssignmentEndpointGroup{ Endpoints: failEndpoints, - OnlyPassing: failTargetConfig.Subset.OnlyPassing, + OnlyPassing: failTarget.Subset.OnlyPassing, }) } } else { @@ -134,7 +121,6 @@ func (s *Server) endpointsFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnaps la := makeLoadAssignment( clusterName, - overprovisioningFactor, endpointGroups, cfgSnap.Datacenter, ) @@ -154,7 +140,6 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh clusterName := DatacenterSNI(dc, cfgSnap) la := makeLoadAssignment( clusterName, - 0, []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, @@ -168,7 +153,6 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh clusterName := ServiceSNI(svc, "", "default", cfgSnap.Datacenter, cfgSnap) la := makeLoadAssignment( clusterName, - 0, []loadAssignmentEndpointGroup{ {Endpoints: endpoints}, }, @@ -200,7 +184,6 @@ func (s *Server) endpointsFromSnapshotMeshGateway(cfgSnap *proxycfg.ConfigSnapsh la := makeLoadAssignment( clusterName, - 0, []loadAssignmentEndpointGroup{ { Endpoints: endpoints, @@ -231,19 +214,17 @@ type loadAssignmentEndpointGroup struct { OnlyPassing bool } -func makeLoadAssignment( - clusterName string, - overprovisioningFactor int, - endpointGroups []loadAssignmentEndpointGroup, - localDatacenter string, -) *envoy.ClusterLoadAssignment { +func makeLoadAssignment(clusterName string, endpointGroups []loadAssignmentEndpointGroup, localDatacenter string) *envoy.ClusterLoadAssignment { cla := &envoy.ClusterLoadAssignment{ ClusterName: clusterName, Endpoints: make([]envoyendpoint.LocalityLbEndpoints, 0, len(endpointGroups)), } - if overprovisioningFactor > 0 { + + if len(endpointGroups) > 1 { cla.Policy = &envoy.ClusterLoadAssignment_Policy{ - OverprovisioningFactor: makeUint32Value(overprovisioningFactor), + // We choose such a large value here that the failover math should + // in effect not happen until zero instances are healthy. + OverprovisioningFactor: makeUint32Value(100000), } } diff --git a/agent/xds/endpoints_test.go b/agent/xds/endpoints_test.go index c280760d0..019da2985 100644 --- a/agent/xds/endpoints_test.go +++ b/agent/xds/endpoints_test.go @@ -96,11 +96,10 @@ func Test_makeLoadAssignment(t *testing.T) { // TODO(rb): test onlypassing tests := []struct { - name string - clusterName string - overprovisioningFactor int - endpoints []loadAssignmentEndpointGroup - want *envoy.ClusterLoadAssignment + name string + clusterName string + endpoints []loadAssignmentEndpointGroup + want *envoy.ClusterLoadAssignment }{ { name: "no instances", @@ -210,7 +209,6 @@ func Test_makeLoadAssignment(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got := makeLoadAssignment( tt.clusterName, - tt.overprovisioningFactor, tt.endpoints, "dc1", ) @@ -255,24 +253,6 @@ func Test_endpointsFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotDiscoveryChainWithFailover, setup: nil, }, - { - name: "connect-proxy-with-chain-and-sliding-failover", - create: proxycfg.TestConfigSnapshotDiscoveryChainWithFailover, - setup: func(snap *proxycfg.ConfigSnapshot) { - chain := snap.ConnectProxy.DiscoveryChain["db"] - - dbTarget := structs.DiscoveryTarget{ - Service: "db", - Namespace: "default", - Datacenter: "dc1", - } - dbResolverNode := chain.Nodes["resolver:"+dbTarget.Identifier()] - - failover := dbResolverNode.Resolver.Failover - - failover.Definition.OverprovisioningFactor = 160 - }, - }, { name: "splitter-with-resolver-redirect", create: proxycfg.TestConfigSnapshotDiscoveryChain_SplitterWithResolverRedirectMultiDC, diff --git a/agent/xds/routes.go b/agent/xds/routes.go index 243c06541..6469c65b4 100644 --- a/agent/xds/routes.go +++ b/agent/xds/routes.go @@ -307,7 +307,9 @@ func makeDefaultRouteMatch() envoyroute.RouteMatch { } } -func makeRouteActionForSingleCluster(target structs.DiscoveryTarget, chain *structs.CompiledDiscoveryChain, cfgSnap *proxycfg.ConfigSnapshot) *envoyroute.Route_Route { +func makeRouteActionForSingleCluster(targetID string, chain *structs.CompiledDiscoveryChain, cfgSnap *proxycfg.ConfigSnapshot) *envoyroute.Route_Route { + target := chain.Targets[targetID] + sni := TargetSNI(target, cfgSnap) clusterName := CustomizeClusterName(sni, chain) @@ -328,7 +330,9 @@ func makeRouteActionForSplitter(splits []*structs.DiscoverySplit, chain *structs if nextNode.Type != structs.DiscoveryGraphNodeTypeResolver { return nil, fmt.Errorf("unexpected splitter destination node type: %s", nextNode.Type) } - target := nextNode.Resolver.Target + targetID := nextNode.Resolver.Target + + target := chain.Targets[targetID] sni := TargetSNI(target, cfgSnap) clusterName := CustomizeClusterName(sni, chain) diff --git a/agent/xds/sni.go b/agent/xds/sni.go index 509824291..dc886feaa 100644 --- a/agent/xds/sni.go +++ b/agent/xds/sni.go @@ -42,7 +42,7 @@ func QuerySNI(service string, datacenter string, cfgSnap *proxycfg.ConfigSnapsho return fmt.Sprintf("%s.default.%s.query.%s", service, datacenter, cfgSnap.Roots.TrustDomain) } -func TargetSNI(target structs.DiscoveryTarget, cfgSnap *proxycfg.ConfigSnapshot) string { +func TargetSNI(target *structs.DiscoveryTarget, cfgSnap *proxycfg.ConfigSnapshot) string { return ServiceSNI(target.Service, target.ServiceSubset, target.Namespace, target.Datacenter, cfgSnap) } diff --git a/agent/xds/testdata/endpoints/connect-proxy-with-chain-and-sliding-failover.golden b/agent/xds/testdata/endpoints/connect-proxy-with-chain-and-sliding-failover.golden deleted file mode 100644 index 5f7883a63..000000000 --- a/agent/xds/testdata/endpoints/connect-proxy-with-chain-and-sliding-failover.golden +++ /dev/null @@ -1,73 +0,0 @@ -{ - "versionInfo": "00000001", - "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "clusterName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul", - "endpoints": [ - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "10.10.1.1", - "portValue": 8080 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "10.10.1.2", - "portValue": 8080 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - } - ] - }, - { - "lbEndpoints": [ - { - "endpoint": { - "address": { - "socketAddress": { - "address": "10.20.1.1", - "portValue": 8080 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - }, - { - "endpoint": { - "address": { - "socketAddress": { - "address": "10.20.1.2", - "portValue": 8080 - } - } - }, - "healthStatus": "HEALTHY", - "loadBalancingWeight": 1 - } - ], - "priority": 1 - } - ], - "policy": { - "overprovisioningFactor": 160 - } - } - ], - "typeUrl": "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment", - "nonce": "00000001" -} \ No newline at end of file diff --git a/api/config_entry.go b/api/config_entry.go index f6fa79234..f2a4b5b85 100644 --- a/api/config_entry.go +++ b/api/config_entry.go @@ -188,7 +188,7 @@ func DecodeConfigEntryFromJSON(data []byte) (ConfigEntry, error) { return DecodeConfigEntry(raw) } -// Config can be used to query the Config endpoints +// ConfigEntries can be used to query the Config endpoints type ConfigEntries struct { c *Client } diff --git a/api/config_entry_discoverychain.go b/api/config_entry_discoverychain.go index 8623618af..e0d3e9b1b 100644 --- a/api/config_entry_discoverychain.go +++ b/api/config_entry_discoverychain.go @@ -118,12 +118,8 @@ type ServiceResolverRedirect struct { } type ServiceResolverFailover struct { - Service string `json:",omitempty"` - ServiceSubset string `json:",omitempty"` - Namespace string `json:",omitempty"` - Datacenters []string `json:",omitempty"` - OverprovisioningFactor int `json:",omitempty"` - - // TODO(rb): bring this back after normal DC failover works - // NearestN int `json:",omitempty"` + Service string `json:",omitempty"` + ServiceSubset string `json:",omitempty"` + Namespace string `json:",omitempty"` + Datacenters []string `json:",omitempty"` } diff --git a/api/discovery_chain.go b/api/discovery_chain.go new file mode 100644 index 000000000..a226f912c --- /dev/null +++ b/api/discovery_chain.go @@ -0,0 +1,190 @@ +package api + +import ( + "fmt" + "time" +) + +// DiscoveryChain can be used to query the discovery-chain endpoints +type DiscoveryChain struct { + c *Client +} + +// DiscoveryChain returns a handle to the discovery-chain endpoints +func (c *Client) DiscoveryChain() *DiscoveryChain { + return &DiscoveryChain{c} +} + +func (d *DiscoveryChain) Get(name string, opts *DiscoveryChainOptions, q *QueryOptions) (*DiscoveryChainResponse, *QueryMeta, error) { + if name == "" { + return nil, nil, fmt.Errorf("Name parameter must not be empty") + } + + method := "GET" + if opts != nil && opts.requiresPOST() { + method = "POST" + } + + r := d.c.newRequest(method, fmt.Sprintf("/v1/discovery-chain/%s", name)) + r.setQueryOptions(q) + + if opts != nil { + if opts.EvaluateInDatacenter != "" { + r.params.Set("compile-dc", opts.EvaluateInDatacenter) + } + // TODO(namespaces): handle possible EvaluateInNamespace here + } + + if method == "POST" { + r.obj = opts + } + + rtt, resp, err := requireOK(d.c.doRequest(r)) + if err != nil { + return nil, nil, err + } + defer resp.Body.Close() + + qm := &QueryMeta{} + parseQueryMeta(resp, qm) + qm.RequestTime = rtt + + var out DiscoveryChainResponse + + if err := decodeBody(resp, &out); err != nil { + return nil, nil, err + } + + return &out, qm, nil +} + +type DiscoveryChainOptions struct { + EvaluateInDatacenter string `json:"-"` + + // OverrideMeshGateway allows for the mesh gateway setting to be overridden + // for any resolver in the compiled chain. + OverrideMeshGateway MeshGatewayConfig `json:",omitempty"` + + // OverrideProtocol allows for the final protocol for the chain to be + // altered. + // + // - If the chain ordinarily would be TCP and an L7 protocol is passed here + // the chain will not include Routers or Splitters. + // + // - If the chain ordinarily would be L7 and TCP is passed here the chain + // will not include Routers or Splitters. + OverrideProtocol string `json:",omitempty"` + + // OverrideConnectTimeout allows for the ConnectTimeout setting to be + // overridden for any resolver in the compiled chain. + OverrideConnectTimeout time.Duration `json:",omitempty"` +} + +func (o *DiscoveryChainOptions) requiresPOST() bool { + if o == nil { + return false + } + return o.OverrideMeshGateway.Mode != "" || + o.OverrideProtocol != "" || + o.OverrideConnectTimeout != 0 +} + +type DiscoveryChainResponse struct { + Chain *CompiledDiscoveryChain +} + +type CompiledDiscoveryChain struct { + ServiceName string + Namespace string + Datacenter string + + // CustomizationHash is a unique hash of any data that affects the + // compilation of the discovery chain other than config entries or the + // name/namespace/datacenter evaluation criteria. + // + // If set, this value should be used to prefix/suffix any generated load + // balancer data plane objects to avoid sharing customized and + // non-customized versions. + CustomizationHash string + + // Protocol is the overall protocol shared by everything in the chain. + Protocol string + + // StartNode is the first key into the Nodes map that should be followed + // when walking the discovery chain. + StartNode string + + // Nodes contains all nodes available for traversal in the chain keyed by a + // unique name. You can walk this by starting with StartNode. + // + // NOTE: The names should be treated as opaque values and are only + // guaranteed to be consistent within a single compilation. + Nodes map[string]*DiscoveryGraphNode + + // Targets is a list of all targets used in this chain. + // + // NOTE: The names should be treated as opaque values and are only + // guaranteed to be consistent within a single compilation. + Targets map[string]*DiscoveryTarget +} + +const ( + DiscoveryGraphNodeTypeRouter = "router" + DiscoveryGraphNodeTypeSplitter = "splitter" + DiscoveryGraphNodeTypeResolver = "resolver" +) + +// DiscoveryGraphNode is a single node in the compiled discovery chain. +type DiscoveryGraphNode struct { + Type string + Name string // this is NOT necessarily a service + + // fields for Type==router + Routes []*DiscoveryRoute + + // fields for Type==splitter + Splits []*DiscoverySplit + + // fields for Type==resolver + Resolver *DiscoveryResolver +} + +// compiled form of ServiceRoute +type DiscoveryRoute struct { + Definition *ServiceRoute + NextNode string +} + +// compiled form of ServiceSplit +type DiscoverySplit struct { + Weight float32 + NextNode string +} + +// compiled form of ServiceResolverConfigEntry +type DiscoveryResolver struct { + Default bool + ConnectTimeout time.Duration + Target string + Failover *DiscoveryFailover +} + +// compiled form of ServiceResolverFailover +type DiscoveryFailover struct { + Targets []string +} + +// DiscoveryTarget represents all of the inputs necessary to use a resolver +// config entry to execute a catalog query to generate a list of service +// instances during discovery. +type DiscoveryTarget struct { + ID string + + Service string + ServiceSubset string + Namespace string + Datacenter string + + MeshGateway MeshGatewayConfig + Subset ServiceResolverSubset +} diff --git a/api/discovery_chain_test.go b/api/discovery_chain_test.go new file mode 100644 index 000000000..5433f60d4 --- /dev/null +++ b/api/discovery_chain_test.go @@ -0,0 +1,180 @@ +package api + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestAPI_DiscoveryChain_Get(t *testing.T) { + t.Parallel() + c, s := makeClient(t) + defer s.Stop() + + config_entries := c.ConfigEntries() + discoverychain := c.DiscoveryChain() + + require.True(t, t.Run("read default chain", func(t *testing.T) { + resp, _, err := discoverychain.Get("web", nil, nil) + require.NoError(t, err) + + expect := &DiscoveryChainResponse{ + Chain: &CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*DiscoveryGraphNode{ + "resolver:web.default.dc1": &DiscoveryGraphNode{ + Type: DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*DiscoveryTarget{ + "web.default.dc1": &DiscoveryTarget{ + ID: "web.default.dc1", + Service: "web", + Namespace: "default", + Datacenter: "dc1", + }, + }, + }, + } + require.Equal(t, expect, resp) + })) + + require.True(t, t.Run("read default chain; evaluate in dc2", func(t *testing.T) { + opts := &DiscoveryChainOptions{ + EvaluateInDatacenter: "dc2", + } + resp, _, err := discoverychain.Get("web", opts, nil) + require.NoError(t, err) + + expect := &DiscoveryChainResponse{ + Chain: &CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc2", + Protocol: "tcp", + StartNode: "resolver:web.default.dc2", + Nodes: map[string]*DiscoveryGraphNode{ + "resolver:web.default.dc2": &DiscoveryGraphNode{ + Type: DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc2", + Resolver: &DiscoveryResolver{ + Default: true, + ConnectTimeout: 5 * time.Second, + Target: "web.default.dc2", + }, + }, + }, + Targets: map[string]*DiscoveryTarget{ + "web.default.dc2": &DiscoveryTarget{ + ID: "web.default.dc2", + Service: "web", + Namespace: "default", + Datacenter: "dc2", + }, + }, + }, + } + require.Equal(t, expect, resp) + })) + + { // Now create one config entry. + ok, _, err := config_entries.Set(&ServiceResolverConfigEntry{ + Kind: ServiceResolver, + Name: "web", + ConnectTimeout: 33 * time.Second, + }, nil) + require.NoError(t, err) + require.True(t, ok) + } + + require.True(t, t.Run("read modified chain", func(t *testing.T) { + resp, _, err := discoverychain.Get("web", nil, nil) + require.NoError(t, err) + + expect := &DiscoveryChainResponse{ + Chain: &CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc1", + Protocol: "tcp", + StartNode: "resolver:web.default.dc1", + Nodes: map[string]*DiscoveryGraphNode{ + "resolver:web.default.dc1": &DiscoveryGraphNode{ + Type: DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc1", + Resolver: &DiscoveryResolver{ + ConnectTimeout: 33 * time.Second, + Target: "web.default.dc1", + }, + }, + }, + Targets: map[string]*DiscoveryTarget{ + "web.default.dc1": &DiscoveryTarget{ + ID: "web.default.dc1", + Service: "web", + Namespace: "default", + Datacenter: "dc1", + }, + }, + }, + } + require.Equal(t, expect, resp) + })) + + require.True(t, t.Run("read modified chain in dc2 with overrides", func(t *testing.T) { + opts := &DiscoveryChainOptions{ + EvaluateInDatacenter: "dc2", + OverrideMeshGateway: MeshGatewayConfig{ + Mode: MeshGatewayModeLocal, + }, + OverrideProtocol: "grpc", + OverrideConnectTimeout: 22 * time.Second, + } + resp, _, err := discoverychain.Get("web", opts, nil) + require.NoError(t, err) + + expect := &DiscoveryChainResponse{ + Chain: &CompiledDiscoveryChain{ + ServiceName: "web", + Namespace: "default", + Datacenter: "dc2", + Protocol: "grpc", + CustomizationHash: "98809527", + StartNode: "resolver:web.default.dc2", + Nodes: map[string]*DiscoveryGraphNode{ + "resolver:web.default.dc2": &DiscoveryGraphNode{ + Type: DiscoveryGraphNodeTypeResolver, + Name: "web.default.dc2", + Resolver: &DiscoveryResolver{ + ConnectTimeout: 22 * time.Second, + Target: "web.default.dc2", + }, + }, + }, + Targets: map[string]*DiscoveryTarget{ + "web.default.dc2": &DiscoveryTarget{ + ID: "web.default.dc2", + Service: "web", + Namespace: "default", + Datacenter: "dc2", + MeshGateway: MeshGatewayConfig{ + Mode: MeshGatewayModeLocal, + }, + }, + }, + }, + } + require.Equal(t, expect, resp) + })) +} diff --git a/command/config/write/config_write_test.go b/command/config/write/config_write_test.go index 4b15ea864..841be30c3 100644 --- a/command/config/write/config_write_test.go +++ b/command/config/write/config_write_test.go @@ -518,7 +518,6 @@ func TestParseConfigEntry(t *testing.T) { service_subset = "sure" namespace = "neighbor" datacenters = ["dc5", "dc14"] - overprovisioning_factor = 150 }, "*" = { datacenters = ["dc7"] @@ -544,7 +543,6 @@ func TestParseConfigEntry(t *testing.T) { ServiceSubset = "sure" Namespace = "neighbor" Datacenters = ["dc5", "dc14"] - OverprovisioningFactor = 150 }, "*" = { Datacenters = ["dc7"] @@ -566,11 +564,10 @@ func TestParseConfigEntry(t *testing.T) { }, Failover: map[string]api.ServiceResolverFailover{ "v2": { - Service: "failcopy", - ServiceSubset: "sure", - Namespace: "neighbor", - Datacenters: []string{"dc5", "dc14"}, - OverprovisioningFactor: 150, + Service: "failcopy", + ServiceSubset: "sure", + Namespace: "neighbor", + Datacenters: []string{"dc5", "dc14"}, }, "*": { Datacenters: []string{"dc7"},