feat(*): add traffic split in permissive mode

resolves #2527

Signed-off-by: Michelle Noorali <minooral@microsoft.com>

pkg/catalog/inbound_traffic_policies.go

@@ -26,16 +26,17 @@ const (
 // 2. for the given service account and upstream services from SMI Traffic Target and Traffic Split
 // Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
 func (mc *MeshCatalog) ListInboundTrafficPolicies(upstreamIdentity identity.ServiceIdentity, upstreamServices []service.MeshService) []*trafficpolicy.InboundTrafficPolicy {
+	inboundPoliciesFromSplits := mc.listInboundPoliciesForTrafficSplits(upstreamIdentity, upstreamServices)
+
 	if mc.configurator.IsPermissiveTrafficPolicyMode() {
 		var inboundPolicies []*trafficpolicy.InboundTrafficPolicy
 		for _, svc := range upstreamServices {
-			inboundPolicies = trafficpolicy.MergeInboundPolicies(DisallowPartialHostnamesMatch, inboundPolicies, mc.buildInboundPermissiveModePolicies(svc)...)
+			inboundPolicies = trafficpolicy.MergeInboundPolicies(DisallowPartialHostnamesMatch, mc.buildInboundPermissiveModePolicies(svc), inboundPoliciesFromSplits...)
 		}
 		return inboundPolicies
 	}
 
 	inbound := mc.listInboundPoliciesFromTrafficTargets(upstreamIdentity, upstreamServices)
-	inboundPoliciesFromSplits := mc.listInboundPoliciesForTrafficSplits(upstreamIdentity, upstreamServices)
 	inbound = trafficpolicy.MergeInboundPolicies(AllowPartialHostnamesMatch, inbound, inboundPoliciesFromSplits...)
 	return inbound
 }

pkg/catalog/outbound_traffic_policies.go

@@ -18,18 +18,17 @@ import (
 // Note: ServiceIdentity must be in the format "name.namespace" [https://github.com/openservicemesh/osm/issues/3188]
 func (mc *MeshCatalog) ListOutboundTrafficPolicies(downstreamIdentity identity.ServiceIdentity) []*trafficpolicy.OutboundTrafficPolicy {
 	downstreamServiceAccount := downstreamIdentity.ToK8sServiceAccount()
-	if mc.configurator.IsPermissiveTrafficPolicyMode() {
-		var outboundPolicies []*trafficpolicy.OutboundTrafficPolicy
-		mergedPolicies := trafficpolicy.MergeOutboundPolicies(DisallowPartialHostnamesMatch, outboundPolicies, mc.buildOutboundPermissiveModePolicies(downstreamServiceAccount.Namespace)...)
-		outboundPolicies = mergedPolicies
-		return outboundPolicies
-	}
 
-	outbound := mc.listOutboundPoliciesForTrafficTargets(downstreamIdentity)
 	outboundPoliciesFromSplits := mc.listOutboundTrafficPoliciesForTrafficSplits(downstreamServiceAccount.Namespace)
-	outbound = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outbound, outboundPoliciesFromSplits...)
 
-	return outbound
+	if mc.configurator.IsPermissiveTrafficPolicyMode() {
+		permissiveOutboundPolicies := mc.buildOutboundPermissiveModePolicies(downstreamServiceAccount.Namespace)
+		//TODO: I don't see a reason to disallow partial hostname match. Will follow up on this.
+		return trafficpolicy.MergeOutboundPolicies(DisallowPartialHostnamesMatch, permissiveOutboundPolicies, outboundPoliciesFromSplits...)
+	}
+
+	allowedOutboundPolicies := mc.listOutboundPoliciesForTrafficTargets(downstreamIdentity)
+	return trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, allowedOutboundPolicies, outboundPoliciesFromSplits...)
 }
 
 // listOutboundPoliciesForTrafficTargets loops through all SMI Traffic Target resources and returns outbound traffic policies

pkg/catalog/outbound_traffic_policies_test.go

@@ -328,6 +328,95 @@ func TestListOutboundTrafficPolicies(t *testing.T) {
 			},
 			permissiveMode: true,
 		},
+		{
+			name:                "permissive mode with traffic splits",
+			downstreamSA:        tests.BookbuyerServiceIdentity,
+			apexMeshServices:    []service.MeshService{tests.BookstoreApexService},
+			meshServices:        []service.MeshService{tests.BookstoreV1Service, tests.BookstoreV2Service, tests.BookbuyerService},
+			meshServiceAccounts: []identity.K8sServiceAccount{tests.BookbuyerServiceAccount, tests.BookstoreServiceAccount},
+			trafficsplits:       []*split.TrafficSplit{&tests.TrafficSplit},
+			traffictargets:      []*access.TrafficTarget{},
+			trafficspecs:        []*spec.HTTPRouteGroup{},
+			expectedOutbound: []*trafficpolicy.OutboundTrafficPolicy{
+				{
+					Name:      "bookstore-apex.default.svc.cluster.local",
+					Hostnames: tests.BookstoreApexHostnames,
+					Routes: []*trafficpolicy.RouteWeightedClusters{
+						{
+							HTTPRouteMatch: tests.WildCardRouteMatch,
+							WeightedClusters: mapset.NewSetFromSlice([]interface{}{
+								service.WeightedCluster{ClusterName: "default/bookstore-v1", Weight: 90},
+								service.WeightedCluster{ClusterName: "default/bookstore-v2", Weight: 10},
+							}),
+						},
+					},
+				},
+				{
+					Name: "bookstore-v1.default.svc.cluster.local",
+					Hostnames: []string{
+						"bookstore-v1",
+						"bookstore-v1.default",
+						"bookstore-v1.default.svc",
+						"bookstore-v1.default.svc.cluster",
+						"bookstore-v1.default.svc.cluster.local",
+						"bookstore-v1:8888",
+						"bookstore-v1.default:8888",
+						"bookstore-v1.default.svc:8888",
+						"bookstore-v1.default.svc.cluster:8888",
+						"bookstore-v1.default.svc.cluster.local:8888",
+					},
+					Routes: []*trafficpolicy.RouteWeightedClusters{
+						{
+							HTTPRouteMatch:   tests.WildCardRouteMatch,
+							WeightedClusters: mapset.NewSet(tests.BookstoreV1DefaultWeightedCluster),
+						},
+					},
+				},
+				{
+					Name: "bookstore-v2.default.svc.cluster.local",
+					Hostnames: []string{
+						"bookstore-v2",
+						"bookstore-v2.default",
+						"bookstore-v2.default.svc",
+						"bookstore-v2.default.svc.cluster",
+						"bookstore-v2.default.svc.cluster.local",
+						"bookstore-v2:8888",
+						"bookstore-v2.default:8888",
+						"bookstore-v2.default.svc:8888",
+						"bookstore-v2.default.svc.cluster:8888",
+						"bookstore-v2.default.svc.cluster.local:8888",
+					},
+					Routes: []*trafficpolicy.RouteWeightedClusters{
+						{
+							HTTPRouteMatch:   tests.WildCardRouteMatch,
+							WeightedClusters: mapset.NewSet(tests.BookstoreV2DefaultWeightedCluster),
+						},
+					},
+				},
+				{
+					Name: "bookbuyer.default.svc.cluster.local",
+					Hostnames: []string{
+						"bookbuyer",
+						"bookbuyer.default",
+						"bookbuyer.default.svc",
+						"bookbuyer.default.svc.cluster",
+						"bookbuyer.default.svc.cluster.local",
+						"bookbuyer:8888",
+						"bookbuyer.default:8888",
+						"bookbuyer.default.svc:8888",
+						"bookbuyer.default.svc.cluster:8888",
+						"bookbuyer.default.svc.cluster.local:8888",
+					},
+					Routes: []*trafficpolicy.RouteWeightedClusters{
+						{
+							HTTPRouteMatch:   tests.WildCardRouteMatch,
+							WeightedClusters: mapset.NewSet(tests.BookbuyerDefaultWeightedCluster),
+						},
+					},
+				},
+			},
+			permissiveMode: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -364,6 +453,7 @@ func TestListOutboundTrafficPolicies(t *testing.T) {
 				}
 				mockKubeController.EXPECT().ListServices().Return(services).AnyTimes()
 				mockKubeController.EXPECT().ListServiceAccounts().Return(serviceAccounts).AnyTimes()
+				mockMeshSpec.EXPECT().ListTrafficSplits().Return(tc.trafficsplits).AnyTimes()
 			} else {
 				mockMeshSpec.EXPECT().ListTrafficSplits().Return(tc.trafficsplits).AnyTimes()
 				mockMeshSpec.EXPECT().ListTrafficTargets().Return(tc.traffictargets).AnyTimes()

pkg/envoy/cds/cluster.go

@@ -81,16 +81,10 @@ func getUpstreamServiceCluster(downstreamIdentity identity.ServiceIdentity, upst
 		},
 	}
 
-	if o.permissive {
-		// Since no traffic policies exist with permissive mode, rely on cluster provided service discovery.
-		remoteCluster.ClusterDiscoveryType = &xds_cluster.Cluster_Type{Type: xds_cluster.Cluster_ORIGINAL_DST}
-		remoteCluster.LbPolicy = xds_cluster.Cluster_CLUSTER_PROVIDED
-	} else {
-		// Configure service discovery based on traffic policies
-		remoteCluster.ClusterDiscoveryType = &xds_cluster.Cluster_Type{Type: xds_cluster.Cluster_EDS}
-		remoteCluster.EdsClusterConfig = &xds_cluster.Cluster_EdsClusterConfig{EdsConfig: envoy.GetADSConfigSource()}
-		remoteCluster.LbPolicy = xds_cluster.Cluster_ROUND_ROBIN
-	}
+	// Configure service discovery based on traffic policies
+	remoteCluster.ClusterDiscoveryType = &xds_cluster.Cluster_Type{Type: xds_cluster.Cluster_EDS}
+	remoteCluster.EdsClusterConfig = &xds_cluster.Cluster_EdsClusterConfig{EdsConfig: envoy.GetADSConfigSource()}
+	remoteCluster.LbPolicy = xds_cluster.Cluster_ROUND_ROBIN
 
 	if o.withActiveHealthChecks {
 		enableHealthChecksOnCluster(remoteCluster, upstreamSvc)

pkg/envoy/rds/response.go

@@ -30,6 +30,7 @@ func NewResponse(cataloger catalog.MeshCataloger, proxy *envoy.Proxy, discoveryR
 	}
 
 	services, err := proxyRegistry.ListProxyServices(proxy)
+
 	if err != nil {
 		log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrFetchingServiceList)).
 			Msgf("Error looking up services for Envoy with serial number=%q", proxy.GetCertificateSerialNumber())

pkg/trafficpolicy/trafficpolicy.go

@@ -190,15 +190,15 @@ func mergeRules(originalRules, latestRules []*Rule) []*Rule {
 
 // mergeRoutesWeightedClusters merges two slices of RouteWeightedClusters and returns a slice where there is one RouteWeightedCluster
 //	for any HTTPRouteMatch. Where there is an overlap in HTTPRouteMatch between the originalRoutes and latestRoutes, the WeightedClusters
-//  will be unioned as there can only be one set of WeightedClusters per HTTPRouteMatch.
+// from latest will be used as there can only be one set of WeightedClusters per HTTPRouteMatch.
 func mergeRoutesWeightedClusters(originalRoutes, latestRoutes []*RouteWeightedClusters) []*RouteWeightedClusters {
 	for _, latest := range latestRoutes {
 		foundRoute := false
 		for _, original := range originalRoutes {
 			if reflect.DeepEqual(original.HTTPRouteMatch, latest.HTTPRouteMatch) {
 				foundRoute = true
 				if !reflect.DeepEqual(original.WeightedClusters, latest.WeightedClusters) {
-					original.WeightedClusters = original.WeightedClusters.Union(latest.WeightedClusters)
+					original.WeightedClusters = latest.WeightedClusters
 				}
 				continue
 			}