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

Jump to bottom

Fix tproxy listeners with discovery chains. #14751

Merged
merged 5 commits into from
Oct 5, 2022
Merged

Fix tproxy listeners with discovery chains. #14751

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
260fdf2
Fix tproxy listeners with discovery chains.
hashi-derek Sep 26, 2022
6074e0a
Tweak test from 1.12 to 1.14 format.
hashi-derek Sep 26, 2022
67bb084
Fix issue with discovery chain cleanup.
hashi-derek Sep 27, 2022
58873e4
Add changelog entry.
hashi-derek Sep 27, 2022
5bf1075
Address PR comments.
hashi-derek Oct 5, 2022
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
3 changes: 3 additions & 0 deletions .changelog/14751.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
connect: Fixed a bug where transparent proxy does not correctly spawn listeners for upstreams to service-resolvers.
```
20 changes: 14 additions & 6 deletions agent/proxycfg/connect_proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,6 @@ func (s *handlerConnectProxy) initialize(ctx context.Context) (ConfigSnapshot, e
if u.Datacenter != "" {
dc = u.Datacenter
}
if s.proxyCfg.Mode == structs.ProxyModeTransparent && (dc == "" || dc == s.source.Datacenter) {
hashi-derek marked this conversation as resolved.
Show resolved Hide resolved
// In transparent proxy mode, watches for upstreams in the local DC
// are handled by the IntentionUpstreams and PeeredUpstreams watch.
continue
}

// Default the partition and namespace to the namespace of this proxy service.
partition := s.proxyID.PartitionOrDefault()
Expand Down Expand Up @@ -305,6 +300,7 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
}

seenUpstreams := make(map[UpstreamID]struct{})

for _, psn := range resp.Services {
uid := NewUpstreamIDFromPeeredServiceName(psn)

Expand Down Expand Up @@ -407,11 +403,16 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
return fmt.Errorf("invalid type for response %T", u.Result)
}

// Contains the list of all implicit / explicit upstreams found.
seenUpstreams := make(map[UpstreamID]struct{})
// Contains only the implicit (intention-based) upstreams.
intentionUpstreams := make(map[UpstreamID]struct{})

for _, svc := range resp.Services {
uid := NewUpstreamIDFromServiceName(svc)

seenUpstreams[uid] = struct{}{}
intentionUpstreams[uid] = struct{}{}

cfgMap := make(map[string]interface{})
u, ok := snap.ConnectProxy.UpstreamConfig[uid]
Expand Down Expand Up @@ -461,7 +462,14 @@ func (s *handlerConnectProxy) handleUpdate(ctx context.Context, u UpdateEvent, s
return fmt.Errorf("failed to watch discovery chain for %s: %v", uid, err)
}
}
snap.ConnectProxy.IntentionUpstreams = seenUpstreams
snap.ConnectProxy.IntentionUpstreams = intentionUpstreams

for uid, upstream := range snap.ConnectProxy.UpstreamConfig {
// Prevent cleanup of discovery chains for explicit upstream definitions.
if !upstream.CentrallyConfigured {
seenUpstreams[uid] = struct{}{}
}
}

// Clean up data from services that were not in the update
for uid, targets := range snap.ConnectProxy.WatchedUpstreams {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if instead we modify the check for cross-datacenter upstreams to instead check !CentrallyConfigured? That way we avoid allocating a new map. We're already checking UpstreamConfig in the reconciliation.

if upstream, ok := snap.ConnectProxy.UpstreamConfig[uid]; ok && upstream.Datacenter != "" && upstream.Datacenter != s.source.Datacenter {
	continue
}

Could become:

if upstream, ok := snap.ConnectProxy.UpstreamConfig[uid]; ok && !upstream.CentrallyConfigured {
	continue
}

This works because a cross-DC watch can only be done explicitly, and not via service-defaults/CentrallyConfigured.

Copy link
Contributor

Choose a reason for hiding this comment

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

(Applies through L549))

Expand Down
72 changes: 71 additions & 1 deletion agent/proxycfg/testing_tproxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package proxycfg

import (
"github.com/hashicorp/consul/api"
"time"

"github.com/hashicorp/consul/api"

"github.com/mitchellh/go-testing-interface"

"github.com/hashicorp/consul/agent/connect"
Expand Down Expand Up @@ -436,6 +437,75 @@ func TestConfigSnapshotTransparentProxyDialDirectly(t testing.T) *ConfigSnapshot
})
}

func TestConfigSnapshotTransparentProxyResolverRedirectUpstream(t testing.T) *ConfigSnapshot {
// Service-Resolver redirect with explicit upstream should spawn an outbound listener.
var (
db = structs.NewServiceName("db-redir", nil)
dbUID = NewUpstreamIDFromServiceName(db)
dbChain = discoverychain.TestCompileConfigEntries(t, "db-redir", "default", "default", "dc1", connect.TestClusterID+".consul", nil,
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "db-redir",
Redirect: &structs.ServiceResolverRedirect{
Service: "db",
},
},
)

google = structs.NewServiceName("google", nil)
googleUID = NewUpstreamIDFromServiceName(google)
googleChain = discoverychain.TestCompileConfigEntries(t, "google", "default", "default", "dc1", connect.TestClusterID+".consul", nil)
)

return TestConfigSnapshot(t, func(ns *structs.NodeService) {
ns.Proxy.Mode = structs.ProxyModeTransparent
ns.Proxy.Upstreams[0].DestinationName = "db-redir"
}, []UpdateEvent{
{
CorrelationID: "discovery-chain:" + dbUID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: dbChain,
},
},
{
CorrelationID: intentionUpstreamsID,
Result: &structs.IndexedServiceList{
Services: structs.ServiceList{
google,
},
},
},
{
CorrelationID: "discovery-chain:" + googleUID.String(),
Result: &structs.DiscoveryChainResponse{
Chain: googleChain,
},
},
{
CorrelationID: "upstream-target:google.default.default.dc1:" + googleUID.String(),
Result: &structs.IndexedCheckServiceNodes{
Nodes: []structs.CheckServiceNode{
{
Node: &structs.Node{
Address: "8.8.8.8",
Datacenter: "dc1",
},
Service: &structs.NodeService{
Service: "google",
Address: "9.9.9.9",
Port: 9090,
TaggedAddresses: map[string]structs.ServiceAddress{
"virtual": {Address: "10.0.0.1"},
structs.TaggedAddressVirtualIP: {Address: "240.0.0.1"},
},
},
},
},
},
},
})
}

func TestConfigSnapshotTransparentProxyTerminatingGatewayCatalogDestinationsOnly(t testing.T) *ConfigSnapshot {
// DiscoveryChain without an UpstreamConfig should yield a
// filter chain when in transparent proxy mode
Expand Down
4 changes: 4 additions & 0 deletions agent/xds/listeners_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,10 @@ func TestListenersFromSnapshot(t *testing.T) {
name: "transparent-proxy-http-upstream",
create: proxycfg.TestConfigSnapshotTransparentProxyHTTPUpstream,
},
{
name: "transparent-proxy-with-resolver-redirect-upstream",
create: proxycfg.TestConfigSnapshotTransparentProxyResolverRedirectUpstream,
},
{
name: "transparent-proxy-catalog-destinations-only",
create: proxycfg.TestConfigSnapshotTransparentProxyCatalogDestinationsOnly,
Expand Down
176 changes: 176 additions & 0 deletions agent/xds/testdata/listeners/transparent-proxy-with-resolver-redirect-upstream.latest.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
{
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"name": "db-redir:127.0.0.1:9191",
"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-redir.default.default.dc1"
}
}
]
}
],
"trafficDirection": "OUTBOUND"
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"name": "outbound_listener:127.0.0.1:15001",
"address": {
"socketAddress": {
"address": "127.0.0.1",
"portValue": 15001
}
},
"filterChains": [
{
"filterChainMatch": {
"prefixRanges": [
{
"addressPrefix": "10.0.0.1",
"prefixLen": 32
},
{
"addressPrefix": "240.0.0.1",
"prefixLen": 32
}
]
},
"filters": [
{
"name": "envoy.filters.network.tcp_proxy",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy",
"statPrefix": "upstream.google.default.default.dc1",
"cluster": "google.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
}
}
]
}
],
"defaultFilterChain": {
"filters": [
{
"name": "envoy.filters.network.tcp_proxy",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy",
"statPrefix": "upstream.original-destination",
"cluster": "original-destination"
}
}
]
},
"listenerFilters": [
{
"name": "envoy.filters.listener.original_dst",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.filters.listener.original_dst.v3.OriginalDst"
}
}
],
"trafficDirection": "OUTBOUND"
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"name": "prepared_query:geo-cache:127.10.10.10:8181",
"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",
"statPrefix": "upstream.prepared_query_geo-cache",
"cluster": "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul"
}
}
]
}
],
"trafficDirection": "OUTBOUND"
},
{
"@type": "type.googleapis.com/envoy.config.listener.v3.Listener",
"name": "public_listener:0.0.0.0:9999",
"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",
"statPrefix": "public_listener",
"cluster": "local_app"
}
}
],
"transportSocket": {
"name": "tls",
"typedConfig": {
"@type": "type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.DownstreamTlsContext",
"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"
}
}
},
"requireClientCertificate": true
}
}
}
],
"trafficDirection": "INBOUND"
}
],
"typeUrl": "type.googleapis.com/envoy.config.listener.v3.Listener",
"nonce": "00000001"
}