diff --git a/agent/connect/sni_test.go b/agent/connect/sni_test.go index 26fae1da7..59e9f41fc 100644 --- a/agent/connect/sni_test.go +++ b/agent/connect/sni_test.go @@ -178,20 +178,43 @@ func TestQuerySNI(t *testing.T) { func TestTargetSNI(t *testing.T) { // empty namespace, empty subset require.Equal(t, "api.default.foo."+testTrustDomainSuffix1, - TargetSNI(structs.NewDiscoveryTarget("api", "", "", "default", "foo"), testTrustDomain1)) + TargetSNI(structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "api", + Partition: "default", + Datacenter: "foo", + }), testTrustDomain1)) require.Equal(t, "api.default.foo."+testTrustDomainSuffix1, - TargetSNI(structs.NewDiscoveryTarget("api", "", "", "", "foo"), testTrustDomain1)) + TargetSNI(structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "api", + Datacenter: "foo", + }), testTrustDomain1)) // set namespace, empty subset require.Equal(t, "api.neighbor.foo."+testTrustDomainSuffix2, - TargetSNI(structs.NewDiscoveryTarget("api", "", "neighbor", "default", "foo"), testTrustDomain2)) + TargetSNI(structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "api", + Namespace: "neighbor", + Partition: "default", + Datacenter: "foo", + }), testTrustDomain2)) // empty namespace, set subset require.Equal(t, "v2.api.default.foo."+testTrustDomainSuffix1, - TargetSNI(structs.NewDiscoveryTarget("api", "v2", "", "default", "foo"), testTrustDomain1)) + TargetSNI(structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "api", + ServiceSubset: "v2", + Partition: "default", + Datacenter: "foo", + }), testTrustDomain1)) // set namespace, set subset require.Equal(t, "canary.api.neighbor.foo."+testTrustDomainSuffix2, - TargetSNI(structs.NewDiscoveryTarget("api", "canary", "neighbor", "default", "foo"), testTrustDomain2)) + TargetSNI(structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "api", + ServiceSubset: "canary", + Namespace: "neighbor", + Partition: "default", + Datacenter: "foo", + }), testTrustDomain2)) } diff --git a/agent/consul/discovery_chain_endpoint_test.go b/agent/consul/discovery_chain_endpoint_test.go index 21c34aa86..c1ad0fef3 100644 --- a/agent/consul/discovery_chain_endpoint_test.go +++ b/agent/consul/discovery_chain_endpoint_test.go @@ -56,8 +56,17 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) { return &resp, nil } - newTarget := func(service, serviceSubset, namespace, partition, datacenter string) *structs.DiscoveryTarget { - t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, partition, datacenter) + newTarget := func(opts structs.DiscoveryTargetOpts) *structs.DiscoveryTarget { + if opts.Namespace == "" { + opts.Namespace = "default" + } + if opts.Partition == "" { + opts.Partition = "default" + } + if opts.Datacenter == "" { + opts.Datacenter = "dc1" + } + t := structs.NewDiscoveryTarget(opts) t.SNI = connect.TargetSNI(t, connect.TestClusterID+".consul") t.Name = t.SNI t.ConnectTimeout = 5 * time.Second // default @@ -119,7 +128,7 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "web.default.default.dc1": newTarget("web", "", "default", "default", "dc1"), + "web.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "web"}), }, }, } @@ -245,7 +254,7 @@ func TestDiscoveryChainEndpoint_Get(t *testing.T) { }, Targets: map[string]*structs.DiscoveryTarget{ "web.default.default.dc1": targetWithConnectTimeout( - newTarget("web", "", "default", "default", "dc1"), + newTarget(structs.DiscoveryTargetOpts{Service: "web"}), 33*time.Second, ), }, diff --git a/agent/consul/discoverychain/compile.go b/agent/consul/discoverychain/compile.go index ed664878b..3a9a1f0ed 100644 --- a/agent/consul/discoverychain/compile.go +++ b/agent/consul/discoverychain/compile.go @@ -8,6 +8,7 @@ import ( "github.com/mitchellh/hashstructure" "github.com/mitchellh/mapstructure" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/configentry" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/structs" @@ -576,7 +577,10 @@ func (c *compiler) assembleChain() error { if router == nil { // If no router is configured, move on down the line to the next hop of // the chain. - node, err := c.getSplitterOrResolverNode(c.newTarget(c.serviceName, "", "", "", "")) + node, err := c.getSplitterOrResolverNode(c.newTarget(structs.DiscoveryTargetOpts{ + Service: c.serviceName, + })) + if err != nil { return err } @@ -626,11 +630,20 @@ func (c *compiler) assembleChain() error { ) if dest.ServiceSubset == "" { node, err = c.getSplitterOrResolverNode( - c.newTarget(svc, "", destNamespace, destPartition, ""), - ) + c.newTarget(structs.DiscoveryTargetOpts{ + Service: svc, + Namespace: destNamespace, + Partition: destPartition, + }, + )) } else { node, err = c.getResolverNode( - c.newTarget(svc, dest.ServiceSubset, destNamespace, destPartition, ""), + c.newTarget(structs.DiscoveryTargetOpts{ + Service: svc, + ServiceSubset: dest.ServiceSubset, + Namespace: destNamespace, + Partition: destPartition, + }), false, ) } @@ -642,7 +655,12 @@ func (c *compiler) assembleChain() error { // If we have a router, we'll add a catch-all route at the end to send // unmatched traffic to the next hop in the chain. - defaultDestinationNode, err := c.getSplitterOrResolverNode(c.newTarget(router.Name, "", router.NamespaceOrDefault(), router.PartitionOrDefault(), "")) + opts := structs.DiscoveryTargetOpts{ + Service: router.Name, + Namespace: router.NamespaceOrDefault(), + Partition: router.PartitionOrDefault(), + } + defaultDestinationNode, err := c.getSplitterOrResolverNode(c.newTarget(opts)) if err != nil { return err } @@ -674,26 +692,36 @@ func newDefaultServiceRoute(serviceName, namespace, partition string) *structs.S } } -func (c *compiler) newTarget(service, serviceSubset, namespace, partition, datacenter string) *structs.DiscoveryTarget { - if service == "" { +func (c *compiler) newTarget(opts structs.DiscoveryTargetOpts) *structs.DiscoveryTarget { + if opts.Service == "" { panic("newTarget called with empty service which makes no sense") } - t := structs.NewDiscoveryTarget( - service, - serviceSubset, - defaultIfEmpty(namespace, c.evaluateInNamespace), - defaultIfEmpty(partition, c.evaluateInPartition), - defaultIfEmpty(datacenter, c.evaluateInDatacenter), - ) + if opts.Peer == "" { + opts.Datacenter = defaultIfEmpty(opts.Datacenter, c.evaluateInDatacenter) + opts.Namespace = defaultIfEmpty(opts.Namespace, c.evaluateInNamespace) + opts.Partition = defaultIfEmpty(opts.Partition, c.evaluateInPartition) + } else { + // Don't allow Peer and Datacenter. + opts.Datacenter = "" + // Peer and Partition cannot both be set. + opts.Partition = acl.PartitionOrDefault("") + // Default to "default" rather than c.evaluateInNamespace. + opts.Namespace = acl.PartitionOrDefault(opts.Namespace) + } - // Set default connect SNI. This will be overridden later if the service - // has an explicit SNI value configured in service-defaults. - t.SNI = connect.TargetSNI(t, c.evaluateInTrustDomain) + t := structs.NewDiscoveryTarget(opts) - // Use the same representation for the name. This will NOT be overridden - // later. - t.Name = t.SNI + // We don't have the peer's trust domain yet so we can't construct the SNI. + if opts.Peer == "" { + // Set default connect SNI. This will be overridden later if the service + // has an explicit SNI value configured in service-defaults. + t.SNI = connect.TargetSNI(t, c.evaluateInTrustDomain) + + // Use the same representation for the name. This will NOT be overridden + // later. + t.Name = t.SNI + } prev, ok := c.loadedTargets[t.ID] if ok { @@ -703,34 +731,30 @@ func (c *compiler) newTarget(service, serviceSubset, namespace, partition, datac return t } -func (c *compiler) rewriteTarget(t *structs.DiscoveryTarget, service, serviceSubset, partition, namespace, datacenter string) *structs.DiscoveryTarget { - var ( - service2 = t.Service - serviceSubset2 = t.ServiceSubset - partition2 = t.Partition - namespace2 = t.Namespace - datacenter2 = t.Datacenter - ) +func (c *compiler) rewriteTarget(t *structs.DiscoveryTarget, opts structs.DiscoveryTargetOpts) *structs.DiscoveryTarget { + mergedOpts := t.ToDiscoveryTargetOpts() - if service != "" && service != service2 { - service2 = service + if opts.Service != "" && opts.Service != mergedOpts.Service { + mergedOpts.Service = opts.Service // Reset the chosen subset if we reference a service other than our own. - serviceSubset2 = "" + mergedOpts.ServiceSubset = "" } - if serviceSubset != "" { - serviceSubset2 = serviceSubset + if opts.ServiceSubset != "" { + mergedOpts.ServiceSubset = opts.ServiceSubset } - if partition != "" { - partition2 = partition + if opts.Partition != "" { + mergedOpts.Partition = opts.Partition } - if namespace != "" { - namespace2 = namespace + // Only use explicit Namespace with Peer + if opts.Namespace != "" || opts.Peer != "" { + mergedOpts.Namespace = opts.Namespace } - if datacenter != "" { - datacenter2 = datacenter + if opts.Datacenter != "" { + mergedOpts.Datacenter = opts.Datacenter } + mergedOpts.Peer = opts.Peer - return c.newTarget(service2, serviceSubset2, namespace2, partition2, datacenter2) + return c.newTarget(mergedOpts) } func (c *compiler) getSplitterOrResolverNode(target *structs.DiscoveryTarget) (*structs.DiscoveryGraphNode, error) { @@ -803,10 +827,13 @@ func (c *compiler) getSplitterNode(sid structs.ServiceID) (*structs.DiscoveryGra // fall through to group-resolver } - node, err := c.getResolverNode( - c.newTarget(splitID.ID, split.ServiceSubset, splitID.NamespaceOrDefault(), splitID.PartitionOrDefault(), ""), - false, - ) + opts := structs.DiscoveryTargetOpts{ + Service: splitID.ID, + ServiceSubset: split.ServiceSubset, + Namespace: splitID.NamespaceOrDefault(), + Partition: splitID.PartitionOrDefault(), + } + node, err := c.getResolverNode(c.newTarget(opts), false) if err != nil { return nil, err } @@ -881,11 +908,7 @@ RESOLVE_AGAIN: redirectedTarget := c.rewriteTarget( target, - redirect.Service, - redirect.ServiceSubset, - redirect.Partition, - redirect.Namespace, - redirect.Datacenter, + redirect.ToDiscoveryTargetOpts(), ) if redirectedTarget.ID != target.ID { target = redirectedTarget @@ -895,14 +918,9 @@ RESOLVE_AGAIN: // Handle default subset. if target.ServiceSubset == "" && resolver.DefaultSubset != "" { - target = c.rewriteTarget( - target, - "", - resolver.DefaultSubset, - "", - "", - "", - ) + target = c.rewriteTarget(target, structs.DiscoveryTargetOpts{ + ServiceSubset: resolver.DefaultSubset, + }) goto RESOLVE_AGAIN } @@ -1027,56 +1045,54 @@ RESOLVE_AGAIN: failover, ok = f["*"] } - if ok { - // Determine which failover definitions apply. - var failoverTargets []*structs.DiscoveryTarget - if len(failover.Datacenters) > 0 { - for _, dc := range failover.Datacenters { - // Rewrite the target as per the failover policy. - failoverTarget := c.rewriteTarget( - target, - failover.Service, - failover.ServiceSubset, - target.Partition, - failover.Namespace, - dc, - ) - if failoverTarget.ID != target.ID { // don't failover to yourself - failoverTargets = append(failoverTargets, failoverTarget) - } - } - } else { + if !ok { + return node, nil + } + + // Determine which failover definitions apply. + var failoverTargets []*structs.DiscoveryTarget + if len(failover.Datacenters) > 0 { + opts := failover.ToDiscoveryTargetOpts() + for _, dc := range failover.Datacenters { // Rewrite the target as per the failover policy. - failoverTarget := c.rewriteTarget( - target, - failover.Service, - failover.ServiceSubset, - target.Partition, - failover.Namespace, - "", - ) + opts.Datacenter = dc + failoverTarget := c.rewriteTarget(target, opts) 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{} - node.Resolver.Failover = df - - // 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 { - return nil, err - } - failoverTarget := failoverResolveNode.Resolver.Target - df.Targets = append(df.Targets, failoverTarget) + } else if len(failover.Targets) > 0 { + for _, t := range failover.Targets { + // Rewrite the target as per the failover policy. + failoverTarget := c.rewriteTarget(target, t.ToDiscoveryTargetOpts()) + if failoverTarget.ID != target.ID { // don't failover to yourself + failoverTargets = append(failoverTargets, failoverTarget) } } + } else { + // Rewrite the target as per the failover policy. + failoverTarget := c.rewriteTarget(target, failover.ToDiscoveryTargetOpts()) + 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{} + node.Resolver.Failover = df + + // 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 { + return nil, err + } + failoverTarget := failoverResolveNode.Resolver.Target + df.Targets = append(df.Targets, failoverTarget) + } } } diff --git a/agent/consul/discoverychain/compile_test.go b/agent/consul/discoverychain/compile_test.go index 221ac757f..6505fdb9e 100644 --- a/agent/consul/discoverychain/compile_test.go +++ b/agent/consul/discoverychain/compile_test.go @@ -46,6 +46,7 @@ func TestCompile(t *testing.T) { "service and subset failover": testcase_ServiceAndSubsetFailover(), "datacenter failover": testcase_DatacenterFailover(), "datacenter failover with mesh gateways": testcase_DatacenterFailover_WithMeshGateways(), + "target failover": testcase_Failover_Targets(), "noop split to resolver with default subset": testcase_NoopSplit_WithDefaultSubset(), "resolver with default subset": testcase_Resolve_WithDefaultSubset(), "default resolver with external sni": testcase_DefaultResolver_ExternalSNI(), @@ -182,7 +183,7 @@ func testcase_JustRouterWithDefaults() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -244,7 +245,7 @@ func testcase_JustRouterWithNoDestination() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -294,7 +295,7 @@ func testcase_RouterWithDefaults_NoSplit_WithResolver() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ "main.default.default.dc1": targetWithConnectTimeout( - newTarget("main", "", "default", "default", "dc1", nil), + newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), 33*time.Second, ), }, @@ -361,7 +362,7 @@ func testcase_RouterWithDefaults_WithNoopSplit_DefaultResolver() compileTestCase }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -426,7 +427,10 @@ func testcase_NoopSplit_DefaultResolver_ProtocolFromProxyDefaults() compileTestC }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc1", + }, nil), }, } @@ -498,7 +502,7 @@ func testcase_RouterWithDefaults_WithNoopSplit_WithResolver() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ "main.default.default.dc1": targetWithConnectTimeout( - newTarget("main", "", "default", "default", "dc1", nil), + newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), 33*time.Second, ), }, @@ -584,8 +588,11 @@ func testcase_RouteBypassesSplit() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "bypass.other.default.default.dc1": newTarget("other", "bypass", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "bypass.other.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "other", + ServiceSubset: "bypass", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == bypass", } @@ -638,7 +645,7 @@ func testcase_NoopSplit_DefaultResolver() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -694,7 +701,7 @@ func testcase_NoopSplit_WithResolver() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ "main.default.default.dc1": targetWithConnectTimeout( - newTarget("main", "", "default", "default", "dc1", nil), + newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), 33*time.Second, ), }, @@ -776,12 +783,19 @@ func testcase_SubsetSplit() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "v2.main.default.default.dc1": newTarget("main", "v2", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + + "v2.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v2", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", } }), - "v1.main.default.default.dc1": newTarget("main", "v1", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v1.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v1", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 1", } @@ -855,8 +869,8 @@ func testcase_ServiceSplit() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "foo.default.default.dc1": newTarget("foo", "", "default", "default", "dc1", nil), - "bar.default.default.dc1": newTarget("bar", "", "default", "default", "dc1", nil), + "foo.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "foo"}, nil), + "bar.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "bar"}, nil), }, } @@ -935,7 +949,10 @@ func testcase_SplitBypassesSplit() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "bypassed.next.default.default.dc1": newTarget("next", "bypassed", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "bypassed.next.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "next", + ServiceSubset: "bypassed", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == bypass", } @@ -973,7 +990,7 @@ func testcase_ServiceRedirect() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "other.default.default.dc1": newTarget("other", "", "default", "default", "dc1", nil), + "other.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "other"}, nil), }, } @@ -1019,7 +1036,10 @@ func testcase_ServiceAndSubsetRedirect() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "v2.other.default.default.dc1": newTarget("other", "v2", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v2.other.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "other", + ServiceSubset: "v2", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", } @@ -1055,7 +1075,10 @@ func testcase_DatacenterRedirect() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc9": newTarget("main", "", "default", "default", "dc9", nil), + "main.default.default.dc9": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc9", + }, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1095,7 +1118,10 @@ func testcase_DatacenterRedirect_WithMeshGateways() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc9": newTarget("main", "", "default", "default", "dc9", func(t *structs.DiscoveryTarget) { + "main.default.default.dc9": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc9", + }, func(t *structs.DiscoveryTarget) { t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, } @@ -1134,8 +1160,8 @@ func testcase_ServiceFailover() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "backup.default.default.dc1": newTarget("backup", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "backup.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "backup"}, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1177,8 +1203,8 @@ func testcase_ServiceFailoverThroughRedirect() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "actual.default.default.dc1": newTarget("actual", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "actual.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "actual"}, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1220,8 +1246,8 @@ func testcase_Resolver_CircularFailover() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "backup.default.default.dc1": newTarget("backup", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "backup.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "backup"}, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1261,8 +1287,11 @@ func testcase_ServiceAndSubsetFailover() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "backup.main.default.default.dc1": newTarget("main", "backup", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "backup.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "backup", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == backup", } @@ -1301,9 +1330,15 @@ func testcase_DatacenterFailover() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), - "main.default.default.dc2": newTarget("main", "", "default", "default", "dc2", nil), - "main.default.default.dc4": newTarget("main", "", "default", "default", "dc4", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), + "main.default.default.dc2": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc2", + }, nil), + "main.default.default.dc4": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc4", + }, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1350,17 +1385,105 @@ func testcase_DatacenterFailover_WithMeshGateways() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, } }), - "main.default.default.dc2": newTarget("main", "", "default", "default", "dc2", func(t *structs.DiscoveryTarget) { + "main.default.default.dc2": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc2", + }, func(t *structs.DiscoveryTarget) { t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, } }), - "main.default.default.dc4": newTarget("main", "", "default", "default", "dc4", func(t *structs.DiscoveryTarget) { + "main.default.default.dc4": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc4", + }, func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), + }, + } + return compileTestCase{entries: entries, expect: expect} +} + +func testcase_Failover_Targets() compileTestCase { + entries := newEntries() + + entries.AddProxyDefaults(&structs.ProxyConfigEntry{ + Kind: structs.ProxyDefaults, + Name: structs.ProxyConfigGlobal, + MeshGateway: structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + }, + }) + + entries.AddResolvers( + &structs.ServiceResolverConfigEntry{ + Kind: "service-resolver", + Name: "main", + Failover: map[string]structs.ServiceResolverFailover{ + "*": { + Targets: []structs.ServiceResolverFailoverTarget{ + {Datacenter: "dc3"}, + {Service: "new-main"}, + {Peer: "cluster-01"}, + }, + }, + }, + }, + ) + + expect := &structs.CompiledDiscoveryChain{ + Protocol: "tcp", + StartNode: "resolver:main.default.default.dc1", + Nodes: map[string]*structs.DiscoveryGraphNode{ + "resolver:main.default.default.dc1": { + Type: structs.DiscoveryGraphNodeTypeResolver, + Name: "main.default.default.dc1", + Resolver: &structs.DiscoveryResolver{ + ConnectTimeout: 5 * time.Second, + Target: "main.default.default.dc1", + Failover: &structs.DiscoveryFailover{ + Targets: []string{ + "main.default.default.dc3", + "new-main.default.default.dc1", + "main.default.default.external.cluster-01", + }, + }, + }, + }, + }, + Targets: map[string]*structs.DiscoveryTarget{ + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), + "main.default.default.dc3": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc3", + }, func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), + "new-main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "new-main"}, func(t *structs.DiscoveryTarget) { + t.MeshGateway = structs.MeshGatewayConfig{ + Mode: structs.MeshGatewayModeRemote, + } + }), + "main.default.default.external.cluster-01": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Peer: "cluster-01", + }, func(t *structs.DiscoveryTarget) { + t.SNI = "" + t.Name = "" + t.Datacenter = "" t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, } @@ -1422,7 +1545,10 @@ func testcase_NoopSplit_WithDefaultSubset() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "v2.main.default.default.dc1": newTarget("main", "v2", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v2.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v2", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", } @@ -1452,7 +1578,7 @@ func testcase_DefaultResolver() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ // TODO-TARGET - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } return compileTestCase{entries: entries, expect: expect} @@ -1488,7 +1614,7 @@ func testcase_DefaultResolver_WithProxyDefaults() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { t.MeshGateway = structs.MeshGatewayConfig{ Mode: structs.MeshGatewayModeRemote, } @@ -1530,7 +1656,7 @@ func testcase_ServiceMetaProjection() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -1588,7 +1714,7 @@ func testcase_ServiceMetaProjectionWithRedirect() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "other.default.default.dc1": newTarget("other", "", "default", "default", "dc1", nil), + "other.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "other"}, nil), }, } @@ -1623,7 +1749,7 @@ func testcase_RedirectToDefaultResolverIsNotDefaultChain() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "other.default.default.dc1": newTarget("other", "", "default", "default", "dc1", nil), + "other.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "other"}, nil), }, } @@ -1658,7 +1784,10 @@ func testcase_Resolve_WithDefaultSubset() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "v2.main.default.default.dc1": newTarget("main", "v2", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v2.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v2", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", } @@ -1692,7 +1821,7 @@ func testcase_DefaultResolver_ExternalSNI() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, func(t *structs.DiscoveryTarget) { t.SNI = "main.some.other.service.mesh" t.External = true }), @@ -1857,11 +1986,17 @@ func testcase_MultiDatacenterCanary() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ "main.default.default.dc2": targetWithConnectTimeout( - newTarget("main", "", "default", "default", "dc2", nil), + newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc2", + }, nil), 33*time.Second, ), "main.default.default.dc3": targetWithConnectTimeout( - newTarget("main", "", "default", "default", "dc3", nil), + newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + Datacenter: "dc3", + }, nil), 33*time.Second, ), }, @@ -2155,27 +2290,42 @@ func testcase_AllBellsAndWhistles() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "prod.redirected.default.default.dc1": newTarget("redirected", "prod", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "prod.redirected.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "redirected", + ServiceSubset: "prod", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "ServiceMeta.env == prod", } }), - "v1.main.default.default.dc1": newTarget("main", "v1", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v1.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v1", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 1", } }), - "v2.main.default.default.dc1": newTarget("main", "v2", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v2.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v2", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 2", } }), - "v3.main.default.default.dc1": newTarget("main", "v3", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "v3.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "v3", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{ Filter: "Service.Meta.version == 3", } }), - "default-subset.main.default.default.dc1": newTarget("main", "default-subset", "default", "default", "dc1", func(t *structs.DiscoveryTarget) { + "default-subset.main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{ + Service: "main", + ServiceSubset: "default-subset", + }, func(t *structs.DiscoveryTarget) { t.Subset = structs.ServiceResolverSubset{OnlyPassing: true} }), }, @@ -2379,7 +2529,7 @@ func testcase_ResolverProtocolOverride() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ // TODO-TARGET - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } return compileTestCase{entries: entries, expect: expect, @@ -2413,7 +2563,7 @@ func testcase_ResolverProtocolOverrideIgnored() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ // TODO-TARGET - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } return compileTestCase{entries: entries, expect: expect, @@ -2451,7 +2601,7 @@ func testcase_RouterIgnored_ResolverProtocolOverride() compileTestCase { }, Targets: map[string]*structs.DiscoveryTarget{ // TODO-TARGET - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } return compileTestCase{entries: entries, expect: expect, @@ -2685,9 +2835,9 @@ func testcase_LBSplitterAndResolver() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "foo.default.default.dc1": newTarget("foo", "", "default", "default", "dc1", nil), - "bar.default.default.dc1": newTarget("bar", "", "default", "default", "dc1", nil), - "baz.default.default.dc1": newTarget("baz", "", "default", "default", "dc1", nil), + "foo.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "foo"}, nil), + "bar.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "bar"}, nil), + "baz.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "baz"}, nil), }, } @@ -2743,7 +2893,7 @@ func testcase_LBResolver() compileTestCase { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "main.default.default.dc1": newTarget("main", "", "default", "default", "dc1", nil), + "main.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "main"}, nil), }, } @@ -2791,8 +2941,17 @@ func newEntries() *configentry.DiscoveryChainSet { } } -func newTarget(service, serviceSubset, namespace, partition, datacenter string, modFn func(t *structs.DiscoveryTarget)) *structs.DiscoveryTarget { - t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, partition, datacenter) +func newTarget(opts structs.DiscoveryTargetOpts, modFn func(t *structs.DiscoveryTarget)) *structs.DiscoveryTarget { + if opts.Namespace == "" { + opts.Namespace = "default" + } + if opts.Partition == "" { + opts.Partition = "default" + } + if opts.Datacenter == "" { + opts.Datacenter = "dc1" + } + t := structs.NewDiscoveryTarget(opts) t.SNI = connect.TargetSNI(t, "trustdomain.consul") t.Name = t.SNI t.ConnectTimeout = 5 * time.Second // default diff --git a/agent/consul/state/peering_test.go b/agent/consul/state/peering_test.go index b48e4f80d..bfce75295 100644 --- a/agent/consul/state/peering_test.go +++ b/agent/consul/state/peering_test.go @@ -1461,7 +1461,13 @@ func TestStateStore_ExportedServicesForPeer(t *testing.T) { } newTarget := func(service, serviceSubset, datacenter string) *structs.DiscoveryTarget { - t := structs.NewDiscoveryTarget(service, serviceSubset, "default", "default", datacenter) + t := structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: service, + ServiceSubset: serviceSubset, + Partition: "default", + Namespace: "default", + Datacenter: datacenter, + }) t.SNI = connect.TargetSNI(t, connect.TestTrustDomain) t.Name = t.SNI t.ConnectTimeout = 5 * time.Second // default diff --git a/agent/discovery_chain_endpoint_test.go b/agent/discovery_chain_endpoint_test.go index 8b4a7e272..42c082591 100644 --- a/agent/discovery_chain_endpoint_test.go +++ b/agent/discovery_chain_endpoint_test.go @@ -27,8 +27,17 @@ func TestDiscoveryChainRead(t *testing.T) { defer a.Shutdown() testrpc.WaitForTestAgent(t, a.RPC, "dc1") - newTarget := func(service, serviceSubset, namespace, partition, datacenter string) *structs.DiscoveryTarget { - t := structs.NewDiscoveryTarget(service, serviceSubset, namespace, partition, datacenter) + newTarget := func(opts structs.DiscoveryTargetOpts) *structs.DiscoveryTarget { + if opts.Namespace == "" { + opts.Namespace = "default" + } + if opts.Partition == "" { + opts.Partition = "default" + } + if opts.Datacenter == "" { + opts.Datacenter = "dc1" + } + t := structs.NewDiscoveryTarget(opts) t.SNI = connect.TargetSNI(t, connect.TestClusterID+".consul") t.Name = t.SNI t.ConnectTimeout = 5 * time.Second // default @@ -99,7 +108,7 @@ func TestDiscoveryChainRead(t *testing.T) { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "web.default.default.dc1": newTarget("web", "", "default", "default", "dc1"), + "web.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "web"}), }, } require.Equal(t, expect, value.Chain) @@ -144,7 +153,7 @@ func TestDiscoveryChainRead(t *testing.T) { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "web.default.default.dc2": newTarget("web", "", "default", "default", "dc2"), + "web.default.default.dc2": newTarget(structs.DiscoveryTargetOpts{Service: "web", Datacenter: "dc2"}), }, } require.Equal(t, expect, value.Chain) @@ -198,7 +207,7 @@ func TestDiscoveryChainRead(t *testing.T) { }, }, Targets: map[string]*structs.DiscoveryTarget{ - "web.default.default.dc1": newTarget("web", "", "default", "default", "dc1"), + "web.default.default.dc1": newTarget(structs.DiscoveryTargetOpts{Service: "web"}), }, } require.Equal(t, expect, value.Chain) @@ -264,11 +273,11 @@ func TestDiscoveryChainRead(t *testing.T) { }, Targets: map[string]*structs.DiscoveryTarget{ "web.default.default.dc1": targetWithConnectTimeout( - newTarget("web", "", "default", "default", "dc1"), + newTarget(structs.DiscoveryTargetOpts{Service: "web"}), 33*time.Second, ), "web.default.default.dc2": targetWithConnectTimeout( - newTarget("web", "", "default", "default", "dc2"), + newTarget(structs.DiscoveryTargetOpts{Service: "web", Datacenter: "dc2"}), 33*time.Second, ), }, @@ -280,7 +289,7 @@ func TestDiscoveryChainRead(t *testing.T) { })) expectTarget_DC1 := targetWithConnectTimeout( - newTarget("web", "", "default", "default", "dc1"), + newTarget(structs.DiscoveryTargetOpts{Service: "web"}), 22*time.Second, ) expectTarget_DC1.MeshGateway = structs.MeshGatewayConfig{ @@ -288,7 +297,7 @@ func TestDiscoveryChainRead(t *testing.T) { } expectTarget_DC2 := targetWithConnectTimeout( - newTarget("web", "", "default", "default", "dc2"), + newTarget(structs.DiscoveryTargetOpts{Service: "web", Datacenter: "dc2"}), 22*time.Second, ) expectTarget_DC2.MeshGateway = structs.MeshGatewayConfig{ diff --git a/agent/proxycfg/naming.go b/agent/proxycfg/naming.go index 3bb0854b0..08ff216ed 100644 --- a/agent/proxycfg/naming.go +++ b/agent/proxycfg/naming.go @@ -63,22 +63,29 @@ func NewUpstreamIDFromServiceID(sid structs.ServiceID) UpstreamID { return id } -// TODO(peering): confirm we don't need peername here func NewUpstreamIDFromTargetID(tid string) UpstreamID { - // Drop the leading subset if one is present in the target ID. - separators := strings.Count(tid, ".") - if separators > 3 { - prefix := tid[:strings.Index(tid, ".")+1] - tid = strings.TrimPrefix(tid, prefix) + var id UpstreamID + split := strings.Split(tid, ".") + + switch { + case split[len(split)-2] == "external": + id = UpstreamID{ + Name: split[0], + EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(split[2], split[1]), + Peer: split[4], + } + case len(split) == 5: + // Drop the leading subset if one is present in the target ID. + split = split[1:] + fallthrough + default: + id = UpstreamID{ + Name: split[0], + EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(split[2], split[1]), + Datacenter: split[3], + } } - split := strings.SplitN(tid, ".", 4) - - id := UpstreamID{ - Name: split[0], - EnterpriseMeta: acl.NewEnterpriseMetaWithPartition(split[2], split[1]), - Datacenter: split[3], - } id.normalize() return id } diff --git a/agent/proxycfg/naming_test.go b/agent/proxycfg/naming_test.go index 23ff24165..2c4f5173a 100644 --- a/agent/proxycfg/naming_test.go +++ b/agent/proxycfg/naming_test.go @@ -35,6 +35,13 @@ func TestUpstreamIDFromTargetID(t *testing.T) { Datacenter: "dc2", }, }, + "peered": { + tid: "foo.default.default.external.cluster-01", + expect: UpstreamID{ + Name: "foo", + Peer: "cluster-01", + }, + }, } for name, tc := range cases { diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go index 8bc0305b0..0ea260955 100644 --- a/agent/structs/config_entry_discoverychain.go +++ b/agent/structs/config_entry_discoverychain.go @@ -1233,6 +1233,16 @@ type ServiceResolverRedirect struct { Datacenter string `json:",omitempty"` } +func (r *ServiceResolverRedirect) ToDiscoveryTargetOpts() DiscoveryTargetOpts { + return DiscoveryTargetOpts{ + Service: r.Service, + ServiceSubset: r.ServiceSubset, + Namespace: r.Namespace, + Partition: r.Partition, + Datacenter: r.Datacenter, + } +} + // There are some restrictions on what is allowed in here: // // - Service, ServiceSubset, Namespace, Datacenters, and Targets cannot all be @@ -1275,6 +1285,14 @@ type ServiceResolverFailover struct { Targets []ServiceResolverFailoverTarget `json:",omitempty"` } +func (t *ServiceResolverFailover) ToDiscoveryTargetOpts() DiscoveryTargetOpts { + return DiscoveryTargetOpts{ + Service: t.Service, + ServiceSubset: t.ServiceSubset, + Namespace: t.Namespace, + } +} + func (f *ServiceResolverFailover) isEmpty() bool { return f.Service == "" && f.ServiceSubset == "" && f.Namespace == "" && len(f.Datacenters) == 0 && len(f.Targets) == 0 } @@ -1299,6 +1317,17 @@ type ServiceResolverFailoverTarget struct { Peer string `json:",omitempty"` } +func (t *ServiceResolverFailoverTarget) ToDiscoveryTargetOpts() DiscoveryTargetOpts { + return DiscoveryTargetOpts{ + Service: t.Service, + ServiceSubset: t.ServiceSubset, + Namespace: t.Namespace, + Partition: t.Partition, + Datacenter: t.Datacenter, + Peer: t.Peer, + } +} + // LoadBalancer determines the load balancing policy and configuration for services // issuing requests to this upstream service. type LoadBalancer struct { diff --git a/agent/structs/discovery_chain.go b/agent/structs/discovery_chain.go index 2bbe88f9e..ca64d070d 100644 --- a/agent/structs/discovery_chain.go +++ b/agent/structs/discovery_chain.go @@ -56,7 +56,12 @@ type CompiledDiscoveryChain struct { // ID returns an ID that encodes the service, namespace, partition, and datacenter. // This ID allows us to compare a discovery chain target to the chain upstream itself. func (c *CompiledDiscoveryChain) ID() string { - return chainID("", c.ServiceName, c.Namespace, c.Partition, c.Datacenter) + return chainID(DiscoveryTargetOpts{ + Service: c.ServiceName, + Namespace: c.Namespace, + Partition: c.Partition, + Datacenter: c.Datacenter, + }) } func (c *CompiledDiscoveryChain) CompoundServiceName() ServiceName { @@ -185,6 +190,7 @@ type DiscoveryTarget struct { Namespace string `json:",omitempty"` Partition string `json:",omitempty"` Datacenter string `json:",omitempty"` + Peer string `json:",omitempty"` MeshGateway MeshGatewayConfig `json:",omitempty"` Subset ServiceResolverSubset `json:",omitempty"` @@ -240,28 +246,52 @@ func (t *DiscoveryTarget) UnmarshalJSON(data []byte) error { return nil } -func NewDiscoveryTarget(service, serviceSubset, namespace, partition, datacenter string) *DiscoveryTarget { +type DiscoveryTargetOpts struct { + Service string + ServiceSubset string + Namespace string + Partition string + Datacenter string + Peer string +} + +func NewDiscoveryTarget(opts DiscoveryTargetOpts) *DiscoveryTarget { t := &DiscoveryTarget{ - Service: service, - ServiceSubset: serviceSubset, - Namespace: namespace, - Partition: partition, - Datacenter: datacenter, + Service: opts.Service, + ServiceSubset: opts.ServiceSubset, + Namespace: opts.Namespace, + Partition: opts.Partition, + Datacenter: opts.Datacenter, + Peer: opts.Peer, } t.setID() return t } -func chainID(subset, service, namespace, partition, dc string) string { - // NOTE: this format is similar to the SNI syntax for simplicity - if subset == "" { - return fmt.Sprintf("%s.%s.%s.%s", service, namespace, partition, dc) +func (t *DiscoveryTarget) ToDiscoveryTargetOpts() DiscoveryTargetOpts { + return DiscoveryTargetOpts{ + Service: t.Service, + ServiceSubset: t.ServiceSubset, + Namespace: t.Namespace, + Partition: t.Partition, + Datacenter: t.Datacenter, + Peer: t.Peer, } - return fmt.Sprintf("%s.%s.%s.%s.%s", subset, service, namespace, partition, dc) +} + +func chainID(opts DiscoveryTargetOpts) string { + // NOTE: this format is similar to the SNI syntax for simplicity + if opts.Peer != "" { + return fmt.Sprintf("%s.%s.default.external.%s", opts.Service, opts.Namespace, opts.Peer) + } + if opts.ServiceSubset == "" { + return fmt.Sprintf("%s.%s.%s.%s", opts.Service, opts.Namespace, opts.Partition, opts.Datacenter) + } + return fmt.Sprintf("%s.%s.%s.%s.%s", opts.ServiceSubset, opts.Service, opts.Namespace, opts.Partition, opts.Datacenter) } func (t *DiscoveryTarget) setID() { - t.ID = chainID(t.ServiceSubset, t.Service, t.Namespace, t.Partition, t.Datacenter) + t.ID = chainID(t.ToDiscoveryTargetOpts()) } func (t *DiscoveryTarget) String() string { diff --git a/agent/xds/failover_math_test.go b/agent/xds/failover_math_test.go index 29ac17ffe..296d1cc77 100644 --- a/agent/xds/failover_math_test.go +++ b/agent/xds/failover_math_test.go @@ -15,15 +15,40 @@ func TestFirstHealthyTarget(t *testing.T) { warning := proxycfg.TestUpstreamNodesInStatus(t, "warning") critical := proxycfg.TestUpstreamNodesInStatus(t, "critical") - warnOnlyPassingTarget := structs.NewDiscoveryTarget("all-warn", "", "default", "default", "dc1") + warnOnlyPassingTarget := structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "all-warn", + Namespace: "default", + Partition: "default", + Datacenter: "dc1", + }) warnOnlyPassingTarget.Subset.OnlyPassing = true - failOnlyPassingTarget := structs.NewDiscoveryTarget("all-fail", "", "default", "default", "dc1") + failOnlyPassingTarget := structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "all-fail", + Namespace: "default", + Partition: "default", + Datacenter: "dc1", + }) failOnlyPassingTarget.Subset.OnlyPassing = true targets := map[string]*structs.DiscoveryTarget{ - "all-ok.default.dc1": structs.NewDiscoveryTarget("all-ok", "", "default", "default", "dc1"), - "all-warn.default.dc1": structs.NewDiscoveryTarget("all-warn", "", "default", "default", "dc1"), - "all-fail.default.default.dc1": structs.NewDiscoveryTarget("all-fail", "", "default", "default", "dc1"), + "all-ok.default.dc1": structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "all-ok", + Namespace: "default", + Partition: "default", + Datacenter: "dc1", + }), + "all-warn.default.dc1": structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "all-warn", + Namespace: "default", + Partition: "default", + Datacenter: "dc1", + }), + "all-fail.default.default.dc1": structs.NewDiscoveryTarget(structs.DiscoveryTargetOpts{ + Service: "all-fail", + Namespace: "default", + Partition: "default", + Datacenter: "dc1", + }), "all-warn-onlypassing.default.dc1": warnOnlyPassingTarget, "all-fail-onlypassing.default.dc1": failOnlyPassingTarget, }