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

XDS V1 should not make routes for TCP Disco Chains. #19496

Merged
merged 2 commits into from
Nov 3, 2023
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
15 changes: 0 additions & 15 deletions agent/xds/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,6 @@ func makeClusterDiscoChainTests(enterprise bool) []clusterTestCase {
// TODO(proxystate): requires custom cluster work
alsoRunTestForV2: false,
},
{
name: "connect-proxy-with-chain",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-http2",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand All @@ -84,14 +77,6 @@ func makeClusterDiscoChainTests(enterprise bool) []clusterTestCase {
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "external-sni", enterprise, nil, nil)
},
//TODO(proxystate): this requires terminating gateway work
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
14 changes: 0 additions & 14 deletions agent/xds/endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,20 +247,6 @@ type endpointTestCase struct {

func makeEndpointDiscoChainTests(enterprise bool) []endpointTestCase {
return []endpointTestCase{
{
name: "connect-proxy-with-chain",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "external-sni", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
7 changes: 0 additions & 7 deletions agent/xds/listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,6 @@ func makeListenerDiscoChainTests(enterprise bool) []listenerTestCase {
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "external-sni", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-and-overrides",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
12 changes: 12 additions & 0 deletions agent/xds/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,18 @@ func TestAllResourcesFromSnapshot(t *testing.T) {
return proxycfg.TestConfigSnapshot(t, nil, nil)
},
},
{
name: "connect-proxy-with-chain",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", false, nil, nil)
},
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "external-sni", false, nil, nil)
},
},
{
name: "connect-proxy-exported-to-peers",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
5 changes: 5 additions & 0 deletions agent/xds/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ func (s *ResourceGenerator) routesForConnectProxy(cfgSnap *proxycfg.ConfigSnapsh
continue
}

if !structs.IsProtocolHTTPLike(chain.Protocol) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This same check happens below when creating routes for Destination Upstreams: https://github.com/hashicorp/consul/pull/19496/files#diff-d98ce89d93316f2ab4fd8ea0561b9121e88e535567b9fd4d0992671d145b4b85R93

The two tests that were moved to resource_test.go in this PR both hit this condition where a TCP discovery chain creates a route resource that is not referenced by a listener filter chain.

// Routes can only be defined for HTTP services
continue
}

virtualHost, err := s.makeUpstreamRouteForDiscoveryChain(cfgSnap, uid, chain, []string{"*"}, false, perRouteFilterBuilder{})
if err != nil {
return nil, err
Expand Down
14 changes: 0 additions & 14 deletions agent/xds/routes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,6 @@ type routeTestCase struct {

func makeRouteDiscoChainTests(enterprise bool) []routeTestCase {
return []routeTestCase{
{
name: "connect-proxy-with-chain",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "simple", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-with-chain-external-sni",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "external-sni", enterprise, nil, nil)
},
alsoRunTestForV2: true,
},
{
name: "connect-proxy-splitter-overweight",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,32 +1,5 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"mostSpecificHeaderMutationsWins": true,
"name": "db",
"validateClusters": true,
"virtualHosts": [
{
"domains": [
"*"
],
"name": "db",
"routes": [
{
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"versionInfo": "00000001"
}
115 changes: 115 additions & 0 deletions agent/xds/testdata/listeners/connect-proxy-with-chain.latest.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"address": {
"socketAddress": {
"address": "127.0.0.1",
"portValue": 9191
}
},
"filterChains": [
{
"filters": [
{
"name": "envoy.filters.network.tcp_proxy",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy",
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"statPrefix": "upstream.db.default.default.dc1"
}
}
]
}
],
"name": "db:127.0.0.1:9191",
"trafficDirection": "OUTBOUND"
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"address": {
"socketAddress": {
"address": "127.10.10.10",
"portValue": 8181
}
},
"filterChains": [
{
"filters": [
{
"name": "envoy.filters.network.tcp_proxy",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy",
"cluster": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul",
"statPrefix": "upstream.prepared_query_geo-cache"
}
}
]
}
],
"name": "prepared_query:geo-cache:127.10.10.10:8181",
"trafficDirection": "OUTBOUND"
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"address": {
"socketAddress": {
"address": "0.0.0.0",
"portValue": 9999
}
},
"filterChains": [
{
"filters": [
{
"name": "envoy.filters.network.rbac",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC",
"rules": {},
"statPrefix": "connect_authz"
}
},
{
"name": "envoy.filters.network.tcp_proxy",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy",
"cluster": "local_app",
"statPrefix": "public_listener"
}
}
],
"transportSocket": {
"name": "tls",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext",
"commonTlsContext": {
"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"
}
}
],
"tlsParams": {},
"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"
}
}
},
"requireClientCertificate": true
}
}
}
],
"name": "public_listener:0.0.0.0:9999",
"trafficDirection": "INBOUND"
}
],
"typeUrl": "type.googleapis.com/envoy.config.listener.v3.Listener",
"versionInfo": "00000001"
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,5 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"validateClusters": true,
"virtualHosts": [
{
"domains": [
"*"
],
"name": "db",
"routes": [
{
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"versionInfo": "00000001"
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,5 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"validateClusters": true,
"virtualHosts": [
{
"domains": [
"*"
],
"name": "db",
"routes": [
{
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.cluster-01.external.peer1.domain",
"timeout": "33s"
}
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"versionInfo": "00000001"
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,5 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"validateClusters": true,
"virtualHosts": [
{
"domains": [
"*"
],
"name": "db",
"routes": [
{
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"versionInfo": "00000001"
}
Original file line number Diff line number Diff line change
@@ -1,31 +1,5 @@
{
"nonce": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"validateClusters": true,
"virtualHosts": [
{
"domains": [
"*"
],
"name": "db",
"routes": [
{
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"versionInfo": "00000001"
}
Loading