Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connect: introduce ExternalSNI field on service-defaults #6324

Merged
merged 4 commits into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions agent/config/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2783,6 +2783,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
"kind": "service-defaults",
"name": "web",
"protocol": "http",
"external_sni": "abc-123",
"mesh_gateway": {
"mode": "remote"
}
Expand All @@ -2796,6 +2797,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
kind = "service-defaults"
name = "web"
protocol = "http"
external_sni = "abc-123"
mesh_gateway {
mode = "remote"
}
Expand All @@ -2805,9 +2807,10 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
rt.DataDir = dataDir
rt.ConfigEntryBootstrap = []structs.ConfigEntry{
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "web",
Protocol: "http",
Kind: structs.ServiceDefaults,
Name: "web",
Protocol: "http",
ExternalSNI: "abc-123",
MeshGateway: structs.MeshGatewayConfig{
Mode: structs.MeshGatewayModeRemote,
},
Expand All @@ -2825,6 +2828,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
"Kind": "service-defaults",
"Name": "web",
"Protocol": "http",
"ExternalSNI": "abc-123",
"MeshGateway": {
"Mode": "remote"
}
Expand All @@ -2838,6 +2842,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
Kind = "service-defaults"
Name = "web"
Protocol = "http"
ExternalSNI = "abc-123"
MeshGateway {
Mode = "remote"
}
Expand All @@ -2847,9 +2852,10 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
rt.DataDir = dataDir
rt.ConfigEntryBootstrap = []structs.ConfigEntry{
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "web",
Protocol: "http",
Kind: structs.ServiceDefaults,
Name: "web",
Protocol: "http",
ExternalSNI: "abc-123",
MeshGateway: structs.MeshGatewayConfig{
Mode: structs.MeshGatewayModeRemote,
},
Expand Down
39 changes: 39 additions & 0 deletions agent/consul/discoverychain/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,9 +813,48 @@ RESOLVE_AGAIN:

target.Subset = resolver.Subsets[target.ServiceSubset]

if serviceDefault := c.entries.GetService(target.Service); serviceDefault != nil && serviceDefault.ExternalSNI != "" {
// Explicitly set the SNI value.
target.SNI = serviceDefault.ExternalSNI
target.External = true
}

// If using external SNI the service is fundamentally external.
if target.External {
if resolver.Redirect != nil {
return nil, &structs.ConfigEntryGraphError{
Message: fmt.Sprintf(
"service %q has an external SNI set; cannot define redirects for external services",
target.Service,
),
}
}
if len(resolver.Subsets) > 0 {
return nil, &structs.ConfigEntryGraphError{
Message: fmt.Sprintf(
"service %q has an external SNI set; cannot define subsets for external services",
target.Service,
),
}
}
if len(resolver.Failover) > 0 {
return nil, &structs.ConfigEntryGraphError{
Message: fmt.Sprintf(
"service %q has an external SNI set; cannot define failover for external services",
target.Service,
),
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, these help make the external thing less confusing when trying to configure features that won't work any more.

}

// TODO (mesh-gateway)- maybe allow using a gateway within a datacenter at some point
if target.Datacenter == c.useInDatacenter {
target.MeshGateway.Mode = structs.MeshGatewayModeDefault

} else if target.External {
// Bypass mesh gateways if it is an external service.
target.MeshGateway.Mode = structs.MeshGatewayModeDefault

} else {
// Default mesh gateway settings
if serviceDefault := c.entries.GetService(target.Service); serviceDefault != nil {
Expand Down
121 changes: 114 additions & 7 deletions agent/consul/discoverychain/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func TestCompile(t *testing.T) {
"datacenter failover with mesh gateways": testcase_DatacenterFailover_WithMeshGateways(),
"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(),
"resolver with no entries and inferring defaults": testcase_DefaultResolver(),
"default resolver with proxy defaults": testcase_DefaultResolver_WithProxyDefaults(),
"service redirect to service with default resolver is not a default chain": testcase_RedirectToDefaultResolverIsNotDefaultChain(),
Expand All @@ -54,13 +55,16 @@ func TestCompile(t *testing.T) {
"multi dc canary": testcase_MultiDatacenterCanary(),

// various errors
"splitter requires valid protocol": testcase_SplitterRequiresValidProtocol(),
"router requires valid protocol": testcase_RouterRequiresValidProtocol(),
"split to unsplittable protocol": testcase_SplitToUnsplittableProtocol(),
"route to unroutable protocol": testcase_RouteToUnroutableProtocol(),
"failover crosses protocols": testcase_FailoverCrossesProtocols(),
"redirect crosses protocols": testcase_RedirectCrossesProtocols(),
"redirect to missing subset": testcase_RedirectToMissingSubset(),
"splitter requires valid protocol": testcase_SplitterRequiresValidProtocol(),
"router requires valid protocol": testcase_RouterRequiresValidProtocol(),
"split to unsplittable protocol": testcase_SplitToUnsplittableProtocol(),
"route to unroutable protocol": testcase_RouteToUnroutableProtocol(),
"failover crosses protocols": testcase_FailoverCrossesProtocols(),
"redirect crosses protocols": testcase_RedirectCrossesProtocols(),
"redirect to missing subset": testcase_RedirectToMissingSubset(),
"resolver with failover and external sni": testcase_Resolver_ExternalSNI_FailoverNotAllowed(),
"resolver with subsets and external sni": testcase_Resolver_ExternalSNI_SubsetsNotAllowed(),
"resolver with redirect and external sni": testcase_Resolver_ExternalSNI_RedirectNotAllowed(),

// overrides
"resolver with protocol from override": testcase_ResolverProtocolOverride(),
Expand Down Expand Up @@ -1435,6 +1439,109 @@ func testcase_Resolve_WithDefaultSubset() compileTestCase {
return compileTestCase{entries: entries, expect: expect}
}

func testcase_DefaultResolver_ExternalSNI() compileTestCase {
entries := newEntries()
entries.AddServices(&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
ExternalSNI: "main.some.other.service.mesh",
})

expect := &structs.CompiledDiscoveryChain{
Protocol: "tcp",
StartNode: "resolver:main.default.dc1",
Nodes: map[string]*structs.DiscoveryGraphNode{
"resolver:main.default.dc1": &structs.DiscoveryGraphNode{
Type: structs.DiscoveryGraphNodeTypeResolver,
Name: "main.default.dc1",
Resolver: &structs.DiscoveryResolver{
Default: true,
ConnectTimeout: 5 * time.Second,
Target: "main.default.dc1",
},
},
},
Targets: map[string]*structs.DiscoveryTarget{
"main.default.dc1": newTarget("main", "", "default", "dc1", func(t *structs.DiscoveryTarget) {
t.SNI = "main.some.other.service.mesh"
t.External = true
}),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: true}
}

func testcase_Resolver_ExternalSNI_FailoverNotAllowed() compileTestCase {
entries := newEntries()
entries.AddServices(&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
ExternalSNI: "main.some.other.service.mesh",
})
entries.AddResolvers(&structs.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
ConnectTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {Service: "backup"},
},
})

return compileTestCase{
entries: entries,
expectErr: `service "main" has an external SNI set; cannot define failover for external services`,
expectGraphErr: true,
}
}

func testcase_Resolver_ExternalSNI_SubsetsNotAllowed() compileTestCase {
entries := newEntries()
entries.AddServices(&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
ExternalSNI: "main.some.other.service.mesh",
})
entries.AddResolvers(&structs.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
ConnectTimeout: 33 * time.Second,
Subsets: map[string]structs.ServiceResolverSubset{
"canary": {
Filter: "Service.Meta.version == canary",
},
},
})

return compileTestCase{
entries: entries,
expectErr: `service "main" has an external SNI set; cannot define subsets for external services`,
expectGraphErr: true,
}
}

func testcase_Resolver_ExternalSNI_RedirectNotAllowed() compileTestCase {
entries := newEntries()
entries.AddServices(&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "main",
ExternalSNI: "main.some.other.service.mesh",
})
entries.AddResolvers(&structs.ServiceResolverConfigEntry{
Kind: "service-resolver",
Name: "main",
ConnectTimeout: 33 * time.Second,
Redirect: &structs.ServiceResolverRedirect{
Datacenter: "dc2",
},
})

return compileTestCase{
entries: entries,
expectErr: `service "main" has an external SNI set; cannot define redirects for external services`,
expectGraphErr: true,
}
}

func testcase_MultiDatacenterCanary() compileTestCase {
entries := newEntries()
setServiceProtocol(entries, "main", "http")
Expand Down
18 changes: 18 additions & 0 deletions agent/proxycfg/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,10 @@ func TestConfigSnapshotDiscoveryChain(t testing.T) *ConfigSnapshot {
return testConfigSnapshotDiscoveryChain(t, "simple")
}

func TestConfigSnapshotDiscoveryChainExternalSNI(t testing.T) *ConfigSnapshot {
return testConfigSnapshotDiscoveryChain(t, "external-sni")
}

func TestConfigSnapshotDiscoveryChainWithOverrides(t testing.T) *ConfigSnapshot {
return testConfigSnapshotDiscoveryChain(t, "simple-with-overrides")
}
Expand Down Expand Up @@ -664,6 +668,19 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE
ConnectTimeout: 33 * time.Second,
},
)
case "external-sni":
entries = append(entries,
&structs.ServiceConfigEntry{
Kind: structs.ServiceDefaults,
Name: "db",
ExternalSNI: "db.some.other.service.mesh",
},
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
},
)
case "failover":
entries = append(entries,
&structs.ServiceResolverConfigEntry{
Expand Down Expand Up @@ -858,6 +875,7 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE
switch variation {
case "simple-with-overrides":
case "simple":
case "external-sni":
case "failover":
snap.ConnectProxy.WatchedUpstreamEndpoints["db"]["fail.default.dc1"] =
TestUpstreamNodesAlternate(t)
Expand Down
3 changes: 3 additions & 0 deletions agent/structs/config_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ type ServiceConfigEntry struct {
Protocol string
MeshGateway MeshGatewayConfig `json:",omitempty"`

ExternalSNI string `json:",omitempty"`

// TODO(banks): enable this once we have upstreams supported too. Enabling
// sidecars actually makes no sense and adds complications when you don't
// allow upstreams to be specified centrally too.
Expand Down Expand Up @@ -306,6 +308,7 @@ func ConfigEntryDecodeRulesForKind(kind string) (skipWhenPatching []string, tran
case ServiceDefaults:
return nil, map[string]string{
"mesh_gateway": "meshgateway",
"external_sni": "externalsni",
}, nil
case ServiceRouter:
return []string{
Expand Down
9 changes: 6 additions & 3 deletions agent/structs/config_entry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func TestDecodeConfigEntry(t *testing.T) {
kind = "service-defaults"
name = "main"
protocol = "http"
external_sni = "abc-123"
mesh_gateway {
mode = "remote"
}
Expand All @@ -101,14 +102,16 @@ func TestDecodeConfigEntry(t *testing.T) {
Kind = "service-defaults"
Name = "main"
Protocol = "http"
ExternalSNI = "abc-123"
MeshGateway {
Mode = "remote"
}
`,
expect: &ServiceConfigEntry{
Kind: "service-defaults",
Name: "main",
Protocol: "http",
Kind: "service-defaults",
Name: "main",
Protocol: "http",
ExternalSNI: "abc-123",
MeshGateway: MeshGatewayConfig{
Mode: MeshGatewayModeRemote,
},
Expand Down
5 changes: 5 additions & 0 deletions agent/structs/discovery_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ type DiscoveryTarget struct {

MeshGateway MeshGatewayConfig `json:",omitempty"`
Subset ServiceResolverSubset `json:",omitempty"`

// SNI if set is the sni field to use when addressing this set of
// endpoints. If not configured then the Connect default should be used.
SNI string `json:",omitempty"`
External bool `json:",omitempty"`
}

func NewDiscoveryTarget(service, serviceSubset, namespace, datacenter string) *DiscoveryTarget {
Expand Down
7 changes: 6 additions & 1 deletion agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,15 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
c.Http2ProtocolOptions = &envoycore.Http2ProtocolOptions{}
}

finalSNI := sni
if target.SNI != "" {
finalSNI = target.SNI
}

// Enable TLS upstream with the configured client certificate.
c.TlsContext = &envoyauth.UpstreamTlsContext{
CommonTlsContext: makeCommonTLSContext(cfgSnap),
Sni: sni,
Sni: finalSNI,
}

out = append(out, c)
Expand Down
5 changes: 5 additions & 0 deletions agent/xds/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ func TestClustersFromSnapshot(t *testing.T) {
create: proxycfg.TestConfigSnapshotDiscoveryChain,
setup: nil,
},
{
name: "connect-proxy-with-chain-external-sni",
create: proxycfg.TestConfigSnapshotDiscoveryChainExternalSNI,
setup: nil,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: proxycfg.TestConfigSnapshotDiscoveryChainWithOverrides,
Expand Down
5 changes: 5 additions & 0 deletions agent/xds/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,11 @@ func Test_endpointsFromSnapshot(t *testing.T) {
create: proxycfg.TestConfigSnapshotDiscoveryChain,
setup: nil,
},
{
name: "connect-proxy-with-chain-external-sni",
create: proxycfg.TestConfigSnapshotDiscoveryChainExternalSNI,
setup: nil,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: proxycfg.TestConfigSnapshotDiscoveryChainWithOverrides,
Expand Down
5 changes: 5 additions & 0 deletions agent/xds/listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ func TestListenersFromSnapshot(t *testing.T) {
},
setup: nil,
},
{
name: "connect-proxy-with-chain-external-sni",
create: proxycfg.TestConfigSnapshotDiscoveryChainExternalSNI,
setup: nil,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: proxycfg.TestConfigSnapshotDiscoveryChainWithOverrides,
Expand Down
Loading