Simplified uniqueness logic for exported services, sorted the respons…

…e in order of service name

agent/config_endpoint_test.go

@@ -813,6 +813,6 @@ func TestConfig_Exported_Services(t *testing.T) {
 				},
 			},
 		}
-		require.ElementsMatch(t, expected, services)
+		require.Equal(t, expected, services)
 	})
 }

agent/consul/state/config_entry_exported_services.go

@@ -5,6 +5,7 @@ package state
 
 import (
 	"fmt"
+	"sort"
 
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/configentry"
@@ -88,21 +89,7 @@ func resolvedExportedServicesTxn(tx ReadTxn, ws memdb.WatchSet, entMeta *acl.Ent
 		return maxIdx, nil, nil
 	}
 
-	// Services -> ServiceConsumers
-	var exportedServices = make(map[structs.ServiceName]map[structs.ServiceConsumer]struct{})
-
-	// Helper function for inserting data and auto-creating maps.
-	insertEntry := func(m map[structs.ServiceName]map[structs.ServiceConsumer]struct{}, service structs.ServiceName, consumers []structs.ServiceConsumer) {
-		for _, c := range consumers {
-			cons, ok := m[service]
-			if !ok {
-				cons = make(map[structs.ServiceConsumer]struct{})
-				m[service] = cons
-			}
-
-			cons[c] = struct{}{}
-		}
-	}
+	var exportedServices []structs.ExportedService
 
 	for _, svc := range exports.Services {
 		// Prevent exporting the "consul" service.
@@ -111,11 +98,10 @@ func resolvedExportedServicesTxn(tx ReadTxn, ws memdb.WatchSet, entMeta *acl.Ent
 		}
 
 		svcEntMeta := acl.NewEnterpriseMetaWithPartition(entMeta.PartitionOrDefault(), svc.Namespace)
-		svcName := structs.NewServiceName(svc.Name, &svcEntMeta)
 
-		// If this isn't a wildcard, we can simply add it to the list of services to watch and move to the next entry.
+		// If this isn't a wildcard, we can simply add it to the list of exportedServices and move to the next entry.
 		if svc.Name != structs.WildcardSpecifier {
-			insertEntry(exportedServices, svcName, svc.Consumers)
+			exportedServices = append(exportedServices, svc)
 			continue
 		}
 
@@ -130,12 +116,59 @@ func resolvedExportedServicesTxn(tx ReadTxn, ws memdb.WatchSet, entMeta *acl.Ent
 		for _, sn := range typicalServices {
 			// Prevent exporting the "consul" service.
 			if sn.Service.Name != structs.ConsulServiceName {
-				insertEntry(exportedServices, sn.Service, svc.Consumers)
+				exportedServices = append(exportedServices, structs.ExportedService{
+					Name:      sn.Service.Name,
+					Namespace: sn.Service.NamespaceOrDefault(),
+					Consumers: svc.Consumers,
+				})
 			}
 		}
 	}
 
-	resp = prepareExportedServicesResponse(exportedServices)
+	uniqueExportedServices := getUniqueExportedServices(exportedServices, entMeta)
+	resp = prepareExportedServicesResponse(uniqueExportedServices, entMeta)
 
 	return maxIdx, resp, nil
 }
+
+// getUniqueExportedServices removes duplicate services and consumers. Services are also sorted in ascending order
+func getUniqueExportedServices(exportedServices []structs.ExportedService, entMeta *acl.EnterpriseMeta) []structs.ExportedService {
+	// Services -> ServiceConsumers
+	var exportedServicesMapper = make(map[structs.ServiceName]map[structs.ServiceConsumer]struct{})
+	for _, svc := range exportedServices {
+		svcEntMeta := acl.NewEnterpriseMetaWithPartition(entMeta.PartitionOrDefault(), svc.Namespace)
+		svcName := structs.NewServiceName(svc.Name, &svcEntMeta)
+
+		for _, c := range svc.Consumers {
+			cons, ok := exportedServicesMapper[svcName]
+			if !ok {
+				cons = make(map[structs.ServiceConsumer]struct{})
+				exportedServicesMapper[svcName] = cons
+			}
+			cons[c] = struct{}{}
+		}
+	}
+
+	uniqueExportedServices := make([]structs.ExportedService, 0, len(exportedServicesMapper))
+
+	for svc, cons := range exportedServicesMapper {
+		consumers := make([]structs.ServiceConsumer, 0, len(cons))
+		for con := range cons {
+			consumers = append(consumers, con)
+		}
+
+		uniqueExportedServices = append(uniqueExportedServices, structs.ExportedService{
+			Name:      svc.Name,
+			Namespace: svc.NamespaceOrDefault(),
+			Consumers: consumers,
+		})
+
+	}
+
+	sort.Slice(uniqueExportedServices, func(i, j int) bool {
+		return (uniqueExportedServices[i].Name < uniqueExportedServices[j].Name) ||
+			(uniqueExportedServices[i].Name == uniqueExportedServices[j].Name && uniqueExportedServices[i].Namespace < uniqueExportedServices[j].Namespace)
+	})
+
+	return uniqueExportedServices
+}

agent/consul/state/config_entry_exported_services_ce.go

@@ -35,26 +35,27 @@ func (s *Store) GetSimplifiedExportedServices(ws memdb.WatchSet, entMeta acl.Ent
 	return getSimplifiedExportedServices(tx, ws, nil, entMeta)
 }
 
-func prepareExportedServicesResponse(exportedServices map[structs.ServiceName]map[structs.ServiceConsumer]struct{}) []*pbconfigentry.ResolvedExportedService {
-	var resp []*pbconfigentry.ResolvedExportedService
+func prepareExportedServicesResponse(exportedServices []structs.ExportedService, entMeta *acl.EnterpriseMeta) []*pbconfigentry.ResolvedExportedService {
 
-	for svc, consumers := range exportedServices {
+	resp := make([]*pbconfigentry.ResolvedExportedService, len(exportedServices))
+
+	for idx, exportedService := range exportedServices {
 		consumerPeers := []string{}
 
-		for consumer := range consumers {
+		for _, consumer := range exportedService.Consumers {
 			if consumer.Peer != "" {
 				consumerPeers = append(consumerPeers, consumer.Peer)
 			}
 		}
 
 		sort.Strings(consumerPeers)
 
-		resp = append(resp, &pbconfigentry.ResolvedExportedService{
-			Service: svc.Name,
+		resp[idx] = &pbconfigentry.ResolvedExportedService{
+			Service: exportedService.Name,
 			Consumers: &pbconfigentry.Consumers{
 				Peers: consumerPeers,
 			},
-		})
+		}
 	}
 
 	return resp

agent/consul/state/config_entry_exported_services_ce_test.go

@@ -15,22 +15,36 @@ import (
 )
 
 func TestStore_prepareExportedServicesResponse(t *testing.T) {
-	var exportedServices = make(map[structs.ServiceName]map[structs.ServiceConsumer]struct{})
 
-	svc1 := structs.NewServiceName("db", nil)
-	exportedServices[svc1] = make(map[structs.ServiceConsumer]struct{})
-	exportedServices[svc1][structs.ServiceConsumer{Peer: "west"}] = struct{}{}
-	exportedServices[svc1][structs.ServiceConsumer{Peer: "east"}] = struct{}{}
-
-	// Adding partition to ensure that it's not included in response
-	exportedServices[svc1][structs.ServiceConsumer{Partition: "east"}] = struct{}{}
-
-	svc2 := structs.NewServiceName("web", nil)
-	exportedServices[svc2] = make(map[structs.ServiceConsumer]struct{})
-	exportedServices[svc2][structs.ServiceConsumer{Peer: "peer-a"}] = struct{}{}
-	exportedServices[svc2][structs.ServiceConsumer{Peer: "peer-b"}] = struct{}{}
+	exportedServices := []structs.ExportedService{
+		{
+			Name: "db",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "west",
+				},
+				{
+					Peer: "east",
+				},
+				{
+					Partition: "part",
+				},
+			},
+		},
+		{
+			Name: "web",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "peer-a",
+				},
+				{
+					Peer: "peer-b",
+				},
+			},
+		},
+	}
 
-	resp := prepareExportedServicesResponse(exportedServices)
+	resp := prepareExportedServicesResponse(exportedServices, nil)
 
 	expected := []*pbconfigentry.ResolvedExportedService{
 		{
@@ -47,7 +61,7 @@ func TestStore_prepareExportedServicesResponse(t *testing.T) {
 		},
 	}
 
-	require.ElementsMatch(t, expected, resp)
+	require.Equal(t, expected, resp)
 }
 
 func TestStore_ResolvedExportingServices(t *testing.T) {
@@ -122,22 +136,22 @@ func TestStore_ResolvedExportingServices(t *testing.T) {
 		idx, services, err := s.ResolvedExportedServices(ws, defaultMeta)
 		require.NoError(t, err)
 		require.Equal(t, tc.idx, idx)
-		require.ElementsMatch(t, tc.expect, services)
+		require.Equal(t, tc.expect, services)
 	}
 
 	t.Run("only exported services are included", func(t *testing.T) {
 		tc := testCase{
 			expect: []*pbconfigentry.ResolvedExportedService{
 				{
-					Service: "db",
+					Service: "cache",
 					Consumers: &pbconfigentry.Consumers{
-						Peers: []string{"east", "west"},
+						Peers: []string{"east"},
 					},
 				},
 				{
-					Service: "cache",
+					Service: "db",
 					Consumers: &pbconfigentry.Consumers{
-						Peers: []string{"east"},
+						Peers: []string{"east", "west"},
 					},
 				},
 			},
@@ -166,7 +180,7 @@ func TestStore_ResolvedExportingServices(t *testing.T) {
 		tc := testCase{
 			expect: []*pbconfigentry.ResolvedExportedService{
 				{
-					Service: "db",
+					Service: "backend",
 					Consumers: &pbconfigentry.Consumers{
 						Peers: []string{"west"},
 					},
@@ -178,13 +192,14 @@ func TestStore_ResolvedExportingServices(t *testing.T) {
 					},
 				},
 				{
-					Service: "frontend",
+					Service: "db",
 					Consumers: &pbconfigentry.Consumers{
 						Peers: []string{"west"},
 					},
 				},
+
 				{
-					Service: "backend",
+					Service: "frontend",
 					Consumers: &pbconfigentry.Consumers{
 						Peers: []string{"west"},
 					},
@@ -207,3 +222,96 @@ func TestStore_ResolvedExportingServices(t *testing.T) {
 		require.Nil(t, result)
 	})
 }
+
+func TestStore_getUniqueExportedServices(t *testing.T) {
+
+	exportedServices := []structs.ExportedService{
+		{
+			Name: "db",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "west",
+				},
+				{
+					Peer: "east",
+				},
+				{
+					Partition: "part",
+				},
+			},
+		},
+		{
+			Name: "web",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "peer-a",
+				},
+				{
+					Peer: "peer-b",
+				},
+			},
+		},
+		{
+			Name: "db",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "west",
+				},
+				{
+					Peer: "west-2",
+				},
+			},
+		},
+		{
+			Name: "db",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "west",
+				},
+				{
+					Peer: "west-2",
+				},
+			},
+		},
+	}
+
+	resp := getUniqueExportedServices(exportedServices, nil)
+
+	expected := []structs.ExportedService{
+		{
+			Name: "db",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "west",
+				},
+				{
+					Peer: "east",
+				},
+				{
+					Partition: "part",
+				},
+				{
+					Peer: "west-2",
+				},
+			},
+		},
+		{
+			Name: "web",
+			Consumers: []structs.ServiceConsumer{
+				{
+					Peer: "peer-a",
+				},
+				{
+					Peer: "peer-b",
+				},
+			},
+		},
+	}
+
+	require.Equal(t, 2, len(resp))
+
+	for idx, expSvc := range expected {
+		require.Equal(t, expSvc.Name, resp[idx].Name)
+		require.ElementsMatch(t, expSvc.Consumers, resp[idx].Consumers)
+	}
+}

agent/grpc-external/services/configentry/server_ce_test.go

@@ -63,21 +63,21 @@ func TestGetResolvedExportedServices(t *testing.T) {
 
 	expected := []*pbconfigentry.ResolvedExportedService{
 		{
-			Service: "db",
+			Service: "cache",
 			Consumers: &pbconfigentry.Consumers{
-				Peers: []string{"east", "west"},
+				Peers: []string{"east"},
 			},
 		},
 		{
-			Service: "cache",
+			Service: "db",
 			Consumers: &pbconfigentry.Consumers{
-				Peers: []string{"east"},
+				Peers: []string{"east", "west"},
 			},
 		},
 	}
 
 	ctx := grpc.NewContextWithServerTransportStream(context.Background(), &testutils.MockServerTransportStream{})
 	resp, err := server.GetResolvedExportedServices(ctx, &pbconfigentry.GetResolvedExportedServicesRequest{})
 	require.NoError(t, err)
-	require.ElementsMatch(t, expected, resp.Services)
+	require.Equal(t, expected, resp.Services)
 }