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.

Sign up for GitHub

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

Fix cluster escape hatch #6378

Merged
merged 6 commits into from
Aug 22, 2019
Merged

Fix cluster escape hatch #6378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ecbff12
connect: allow 'envoy_cluster_json' escape hatch to continue to function
rboyer Aug 22, 2019
080dc05
foo
rboyer Aug 22, 2019
a69c0b9
differnet fix
rboyer Aug 22, 2019
dc86f42
Add test for default disco chain + escape hatch
mkeeler Aug 22, 2019
ff8752b
update test to make it obvious when the override is applied
rboyer Aug 22, 2019
f58b8ef
get another test to pass
rboyer Aug 22, 2019
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
22 changes: 21 additions & 1 deletion agent/proxycfg/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,12 @@ func TestGatewayServiceGroupFooDC1(t testing.T) structs.CheckServiceNodes {
// TestConfigSnapshot returns a fully populated snapshot
func TestConfigSnapshot(t testing.T) *ConfigSnapshot {
roots, leaf := TestCerts(t)

// no entries implies we'll get a default chain
dbChain := discoverychain.TestCompileConfigEntries(
t, "db", "default", "dc1",
connect.TestClusterID+".consul", "dc1", nil)

return &ConfigSnapshot{
Kind: structs.ServiceKindConnectProxy,
Service: "web-sidecar-proxy",
Expand All @@ -578,10 +584,17 @@ func TestConfigSnapshot(t testing.T) *ConfigSnapshot {
Roots: roots,
ConnectProxy: configSnapshotConnectProxy{
Leaf: leaf,
DiscoveryChain: map[string]*structs.CompiledDiscoveryChain{
"db": dbChain,
},
UpstreamEndpoints: map[string]structs.CheckServiceNodes{
"db": TestUpstreamNodes(t),
"prepared_query:geo-cache": TestUpstreamNodes(t),
},
WatchedUpstreamEndpoints: map[string]map[string]structs.CheckServiceNodes{
"db": map[string]structs.CheckServiceNodes{
"db.default.dc1": TestUpstreamNodes(t),
},
},
},
Datacenter: "dc1",
}
Expand Down Expand Up @@ -644,6 +657,10 @@ func TestConfigSnapshotDiscoveryChainWithEntries(t testing.T, additionalEntries
return testConfigSnapshotDiscoveryChain(t, "simple", additionalEntries...)
}

func TestConfigSnapshotDiscoveryChainDefault(t testing.T) *ConfigSnapshot {
return testConfigSnapshotDiscoveryChain(t, "default")
}

func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalEntries ...structs.ConfigEntry) *ConfigSnapshot {
roots, leaf := TestCerts(t)

Expand All @@ -653,6 +670,8 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE
compileSetup func(req *discoverychain.CompileRequest)
)
switch variation {
case "default":
// no config entries
case "simple-with-overrides":
compileSetup = func(req *discoverychain.CompileRequest) {
req.OverrideMeshGateway.Mode = structs.MeshGatewayModeLocal
Expand Down Expand Up @@ -873,6 +892,7 @@ func testConfigSnapshotDiscoveryChain(t testing.T, variation string, additionalE
}

switch variation {
case "default":
case "simple-with-overrides":
case "simple":
case "external-sni":
Expand Down
48 changes: 33 additions & 15 deletions agent/xds/clusters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,16 @@ func (s *Server) clustersFromSnapshotConnectProxy(cfgSnap *proxycfg.ConfigSnapsh

for _, u := range cfgSnap.Proxy.Upstreams {
id := u.Identifier()
var chain *structs.CompiledDiscoveryChain
if u.DestinationType != structs.UpstreamDestTypePreparedQuery {
chain = cfgSnap.ConnectProxy.DiscoveryChain[id]
}

if chain == nil {
upstreamCluster, err := s.makeUpstreamCluster(u, cfgSnap)
if u.DestinationType == structs.UpstreamDestTypePreparedQuery {
upstreamCluster, err := s.makeUpstreamClusterForPreparedQuery(u, cfgSnap)
if err != nil {
return nil, err
}
clusters = append(clusters, upstreamCluster)

} else {
chain := cfgSnap.ConnectProxy.DiscoveryChain[id]
upstreamClusters, err := s.makeUpstreamClustersForDiscoveryChain(u, chain, cfgSnap)
if err != nil {
return nil, err
Expand Down Expand Up @@ -169,7 +166,7 @@ func (s *Server) makeAppCluster(cfgSnap *proxycfg.ConfigSnapshot) (*envoy.Cluste
return c, err
}

func (s *Server) makeUpstreamCluster(upstream structs.Upstream, cfgSnap *proxycfg.ConfigSnapshot) (*envoy.Cluster, error) {
func (s *Server) makeUpstreamClusterForPreparedQuery(upstream structs.Upstream, cfgSnap *proxycfg.ConfigSnapshot) (*envoy.Cluster, error) {
var c *envoy.Cluster
var err error

Expand Down Expand Up @@ -228,20 +225,31 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
chain *structs.CompiledDiscoveryChain,
cfgSnap *proxycfg.ConfigSnapshot,
) ([]*envoy.Cluster, error) {
if chain == nil {
return nil, fmt.Errorf("cannot create upstream cluster without discovery chain")
}

cfg, err := ParseUpstreamConfigNoDefaults(upstream.Config)
if err != nil {
// Don't hard fail on a config typo, just warn. The parse func returns
// default config if there is an error so it's safe to continue.
s.Logger.Printf("[WARN] envoy: failed to parse Upstream[%s].Config: %s",
upstream.Identifier(), err)
}
if cfg.ClusterJSON != "" {
s.Logger.Printf("[WARN] envoy: ignoring escape hatch setting Upstream[%s].Config[%s] because a discovery chain for %q is configured",
upstream.Identifier(), "envoy_cluster_json", chain.ServiceName)
}

if chain == nil {
panic("chain must be provided")
var escapeHatchCluster *envoy.Cluster
if cfg.ClusterJSON != "" {
if chain.IsDefault() {
// If you haven't done anything to setup the discovery chain, then
// you can use the envoy_cluster_json escape hatch.
escapeHatchCluster, err = makeClusterFromUserConfig(cfg.ClusterJSON)
if err != nil {
return nil, err
}
} else {
s.Logger.Printf("[WARN] envoy: ignoring escape hatch setting Upstream[%s].Config[%s] because a discovery chain for %q is configured",
upstream.Identifier(), "envoy_cluster_json", chain.ServiceName)
}
}

id := upstream.Identifier()
Expand All @@ -251,8 +259,6 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
return nil, fmt.Errorf("no endpoint map for upstream %q", id)
}

// TODO(rb): make escape hatches work with chains

var out []*envoy.Cluster

for _, node := range chain.Nodes {
Expand Down Expand Up @@ -328,6 +334,18 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
out = append(out, c)
}

if escapeHatchCluster != nil {
if len(out) != 1 {
return nil, fmt.Errorf("cannot inject escape hatch cluster when discovery chain had no nodes")
}
defaultCluster := out[0]

// Overlay what the user provided.
escapeHatchCluster.TlsContext = defaultCluster.TlsContext

out = []*envoy.Cluster{escapeHatchCluster}
}

return out, nil
}

Expand Down
19 changes: 17 additions & 2 deletions agent/xds/clusters_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ func TestClustersFromSnapshot(t *testing.T) {
})
},
},
{
name: "custom-upstream-default-chain",
create: proxycfg.TestConfigSnapshotDiscoveryChainDefault,
setup: func(snap *proxycfg.ConfigSnapshot) {
snap.Proxy.Upstreams[0].Config["envoy_cluster_json"] =
customAppClusterJSON(t, customClusterJSONOptions{
Name: "myservice",
IncludeType: false,
})
},
},
{
name: "custom-upstream-typed",
create: proxycfg.TestConfigSnapshot,
Expand Down Expand Up @@ -281,7 +292,11 @@ func expectClustersJSONResources(t *testing.T, snap *proxycfg.ConfigSnapshot, to
"outlierDetection": {

},
"connectTimeout": "1s",
"altStatName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"commonLbConfig": {
"healthyPanicThreshold": {}
},
"connectTimeout": "5s",
"tlsContext": ` + expectedUpstreamTLSContextJSON(t, snap, "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul") + `
}`,
"prepared_query:geo-cache": `
Expand Down Expand Up @@ -381,7 +396,7 @@ var customAppClusterJSONTpl = `{
"tlsContext": {{ .TLSContext }},
{{- end }}
"name": "{{ .Name }}",
"connectTimeout": "5s",
"connectTimeout": "15s",
"hosts": [
{
"socketAddress": {
Expand Down
10 changes: 8 additions & 2 deletions agent/xds/testdata/clusters/custom-local-app.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"altStatName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"type": "EDS",
"edsClusterConfig": {
"edsConfig": {
Expand All @@ -12,7 +13,7 @@
}
}
},
"connectTimeout": "1s",
"connectTimeout": "5s",
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand All @@ -38,6 +39,11 @@
},
"outlierDetection": {

},
"commonLbConfig": {
"healthyPanicThreshold": {

}
}
},
{
Expand Down Expand Up @@ -82,7 +88,7 @@
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "mylocal",
"connectTimeout": "5s",
"connectTimeout": "15s",
"hosts": [
{
"socketAddress": {
Expand Down
8 changes: 7 additions & 1 deletion agent/xds/testdata/clusters/custom-timeouts.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"altStatName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"type": "EDS",
"edsClusterConfig": {
"edsConfig": {
Expand All @@ -12,7 +13,7 @@
}
}
},
"connectTimeout": "2.345s",
"connectTimeout": "5s",
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand All @@ -38,6 +39,11 @@
},
"outlierDetection": {

},
"commonLbConfig": {
"healthyPanicThreshold": {

}
}
},
{
Expand Down
107 changes: 107 additions & 0 deletions agent/xds/testdata/clusters/custom-upstream-default-chain.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
{
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul",
"type": "EDS",
"edsClusterConfig": {
"edsConfig": {
"ads": {

}
}
},
"connectTimeout": "5s",
"tlsContext": {
"commonTlsContext": {
"tlsParams": {

},
"tlsCertificates": [
{
"certificateChain": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"
},
"privateKey": {
"inlineString": "-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"
}
}
],
"validationContext": {
"trustedCa": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"
}
}
},
"sni": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul"
},
"outlierDetection": {

}
},
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "local_app",
"type": "STATIC",
"connectTimeout": "5s",
"loadAssignment": {
"clusterName": "local_app",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "127.0.0.1",
"portValue": 8080
}
}
}
}
]
}
]
}
},
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "myservice",
"connectTimeout": "15s",
"hosts": [
{
"socketAddress": {
"address": "127.0.0.1",
"portValue": 8080
}
}
],
"tlsContext": {
"commonTlsContext": {
"tlsParams": {

},
"tlsCertificates": [
{
"certificateChain": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG\nA1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR\nAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7\nSkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD\nAgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6\nNDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6\nNWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf\nZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6\nZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw\nWQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1\nNTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG\nSM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA\npY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g=\n-----END CERTIFICATE-----\n"
},
"privateKey": {
"inlineString": "-----BEGIN EC PRIVATE KEY-----\nMHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49\nAwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav\nq5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ==\n-----END EC PRIVATE KEY-----\n"
}
}
],
"validationContext": {
"trustedCa": {
"inlineString": "-----BEGIN CERTIFICATE-----\nMIICXDCCAgKgAwIBAgIICpZq70Z9LyUwCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ\nVGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowFDESMBAG\nA1UEAxMJVGVzdCBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEIhywH1gx\nAsMwuF3ukAI5YL2jFxH6Usnma1HFSfVyxbXX1/uoZEYrj8yCAtdU2yoHETyd+Zx2\nThhRLP79pYegCaOCATwwggE4MA4GA1UdDwEB/wQEAwIBhjAPBgNVHRMBAf8EBTAD\nAQH/MGgGA1UdDgRhBF9kMToxMToxMTphYzoyYTpiYTo5NzpiMjozZjphYzo3Yjpi\nZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1ZTo0MTo2ZjpmMjo3\nMzo5NTo1ODowYzpkYjBqBgNVHSMEYzBhgF9kMToxMToxMTphYzoyYTpiYTo5Nzpi\nMjozZjphYzo3YjpiZDpkYTpiZTpiMTo4YTpmYzo5YTpiYTpiNTpiYzo4MzplNzo1\nZTo0MTo2ZjpmMjo3Mzo5NTo1ODowYzpkYjA/BgNVHREEODA2hjRzcGlmZmU6Ly8x\nMTExMTExMS0yMjIyLTMzMzMtNDQ0NC01NTU1NTU1NTU1NTUuY29uc3VsMAoGCCqG\nSM49BAMCA0gAMEUCICOY0i246rQHJt8o8Oya0D5PLL1FnmsQmQqIGCi31RwnAiEA\noR5f6Ku+cig2Il8T8LJujOp2/2A72QcHZA57B13y+8o=\n-----END CERTIFICATE-----\n"
}
}
},
"sni": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
}
}
],
"typeUrl": "type.googleapis.com/envoy.api.v2.Cluster",
"nonce": "00000001"
}
2 changes: 1 addition & 1 deletion agent/xds/testdata/clusters/custom-upstream.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
{
"@type": "type.googleapis.com/envoy.api.v2.Cluster",
"name": "myservice",
"connectTimeout": "5s",
"connectTimeout": "15s",
"hosts": [
{
"socketAddress": {
Expand Down
Loading