From c4b75fde6131fc259ce5489384f8ebc4a0624621 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 18:42:16 -0600 Subject: [PATCH 01/16] Remove unused param --- agent/xds/listeners.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 276ea58f7558..e28670506308 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -116,7 +116,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. upstreamCfg, chain, cfgSnap, - nil, ) if err != nil { return nil, err @@ -142,7 +141,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. upstreamCfg, chain, cfgSnap, - nil, ) if err != nil { return nil, err @@ -197,7 +195,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. &u, nil, cfgSnap, - nil, ) if err != nil { return nil, err @@ -226,7 +223,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. nil, nil, cfgSnap, - nil, ) if err != nil { return nil, err @@ -275,7 +271,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. u, nil, cfgSnap, - nil, ) if err != nil { return nil, err @@ -1219,7 +1214,6 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( u *structs.Upstream, chain *structs.CompiledDiscoveryChain, cfgSnap *proxycfg.ConfigSnapshot, - tlsContext *envoy_tls_v3.DownstreamTlsContext, ) (*envoy_listener_v3.FilterChain, error) { // TODO (freddy) Make this actually legible useRDS := true @@ -1313,16 +1307,11 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( if err != nil { return nil, err } - transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext) - if err != nil { - return nil, err - } return &envoy_listener_v3.FilterChain{ Filters: []*envoy_listener_v3.Filter{ filter, }, - TransportSocket: transportSocket, }, nil } From 47c19c88d6e2f0fae7dd57298917cd916424a848 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 19:06:29 -0600 Subject: [PATCH 02/16] Extract function for disco target from tcp chain --- agent/xds/listeners.go | 46 +++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index e28670506308..430355ca716a 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -1227,7 +1227,7 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( if chain != nil { destination, datacenter, partition, namespace = chain.ServiceName, chain.Datacenter, chain.Partition, chain.Namespace } - if (chain == nil || chain.IsDefault()) && u != nil { + if (chain == nil || chain.IsDefault()) && u != nil && !u.CentrallyConfigured { useRDS = false if datacenter == "" { @@ -1253,16 +1253,10 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( if protocol == "tcp" && chain != nil { useRDS = false - startNode := chain.Nodes[chain.StartNode] - if startNode == nil { - return nil, fmt.Errorf("missing first node in compiled discovery chain for: %s", chain.ServiceName) - } - if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { - return nil, fmt.Errorf("unexpected first node in discovery chain using protocol=%q: %s", protocol, startNode.Type) + target, err := tcpChainTarget(chain) + if err != nil { + return nil, err } - targetID := startNode.Resolver.Target - target := chain.Targets[targetID] - clusterName = CustomizeClusterName(target.Name, chain) } } @@ -1293,15 +1287,12 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( } opts := listenerFilterOpts{ - useRDS: useRDS, - protocol: protocol, - filterName: filterName, - routeName: id, - cluster: clusterName, - statPrefix: "upstream.", - routePath: "", - ingressGateway: false, - httpAuthzFilter: nil, + useRDS: useRDS, + protocol: protocol, + filterName: filterName, + routeName: id, + cluster: clusterName, + statPrefix: "upstream.", } filter, err := makeListenerFilter(opts) if err != nil { @@ -1315,6 +1306,23 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( }, nil } +// tcpChainTarget returns the discovery target for a TCP chain. Since TCP services +// cannot have splits or routes, there will be a single target. +func tcpChainTarget(chain *structs.CompiledDiscoveryChain) (*structs.DiscoveryTarget, error) { + if chain.Protocol != "tcp" { + return nil, fmt.Errorf(`expected chain with tcp protocol, got %q`, chain.Protocol) + } + startNode := chain.Nodes[chain.StartNode] + if startNode == nil { + return nil, fmt.Errorf("missing first node in compiled discovery chain for: %s", chain.ServiceName) + } + if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { + return nil, fmt.Errorf("unexpected first node in discovery chain for tcp service: %s", startNode.Type) + } + targetID := startNode.Resolver.Target + return chain.Targets[targetID], nil +} + // TODO(freddy) Replace in favor of new function above. Currently in use for ingress gateways. func (s *ResourceGenerator) makeUpstreamListenerForDiscoveryChain( u *structs.Upstream, From 4f91727583272d3dc38dbeea4f16dfdeef4cbfbd Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 19:17:09 -0600 Subject: [PATCH 03/16] Consolidate RDS logic --- agent/xds/listeners.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 430355ca716a..c608672f36a2 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -1216,7 +1216,14 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( cfgSnap *proxycfg.ConfigSnapshot, ) (*envoy_listener_v3.FilterChain, error) { // TODO (freddy) Make this actually legible - useRDS := true + var useRDS bool + + // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized + // discovery chain. Default HTTP chains have no routes, and TCP services can have customized chains + // when they redirect or fail-over to other services. + if chain != nil && !chain.IsDefault() && chain.Protocol != "tcp" { + useRDS = true + } var ( clusterName string @@ -1228,8 +1235,6 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( destination, datacenter, partition, namespace = chain.ServiceName, chain.Datacenter, chain.Partition, chain.Namespace } if (chain == nil || chain.IsDefault()) && u != nil && !u.CentrallyConfigured { - useRDS = false - if datacenter == "" { datacenter = u.Datacenter } @@ -1251,8 +1256,6 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( } else { if protocol == "tcp" && chain != nil { - useRDS = false - target, err := tcpChainTarget(chain) if err != nil { return nil, err @@ -1278,7 +1281,6 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( filterName = id } if overrideCluster != "" { - useRDS = false clusterName = overrideCluster if destination == "" { From 79b8e0042d2555d66304b07a93074ca1aabb8211 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 20:09:11 -0600 Subject: [PATCH 04/16] Shuffle around coalescing of name/ns/dc/ap --- agent/xds/listeners.go | 75 ++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 43 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index c608672f36a2..1461b78a4dfa 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -1218,63 +1218,50 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( // TODO (freddy) Make this actually legible var useRDS bool - // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized - // discovery chain. Default HTTP chains have no routes, and TCP services can have customized chains - // when they redirect or fail-over to other services. + // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. + // Default HTTP chains have no routes, and TCP services can have customized chains when they redirect + // or fail-over to other services. if chain != nil && !chain.IsDefault() && chain.Protocol != "tcp" { useRDS = true } var ( - clusterName string + sni, clusterName string destination, datacenter, partition, namespace string ) - // TODO (SNI partition) add partition for SNI + // Coalesce service details from the given discovery chain or upstream configuration if chain != nil { destination, datacenter, partition, namespace = chain.ServiceName, chain.Datacenter, chain.Partition, chain.Namespace - } - if (chain == nil || chain.IsDefault()) && u != nil && !u.CentrallyConfigured { - if datacenter == "" { - datacenter = u.Datacenter + + // When not using RDS we must generate a cluster name to attach to the filter chain + if !useRDS { + target, err := simpleChainTarget(chain) + if err != nil { + return nil, err + } + sni = target.Name } + } else if u != nil && !u.CentrallyConfigured { + destination, datacenter, partition, namespace = u.DestinationName, u.Datacenter, u.DestinationPartition, u.DestinationNamespace + if datacenter == "" { datacenter = cfgSnap.Datacenter } - if destination == "" { - destination = u.DestinationName - } - if partition == "" { - partition = u.DestinationPartition - } if namespace == "" { - namespace = u.DestinationNamespace + namespace = structs.IntentionDefaultNamespace } - - sni := connect.UpstreamSNI(u, "", datacenter, cfgSnap.Roots.TrustDomain) - clusterName = CustomizeClusterName(sni, chain) - - } else { - if protocol == "tcp" && chain != nil { - target, err := tcpChainTarget(chain) - if err != nil { - return nil, err - } - clusterName = CustomizeClusterName(target.Name, chain) + if partition == "" { + partition = structs.IntentionDefaultNamespace } - } - - // Default the namespace to match how SNIs are generated - if namespace == "" { - namespace = structs.IntentionDefaultNamespace - } - // Default the partition to match how SNIs are generated - if partition == "" { - partition = structs.IntentionDefaultNamespace + // TODO (SNI partition) add partition for upstream SNI + sni = connect.UpstreamSNI(u, "", datacenter, cfgSnap.Roots.TrustDomain) } + clusterName = CustomizeClusterName(sni, chain) filterName := fmt.Sprintf("%s.%s.%s.%s", destination, namespace, partition, datacenter) + if u != nil && u.DestinationType == structs.UpstreamDestTypePreparedQuery { // Avoid encoding dc and namespace for prepared queries. // Those are defined in the query itself and are not available here. @@ -1288,6 +1275,10 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( } } + // When using RDS the cluster name needs to be attached to the routes, not the listener. + if useRDS { + clusterName = "" + } opts := listenerFilterOpts{ useRDS: useRDS, protocol: protocol, @@ -1308,18 +1299,16 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( }, nil } -// tcpChainTarget returns the discovery target for a TCP chain. Since TCP services -// cannot have splits or routes, there will be a single target. -func tcpChainTarget(chain *structs.CompiledDiscoveryChain) (*structs.DiscoveryTarget, error) { - if chain.Protocol != "tcp" { - return nil, fmt.Errorf(`expected chain with tcp protocol, got %q`, chain.Protocol) - } +// simpleChainTarget returns the discovery target for a chain with a single node. +// A chain can have a single target if it is for a TCP service or an HTTP service without +// multiple splits/routes/failovers. +func simpleChainTarget(chain *structs.CompiledDiscoveryChain) (*structs.DiscoveryTarget, error) { startNode := chain.Nodes[chain.StartNode] if startNode == nil { return nil, fmt.Errorf("missing first node in compiled discovery chain for: %s", chain.ServiceName) } if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { - return nil, fmt.Errorf("unexpected first node in discovery chain for tcp service: %s", startNode.Type) + return nil, fmt.Errorf("expected discovery chain with single node, found unexpected start node: %s", startNode.Type) } targetID := startNode.Resolver.Target return chain.Targets[targetID], nil From 6e427b9f141b786c1ba0ce082feada611f3f5df7 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 20:14:28 -0600 Subject: [PATCH 05/16] Simplify filter name overriding logic --- agent/xds/listeners.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 1461b78a4dfa..8449eb67f1d7 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -112,6 +112,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( id, "", + "", cfg.Protocol, upstreamCfg, chain, @@ -137,6 +138,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( id, "", + "", cfg.Protocol, upstreamCfg, chain, @@ -189,6 +191,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( "", "passthrough~"+passthrough.SNI, + "", // TODO(tproxy) This should use the protocol configured on the upstream's config entry "tcp", @@ -219,6 +222,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( "", OriginalDestinationClusterName, + OriginalDestinationClusterName, "tcp", nil, nil, @@ -267,6 +271,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( id, "", + id, cfg.Protocol, u, nil, @@ -1210,6 +1215,7 @@ func (s *ResourceGenerator) makeMeshGatewayListener(name, addr string, port int, func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( id string, overrideCluster string, + overrideFilterName string, protocol string, u *structs.Upstream, chain *structs.CompiledDiscoveryChain, @@ -1259,20 +1265,14 @@ func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( sni = connect.UpstreamSNI(u, "", datacenter, cfgSnap.Roots.TrustDomain) } - clusterName = CustomizeClusterName(sni, chain) filterName := fmt.Sprintf("%s.%s.%s.%s", destination, namespace, partition, datacenter) - - if u != nil && u.DestinationType == structs.UpstreamDestTypePreparedQuery { - // Avoid encoding dc and namespace for prepared queries. - // Those are defined in the query itself and are not available here. - filterName = id + if overrideFilterName != "" { + filterName = overrideFilterName } + + clusterName = CustomizeClusterName(sni, chain) if overrideCluster != "" { clusterName = overrideCluster - - if destination == "" { - filterName = overrideCluster - } } // When using RDS the cluster name needs to be attached to the routes, not the listener. From 725d5a46797c03422b2a566227c55ca593d39525 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 20:19:49 -0600 Subject: [PATCH 06/16] Rename function since it's not only used with discovery chains --- agent/xds/listeners.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 8449eb67f1d7..9aad28454424 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -109,7 +109,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // Generate the upstream listeners for when they are explicitly set with a local bind port or socket path if outboundListener == nil || (upstreamCfg != nil && upstreamCfg.HasLocalPortOrSocket()) { - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( + filterChain, err := s.makeUpstreamFilterChain( id, "", "", @@ -135,7 +135,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // The rest of this loop is used exclusively for transparent proxies. // Below we create a filter chain per upstream, rather than a listener per upstream // as we do for explicit upstreams above. - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( + filterChain, err := s.makeUpstreamFilterChain( id, "", "", @@ -188,7 +188,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. DestinationPartition: sn.PartitionOrDefault(), } - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( + filterChain, err := s.makeUpstreamFilterChain( "", "passthrough~"+passthrough.SNI, "", @@ -219,7 +219,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. if cfgSnap.ConnectProxy.MeshConfig == nil || !cfgSnap.ConnectProxy.MeshConfig.TransparentProxy.MeshDestinationsOnly { - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( + filterChain, err := s.makeUpstreamFilterChain( "", OriginalDestinationClusterName, OriginalDestinationClusterName, @@ -268,7 +268,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. upstreamListener := makeListener(id, u, envoy_core_v3.TrafficDirection_OUTBOUND) - filterChain, err := s.makeUpstreamFilterChainForDiscoveryChain( + filterChain, err := s.makeUpstreamFilterChain( id, "", id, @@ -1212,7 +1212,7 @@ func (s *ResourceGenerator) makeMeshGatewayListener(name, addr string, port int, return l, nil } -func (s *ResourceGenerator) makeUpstreamFilterChainForDiscoveryChain( +func (s *ResourceGenerator) makeUpstreamFilterChain( id string, overrideCluster string, overrideFilterName string, From ac79dc79f0bb4d783c453db6f6f07f025c22b047 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 20:37:00 -0600 Subject: [PATCH 07/16] Extract RDS logic since it's only relevant to disco chains --- agent/xds/listeners.go | 180 +++++++++++++++++++---------------------- 1 file changed, 85 insertions(+), 95 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 9aad28454424..074242129511 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -107,17 +107,24 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. continue } + // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. + // Default HTTP chains have no routes, and TCP services can have customized chains when they redirect + // or fail-over to other services. + var useRDS bool + if !chain.IsDefault() && chain.Protocol != "tcp" { + useRDS = true + } + // Generate the upstream listeners for when they are explicitly set with a local bind port or socket path - if outboundListener == nil || (upstreamCfg != nil && upstreamCfg.HasLocalPortOrSocket()) { - filterChain, err := s.makeUpstreamFilterChain( - id, - "", - "", - cfg.Protocol, - upstreamCfg, - chain, - cfgSnap, - ) + if upstreamCfg != nil && upstreamCfg.HasLocalPortOrSocket() { + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + id: id, + protocol: cfg.Protocol, + useRDS: useRDS, + upstream: upstreamCfg, + chain: chain, + cfgSnap: cfgSnap, + }) if err != nil { return nil, err } @@ -135,15 +142,15 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // The rest of this loop is used exclusively for transparent proxies. // Below we create a filter chain per upstream, rather than a listener per upstream // as we do for explicit upstreams above. - filterChain, err := s.makeUpstreamFilterChain( - id, - "", - "", - cfg.Protocol, - upstreamCfg, - chain, - cfgSnap, - ) + + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + id: id, + protocol: cfg.Protocol, + useRDS: useRDS, + upstream: upstreamCfg, + chain: chain, + cfgSnap: cfgSnap, + }) if err != nil { return nil, err } @@ -188,17 +195,12 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. DestinationPartition: sn.PartitionOrDefault(), } - filterChain, err := s.makeUpstreamFilterChain( - "", - "passthrough~"+passthrough.SNI, - "", - - // TODO(tproxy) This should use the protocol configured on the upstream's config entry - "tcp", - &u, - nil, - cfgSnap, - ) + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + overrideCluster: "passthrough~" + passthrough.SNI, + protocol: "tcp", + upstream: &u, + cfgSnap: cfgSnap, + }) if err != nil { return nil, err } @@ -219,15 +221,12 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. if cfgSnap.ConnectProxy.MeshConfig == nil || !cfgSnap.ConnectProxy.MeshConfig.TransparentProxy.MeshDestinationsOnly { - filterChain, err := s.makeUpstreamFilterChain( - "", - OriginalDestinationClusterName, - OriginalDestinationClusterName, - "tcp", - nil, - nil, - cfgSnap, - ) + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + overrideCluster: OriginalDestinationClusterName, + overrideFilterName: OriginalDestinationClusterName, + protocol: "tcp", + cfgSnap: cfgSnap, + }) if err != nil { return nil, err } @@ -268,15 +267,13 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. upstreamListener := makeListener(id, u, envoy_core_v3.TrafficDirection_OUTBOUND) - filterChain, err := s.makeUpstreamFilterChain( - id, - "", - id, - cfg.Protocol, - u, - nil, - cfgSnap, - ) + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + id: id, + overrideFilterName: id, + protocol: cfg.Protocol, + upstream: u, + cfgSnap: cfgSnap, + }) if err != nil { return nil, err } @@ -1212,82 +1209,75 @@ func (s *ResourceGenerator) makeMeshGatewayListener(name, addr string, port int, return l, nil } -func (s *ResourceGenerator) makeUpstreamFilterChain( - id string, - overrideCluster string, - overrideFilterName string, - protocol string, - u *structs.Upstream, - chain *structs.CompiledDiscoveryChain, - cfgSnap *proxycfg.ConfigSnapshot, -) (*envoy_listener_v3.FilterChain, error) { - // TODO (freddy) Make this actually legible - var useRDS bool - - // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. - // Default HTTP chains have no routes, and TCP services can have customized chains when they redirect - // or fail-over to other services. - if chain != nil && !chain.IsDefault() && chain.Protocol != "tcp" { - useRDS = true - } +type filterChainOpts struct { + id string + overrideCluster string + overrideFilterName string + protocol string + useRDS bool + upstream *structs.Upstream + chain *structs.CompiledDiscoveryChain + cfgSnap *proxycfg.ConfigSnapshot +} +func (s *ResourceGenerator) makeUpstreamFilterChain(opts filterChainOpts) (*envoy_listener_v3.FilterChain, error) { + // TODO (freddy) Make this actually legible var ( - sni, clusterName string - destination, datacenter, partition, namespace string + sni, clusterName string + name, dc, partition, ns string ) - // Coalesce service details from the given discovery chain or upstream configuration - if chain != nil { - destination, datacenter, partition, namespace = chain.ServiceName, chain.Datacenter, chain.Partition, chain.Namespace + // Coalesce upstream's identifying details from the given discovery chain or upstream configuration + if opts.chain != nil { + name, dc, partition, ns = opts.chain.ServiceName, opts.chain.Datacenter, opts.chain.Partition, opts.chain.Namespace // When not using RDS we must generate a cluster name to attach to the filter chain - if !useRDS { - target, err := simpleChainTarget(chain) + if !opts.useRDS { + target, err := simpleChainTarget(opts.chain) if err != nil { return nil, err } sni = target.Name } - } else if u != nil && !u.CentrallyConfigured { - destination, datacenter, partition, namespace = u.DestinationName, u.Datacenter, u.DestinationPartition, u.DestinationNamespace + } else if opts.upstream != nil && !opts.upstream.CentrallyConfigured { + name, dc, partition, ns = opts.upstream.DestinationName, opts.upstream.Datacenter, opts.upstream.DestinationPartition, opts.upstream.DestinationNamespace - if datacenter == "" { - datacenter = cfgSnap.Datacenter + if dc == "" { + dc = opts.cfgSnap.Datacenter } - if namespace == "" { - namespace = structs.IntentionDefaultNamespace + if ns == "" { + ns = structs.IntentionDefaultNamespace } if partition == "" { partition = structs.IntentionDefaultNamespace } // TODO (SNI partition) add partition for upstream SNI - sni = connect.UpstreamSNI(u, "", datacenter, cfgSnap.Roots.TrustDomain) + sni = connect.UpstreamSNI(opts.upstream, "", dc, opts.cfgSnap.Roots.TrustDomain) } - filterName := fmt.Sprintf("%s.%s.%s.%s", destination, namespace, partition, datacenter) - if overrideFilterName != "" { - filterName = overrideFilterName + filterName := fmt.Sprintf("%s.%s.%s.%s", name, ns, partition, dc) + if opts.overrideFilterName != "" { + filterName = opts.overrideFilterName } - clusterName = CustomizeClusterName(sni, chain) - if overrideCluster != "" { - clusterName = overrideCluster + clusterName = CustomizeClusterName(sni, opts.chain) + if opts.overrideCluster != "" { + clusterName = opts.overrideCluster } - - // When using RDS the cluster name needs to be attached to the routes, not the listener. - if useRDS { + if opts.useRDS { + // When using RDS the cluster name is attached to routes, not the filter chain. clusterName = "" } - opts := listenerFilterOpts{ - useRDS: useRDS, - protocol: protocol, + + filter, err := makeListenerFilter(listenerFilterOpts{ + useRDS: opts.useRDS, + protocol: opts.protocol, filterName: filterName, - routeName: id, + routeName: opts.id, cluster: clusterName, statPrefix: "upstream.", - } - filter, err := makeListenerFilter(opts) + }) if err != nil { return nil, err } From b8f3a1a4eb40147e7cbd584bfe57dc9046529212 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 21:23:17 -0600 Subject: [PATCH 08/16] Move ns/partition defaulting upstream --- agent/proxycfg/state.go | 3 ++- agent/xds/listeners.go | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/agent/proxycfg/state.go b/agent/proxycfg/state.go index cd30003531b2..237279b99791 100644 --- a/agent/proxycfg/state.go +++ b/agent/proxycfg/state.go @@ -114,11 +114,12 @@ func copyProxyConfig(ns *structs.NodeService) (structs.ConnectProxyConfig, error for idx := range proxyCfg.Upstreams { us := &proxyCfg.Upstreams[idx] if us.DestinationType != structs.UpstreamDestTypePreparedQuery && us.DestinationNamespace == "" { - // default the upstreams target namespace to the namespace of the proxy + // default the upstreams target namespace and partition to those of the proxy // doing this here prevents needing much more complex logic a bunch of other // places and makes tracking these upstreams simpler as we can dedup them // with the maps tracking upstream ids being watched. proxyCfg.Upstreams[idx].DestinationNamespace = ns.EnterpriseMeta.NamespaceOrDefault() + proxyCfg.Upstreams[idx].DestinationPartition = ns.EnterpriseMeta.PartitionOrDefault() } } diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index 074242129511..e3362b6ac3ce 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -1245,12 +1245,6 @@ func (s *ResourceGenerator) makeUpstreamFilterChain(opts filterChainOpts) (*envo if dc == "" { dc = opts.cfgSnap.Datacenter } - if ns == "" { - ns = structs.IntentionDefaultNamespace - } - if partition == "" { - partition = structs.IntentionDefaultNamespace - } // TODO (SNI partition) add partition for upstream SNI sni = connect.UpstreamSNI(opts.upstream, "", dc, opts.cfgSnap.Roots.TrustDomain) From 244f405e01dd38b077559f3c26d3a26a18631006 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 5 Oct 2021 22:01:55 -0600 Subject: [PATCH 09/16] Extract cluster and filter name creation --- agent/xds/listeners.go | 140 ++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 79 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index e3362b6ac3ce..a69edb8fa24f 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -115,15 +115,30 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. useRDS = true } + var clusterName string + + // When not using RDS we must generate a cluster name to attach to the filter chain + if !useRDS { + target, err := simpleChainTarget(chain) + if err != nil { + return nil, err + } + clusterName = CustomizeClusterName(target.Name, chain) + } + + filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) + // Generate the upstream listeners for when they are explicitly set with a local bind port or socket path if upstreamCfg != nil && upstreamCfg.HasLocalPortOrSocket() { filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ - id: id, - protocol: cfg.Protocol, - useRDS: useRDS, - upstream: upstreamCfg, - chain: chain, - cfgSnap: cfgSnap, + routeName: id, + clusterName: clusterName, + filterName: filterName, + protocol: cfg.Protocol, + useRDS: useRDS, + upstream: upstreamCfg, + chain: chain, + cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -144,12 +159,14 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // as we do for explicit upstreams above. filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ - id: id, - protocol: cfg.Protocol, - useRDS: useRDS, - upstream: upstreamCfg, - chain: chain, - cfgSnap: cfgSnap, + routeName: id, + clusterName: clusterName, + filterName: filterName, + protocol: cfg.Protocol, + useRDS: useRDS, + upstream: upstreamCfg, + chain: chain, + cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -195,11 +212,18 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. DestinationPartition: sn.PartitionOrDefault(), } + dc := u.Datacenter + if dc == "" { + dc = cfgSnap.Datacenter + } + filterName := fmt.Sprintf("%s.%s.%s.%s", u.DestinationName, u.DestinationNamespace, u.DestinationPartition, dc) + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ - overrideCluster: "passthrough~" + passthrough.SNI, - protocol: "tcp", - upstream: &u, - cfgSnap: cfgSnap, + clusterName: "passthrough~" + passthrough.SNI, + filterName: filterName, + protocol: "tcp", + upstream: &u, + cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -222,10 +246,10 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. !cfgSnap.ConnectProxy.MeshConfig.TransparentProxy.MeshDestinationsOnly { filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ - overrideCluster: OriginalDestinationClusterName, - overrideFilterName: OriginalDestinationClusterName, - protocol: "tcp", - cfgSnap: cfgSnap, + clusterName: OriginalDestinationClusterName, + filterName: OriginalDestinationClusterName, + protocol: "tcp", + cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -268,11 +292,12 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. upstreamListener := makeListener(id, u, envoy_core_v3.TrafficDirection_OUTBOUND) filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ - id: id, - overrideFilterName: id, - protocol: cfg.Protocol, - upstream: u, - cfgSnap: cfgSnap, + // TODO (SNI partition) add partition for upstream SNI + clusterName: connect.UpstreamSNI(u, "", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain), + filterName: id, + protocol: cfg.Protocol, + upstream: u, + cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -1210,66 +1235,23 @@ func (s *ResourceGenerator) makeMeshGatewayListener(name, addr string, port int, } type filterChainOpts struct { - id string - overrideCluster string - overrideFilterName string - protocol string - useRDS bool - upstream *structs.Upstream - chain *structs.CompiledDiscoveryChain - cfgSnap *proxycfg.ConfigSnapshot + routeName string + clusterName string + filterName string + protocol string + useRDS bool + upstream *structs.Upstream + chain *structs.CompiledDiscoveryChain + cfgSnap *proxycfg.ConfigSnapshot } func (s *ResourceGenerator) makeUpstreamFilterChain(opts filterChainOpts) (*envoy_listener_v3.FilterChain, error) { - // TODO (freddy) Make this actually legible - var ( - sni, clusterName string - name, dc, partition, ns string - ) - - // Coalesce upstream's identifying details from the given discovery chain or upstream configuration - if opts.chain != nil { - name, dc, partition, ns = opts.chain.ServiceName, opts.chain.Datacenter, opts.chain.Partition, opts.chain.Namespace - - // When not using RDS we must generate a cluster name to attach to the filter chain - if !opts.useRDS { - target, err := simpleChainTarget(opts.chain) - if err != nil { - return nil, err - } - sni = target.Name - } - } else if opts.upstream != nil && !opts.upstream.CentrallyConfigured { - name, dc, partition, ns = opts.upstream.DestinationName, opts.upstream.Datacenter, opts.upstream.DestinationPartition, opts.upstream.DestinationNamespace - - if dc == "" { - dc = opts.cfgSnap.Datacenter - } - - // TODO (SNI partition) add partition for upstream SNI - sni = connect.UpstreamSNI(opts.upstream, "", dc, opts.cfgSnap.Roots.TrustDomain) - } - - filterName := fmt.Sprintf("%s.%s.%s.%s", name, ns, partition, dc) - if opts.overrideFilterName != "" { - filterName = opts.overrideFilterName - } - - clusterName = CustomizeClusterName(sni, opts.chain) - if opts.overrideCluster != "" { - clusterName = opts.overrideCluster - } - if opts.useRDS { - // When using RDS the cluster name is attached to routes, not the filter chain. - clusterName = "" - } - filter, err := makeListenerFilter(listenerFilterOpts{ useRDS: opts.useRDS, protocol: opts.protocol, - filterName: filterName, - routeName: opts.id, - cluster: clusterName, + filterName: opts.filterName, + routeName: opts.routeName, + cluster: opts.clusterName, statPrefix: "upstream.", }) if err != nil { From deb9e794d0b5d9a047781e094d6fe5e19051d896 Mon Sep 17 00:00:00 2001 From: freddygv Date: Wed, 6 Oct 2021 09:57:56 -0600 Subject: [PATCH 10/16] Migrate ingress listener gen away from old fn --- agent/xds/listeners.go | 131 +----------------- agent/xds/listeners_ingress.go | 40 ++++-- agent/xds/listeners_test.go | 28 +++- ...istener-listener-level.envoy-1-20-x.golden | 6 +- 4 files changed, 67 insertions(+), 138 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index a69edb8fa24f..fd1e1a31ca54 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -108,8 +108,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. } // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. - // Default HTTP chains have no routes, and TCP services can have customized chains when they redirect - // or fail-over to other services. var useRDS bool if !chain.IsDefault() && chain.Protocol != "tcp" { useRDS = true @@ -117,7 +115,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. var clusterName string - // When not using RDS we must generate a cluster name to attach to the filter chain + // When not using RDS we must generate a cluster name to attach to the filter chain. if !useRDS { target, err := simpleChainTarget(chain) if err != nil { @@ -136,9 +134,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterName: filterName, protocol: cfg.Protocol, useRDS: useRDS, - upstream: upstreamCfg, - chain: chain, - cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -164,9 +159,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. filterName: filterName, protocol: cfg.Protocol, useRDS: useRDS, - upstream: upstreamCfg, - chain: chain, - cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -222,8 +214,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. clusterName: "passthrough~" + passthrough.SNI, filterName: filterName, protocol: "tcp", - upstream: &u, - cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -249,7 +239,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. clusterName: OriginalDestinationClusterName, filterName: OriginalDestinationClusterName, protocol: "tcp", - cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -296,8 +285,6 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. clusterName: connect.UpstreamSNI(u, "", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain), filterName: id, protocol: cfg.Protocol, - upstream: u, - cfgSnap: cfgSnap, }) if err != nil { return nil, err @@ -1240,9 +1227,7 @@ type filterChainOpts struct { filterName string protocol string useRDS bool - upstream *structs.Upstream - chain *structs.CompiledDiscoveryChain - cfgSnap *proxycfg.ConfigSnapshot + tlsContext *envoy_tls_v3.DownstreamTlsContext } func (s *ResourceGenerator) makeUpstreamFilterChain(opts filterChainOpts) (*envoy_listener_v3.FilterChain, error) { @@ -1258,10 +1243,15 @@ func (s *ResourceGenerator) makeUpstreamFilterChain(opts filterChainOpts) (*envo return nil, err } + transportSocket, err := makeDownstreamTLSTransportSocket(opts.tlsContext) + if err != nil { + return nil, err + } return &envoy_listener_v3.FilterChain{ Filters: []*envoy_listener_v3.Filter{ filter, }, + TransportSocket: transportSocket, }, nil } @@ -1280,113 +1270,6 @@ func simpleChainTarget(chain *structs.CompiledDiscoveryChain) (*structs.Discover return chain.Targets[targetID], nil } -// TODO(freddy) Replace in favor of new function above. Currently in use for ingress gateways. -func (s *ResourceGenerator) makeUpstreamListenerForDiscoveryChain( - u *structs.Upstream, - address string, - chain *structs.CompiledDiscoveryChain, - cfgSnap *proxycfg.ConfigSnapshot, - tlsContext *envoy_tls_v3.DownstreamTlsContext, -) (proto.Message, error) { - - // Best understanding is this only makes sense for port listeners.... - if u.LocalBindSocketPath != "" { - return nil, fmt.Errorf("makeUpstreamListenerForDiscoveryChain not supported for unix domain sockets %s %+v", - address, u) - } - - upstreamID := u.Identifier() - l := makePortListenerWithDefault(upstreamID, address, u.LocalBindPort, envoy_core_v3.TrafficDirection_OUTBOUND) - cfg := s.getAndModifyUpstreamConfigForListener(upstreamID, u, chain) - if cfg.EnvoyListenerJSON != "" { - return makeListenerFromUserConfig(cfg.EnvoyListenerJSON) - } - - useRDS := true - var ( - clusterName string - destination, datacenter, partition, namespace string - ) - if chain == nil || chain.IsDefault() { - useRDS = false - - dc := u.Datacenter - if dc == "" { - dc = cfgSnap.Datacenter - } - destination, datacenter, partition, namespace = u.DestinationName, dc, u.DestinationPartition, u.DestinationNamespace - - sni := connect.UpstreamSNI(u, "", dc, cfgSnap.Roots.TrustDomain) - clusterName = CustomizeClusterName(sni, chain) - - } else { - destination, datacenter, partition, namespace = chain.ServiceName, chain.Datacenter, chain.Partition, chain.Namespace - - if cfg.Protocol == "tcp" { - useRDS = false - - startNode := chain.Nodes[chain.StartNode] - if startNode == nil { - return nil, fmt.Errorf("missing first node in compiled discovery chain for: %s", chain.ServiceName) - } - if startNode.Type != structs.DiscoveryGraphNodeTypeResolver { - return nil, fmt.Errorf("unexpected first node in discovery chain using protocol=%q: %s", cfg.Protocol, startNode.Type) - } - targetID := startNode.Resolver.Target - target := chain.Targets[targetID] - - clusterName = CustomizeClusterName(target.Name, chain) - } - } - - // Default the namespace to match how SNIs are generated - if namespace == "" { - namespace = structs.IntentionDefaultNamespace - } - - // Default the partition to match how SNIs are generated - if partition == "" { - partition = structs.IntentionDefaultNamespace - } - filterName := fmt.Sprintf("%s.%s.%s.%s", destination, namespace, partition, datacenter) - - if u.DestinationType == structs.UpstreamDestTypePreparedQuery { - // Avoid encoding dc and namespace for prepared queries. - // Those are defined in the query itself and are not available here. - filterName = upstreamID - } - - opts := listenerFilterOpts{ - useRDS: useRDS, - protocol: cfg.Protocol, - filterName: filterName, - routeName: upstreamID, - cluster: clusterName, - statPrefix: "upstream.", - routePath: "", - httpAuthzFilter: nil, - } - filter, err := makeListenerFilter(opts) - if err != nil { - return nil, err - } - - transportSocket, err := makeDownstreamTLSTransportSocket(tlsContext) - if err != nil { - return nil, err - } - - l.FilterChains = []*envoy_listener_v3.FilterChain{ - { - Filters: []*envoy_listener_v3.Filter{ - filter, - }, - TransportSocket: transportSocket, - }, - } - return l, nil -} - func (s *ResourceGenerator) getAndModifyUpstreamConfigForListener(id string, u *structs.Upstream, chain *structs.CompiledDiscoveryChain) structs.UpstreamConfig { var ( cfg structs.UpstreamConfig diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index 5a494fe99205..0b20c7b364dd 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -2,7 +2,6 @@ package xds import ( "fmt" - envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" envoy_listener_v3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" envoy_tls_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" @@ -54,19 +53,40 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap id := u.Identifier() chain := cfgSnap.IngressGateway.DiscoveryChain[id] + cfg := s.getAndModifyUpstreamConfigForListener(id, &u, chain) + + // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. + // TODO(freddy): Why can the protocol of the listener be overridden here? + useRDS := !chain.IsDefault() && cfg.Protocol != "tcp" + var clusterName string - var upstreamListener proto.Message - upstreamListener, err := s.makeUpstreamListenerForDiscoveryChain( - &u, - address, - chain, - cfgSnap, - tlsContext, - ) + // When not using RDS we must generate a cluster name to attach to the filter chain. + if !useRDS { + target, err := simpleChainTarget(chain) + if err != nil { + return nil, err + } + clusterName = CustomizeClusterName(target.Name, chain) + } + filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) + + l := makePortListenerWithDefault(id, address, u.LocalBindPort, envoy_core_v3.TrafficDirection_OUTBOUND) + filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ + routeName: id, + useRDS: useRDS, + clusterName: clusterName, + filterName: filterName, + protocol: cfg.Protocol, + tlsContext: tlsContext, + }) if err != nil { return nil, err } - resources = append(resources, upstreamListener) + l.FilterChains = []*envoy_listener_v3.FilterChain{ + filterChain, + } + resources = append(resources, l) + } else { // If multiple upstreams share this port, make a special listener for the protocol. listener := makePortListener(listenerKey.Protocol, address, listenerKey.Port, envoy_core_v3.TrafficDirection_OUTBOUND) diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index aa7bf512a496..42d437c2bd17 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -567,7 +567,7 @@ func TestListenersFromSnapshot(t *testing.T) { snap.IngressGateway.Upstreams = map[proxycfg.IngressListenerKey]structs.Upstreams{ {Protocol: "tcp", Port: 8080}: { { - DestinationName: "foo", + DestinationName: "db", LocalBindPort: 8080, }, }, @@ -640,6 +640,32 @@ func TestListenersFromSnapshot(t *testing.T) { }, }, } + + // Every ingress upstream has an associated discovery chain in the snapshot + secureChain := discoverychain.TestCompileConfigEntries( + t, + "secure", + "default", + "default", + "dc1", + connect.TestClusterID+".consul", + "dc1", + nil, + ) + snap.IngressGateway.DiscoveryChain["secure"] = secureChain + + insecureChain := discoverychain.TestCompileConfigEntries( + t, + "insecure", + "default", + "default", + "dc1", + connect.TestClusterID+".consul", + "dc1", + nil, + ) + snap.IngressGateway.DiscoveryChain["insecure"] = insecureChain + snap.IngressGateway.Listeners = map[proxycfg.IngressListenerKey]structs.IngressListener{ {Protocol: "tcp", Port: 8080}: { Port: 8080, diff --git a/agent/xds/testdata/listeners/ingress-with-sds-listener-listener-level.envoy-1-20-x.golden b/agent/xds/testdata/listeners/ingress-with-sds-listener-listener-level.envoy-1-20-x.golden index 8e223cf3ea71..aabf3d6f9884 100644 --- a/agent/xds/testdata/listeners/ingress-with-sds-listener-listener-level.envoy-1-20-x.golden +++ b/agent/xds/testdata/listeners/ingress-with-sds-listener-listener-level.envoy-1-20-x.golden @@ -3,7 +3,7 @@ "resources": [ { "@type": "type.googleapis.com/envoy.config.listener.v3.Listener", - "name": "foo:1.2.3.4:8080", + "name": "db:1.2.3.4:8080", "address": { "socketAddress": { "address": "1.2.3.4", @@ -17,8 +17,8 @@ "name": "envoy.filters.network.tcp_proxy", "typedConfig": { "@type": "type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy", - "statPrefix": "upstream.foo.default.default.dc1", - "cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + "statPrefix": "upstream.db.default.default.dc1", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" } } ], From 8bb3597fac93a795999f7311b78f10a55b4dc741 Mon Sep 17 00:00:00 2001 From: freddygv Date: Wed, 6 Oct 2021 18:14:26 -0600 Subject: [PATCH 11/16] Add test for upstream defaults --- agent/xds/listeners_test.go | 17 +++ ...roxy-upstream-defaults.envoy-1-19-x.golden | 119 ++++++++++++++++++ ...ream-defaults.v2compat.envoy-1-16-x.golden | 119 ++++++++++++++++++ 3 files changed, 255 insertions(+) create mode 100644 agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden create mode 100644 agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index 42d437c2bd17..b89def22c609 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -259,6 +259,23 @@ func TestListenersFromSnapshot(t *testing.T) { create: proxycfg.TestConfigSnapshotDiscoveryChainWithFailoverThroughLocalGateway, setup: nil, }, + { + name: "connect-proxy-upstream-defaults", + create: proxycfg.TestConfigSnapshot, + setup: func(snap *proxycfg.ConfigSnapshot) { + for _, v := range snap.ConnectProxy.UpstreamConfig { + // Prepared queries do not get centrally configured upstream defaults merged into them. + if v.DestinationType == structs.UpstreamDestTypePreparedQuery { + continue + } + // Represent upstream config as if it came from centrally configured upstream defaults. + // The name/namespace must not make it onto the cluster name attached to the outbound listener. + v.CentrallyConfigured = true + v.DestinationNamespace = structs.WildcardSpecifier + v.DestinationName = structs.WildcardSpecifier + } + }, + }, { name: "expose-paths-local-app-paths", create: proxycfg.TestConfigSnapshotExposeConfig, diff --git a/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden new file mode 100644 index 000000000000..57d50f71c367 --- /dev/null +++ b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden @@ -0,0 +1,119 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.config.listener.v3.Listener", + "name": "db: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", + "statPrefix": "upstream.db.default.default.dc1", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + } + ], + "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" +} \ No newline at end of file diff --git a/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden new file mode 100644 index 000000000000..8ed1490a9974 --- /dev/null +++ b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden @@ -0,0 +1,119 @@ +{ + "versionInfo": "00000001", + "resources": [ + { + "@type": "type.googleapis.com/envoy.api.v2.Listener", + "name": "db: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.config.filter.network.tcp_proxy.v2.TcpProxy", + "statPrefix": "upstream.db.default.default.dc1", + "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" + } + } + ] + } + ], + "trafficDirection": "OUTBOUND" + }, + { + "@type": "type.googleapis.com/envoy.api.v2.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.config.filter.network.tcp_proxy.v2.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.api.v2.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.config.filter.network.rbac.v2.RBAC", + "rules": { + + }, + "statPrefix": "connect_authz" + } + }, + { + "name": "envoy.filters.network.tcp_proxy", + "typedConfig": { + "@type": "type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy", + "statPrefix": "public_listener", + "cluster": "local_app" + } + } + ], + "transportSocket": { + "name": "tls", + "typedConfig": { + "@type": "type.googleapis.com/envoy.api.v2.auth.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.api.v2.Listener", + "nonce": "00000001" +} \ No newline at end of file From d76faa11d9c545343cdc6363d899a70202a315b1 Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 7 Oct 2021 09:16:27 -0600 Subject: [PATCH 12/16] Update test --- agent/proxycfg/state_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/proxycfg/state_test.go b/agent/proxycfg/state_test.go index b4d1fc202f28..b172fdee8600 100644 --- a/agent/proxycfg/state_test.go +++ b/agent/proxycfg/state_test.go @@ -1686,6 +1686,7 @@ func TestState_WatchesAndUpdates(t *testing.T) { db.String(): { DestinationName: "db", DestinationNamespace: structs.IntentionDefaultNamespace, + DestinationPartition: structs.IntentionDefaultNamespace, }, } require.Equal(t, expectUpstreams, snap.ConnectProxy.UpstreamConfig) From 2c2160e8544db85eea14688c8a56870467454e1d Mon Sep 17 00:00:00 2001 From: freddygv Date: Thu, 7 Oct 2021 09:35:04 -0600 Subject: [PATCH 13/16] Minor changes to useRDS --- agent/xds/listeners.go | 9 +++------ agent/xds/listeners_ingress.go | 8 +++++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index fd1e1a31ca54..f9ac5015763c 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -108,15 +108,12 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. } // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. - var useRDS bool - if !chain.IsDefault() && chain.Protocol != "tcp" { - useRDS = true - } + useRDS := chain.Protocol != "tcp" && !chain.IsDefault() var clusterName string - - // When not using RDS we must generate a cluster name to attach to the filter chain. if !useRDS { + // When not using RDS we must generate a cluster name to attach to the filter chain. + // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) if err != nil { return nil, err diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index 0b20c7b364dd..6b7e5344bac4 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -57,17 +57,19 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. // TODO(freddy): Why can the protocol of the listener be overridden here? - useRDS := !chain.IsDefault() && cfg.Protocol != "tcp" - var clusterName string + useRDS := cfg.Protocol != "tcp" && !chain.IsDefault() - // When not using RDS we must generate a cluster name to attach to the filter chain. + var clusterName string if !useRDS { + // When not using RDS we must generate a cluster name to attach to the filter chain. + // With RDS, cluster names get attached to the dynamic routes instead. target, err := simpleChainTarget(chain) if err != nil { return nil, err } clusterName = CustomizeClusterName(target.Name, chain) } + filterName := fmt.Sprintf("%s.%s.%s.%s", chain.ServiceName, chain.Namespace, chain.Partition, chain.Datacenter) l := makePortListenerWithDefault(id, address, u.LocalBindPort, envoy_core_v3.TrafficDirection_OUTBOUND) From e62cbd1b0d381fda71ff60ea0b3ba44199238701 Mon Sep 17 00:00:00 2001 From: freddygv Date: Mon, 8 Nov 2021 23:48:04 -0700 Subject: [PATCH 14/16] PR comments --- agent/xds/listeners.go | 7 ++----- agent/xds/listeners_ingress.go | 5 +++++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go index f9ac5015763c..1b1292d64b5b 100644 --- a/agent/xds/listeners.go +++ b/agent/xds/listeners.go @@ -201,11 +201,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. DestinationPartition: sn.PartitionOrDefault(), } - dc := u.Datacenter - if dc == "" { - dc = cfgSnap.Datacenter - } - filterName := fmt.Sprintf("%s.%s.%s.%s", u.DestinationName, u.DestinationNamespace, u.DestinationPartition, dc) + filterName := fmt.Sprintf("%s.%s.%s.%s", u.DestinationName, u.DestinationNamespace, u.DestinationPartition, cfgSnap.Datacenter) filterChain, err := s.makeUpstreamFilterChain(filterChainOpts{ clusterName: "passthrough~" + passthrough.SNI, @@ -281,6 +277,7 @@ func (s *ResourceGenerator) listenersFromSnapshotConnectProxy(cfgSnap *proxycfg. // TODO (SNI partition) add partition for upstream SNI clusterName: connect.UpstreamSNI(u, "", cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain), filterName: id, + routeName: id, protocol: cfg.Protocol, }) if err != nil { diff --git a/agent/xds/listeners_ingress.go b/agent/xds/listeners_ingress.go index 6b7e5344bac4..7668bd02ea89 100644 --- a/agent/xds/listeners_ingress.go +++ b/agent/xds/listeners_ingress.go @@ -53,6 +53,11 @@ func (s *ResourceGenerator) makeIngressGatewayListeners(address string, cfgSnap id := u.Identifier() chain := cfgSnap.IngressGateway.DiscoveryChain[id] + if chain == nil { + // Wait until a chain is present in the snapshot. + continue + } + cfg := s.getAndModifyUpstreamConfigForListener(id, &u, chain) // RDS, Envoy's Route Discovery Service, is only used for HTTP services with a customized discovery chain. From 0c97455c96d89d3d59f87499b752511ce23be78b Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 9 Nov 2021 08:00:48 -0700 Subject: [PATCH 15/16] Regen golden files --- agent/xds/listeners_test.go | 2 - ...oxy-upstream-defaults.envoy-1-20-x.golden} | 0 ...ream-defaults.v2compat.envoy-1-16-x.golden | 119 ------------------ 3 files changed, 121 deletions(-) rename agent/xds/testdata/listeners/{connect-proxy-upstream-defaults.envoy-1-19-x.golden => connect-proxy-upstream-defaults.envoy-1-20-x.golden} (100%) delete mode 100644 agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go index b89def22c609..c082d41ea66d 100644 --- a/agent/xds/listeners_test.go +++ b/agent/xds/listeners_test.go @@ -666,7 +666,6 @@ func TestListenersFromSnapshot(t *testing.T) { "default", "dc1", connect.TestClusterID+".consul", - "dc1", nil, ) snap.IngressGateway.DiscoveryChain["secure"] = secureChain @@ -678,7 +677,6 @@ func TestListenersFromSnapshot(t *testing.T) { "default", "dc1", connect.TestClusterID+".consul", - "dc1", nil, ) snap.IngressGateway.DiscoveryChain["insecure"] = insecureChain diff --git a/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-20-x.golden similarity index 100% rename from agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-19-x.golden rename to agent/xds/testdata/listeners/connect-proxy-upstream-defaults.envoy-1-20-x.golden diff --git a/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden b/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden deleted file mode 100644 index 8ed1490a9974..000000000000 --- a/agent/xds/testdata/listeners/connect-proxy-upstream-defaults.v2compat.envoy-1-16-x.golden +++ /dev/null @@ -1,119 +0,0 @@ -{ - "versionInfo": "00000001", - "resources": [ - { - "@type": "type.googleapis.com/envoy.api.v2.Listener", - "name": "db: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.config.filter.network.tcp_proxy.v2.TcpProxy", - "statPrefix": "upstream.db.default.default.dc1", - "cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul" - } - } - ] - } - ], - "trafficDirection": "OUTBOUND" - }, - { - "@type": "type.googleapis.com/envoy.api.v2.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.config.filter.network.tcp_proxy.v2.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.api.v2.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.config.filter.network.rbac.v2.RBAC", - "rules": { - - }, - "statPrefix": "connect_authz" - } - }, - { - "name": "envoy.filters.network.tcp_proxy", - "typedConfig": { - "@type": "type.googleapis.com/envoy.config.filter.network.tcp_proxy.v2.TcpProxy", - "statPrefix": "public_listener", - "cluster": "local_app" - } - } - ], - "transportSocket": { - "name": "tls", - "typedConfig": { - "@type": "type.googleapis.com/envoy.api.v2.auth.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.api.v2.Listener", - "nonce": "00000001" -} \ No newline at end of file From b4c2d5cf7271019fbf8d03d00a8e09f0e681e002 Mon Sep 17 00:00:00 2001 From: freddygv Date: Tue, 9 Nov 2021 14:38:54 -0700 Subject: [PATCH 16/16] Add changelog entry --- .changelog/11245.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/11245.txt diff --git a/.changelog/11245.txt b/.changelog/11245.txt new file mode 100644 index 000000000000..aacdb1def509 --- /dev/null +++ b/.changelog/11245.txt @@ -0,0 +1,3 @@ +```release-note:bug +connect: fix issue with attempting to generate an invalid upstream cluster from UpstreamConfig.Defaults. +``` \ No newline at end of file