From 53cdda8d17dcdebad7385e2c0c521822fe522f3b Mon Sep 17 00:00:00 2001
From: Eric Haberkorn <erichaberkorn@gmail.com>
Date: Fri, 21 Apr 2023 09:18:32 -0400
Subject: [PATCH] Fix a bug with disco chain config entry fetching (#17078)

Before this change, we were not fetching service resolvers (and therefore
service defaults) configuration entries for services on members of sameness
groups.
---
 agent/consul/state/config_entry.go           | 104 +++++++++++--------
 agent/structs/config_entry_sameness_group.go |   4 +-
 api/config_entry_sameness_group.go           |   4 +-
 3 files changed, 63 insertions(+), 49 deletions(-)

diff --git a/agent/consul/state/config_entry.go b/agent/consul/state/config_entry.go
index 0eed8a36c8..91792c725b 100644
--- a/agent/consul/state/config_entry.go
+++ b/agent/consul/state/config_entry.go
@@ -1554,14 +1554,14 @@ func readDiscoveryChainConfigEntriesTxn(
 	// the end of this function to indicate "no such entry".
 
 	var (
-		todoSplitters      = make(map[structs.ServiceID]struct{})
-		todoResolvers      = make(map[structs.ServiceID]struct{})
-		todoDefaults       = make(map[structs.ServiceID]struct{})
-		todoPeers          = make(map[string]struct{})
-		todoSamenessGroups = make(map[string]struct{})
+		todoSplitters = make(map[structs.ServiceID]struct{})
+		todoResolvers = make(map[structs.ServiceID]struct{})
+		todoDefaults  = make(map[structs.ServiceID]struct{})
+		todoPeers     = make(map[string]struct{})
 	)
 
 	sid := structs.NewServiceID(serviceName, entMeta)
+	peerEntMeta := structs.DefaultEnterpriseMetaInPartition(entMeta.PartitionOrDefault())
 
 	// At every step we'll need service and proxy defaults.
 	todoDefaults[sid] = struct{}{}
@@ -1628,6 +1628,31 @@ func readDiscoveryChainConfigEntriesTxn(
 		}
 	}
 
+	processSamenessGroup := func(sg *structs.SamenessGroupConfigEntry, resolverID structs.ServiceID) error {
+		if sg == nil {
+			return nil
+		}
+
+		res.SamenessGroups[sg.Name] = sg
+
+		for _, peer := range sg.RelatedPeers() {
+			todoPeers[peer] = struct{}{}
+		}
+
+		for _, m := range sg.AllMembers() {
+			if m.Peer != "" {
+				continue
+			}
+
+			// Disco chains preserve the name and namespace from the resolver.
+			em := acl.NewEnterpriseMetaWithPartition(m.Partition, resolverID.NamespaceOrDefault())
+			s := structs.NewServiceID(resolverID.ID, &em)
+			todoResolvers[s] = struct{}{}
+		}
+
+		return nil
+	}
+
 	for {
 		resolverID, ok := anyKey(todoResolvers)
 		if !ok {
@@ -1650,13 +1675,24 @@ func readDiscoveryChainConfigEntriesTxn(
 			maxIdx = idx
 		}
 
+		res.Resolvers[resolverID] = resolver
 		if resolver == nil {
-			res.Resolvers[resolverID] = nil
+			idx, sg, err := getDefaultSamenessGroup(tx, ws, resolverID.PartitionOrDefault())
+			if err != nil {
+				return 0, nil, err
+			}
+			if idx > maxIdx {
+				maxIdx = idx
+			}
+			processSamenessGroup(sg, resolverID)
+
+			if resolverID == sid {
+				res.DefaultSamenessGroup = sg
+			}
+
 			continue
 		}
 
-		res.Resolvers[resolverID] = resolver
-
 		for _, svc := range resolver.ListRelatedServices() {
 			todoResolvers[svc] = struct{}{}
 		}
@@ -1665,8 +1701,22 @@ func readDiscoveryChainConfigEntriesTxn(
 			todoPeers[peer] = struct{}{}
 		}
 
+		// We fetch sameness groups here rather than in another loop because we need the resolvers
+		// for services in different partitions of the local datacenter.
 		for _, sg := range resolver.RelatedSamenessGroups() {
-			todoSamenessGroups[sg] = struct{}{}
+			if _, ok := res.SamenessGroups[sg]; ok {
+				continue
+			}
+
+			idx, entry, err := getSamenessGroupConfigEntryTxn(tx, ws, sg, overrides, peerEntMeta.PartitionOrDefault())
+			if err != nil {
+				return 0, nil, err
+			}
+			if idx > maxIdx {
+				maxIdx = idx
+			}
+
+			processSamenessGroup(entry, resolverID)
 		}
 	}
 
@@ -1709,42 +1759,6 @@ func readDiscoveryChainConfigEntriesTxn(
 		res.Services[svcID] = entry
 	}
 
-	peerEntMeta := structs.DefaultEnterpriseMetaInPartition(entMeta.PartitionOrDefault())
-	for sg := range todoSamenessGroups {
-		idx, entry, err := getSamenessGroupConfigEntryTxn(tx, ws, sg, overrides, peerEntMeta.PartitionOrDefault())
-		if err != nil {
-			return 0, nil, err
-		}
-		if idx > maxIdx {
-			maxIdx = idx
-		}
-		if entry == nil {
-			continue
-		}
-
-		for _, peer := range entry.RelatedPeers() {
-			todoPeers[peer] = struct{}{}
-		}
-		res.SamenessGroups[sg] = entry
-	}
-
-	if r := res.Resolvers[sid]; r == nil {
-		idx, sg, err := getDefaultSamenessGroup(tx, ws, sid.PartitionOrDefault())
-		if err != nil {
-			return 0, nil, err
-		}
-		if idx > maxIdx {
-			maxIdx = idx
-		}
-		if sg != nil {
-			res.DefaultSamenessGroup = sg
-			res.SamenessGroups[sg.Name] = sg
-			for _, peer := range sg.RelatedPeers() {
-				todoPeers[peer] = struct{}{}
-			}
-		}
-	}
-
 	for peerName := range todoPeers {
 		q := Query{
 			Value:          peerName,
diff --git a/agent/structs/config_entry_sameness_group.go b/agent/structs/config_entry_sameness_group.go
index 40f88d5175..a1d0dccb3c 100644
--- a/agent/structs/config_entry_sameness_group.go
+++ b/agent/structs/config_entry_sameness_group.go
@@ -71,6 +71,6 @@ func (s *SamenessGroupConfigEntry) MarshalJSON() ([]byte, error) {
 }
 
 type SamenessGroupMember struct {
-	Partition string
-	Peer      string
+	Partition string `json:",omitempty"`
+	Peer      string `json:",omitempty"`
 }
diff --git a/api/config_entry_sameness_group.go b/api/config_entry_sameness_group.go
index ffc8bd416b..1217efe7d2 100644
--- a/api/config_entry_sameness_group.go
+++ b/api/config_entry_sameness_group.go
@@ -16,8 +16,8 @@ type SamenessGroupConfigEntry struct {
 }
 
 type SamenessGroupMember struct {
-	Partition string
-	Peer      string
+	Partition string `json:",omitempty"`
+	Peer      string `json:",omitempty"`
 }
 
 func (s *SamenessGroupConfigEntry) GetKind() string            { return s.Kind }