Skip to content

Commit

Permalink
feat(*): traffic split in permissive mode
Browse files Browse the repository at this point in the history 
resolves openservicemesh#2527

Signed-off-by: Michelle Noorali <minooral@microsoft.com>
  • Loading branch information
Michelle Noorali committed Sep 1, 2021
1 parent 72976e4 commit 843bc1a
Show file tree
Hide file tree
Showing 9 changed files with 426 additions and 76 deletions.
33 changes: 32 additions & 1 deletion pkg/catalog/inbound_traffic_policies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,18 @@ 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, inboundPolicies, inboundPoliciesFromSplits...)
return inboundPolicies
}

inbound := mc.listInboundPoliciesFromTrafficTargets(upstreamIdentity, upstreamServices)
inboundPoliciesFromSplits := mc.listInboundPoliciesForTrafficSplits(upstreamIdentity, upstreamServices)
inbound = trafficpolicy.MergeInboundPolicies(AllowPartialHostnamesMatch, inbound, inboundPoliciesFromSplits...)
return inbound
}
Expand Down Expand Up @@ -73,6 +75,35 @@ func (mc *MeshCatalog) listInboundPoliciesForTrafficSplits(upstreamIdentity iden
upstreamServiceAccount := upstreamIdentity.ToK8sServiceAccount()
var inboundPolicies []*trafficpolicy.InboundTrafficPolicy

if mc.configurator.IsPermissiveTrafficPolicyMode() {
for _, upstreamSvc := range upstreamServices {
//check if the upstream service belong to a traffic split
if !mc.isTrafficSplitBackendService(upstreamSvc) {
continue
}

apexServices := mc.getApexServicesForBackendService(upstreamSvc)
for _, apexService := range apexServices {
// build an inbound policy for every apex service
locality := service.LocalCluster
if apexService.Namespace == upstreamServiceAccount.Namespace {
locality = service.LocalNS
}
hostnames, err := mc.GetServiceHostnames(apexService, locality)
if err != nil {
log.Error().Err(err).Str(errcode.Kind, errcode.GetErrCodeWithMetric(errcode.ErrServiceHostnames)).
Msgf("Error getting service hostnames for apex service %v", apexService)
continue
}
servicePolicy := trafficpolicy.NewInboundTrafficPolicy(apexService.FQDN(), hostnames)
weightedCluster := getDefaultWeightedClusterForService(upstreamSvc)
servicePolicy.AddRule(*trafficpolicy.NewRouteWeightedCluster(trafficpolicy.WildCardRouteMatch, []service.WeightedCluster{weightedCluster}), identity.WildcardServiceIdentity)
inboundPolicies = trafficpolicy.MergeInboundPolicies(AllowPartialHostnamesMatch, inboundPolicies, servicePolicy)
}
}
return inboundPolicies
}

for _, t := range mc.meshSpec.ListTrafficTargets() { // loop through all traffic targets
if !isValidTrafficTarget(t) {
continue
Expand Down
183 changes: 175 additions & 8 deletions pkg/catalog/inbound_traffic_policies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,90 @@ func TestListInboundTrafficPolicies(t *testing.T) {
},
permissiveMode: true,
},
{
name: "permissive mode with traffic split",
downstreamSA: tests.BookbuyerServiceIdentity,
upstreamSA: tests.BookstoreV2ServiceIdentity,
upstreamServices: []service.MeshService{tests.BookstoreV2Service},
meshServices: []service.MeshService{tests.BookbuyerService, tests.BookstoreV1Service, tests.BookstoreV2Service, tests.BookstoreApexService},
meshServiceAccounts: []identity.K8sServiceAccount{tests.BookbuyerServiceAccount, tests.BookstoreV2ServiceAccount},
trafficSpec: spec.HTTPRouteGroup{},
trafficSplit: split.TrafficSplit{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: split.TrafficSplitSpec{
Service: "bookstore-apex",
Backends: []split.TrafficSplitBackend{
{
Service: tests.BookstoreV1ServiceName,
Weight: tests.Weight90,
},
{
Service: tests.BookstoreV2ServiceName,
Weight: tests.Weight10,
},
},
},
},
expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
{
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",
},
Rules: []*trafficpolicy.Rule{
{
Route: trafficpolicy.RouteWeightedClusters{
HTTPRouteMatch: tests.WildCardRouteMatch,
WeightedClusters: mapset.NewSet(service.WeightedCluster{
ClusterName: "default/bookstore-v2",
Weight: 100,
}),
},
AllowedServiceIdentities: mapset.NewSet(identity.WildcardServiceIdentity),
},
},
},
{
Name: "bookstore-apex.default.svc.cluster.local",
Hostnames: []string{
"bookstore-apex",
"bookstore-apex.default",
"bookstore-apex.default.svc",
"bookstore-apex.default.svc.cluster",
"bookstore-apex.default.svc.cluster.local",
"bookstore-apex:8888",
"bookstore-apex.default:8888",
"bookstore-apex.default.svc:8888",
"bookstore-apex.default.svc.cluster:8888",
"bookstore-apex.default.svc.cluster.local:8888",
},
Rules: []*trafficpolicy.Rule{
{
Route: trafficpolicy.RouteWeightedClusters{
HTTPRouteMatch: tests.WildCardRouteMatch,
WeightedClusters: mapset.NewSet(service.WeightedCluster{
ClusterName: "default/bookstore-v2",
Weight: 100,
}),
},
AllowedServiceIdentities: mapset.NewSet(identity.WildcardServiceIdentity),
},
},
},
},
permissiveMode: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -501,6 +585,8 @@ func TestListInboundTrafficPolicies(t *testing.T) {

mockEndpointProvider.EXPECT().GetID().Return("fake").AnyTimes()

mockMeshSpec.EXPECT().ListTrafficSplits().Return([]*split.TrafficSplit{&tc.trafficSplit}).AnyTimes()

if tc.permissiveMode {
var serviceAccounts []*corev1.ServiceAccount
for _, sa := range tc.meshServiceAccounts {
Expand All @@ -511,7 +597,6 @@ func TestListInboundTrafficPolicies(t *testing.T) {
mockKubeController.EXPECT().ListServiceAccounts().Return(serviceAccounts).AnyTimes()
} else {
mockMeshSpec.EXPECT().ListHTTPTrafficSpecs().Return([]*spec.HTTPRouteGroup{&tc.trafficSpec}).AnyTimes()
mockMeshSpec.EXPECT().ListTrafficSplits().Return([]*split.TrafficSplit{&tc.trafficSplit}).AnyTimes()
trafficTarget := tests.NewSMITrafficTarget(tc.downstreamSA, tc.upstreamSA)
mockMeshSpec.EXPECT().ListTrafficTargets().Return([]*access.TrafficTarget{&trafficTarget}).AnyTimes()
}
Expand Down Expand Up @@ -541,6 +626,7 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {

testCases := []struct {
name string
permissiveMode bool
downstreamSA identity.ServiceIdentity
upstreamSA identity.ServiceIdentity
upstreamServices []service.MeshService
Expand All @@ -551,7 +637,8 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
}{
// TODO(draychev): use ServiceIdentity in the rest of the tests [https://github.com/openservicemesh/osm/issues/2218]
{
name: "inbound policies in same namespaces, without traffic split",
name: "inbound policies in same namespaces, without traffic split",
permissiveMode: false,
downstreamSA: identity.K8sServiceAccount{
Name: "bookbuyer",
Namespace: "default",
Expand Down Expand Up @@ -603,7 +690,8 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{},
},
{
name: "inbound policies in same namespaces, with traffic split",
name: "inbound policies in same namespaces, with traffic split",
permissiveMode: false,
downstreamSA: identity.K8sServiceAccount{
Name: "bookbuyer",
Namespace: "default",
Expand Down Expand Up @@ -719,7 +807,8 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
},
},
{
name: "inbound policies in same namespaces, with traffic split with namespaced root service",
name: "inbound policies in same namespaces, with traffic split with namespaced root service",
permissiveMode: false,
downstreamSA: identity.K8sServiceAccount{
Name: "bookbuyer",
Namespace: "default",
Expand Down Expand Up @@ -835,7 +924,8 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
},
},
{
name: "inbound policies in same namespaces: with traffic split (namespaced root service) and traffic target (having host header)",
name: "inbound policies in same namespaces: with traffic split (namespaced root service) and traffic target (having host header)",
permissiveMode: false,
downstreamSA: identity.K8sServiceAccount{
Name: "bookbuyer",
Namespace: "default",
Expand Down Expand Up @@ -957,6 +1047,77 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
},
},
},
{
name: "inbound policies with traffic split in permissive mode",
permissiveMode: true,
downstreamSA: identity.K8sServiceAccount{
Name: "bookbuyer",
Namespace: "default",
}.ToServiceIdentity(),
upstreamSA: identity.K8sServiceAccount{
Name: "bookstore",
Namespace: "default",
}.ToServiceIdentity(),
upstreamServices: []service.MeshService{{
Name: "bookstore",
Namespace: "default",
}},
meshServices: []service.MeshService{{
Name: "bookstore",
Namespace: "default",
}, {
Name: "bookstore-apex",
Namespace: "default",
}},
trafficSpec: spec.HTTPRouteGroup{},
trafficSplit: split.TrafficSplit{
ObjectMeta: v1.ObjectMeta{
Namespace: "default",
},
Spec: split.TrafficSplitSpec{
Service: "bookstore-apex",
Backends: []split.TrafficSplitBackend{
{
Service: "bookstore",
Weight: tests.Weight90,
},
{
Service: tests.BookstoreV2ServiceName,
Weight: tests.Weight10,
},
},
},
},
expectedInboundPolicies: []*trafficpolicy.InboundTrafficPolicy{
{
Name: "bookstore-apex.default.svc.cluster.local",
Hostnames: []string{
"bookstore-apex",
"bookstore-apex.default",
"bookstore-apex.default.svc",
"bookstore-apex.default.svc.cluster",
"bookstore-apex.default.svc.cluster.local",
"bookstore-apex:8888",
"bookstore-apex.default:8888",
"bookstore-apex.default.svc:8888",
"bookstore-apex.default.svc.cluster:8888",
"bookstore-apex.default.svc.cluster.local:8888",
},
Rules: []*trafficpolicy.Rule{
{
Route: trafficpolicy.RouteWeightedClusters{
HTTPRouteMatch: tests.WildCardRouteMatch,
WeightedClusters: mapset.NewSet(service.WeightedCluster{
ClusterName: "default/bookstore",
Weight: 100,
}),
},
AllowedServiceIdentities: mapset.NewSet(identity.WildcardServiceIdentity),
},
},
},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -967,12 +1128,14 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
mockMeshSpec := smi.NewMockMeshSpec(mockCtrl)
mockEndpointProvider := endpoint.NewMockProvider(mockCtrl)
mockServiceProvider := service.NewMockProvider(mockCtrl)
mockConfigurator := configurator.NewMockConfigurator(mockCtrl)

mc := MeshCatalog{
kubeController: mockKubeController,
meshSpec: mockMeshSpec,
endpointsProviders: []endpoint.Provider{mockEndpointProvider},
serviceProviders: []service.Provider{mockServiceProvider},
configurator: mockConfigurator,
}

for _, meshSvc := range tc.meshServices {
Expand All @@ -984,9 +1147,12 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
mockMeshSpec.EXPECT().ListTrafficSplits().Return([]*split.TrafficSplit{&tc.trafficSplit}).AnyTimes()
mockEndpointProvider.EXPECT().GetID().Return("fake").AnyTimes()

trafficTarget := tests.NewSMITrafficTarget(tc.downstreamSA, tc.upstreamSA)
mockMeshSpec.EXPECT().ListTrafficTargets().Return([]*access.TrafficTarget{&trafficTarget}).AnyTimes()

if tc.permissiveMode {
mockMeshSpec.EXPECT().ListTrafficTargets().Return([]*access.TrafficTarget{}).AnyTimes()
} else {
trafficTarget := tests.NewSMITrafficTarget(tc.downstreamSA, tc.upstreamSA)
mockMeshSpec.EXPECT().ListTrafficTargets().Return([]*access.TrafficTarget{&trafficTarget}).AnyTimes()
}
for _, ms := range tc.meshServices {
locality := service.LocalCluster
if ms.Namespace == tc.downstreamSA.ToK8sServiceAccount().Namespace {
Expand All @@ -998,6 +1164,7 @@ func TestListInboundPoliciesForTrafficSplits(t *testing.T) {
}
}
}
mockConfigurator.EXPECT().IsPermissiveTrafficPolicyMode().Return(tc.permissiveMode).AnyTimes()

actual := mc.listInboundPoliciesForTrafficSplits(tc.upstreamSA, tc.upstreamServices)
assert.ElementsMatch(tc.expectedInboundPolicies, actual)
Expand Down
23 changes: 11 additions & 12 deletions pkg/catalog/outbound_traffic_policies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,17 @@ var (
// 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, trafficpolicy.OverwriteWeightedClusters, permissiveOutboundPolicies, outboundPoliciesFromSplits...)
}

allowedOutboundPolicies := mc.listOutboundPoliciesForTrafficTargets(downstreamIdentity)
return trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, trafficpolicy.OverwriteWeightedClusters, allowedOutboundPolicies, outboundPoliciesFromSplits...)
}

// listOutboundPoliciesForTrafficTargets loops through all SMI Traffic Target resources and returns outbound traffic policies
Expand All @@ -70,7 +69,7 @@ func (mc *MeshCatalog) listOutboundPoliciesForTrafficTargets(downstreamIdentity
for _, source := range t.Spec.Sources {
// TODO(draychev): must check for the correct type of ServiceIdentity as well
if source.Name == downstreamServiceAccount.Name && source.Namespace == downstreamServiceAccount.Namespace { // found outbound
outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, mc.buildOutboundPolicies(downstreamIdentity, t)...)
outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, trafficpolicy.UnionWeightedClusters, outboundPolicies, mc.buildOutboundPolicies(downstreamIdentity, t)...)
break
}
}
Expand Down Expand Up @@ -262,7 +261,7 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv
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)
outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, trafficpolicy.UnionWeightedClusters, outboundPolicies, policyWithHostHeader)
} else {
needWildCardRoute = true
}
Expand All @@ -275,7 +274,7 @@ func (mc *MeshCatalog) buildOutboundPolicies(sourceServiceIdentity identity.Serv
}
}

outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, outboundPolicies, policy)
outboundPolicies = trafficpolicy.MergeOutboundPolicies(AllowPartialHostnamesMatch, trafficpolicy.UnionWeightedClusters, outboundPolicies, policy)
}
return outboundPolicies
}
Expand Down
Loading

0 comments on commit 843bc1a

Please sign in to comment.