From 05d767b8d6501afaa36451f5424cdc710927d07b Mon Sep 17 00:00:00 2001
From: "R.B. Boyer" <rb@hashicorp.com>
Date: Fri, 5 Feb 2021 16:28:07 -0600
Subject: [PATCH] xds: deduplicate mesh gateway listeners in a stable way
 (#9650)

In a situation where the mesh gateway is configured to bind to multiple
network interfaces, we use a feature called 'tagged addresses'.
Sometimes an address is duplicated across multiple tags such as 'lan'
and 'lan_ipv4'.

There is code to deduplicate these things when creating envoy listeners,
but that code doesn't ensure that the same tag wins every time. If the
winning tag flaps between xDS discovery requests it will cause the
listener to be drained and replaced.
---
 .changelog/9650.txt         |  3 +++
 agent/xds/listeners.go      | 38 +++++++++++++++++++++----------------
 agent/xds/listeners_test.go | 10 +++++-----
 3 files changed, 30 insertions(+), 21 deletions(-)
 create mode 100644 .changelog/9650.txt

diff --git a/.changelog/9650.txt b/.changelog/9650.txt
new file mode 100644
index 000000000..f0d404a68
--- /dev/null
+++ b/.changelog/9650.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+xds: deduplicate mesh gateway listeners by address in a stable way to prevent some LDS churn
+```
diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go
index 78df10a92..23f6613d8 100644
--- a/agent/xds/listeners.go
+++ b/agent/xds/listeners.go
@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/url"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -199,13 +200,11 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 		s.Logger.Warn("failed to parse Connect.Proxy.Config", "error", err)
 	}
 
-	// Prevent invalid configurations of binding to the same port/addr twice
-	// including with the any addresses
+	// We'll collect all of the desired listeners first, and deduplicate them later.
 	type namedAddress struct {
 		name string
 		structs.ServiceAddress
 	}
-	seen := make(map[structs.ServiceAddress]bool)
 	addrs := make([]namedAddress, 0)
 
 	var resources []proto.Message
@@ -219,10 +218,7 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 			Address: addr,
 			Port:    cfgSnap.Port,
 		}
-		if !seen[a] {
-			addrs = append(addrs, namedAddress{name: "default", ServiceAddress: a})
-			seen[a] = true
-		}
+		addrs = append(addrs, namedAddress{name: "default", ServiceAddress: a})
 	}
 
 	if cfg.BindTaggedAddresses {
@@ -231,10 +227,7 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 				Address: addrCfg.Address,
 				Port:    addrCfg.Port,
 			}
-			if !seen[a] {
-				addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
-				seen[a] = true
-			}
+			addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
 		}
 	}
 
@@ -243,14 +236,27 @@ func (s *Server) listenersFromSnapshotGateway(cInfo connectionInfo, cfgSnap *pro
 			Address: addrCfg.Address,
 			Port:    addrCfg.Port,
 		}
-		if !seen[a] {
-			addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
-			seen[a] = true
-		}
+		addrs = append(addrs, namedAddress{name: name, ServiceAddress: a})
 	}
 
-	// Make listeners once deduplicated
+	// Prevent invalid configurations of binding to the same port/addr twice
+	// including with the any addresses
+	//
+	// Sort the list and then if two items share a service address, take the
+	// first one to ensure we generate one listener per address and it's
+	// stable.
+	sort.Slice(addrs, func(i, j int) bool {
+		return addrs[i].name < addrs[j].name
+	})
+
+	// Make listeners and deduplicate on the fly.
+	seen := make(map[structs.ServiceAddress]bool)
 	for _, a := range addrs {
+		if seen[a.ServiceAddress] {
+			continue
+		}
+		seen[a.ServiceAddress] = true
+
 		var l *envoy.Listener
 
 		switch cfgSnap.Kind {
diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go
index 9ac9a5e24..0e9c4b932 100644
--- a/agent/xds/listeners_test.go
+++ b/agent/xds/listeners_test.go
@@ -418,15 +418,15 @@ func TestListenersFromSnapshot(t *testing.T) {
 					"envoy_gateway_no_default_bind":       true,
 					"envoy_gateway_bind_tagged_addresses": true,
 					"envoy_gateway_bind_addresses": map[string]structs.ServiceAddress{
+						// This bind address should not get a listener due to deduplication and it sorts to the end
+						"z-duplicate-of-tagged-wan-addr": {
+							Address: "198.18.0.1",
+							Port:    443,
+						},
 						"foo": {
 							Address: "198.17.2.3",
 							Port:    8080,
 						},
-						// This bind address should not get a listener due to deduplication
-						"duplicate-of-tagged-wan-addr": {
-							Address: "198.18.0.1",
-							Port:    443,
-						},
 					},
 				}
 			},