Skip to content

Commit

Permalink
Avoid panic applying TProxy Envoy extensions (#17537)
Browse files Browse the repository at this point in the history
When UpstreamEnvoyExtender was introduced, some code was left duplicated
between it and BasicEnvoyExtender. One path in that code panics when a
TProxy listener patch is attempted due to no upstream data in
RuntimeConfig matching the local service (which would only happen in
rare cases).

Instead, we can remove the special handling of upstream VIPs from
BasicEnvoyExtender entirely, greatly simplifying the listener filter
patch code and avoiding the panic. UpstreamEnvoyExtender, which needs
this code to function, is modified to ensure a panic does not occur.

This also fixes a second regression in which the Lua extension was not
applied to TProxy outbound listeners.
  • Loading branch information
zalimeni authored Jun 1, 2023
1 parent ca12ce9 commit ad03a5d
Show file tree
Hide file tree
Showing 9 changed files with 928 additions and 64 deletions.
5 changes: 4 additions & 1 deletion agent/proxycfg/testing_tproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ func TestConfigSnapshotTransparentProxyDestination(t testing.T) *ConfigSnapshot
})
}

func TestConfigSnapshotTransparentProxyDestinationHTTP(t testing.T) *ConfigSnapshot {
func TestConfigSnapshotTransparentProxyDestinationHTTP(t testing.T, nsFn func(ns *structs.NodeService)) *ConfigSnapshot {
// DiscoveryChain without an UpstreamConfig should yield a
// filter chain when in transparent proxy mode
var (
Expand Down Expand Up @@ -773,6 +773,9 @@ func TestConfigSnapshotTransparentProxyDestinationHTTP(t testing.T) *ConfigSnaps
},
}
return TestConfigSnapshot(t, func(ns *structs.NodeService) {
if nsFn != nil {
nsFn(ns)
}
ns.Proxy.Mode = structs.ProxyModeTransparent
}, []UpdateEvent{
{
Expand Down
7 changes: 7 additions & 0 deletions agent/xds/delta_envoy_extender_oss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,13 @@ end`,
return proxycfg.TestConfigSnapshotDiscoveryChain(t, "default", false, makeLuaNsFunc(false), nil)
},
},
{
name: "lua-outbound-applies-to-local-upstreams-tproxy",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
// upstreams need to be http in order for lua to be applied to listeners.
return proxycfg.TestConfigSnapshotTransparentProxyDestinationHTTP(t, makeLuaNsFunc(false))
},
},
{
name: "lua-connect-proxy-with-terminating-gateway-upstream",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
Expand Down
6 changes: 4 additions & 2 deletions agent/xds/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,10 @@ func getConnectProxyTransparentProxyGoldenTestCases() []goldenTestCase {
create: proxycfg.TestConfigSnapshotTransparentProxyDestination,
},
{
name: "transparent-proxy-destination-http",
create: proxycfg.TestConfigSnapshotTransparentProxyDestinationHTTP,
name: "transparent-proxy-destination-http",
create: func(t testinf.T) *proxycfg.ConfigSnapshot {
return proxycfg.TestConfigSnapshotTransparentProxyDestinationHTTP(t, nil)
},
},
{
name: "transparent-proxy-terminating-gateway-destinations-only",
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
{
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.10.1.1",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
},
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.10.1.2",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
},
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "destination.192-168-2-1.kafka.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "172.168.0.1",
"portValue": 8443
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
},
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "destination.192-168-2-2.kafka2.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "172.168.0.1",
"portValue": 8443
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
},
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "destination.192-168-2-3.kafka2.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "172.168.0.1",
"portValue": 8443
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
},
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "destination.www-google-com.google.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "172.168.0.1",
"portValue": 8443
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
},
{
"@type": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"clusterName": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul",
"endpoints": [
{
"lbEndpoints": [
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.10.1.1",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
},
{
"endpoint": {
"address": {
"socketAddress": {
"address": "10.20.1.2",
"portValue": 8080
}
}
},
"healthStatus": "HEALTHY",
"loadBalancingWeight": 1
}
]
}
]
}
],
"typeUrl": "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment",
"nonce": "00000001"
}
Loading

0 comments on commit ad03a5d

Please sign in to comment.