feat(pkg/*): Lay some groundwork for multi cluster gateways.

Multi cluster gateway configurations will have multiple routes with the same service name. This PR introduces a cluster field to the MeshService, which currently always returns true for local. We also change TrafficPolicy names to *always* include the cluster id and the namespace, export a few functions, and introduce a Locality type to determine if a service is being reached from which locality. Signed-off-by: Sean Teeling <[email protected]>
openservicemesh · Jun 12, 2021 · daf03b4 · daf03b4
1 parent c086a47
commit daf03b4
Show file tree

Hide file tree

Showing 13 changed files with 159 additions and 132 deletions.
diff --git a/pkg/catalog/inbound_traffic_policies.go b/pkg/catalog/inbound_traffic_policies.go
@@ -98,12 +98,16 @@ func (mc *MeshCatalog) listInboundPoliciesForTrafficSplits(upstreamIdentity iden
 			apexServices := mc.getApexServicesForBackendService(upstreamSvc)
 			for _, apexService := range apexServices {
 				// build an inbound policy for every apex service
-				hostnames, err := mc.getServiceHostnames(apexService, apexService.Namespace == upstreamServiceAccount.Namespace)
+				locality := service.LocalCluster
+				if apexService.Namespace == upstreamServiceAccount.Namespace {
+					locality = service.LocalNS
+				}
+				hostnames, err := mc.GetServiceHostnames(apexService, locality)
 				if err != nil {
 					log.Error().Err(err).Msgf("Error getting service hostnames for apex service %v", apexService)
 					continue
 				}
-				servicePolicy := trafficpolicy.NewInboundTrafficPolicy(buildPolicyName(apexService, apexService.Namespace == upstreamServiceAccount.Namespace), hostnames)
+				servicePolicy := trafficpolicy.NewInboundTrafficPolicy(apexService.FQDN(), hostnames)
 				weightedCluster := getDefaultWeightedClusterForService(upstreamSvc)
 
 				for _, sourceServiceAccount := range trafficTargetIdentitiesToSvcAccounts(t.Spec.Sources) {
@@ -137,13 +141,13 @@ func (mc *MeshCatalog) buildInboundPolicies(t *access.TrafficTarget, svc service
 		return inboundPolicies
 	}
 
-	hostnames, err := mc.getServiceHostnames(svc, true)
+	hostnames, err := mc.GetServiceHostnames(svc, service.LocalNS)
 	if err != nil {
 		log.Error().Err(err).Msgf("Error getting service hostnames for service %s", svc)
 		return inboundPolicies
 	}
 
-	servicePolicy := trafficpolicy.NewInboundTrafficPolicy(buildPolicyName(svc, false), hostnames)
+	servicePolicy := trafficpolicy.NewInboundTrafficPolicy(svc.FQDN(), hostnames)
 	weightedCluster := getDefaultWeightedClusterForService(svc)
 
 	for _, sourceServiceAccount := range trafficTargetIdentitiesToSvcAccounts(t.Spec.Sources) {
@@ -169,13 +173,13 @@ func (mc *MeshCatalog) buildInboundPolicies(t *access.TrafficTarget, svc service
 func (mc *MeshCatalog) buildInboundPermissiveModePolicies(svc service.MeshService) []*trafficpolicy.InboundTrafficPolicy {
 	var inboundPolicies []*trafficpolicy.InboundTrafficPolicy
 
-	hostnames, err := mc.getServiceHostnames(svc, true)
+	hostnames, err := mc.GetServiceHostnames(svc, service.LocalNS)
 	if err != nil {
 		log.Error().Err(err).Msgf("Error getting service hostnames for service %s", svc)
 		return inboundPolicies
 	}
 
-	servicePolicy := trafficpolicy.NewInboundTrafficPolicy(buildPolicyName(svc, false), hostnames)
+	servicePolicy := trafficpolicy.NewInboundTrafficPolicy(svc.FQDN(), hostnames)
 	weightedCluster := getDefaultWeightedClusterForService(svc)
 
 	// Add a wildcard route to accept traffic from any service account (wildcard service account)
@@ -254,12 +258,3 @@ func (mc *MeshCatalog) getTrafficSpecName(trafficSpecKind string, trafficSpecNam
 	specKey := fmt.Sprintf("%s/%s/%s", trafficSpecKind, trafficSpecNamespace, trafficSpecName)
 	return trafficpolicy.TrafficSpecName(specKey)
 }
-
-// buildPolicyName creates a name for a policy associated with the given service
-func buildPolicyName(svc service.MeshService, sameNamespace bool) string {
-	name := svc.Name
-	if !sameNamespace {
-		return name + "." + svc.Namespace
-	}
-	return name
-}
diff --git a/pkg/catalog/inbound_traffic_policies_test.go b/pkg/catalog/inbound_traffic_policies_test.go
@@ -86,7 +86,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 			trafficSplit: split.TrafficSplit{},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -198,7 +198,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -241,7 +241,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 					},
 				},
 				{
-					Name: "bookstore-apex",
+					Name: "bookstore-apex.default.local",
 					Hostnames: []string{
 						"bookstore-apex",
 						"bookstore-apex.default",
@@ -354,7 +354,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -384,7 +384,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 					},
 				},
 				{
-					Name: "bookstore-apex",
+					Name: "bookstore-apex.default.local",
 					Hostnames: []string{
 						"bookstore-apex",
 						"bookstore-apex.default",
@@ -446,7 +446,7 @@ func TestListInboundTrafficPolicies(t *testing.T) {
 			trafficSplit:        split.TrafficSplit{},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookbuyer.default",
+					Name: "bookbuyer.default.local",
 					Hostnames: []string{
 						"bookbuyer",
 						"bookbuyer.default",
@@ -659,7 +659,7 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore-apex",
+					Name: "bookstore-apex.default.local",
 					Hostnames: []string{
 						"bookstore-apex",
 						"bookstore-apex.default",
@@ -775,7 +775,7 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore-apex",
+					Name: "bookstore-apex.default.local",
 					Hostnames: []string{
 						"bookstore-apex",
 						"bookstore-apex.default",
@@ -892,7 +892,7 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore-apex",
+					Name: "bookstore-apex.default.local",
 					Hostnames: []string{
 						"bookstore-apex",
 						"bookstore-apex.default",
@@ -1034,7 +1034,7 @@ func TestBuildInboundPolicies(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.bookstore-ns",
+					Name: "bookstore.bookstore-ns.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.bookstore-ns",
@@ -1125,7 +1125,7 @@ func TestBuildInboundPolicies(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -1208,7 +1208,7 @@ func TestBuildInboundPolicies(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name:      tests.BookstoreV1Service.Name + "." + tests.BookstoreV1Service.Namespace,
+					Name:      tests.BookstoreV1Service.Name + "." + tests.BookstoreV1Service.Namespace + ".local",
 					Hostnames: tests.BookstoreV1Hostnames,
 					Rules: []*trafficpolicy.Rule{
 						{
@@ -1291,7 +1291,7 @@ func TestBuildInboundPermissiveModePolicies(t *testing.T) {
 			name: "inbound traffic policies for permissive mode",
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.bookstore-ns",
+					Name: "bookstore.bookstore-ns.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.bookstore-ns",
@@ -1412,7 +1412,7 @@ func TestListInboundPoliciesFromTrafficTargets(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -1504,7 +1504,7 @@ func TestListInboundPoliciesFromTrafficTargets(t *testing.T) {
 			},
 			expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
 				{
-					Name: "bookstore.default",
+					Name: "bookstore.default.local",
 					Hostnames: []string{
 						"bookstore",
 						"bookstore.default",
@@ -1844,39 +1844,3 @@ func TestGetTrafficSpecName(t *testing.T) {
 	expected := trafficpolicy.TrafficSpecName(fmt.Sprintf("HTTPRouteGroup/%s/%s", tests.Namespace, tests.RouteGroupName))
 	assert.Equal(actual, expected)
 }
-
-func TestBuildPolicyName(t *testing.T) {
-	assert := tassert.New(t)
-
-	svc := service.MeshService{
-		Namespace: "default",
-		Name:      "foo",
-	}
-
-	testCases := []struct {
-		name          string
-		svc           service.MeshService
-		sameNamespace bool
-		expectedName  string
-	}{
-		{
-			name:          "same namespace",
-			svc:           svc,
-			sameNamespace: true,
-			expectedName:  "foo",
-		},
-		{
-			name:          "different namespace",
-			svc:           svc,
-			sameNamespace: false,
-			expectedName:  "foo.default",
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			actual := buildPolicyName(tc.svc, tc.sameNamespace)
-			assert.Equal(tc.expectedName, actual)
-		})
-	}
-}
diff --git a/pkg/catalog/mock_catalog_generated.go b/pkg/catalog/mock_catalog_generated.go
diff --git a/pkg/catalog/outbound_traffic_policies.go b/pkg/catalog/outbound_traffic_policies.go
@@ -66,12 +66,16 @@ func (mc *MeshCatalog) listOutboundTrafficPoliciesForTrafficSplits(sourceNamespa
 			Namespace: split.Namespace,
 		}
 
-		hostnames, err := mc.getServiceHostnames(svc, svc.Namespace == sourceNamespace)
+		locality := service.LocalCluster
+		if svc.Namespace == sourceNamespace {
+			locality = service.LocalNS
+		}
+		hostnames, err := mc.GetServiceHostnames(svc, locality)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error getting service hostnames for apex service %v", svc)
 			continue
 		}
-		policy := trafficpolicy.NewOutboundTrafficPolicy(buildPolicyName(svc, sourceNamespace == svc.Namespace), hostnames)
+		policy := trafficpolicy.NewOutboundTrafficPolicy(svc.FQDN(), hostnames)
 
 		var weightedClusters []service.WeightedCluster
 		for _, backend := range split.Spec.Backends {
@@ -141,14 +145,15 @@ func (mc *MeshCatalog) buildOutboundPermissiveModePolicies() []*trafficpolicy.Ou
 	}
 
 	for _, destService := range destServices {
-		hostnames, err := mc.getServiceHostnames(destService, false)
+		// TODO(steeling): shouldn't this check the source namespace.... not relevant to this PR though.
+		hostnames, err := mc.GetServiceHostnames(destService, service.LocalCluster)
 		if err != nil {
 			log.Error().Err(err).Msgf("Error getting service hostnames for service %s", destService)
 			continue
 		}
 
 		weightedCluster := getDefaultWeightedClusterForService(destService)
-		policy := trafficpolicy.NewOutboundTrafficPolicy(buildPolicyName(destService, false), hostnames)
+		policy := trafficpolicy.NewOutboundTrafficPolicy(destService.FQDN(), hostnames)
 		if err := policy.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil {
 			log.Error().Err(err).Msgf("Error adding route to outbound policy in permissive mode for destination %s(%s)", destService.Name, destService.Namespace)
 			continue
@@ -182,14 +187,18 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv
 		// Do not build an outbound policy if the destination service is an apex service in a traffic target
 		// this will be handled while building policies from traffic split (with the backend services as weighted clusters)
 		if !mc.isTrafficSplitApexService(destService) {
-			hostnames, err := mc.getServiceHostnames(destService, source.Namespace == destService.Namespace)
+			locality := service.LocalCluster
+			if destService.Namespace == source.Namespace {
+				locality = service.LocalNS
+			}
+			hostnames, err := mc.GetServiceHostnames(destService, locality)
 			if err != nil {
 				log.Error().Err(err).Msgf("Error getting service hostnames for service %s", destService)
 				continue
 			}
 			weightedCluster := getDefaultWeightedClusterForService(destService)
 
-			policy := trafficpolicy.NewOutboundTrafficPolicy(buildPolicyName(destService, source.Namespace == destService.Namespace), hostnames)
+			policy := trafficpolicy.NewOutboundTrafficPolicy(destService.FQDN(), hostnames)
 			needWildCardRoute := false
 			for _, routeMatch := range routeMatches {
 				// If the traffic target has a route with host headers
@@ -292,6 +301,10 @@ func (mc *MeshCatalog) ListMeshServicesForIdentity(identity identity.ServiceIden
 	splitPolicy := mc.meshSpec.ListTrafficSplits()
 
 	for upstreamSvc := range dstServicesSet {
+		// Traffic Splits aren't yet supported for non-local services.
+		if !upstreamSvc.Local() {
+			continue
+		}
 		for _, split := range splitPolicy {
 			// Split policy must be in the same namespace as the upstream service that is a backend
 			if split.Namespace != upstreamSvc.Namespace {