Skip to content

Commit

Permalink
connect: introduce ExternalSNI field on service-defaults
Browse files Browse the repository at this point in the history
Compiling this will set an optional SNI field on each DiscoveryTarget.
When set this value should be used for TLS connections to the instances
of the target. If not set the default should be used.

Setting ExternalSNI will disable mesh gateway use for that target.
  • Loading branch information
rboyer committed Aug 14, 2019
1 parent acac627 commit c69055c
Show file tree
Hide file tree
Showing 20 changed files with 431 additions and 16 deletions.
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
12 changes: 12 additions & 0 deletions agent/consul/discoverychain/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -813,9 +813,21 @@ RESOLVE_AGAIN:

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

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

// 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 usingExternalSNI {
// Bypass mesh gateways if external SNI is configured.
target.MeshGateway.Mode = structs.MeshGatewayModeDefault

} else {
// Default mesh gateway settings
if serviceDefault := c.entries.GetService(target.Service); serviceDefault != nil {
Expand Down
32 changes: 32 additions & 0 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 Down Expand Up @@ -1435,6 +1436,37 @@ 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"
}),
},
}
return compileTestCase{entries: entries, expect: expect, expectIsDefault: 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
4 changes: 4 additions & 0 deletions agent/structs/discovery_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ 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 default should be used.
SNI string `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
5 changes: 5 additions & 0 deletions agent/xds/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ func TestRoutesFromSnapshot(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
Loading

0 comments on commit c69055c

Please sign in to comment.