Skip to content

Commit

Permalink
connect: allow 'envoy_cluster_json' escape hatch to continue to funct…
Browse files Browse the repository at this point in the history 
…ion (#6378)
  • Loading branch information
@rboyer
rboyer authored Aug 22, 2019
1 parent fbb3b8d commit dfcdc41
Show file tree
Hide file tree
Showing 8 changed files with 201 additions and 23 deletions.
22 changes: 21 additions & 1 deletion agent/proxycfg/testing.go
View file
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
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
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
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
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
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
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

0 comments on commit dfcdc41

Please sign in to comment.