From 98dc578e04b75e428c34a43f787955d2f6d98410 Mon Sep 17 00:00:00 2001
From: Andrew Stucki <andrew.stucki@hashicorp.com>
Date: Mon, 18 Sep 2023 16:19:17 -0400
Subject: [PATCH] Fix gateway services cleanup where proxy deregistration
 happens after service deregistration (#18831)

* Fix gateway services cleanup where proxy deregistration happens after service deregistration

* Add test

* Add changelog

* Fix comment
---
 .changelog/18831.txt               |  3 ++
 agent/consul/state/catalog.go      |  6 +++
 agent/consul/state/catalog_test.go | 79 ++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 .changelog/18831.txt

diff --git a/.changelog/18831.txt b/.changelog/18831.txt
new file mode 100644
index 0000000000..0981fb6a36
--- /dev/null
+++ b/.changelog/18831.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+gateways: Fix a bug where gateway to service mappings weren't being cleaned up properly when externally registered proxies were being deregistered.
+```
diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go
index 003bf409f4..d42d9fc4c8 100644
--- a/agent/consul/state/catalog.go
+++ b/agent/consul/state/catalog.go
@@ -1985,6 +1985,12 @@ func (s *Store) deleteServiceTxn(tx WriteTxn, idx uint64, nodeName, serviceID st
 			if err := cleanupKindServiceName(tx, idx, sn, structs.ServiceKindConnectEnabled); err != nil {
 				return fmt.Errorf("failed to cleanup connect-enabled service name: %v", err)
 			}
+			// we need to do this if the proxy is deleted after the service itself
+			// as the guard after this might not be 1-1 between proxy and service
+			// names.
+			if err := cleanupGatewayWildcards(tx, idx, sn, false); err != nil {
+				return fmt.Errorf("failed to clean up gateway-service associations for %q: %v", psn.String(), err)
+			}
 		}
 	}
 
diff --git a/agent/consul/state/catalog_test.go b/agent/consul/state/catalog_test.go
index b5976bbd2b..501d99aac0 100644
--- a/agent/consul/state/catalog_test.go
+++ b/agent/consul/state/catalog_test.go
@@ -8201,6 +8201,85 @@ func TestCatalog_cleanupGatewayWildcards_panic(t *testing.T) {
 	require.NoError(t, s.DeleteNode(6, "foo", nil, ""))
 }
 
+func TestCatalog_cleanupGatewayWildcards_proxy(t *testing.T) {
+	s := testStateStore(t)
+
+	require.NoError(t, s.EnsureNode(0, &structs.Node{
+		ID:   "c73b8fdf-4ef8-4e43-9aa2-59e85cc6a70c",
+		Node: "foo",
+	}))
+	require.NoError(t, s.EnsureConfigEntry(1, &structs.ProxyConfigEntry{
+		Kind: structs.ProxyDefaults,
+		Name: structs.ProxyConfigGlobal,
+		Config: map[string]interface{}{
+			"protocol": "http",
+		},
+	}))
+
+	defaultMeta := structs.DefaultEnterpriseMetaInDefaultPartition()
+
+	require.NoError(t, s.EnsureConfigEntry(3, &structs.IngressGatewayConfigEntry{
+		Kind: "ingress-gateway",
+		Name: "my-gateway-2-ingress",
+		Listeners: []structs.IngressListener{
+			{
+				Port:     1111,
+				Protocol: "http",
+				Services: []structs.IngressService{
+					{
+						Name:           "*",
+						EnterpriseMeta: *defaultMeta,
+					},
+				},
+			},
+		},
+	}))
+
+	// Register two services, a regular service, and a sidecar proxy for it
+	api := structs.NodeService{
+		ID:             "api",
+		Service:        "api",
+		Address:        "127.0.0.2",
+		Port:           443,
+		EnterpriseMeta: *defaultMeta,
+	}
+	require.NoError(t, s.EnsureService(4, "foo", &api))
+	proxy := structs.NodeService{
+		Kind:    structs.ServiceKindConnectProxy,
+		ID:      "api-proxy",
+		Service: "api-proxy",
+		Address: "127.0.0.3",
+		Port:    443,
+		Proxy: structs.ConnectProxyConfig{
+			DestinationServiceName: "api",
+			DestinationServiceID:   "api",
+		},
+		EnterpriseMeta: *defaultMeta,
+	}
+	require.NoError(t, s.EnsureService(5, "foo", &proxy))
+
+	// make sure we have only one gateway service
+	_, services, err := s.GatewayServices(nil, "my-gateway-2-ingress", defaultMeta)
+	require.NoError(t, err)
+	require.Len(t, services, 1)
+
+	// now delete the target service
+	require.NoError(t, s.DeleteService(6, "foo", "api", nil, ""))
+
+	// at this point we still have the gateway services because we have a connect proxy still
+	_, services, err = s.GatewayServices(nil, "my-gateway-2-ingress", defaultMeta)
+	require.NoError(t, err)
+	require.Len(t, services, 1)
+
+	// now delete the connect proxy
+	require.NoError(t, s.DeleteService(7, "foo", "api-proxy", nil, ""))
+
+	// make sure we no longer have any services
+	_, services, err = s.GatewayServices(nil, "my-gateway-2-ingress", defaultMeta)
+	require.NoError(t, err)
+	require.Len(t, services, 0)
+}
+
 func TestCatalog_DownstreamsForService(t *testing.T) {
 	defaultMeta := structs.DefaultEnterpriseMetaInDefaultPartition()