From 6c9cf7fab3783e900b57ccba585b76fecf6133ec Mon Sep 17 00:00:00 2001
From: Delyan Raychev <delyan.raychev@microsoft.com>
Date: Mon, 19 Jul 2021 14:28:11 -0700
Subject: [PATCH] catalog: Refactor buildOutboundPolicies()

Signed-off-by: Delyan Raychev <delyan.raychev@microsoft.com>
---
 pkg/catalog/outbound_traffic_policies.go | 75 ++++++++++++------------
 1 file changed, 39 insertions(+), 36 deletions(-)

diff --git a/pkg/catalog/outbound_traffic_policies.go b/pkg/catalog/outbound_traffic_policies.go
index 7db9f67d81..fac64f11cf 100644
--- a/pkg/catalog/outbound_traffic_policies.go
+++ b/pkg/catalog/outbound_traffic_policies.go
@@ -210,47 +210,50 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv
 	for _, destService := range destServices {
 		// 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) {
-			locality := service.LocalCluster
-			if destService.Namespace == source.Namespace {
-				locality = service.LocalNS
-			}
-			hostnames, err := mc.GetServiceHostnames(destService, locality)
-			if err != nil {
-				log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()).
-					Msgf("Error getting service hostnames for service %s", destService)
-				continue
-			}
-			weightedCluster := getDefaultWeightedClusterForService(destService)
-
-			policy := trafficpolicy.NewOutboundTrafficPolicy(destService.FQDN(), hostnames)
-			needWildCardRoute := false
-			for _, routeMatch := range routeMatches {
-				// If the traffic target has a route with host headers
-				// we need to create a new outbound traffic policy with the host header as the required hostnames
-				// else the hosnames will be hostnames corresponding to the service
-				if _, ok := routeMatch.Headers[hostHeaderKey]; ok {
-					policyWithHostHeader := trafficpolicy.NewOutboundTrafficPolicy(routeMatch.Headers[hostHeaderKey], []string{routeMatch.Headers[hostHeaderKey]})
-					if err := policyWithHostHeader.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil {
-						log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()).
-							Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s with host header %s", source.Namespace, source.Name, destService.Namespace, destService.Name, routeMatch.Headers[hostHeaderKey])
-						continue
-					}
-					outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policyWithHostHeader)
-				} else {
-					needWildCardRoute = true
-				}
-			}
-			if needWildCardRoute {
-				if err := policy.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil {
+		if mc.isTrafficSplitApexService(destService) {
+			continue
+		}
+
+		locality := service.LocalCluster
+		if destService.Namespace == source.Namespace {
+			locality = service.LocalNS
+		}
+		hostnames, err := mc.GetServiceHostnames(destService, locality)
+		if err != nil {
+			log.Error().Err(err).Str(errcode.Kind, errcode.ErrServiceHostnames.String()).
+				Msgf("Error getting service hostnames for service %s", destService)
+			continue
+		}
+		weightedCluster := getDefaultWeightedClusterForService(destService)
+
+		policy := trafficpolicy.NewOutboundTrafficPolicy(destService.FQDN(), hostnames)
+		needWildCardRoute := false
+		for _, routeMatch := range routeMatches {
+			// If the traffic target has a route with host headers
+			// we need to create a new outbound traffic policy with the host header as the required hostnames
+			// else the hosnames will be hostnames corresponding to the service
+			if _, ok := routeMatch.Headers[hostHeaderKey]; ok {
+				policyWithHostHeader := trafficpolicy.NewOutboundTrafficPolicy(routeMatch.Headers[hostHeaderKey], []string{routeMatch.Headers[hostHeaderKey]})
+				if err := policyWithHostHeader.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil {
 					log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()).
-						Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s", source.Namespace, source.Name, destService.Namespace, destService.Name)
+						Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s with host header %s", source.Namespace, source.Name, destService.Namespace, destService.Name, routeMatch.Headers[hostHeaderKey])
 					continue
 				}
+				outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policyWithHostHeader)
+			} else {
+				needWildCardRoute = true
+			}
+		}
+		if needWildCardRoute {
+			if err := policy.AddRoute(trafficpolicy.WildCardRouteMatch, weightedCluster); err != nil {
+				log.Error().Err(err).Str(errcode.Kind, errcode.ErrAddingRouteToOutboundTrafficPolicy.String()).
+					Msgf("Error adding Route to outbound policy for source %s/%s and destination %s/%s", source.Namespace, source.Name, destService.Namespace, destService.Name)
+				continue
 			}
-
-			outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policy)
 		}
+
+		outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policy)
+
 	}
 	return outboundPolicies
 }