From 119f7b1cd10b76c4cfc9e05f607c9bd96bb2efd5 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Thu, 1 Dec 2022 07:38:30 -0600
Subject: [PATCH] consul: fixup expected consul tagged_addresses when using
 ipv6 (#15411)

This PR is a continuation of #14917, where we missed the ipv6 cases.

Consul auto-inserts tagged_addresses for keys
- lan_ipv4
- wan_ipv4
- lan_ipv6
- wan_ipv6

even though the service registration coming from Nomad does not contain such
elements. When doing the differential between services Nomad expects to be
registered vs. the services actually registered into Consul, we must first
purge these automatically inserted tagged_addresses if they do not exist in
the Nomad view of the Consul service.
---
 .changelog/15411.txt                        |   3 +
 command/agent/consul/service_client.go      |   6 ++
 command/agent/consul/service_client_test.go | 107 ++++++++++++++++++++
 3 files changed, 116 insertions(+)
 create mode 100644 .changelog/15411.txt

diff --git a/.changelog/15411.txt b/.changelog/15411.txt
new file mode 100644
index 000000000..369cede3e
--- /dev/null
+++ b/.changelog/15411.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+consul: Fixed a bug where services would continuously re-register when using ipv6
+```
diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go
index 2a72a3439..fd42d75e8 100644
--- a/command/agent/consul/service_client.go
+++ b/command/agent/consul/service_client.go
@@ -225,6 +225,12 @@ func maybeTweakTaggedAddresses(wanted *api.AgentServiceRegistration, existing *a
 		if _, exists := wanted.TaggedAddresses["wan_ipv4"]; !exists {
 			delete(existing.TaggedAddresses, "wan_ipv4")
 		}
+		if _, exists := wanted.TaggedAddresses["lan_ipv6"]; !exists {
+			delete(existing.TaggedAddresses, "lan_ipv6")
+		}
+		if _, exists := wanted.TaggedAddresses["wan_ipv6"]; !exists {
+			delete(existing.TaggedAddresses, "wan_ipv6")
+		}
 	}
 }
 
diff --git a/command/agent/consul/service_client_test.go b/command/agent/consul/service_client_test.go
index 3723e3db9..1199e7192 100644
--- a/command/agent/consul/service_client_test.go
+++ b/command/agent/consul/service_client_test.go
@@ -13,8 +13,115 @@ import (
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/shoenig/test/must"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
 )
 
+func TestSyncLogic_maybeTweakTaggedAddresses(t *testing.T) {
+	ci.Parallel(t)
+
+	cases := []struct {
+		name     string
+		wanted   map[string]api.ServiceAddress
+		existing map[string]api.ServiceAddress
+		id       string
+		exp      []string
+	}{
+		{
+			name:   "not managed by nomad",
+			id:     "_nomad-other-hello",
+			wanted: map[string]api.ServiceAddress{
+				// empty
+			},
+			existing: map[string]api.ServiceAddress{
+				"lan_ipv4": {},
+				"wan_ipv4": {},
+				"custom":   {},
+			},
+			exp: []string{"lan_ipv4", "wan_ipv4", "custom"},
+		},
+		{
+			name: "remove defaults",
+			id:   "_nomad-task-hello",
+			wanted: map[string]api.ServiceAddress{
+				"lan_custom": {},
+				"wan_custom": {},
+			},
+			existing: map[string]api.ServiceAddress{
+				"lan_ipv4":   {},
+				"wan_ipv4":   {},
+				"lan_ipv6":   {},
+				"wan_ipv6":   {},
+				"lan_custom": {},
+				"wan_custom": {},
+			},
+			exp: []string{"lan_custom", "wan_custom"},
+		},
+		{
+			name: "overridden defaults",
+			id:   "_nomad-task-hello",
+			wanted: map[string]api.ServiceAddress{
+				"lan_ipv4": {},
+				"wan_ipv4": {},
+				"lan_ipv6": {},
+				"wan_ipv6": {},
+				"custom":   {},
+			},
+			existing: map[string]api.ServiceAddress{
+				"lan_ipv4": {},
+				"wan_ipv4": {},
+				"lan_ipv6": {},
+				"wan_ipv6": {},
+				"custom":   {},
+			},
+			exp: []string{"lan_ipv4", "wan_ipv4", "lan_ipv6", "wan_ipv6", "custom"},
+		},
+		{
+			name: "applies to nomad client",
+			id:   "_nomad-client-12345",
+			wanted: map[string]api.ServiceAddress{
+				"custom": {},
+			},
+			existing: map[string]api.ServiceAddress{
+				"lan_ipv4": {},
+				"wan_ipv4": {},
+				"lan_ipv6": {},
+				"wan_ipv6": {},
+				"custom":   {},
+			},
+			exp: []string{"custom"},
+		},
+		{
+			name: "applies to nomad server",
+			id:   "_nomad-server-12345",
+			wanted: map[string]api.ServiceAddress{
+				"custom": {},
+			},
+			existing: map[string]api.ServiceAddress{
+				"lan_ipv4": {},
+				"wan_ipv4": {},
+				"lan_ipv6": {},
+				"wan_ipv6": {},
+				"custom":   {},
+			},
+			exp: []string{"custom"},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			asr := &api.AgentServiceRegistration{
+				ID:              tc.id,
+				TaggedAddresses: maps.Clone(tc.wanted),
+			}
+			as := &api.AgentService{
+				TaggedAddresses: maps.Clone(tc.existing),
+			}
+			maybeTweakTaggedAddresses(asr, as)
+			must.MapContainsKeys(t, as.TaggedAddresses, tc.exp)
+		})
+	}
+}
+
 func TestSyncLogic_agentServiceUpdateRequired(t *testing.T) {
 	ci.Parallel(t)