From fa3a2b76d557a4638ea5fde9cc3b0e8612ef7696 Mon Sep 17 00:00:00 2001
From: Seth Hoenig <shoenig@duck.com>
Date: Mon, 28 Nov 2022 11:19:08 -0600
Subject: [PATCH] consul: fixup expected consul tagged_addresses when using
 ipv6

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 000000000000..369cede3e2e3
--- /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 2a72a3439df6..fd42d75e83b0 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 3723e3db9899..1199e7192754 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)