Add Import Policy for Service VIPs (#721)

* rename export policies to make it direction independent

* split creating neighborsets and prefixsets from applying export policy

* add bgp import policy to deny service VIPs

* add tests for addition of import policy
This commit is contained in:
Aaron U'Ren 2019-05-26 19:59:10 +02:00 committed by Murali Reddy
parent 4be51ba193
commit 8fe9f70dd5
5 changed files with 306 additions and 132 deletions

View File

@ -350,9 +350,9 @@ func (nrc *NetworkRoutingController) OnNodeUpdate(obj interface{}) {
}
// update export policies so that NeighborSet gets updated with new set of nodes
err := nrc.addExportPolicies()
err := nrc.AddPolicies()
if err != nil {
glog.Errorf("Error adding BGP export policies: %s", err.Error())
glog.Errorf("Error adding BGP policies: %s", err.Error())
}
if nrc.bgpEnableInternal {

View File

@ -10,22 +10,10 @@ import (
v1core "k8s.io/api/core/v1"
)
// BGP export policies are added so that following conditions are met
//
// - by default export of all routes from the RIB to the neighbour's is denied, and explicity statements are added i
// to permit the desired routes to be exported
// - each node is allowed to advertise its assigned pod CIDR's to all of its iBGP peer neighbours with same ASN if --enable-ibgp=true
// - each node is allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
// only if --advertise-pod-cidr flag is set to true
// - each node is NOT allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
// only if --advertise-pod-cidr flag is set to false
// - each node is allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) ONLY to external
// BGP peers
// - each node is NOT allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) to
// iBGP peers
// - an option to allow overriding the next-hop-address with the outgoing ip for external bgp peers
func (nrc *NetworkRoutingController) addExportPolicies() error {
// First create all prefix and neighbor sets
// Then apply export policies
// Then apply import policies
func (nrc *NetworkRoutingController) AddPolicies() error {
// we are rr server do not add export policies
if nrc.bgpRRServer {
return nil
@ -51,30 +39,18 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
}
// creates prefix set to represent all the advertisable IP associated with the services
advIpPrefixList := make([]config.Prefix, 0)
advIPPrefixList := make([]config.Prefix, 0)
advIps, _, _ := nrc.getAllVIPs()
for _, ip := range advIps {
advIpPrefixList = append(advIpPrefixList, config.Prefix{IpPrefix: ip + "/32"})
advIPPrefixList = append(advIPPrefixList, config.Prefix{IpPrefix: ip + "/32"})
}
clusterIpPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
clusterIPPrefixSet, err := table.NewPrefixSet(config.PrefixSet{
PrefixSetName: "clusteripprefixset",
PrefixList: advIpPrefixList,
PrefixList: advIPPrefixList,
})
err = nrc.bgpServer.ReplaceDefinedSet(clusterIpPrefixSet)
err = nrc.bgpServer.ReplaceDefinedSet(clusterIPPrefixSet)
if err != nil {
nrc.bgpServer.AddDefinedSet(clusterIpPrefixSet)
}
statements := make([]config.Statement, 0)
var bgpActions config.BgpActions
if nrc.pathPrepend {
bgpActions = config.BgpActions{
SetAsPathPrepend: config.SetAsPathPrepend{
As: nrc.pathPrependAS,
RepeatN: nrc.pathPrependCount,
},
}
nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet)
}
if nrc.bgpEnableInternal {
@ -93,10 +69,75 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
NeighborSetName: "iBGPpeerset",
NeighborInfoList: iBGPPeers,
})
err = nrc.bgpServer.ReplaceDefinedSet(iBGPPeerNS)
err := nrc.bgpServer.ReplaceDefinedSet(iBGPPeerNS)
if err != nil {
nrc.bgpServer.AddDefinedSet(iBGPPeerNS)
}
}
externalBgpPeers := make([]string, 0)
if len(nrc.globalPeerRouters) > 0 {
for _, peer := range nrc.globalPeerRouters {
externalBgpPeers = append(externalBgpPeers, peer.Config.NeighborAddress)
}
}
if len(nrc.nodePeerRouters) > 0 {
for _, peer := range nrc.nodePeerRouters {
externalBgpPeers = append(externalBgpPeers, peer)
}
}
if len(externalBgpPeers) > 0 {
ns, _ := table.NewNeighborSet(config.NeighborSet{
NeighborSetName: "externalpeerset",
NeighborInfoList: externalBgpPeers,
})
err := nrc.bgpServer.ReplaceDefinedSet(ns)
if err != nil {
nrc.bgpServer.AddDefinedSet(ns)
}
}
err = nrc.addExportPolicies()
if err != nil {
return err
}
err = nrc.addImportPolicies()
if err != nil {
return err
}
return nil
}
// BGP export policies are added so that following conditions are met:
//
// - by default export of all routes from the RIB to the neighbour's is denied, and explicity statements are added i
// to permit the desired routes to be exported
// - each node is allowed to advertise its assigned pod CIDR's to all of its iBGP peer neighbours with same ASN if --enable-ibgp=true
// - each node is allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
// only if --advertise-pod-cidr flag is set to true
// - each node is NOT allowed to advertise its assigned pod CIDR's to all of its external BGP peer neighbours
// only if --advertise-pod-cidr flag is set to false
// - each node is allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) ONLY to external
// BGP peers
// - each node is NOT allowed to advertise service VIP's (cluster ip, load balancer ip, external IP) to
// iBGP peers
// - an option to allow overriding the next-hop-address with the outgoing ip for external bgp peers
func (nrc *NetworkRoutingController) addExportPolicies() error {
statements := make([]config.Statement, 0)
var bgpActions config.BgpActions
if nrc.pathPrepend {
bgpActions = config.BgpActions{
SetAsPathPrepend: config.SetAsPathPrepend{
As: nrc.pathPrependAS,
RepeatN: nrc.pathPrependCount,
},
}
}
if nrc.bgpEnableInternal {
actions := config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_ACCEPT_ROUTE,
}
@ -118,26 +159,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
})
}
externalBgpPeers := make([]string, 0)
if len(nrc.globalPeerRouters) != 0 {
for _, peer := range nrc.globalPeerRouters {
externalBgpPeers = append(externalBgpPeers, peer.Config.NeighborAddress)
}
}
if len(nrc.nodePeerRouters) != 0 {
for _, peer := range nrc.nodePeerRouters {
externalBgpPeers = append(externalBgpPeers, peer)
}
}
if len(externalBgpPeers) > 0 {
ns, _ := table.NewNeighborSet(config.NeighborSet{
NeighborSetName: "externalpeerset",
NeighborInfoList: externalBgpPeers,
})
err = nrc.bgpServer.ReplaceDefinedSet(ns)
if err != nil {
nrc.bgpServer.AddDefinedSet(ns)
}
if len(nrc.globalPeerRouters) > 0 || len(nrc.nodePeerRouters) > 0 {
if nrc.overrideNextHop {
bgpActions.SetNextHop = "self"
}
@ -179,7 +201,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
}
definition := config.PolicyDefinition{
Name: "kube_router",
Name: "kube_router_export",
Statements: statements,
}
@ -191,7 +213,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
policyAlreadyExists := false
policyList := nrc.bgpServer.GetPolicy()
for _, existingPolicy := range policyList {
if existingPolicy.Name == "kube_router" {
if existingPolicy.Name == "kube_router_export" {
policyAlreadyExists = true
}
}
@ -207,7 +229,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
_, existingPolicyAssignments, err := nrc.bgpServer.GetPolicyAssignment("", table.POLICY_DIRECTION_EXPORT)
if err == nil {
for _, existingPolicyAssignment := range existingPolicyAssignments {
if existingPolicyAssignment.Name == "kube_router" {
if existingPolicyAssignment.Name == "kube_router_export" {
policyAssignmentExists = true
}
}
@ -234,3 +256,76 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
return nil
}
// BGP import policies are added so that the following conditions are met:
// - do not import Service VIPs at all, instead traffic to service VIPs should be sent to the gateway and ECMPed from there
func (nrc *NetworkRoutingController) addImportPolicies() error {
statements := make([]config.Statement, 0)
statements = append(statements, config.Statement{
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
})
definition := config.PolicyDefinition{
Name: "kube_router_import",
Statements: statements,
}
policy, err := table.NewPolicy(definition)
if err != nil {
return errors.New("Failed to create new policy: " + err.Error())
}
policyAlreadyExists := false
policyList := nrc.bgpServer.GetPolicy()
for _, existingPolicy := range policyList {
if existingPolicy.Name == "kube_router_import" {
policyAlreadyExists = true
}
}
if !policyAlreadyExists {
err = nrc.bgpServer.AddPolicy(policy, false)
if err != nil {
return errors.New("Failed to add policy: " + err.Error())
}
}
policyAssignmentExists := false
_, existingPolicyAssignments, err := nrc.bgpServer.GetPolicyAssignment("", table.POLICY_DIRECTION_IMPORT)
if err == nil {
for _, existingPolicyAssignment := range existingPolicyAssignments {
if existingPolicyAssignment.Name == "kube_router_import" {
policyAssignmentExists = true
}
}
}
// Default policy is to accept
if !policyAssignmentExists {
err = nrc.bgpServer.AddPolicyAssignment("",
table.POLICY_DIRECTION_IMPORT,
[]*config.PolicyDefinition{&definition},
table.ROUTE_TYPE_ACCEPT)
if err != nil {
return errors.New("Failed to add policy assignment: " + err.Error())
}
} else {
err = nrc.bgpServer.ReplacePolicyAssignment("",
table.POLICY_DIRECTION_IMPORT,
[]*config.PolicyDefinition{&definition},
table.ROUTE_TYPE_ACCEPT)
if err != nil {
return errors.New("Failed to replace policy assignment: " + err.Error())
}
}
return nil
}

View File

@ -94,9 +94,9 @@ func (nrc *NetworkRoutingController) handleServiceUpdate(svc *v1core.Service) {
}
// update export policies so that new VIP's gets addedd to clusteripprefixsit and vip gets advertised to peers
err = nrc.addExportPolicies()
err = nrc.AddPolicies()
if err != nil {
glog.Errorf("Error adding BGP export policies: %s", err.Error())
glog.Errorf("Error adding BGP policies: %s", err.Error())
}
nrc.advertiseVIPs(toAdvertise)
@ -162,7 +162,7 @@ func (nrc *NetworkRoutingController) newEndpointsEventHandler() cache.ResourceEv
// OnEndpointsAdd handles endpoint add events from apiserver
// This method calls OnEndpointsUpdate with the addition of updating BGP export policies
// Calling addExportPolicies here covers the edge case where addExportPolicies fails in
// Calling AddPolicies here covers the edge case where AddPolicies fails in
// OnServiceUpdate because the corresponding Endpoint resource for the
// Service was not created yet.
func (nrc *NetworkRoutingController) OnEndpointsAdd(obj interface{}) {
@ -171,9 +171,9 @@ func (nrc *NetworkRoutingController) OnEndpointsAdd(obj interface{}) {
return
}
err := nrc.addExportPolicies()
err := nrc.AddPolicies()
if err != nil {
glog.Errorf("error adding BGP export policies: %s", err)
glog.Errorf("error adding BGP policies: %s", err)
}
nrc.OnEndpointsUpdate(obj)

View File

@ -286,9 +286,9 @@ func (nrc *NetworkRoutingController) Run(healthChan chan<- *healthcheck.Controll
glog.Errorf("Error advertising route: %s", err.Error())
}
err = nrc.addExportPolicies()
err = nrc.AddPolicies()
if err != nil {
glog.Errorf("Error adding BGP export policies: %s", err.Error())
glog.Errorf("Error adding BGP policies: %s", err.Error())
}
if nrc.bgpEnableInternal {

View File

@ -1482,18 +1482,21 @@ func Test_OnNodeUpdate(t *testing.T) {
}
*/
func Test_addExportPolicies(t *testing.T) {
testcases := []struct {
name string
nrc *NetworkRoutingController
existingNodes []*v1core.Node
existingServices []*v1core.Service
podDefinedSet *config.DefinedSets
clusterIPDefinedSet *config.DefinedSets
externalPeerDefinedSet *config.DefinedSets
policyStatements []*config.Statement
err error
}{
type PolicyTestCase struct {
name string
nrc *NetworkRoutingController
existingNodes []*v1core.Node
existingServices []*v1core.Service
podDefinedSet *config.DefinedSets
clusterIPDefinedSet *config.DefinedSets
externalPeerDefinedSet *config.DefinedSets
exportPolicyStatements []*config.Statement
importPolicyStatements []*config.Statement
err error
}
func Test_AddPolicies(t *testing.T) {
testcases := []PolicyTestCase{
{
"has nodes and services",
&NetworkRoutingController{
@ -1577,7 +1580,7 @@ func Test_addExportPolicies(t *testing.T) {
&config.DefinedSets{},
[]*config.Statement{
{
Name: "kube_router_stmt0",
Name: "kube_router_export_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "podcidrprefixset",
@ -1593,6 +1596,20 @@ func Test_addExportPolicies(t *testing.T) {
},
},
},
[]*config.Statement{
{
Name: "kube_router_import_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
},
},
nil,
},
{
@ -1696,7 +1713,7 @@ func Test_addExportPolicies(t *testing.T) {
},
[]*config.Statement{
{
Name: "kube_router_stmt0",
Name: "kube_router_export_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "podcidrprefixset",
@ -1712,7 +1729,7 @@ func Test_addExportPolicies(t *testing.T) {
},
},
{
Name: "kube_router_stmt1",
Name: "kube_router_export_stmt1",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
@ -1728,6 +1745,20 @@ func Test_addExportPolicies(t *testing.T) {
},
},
},
[]*config.Statement{
{
Name: "kube_router_import_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
},
},
nil,
},
{
@ -1831,7 +1862,7 @@ func Test_addExportPolicies(t *testing.T) {
},
[]*config.Statement{
{
Name: "kube_router_stmt0",
Name: "kube_router_export_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
@ -1847,6 +1878,20 @@ func Test_addExportPolicies(t *testing.T) {
},
},
},
[]*config.Statement{
{
Name: "kube_router_import_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
},
},
nil,
},
{
@ -1953,7 +1998,7 @@ func Test_addExportPolicies(t *testing.T) {
},
[]*config.Statement{
{
Name: "kube_router_stmt0",
Name: "kube_router_export_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "podcidrprefixset",
@ -1969,7 +2014,7 @@ func Test_addExportPolicies(t *testing.T) {
},
},
{
Name: "kube_router_stmt1",
Name: "kube_router_export_stmt1",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
@ -1991,6 +2036,20 @@ func Test_addExportPolicies(t *testing.T) {
},
},
},
[]*config.Statement{
{
Name: "kube_router_import_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
},
},
nil,
},
{
@ -2096,7 +2155,7 @@ func Test_addExportPolicies(t *testing.T) {
},
[]*config.Statement{
{
Name: "kube_router_stmt0",
Name: "kube_router_export_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "podcidrprefixset",
@ -2112,7 +2171,7 @@ func Test_addExportPolicies(t *testing.T) {
},
},
{
Name: "kube_router_stmt1",
Name: "kube_router_export_stmt1",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
@ -2128,6 +2187,20 @@ func Test_addExportPolicies(t *testing.T) {
},
},
},
[]*config.Statement{
{
Name: "kube_router_import_stmt0",
Conditions: config.Conditions{
MatchPrefixSet: config.MatchPrefixSet{
PrefixSet: "clusteripprefixset",
MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
},
},
Actions: config.Actions{
RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
},
},
},
nil,
},
}
@ -2167,7 +2240,7 @@ func Test_addExportPolicies(t *testing.T) {
informerFactory := informers.NewSharedInformerFactory(testcase.nrc.clientset, 0)
nodeInformer := informerFactory.Core().V1().Nodes().Informer()
testcase.nrc.nodeLister = nodeInformer.GetIndexer()
err = testcase.nrc.addExportPolicies()
err = testcase.nrc.AddPolicies()
if !reflect.DeepEqual(err, testcase.err) {
t.Logf("expected err %v", testcase.err)
t.Logf("actual err %v", err)
@ -2207,54 +2280,60 @@ func Test_addExportPolicies(t *testing.T) {
t.Error("unexpected external peer defined set")
}
policies := testcase.nrc.bgpServer.GetPolicy()
policyExists := false
for _, policy := range policies {
if policy.Name == "kube_router" {
policyExists = true
break
}
}
if !policyExists {
t.Errorf("policy 'kube_router' was not added")
}
routeType, policyAssignments, err := testcase.nrc.bgpServer.GetPolicyAssignment("", table.POLICY_DIRECTION_EXPORT)
if routeType != table.ROUTE_TYPE_REJECT {
t.Errorf("expected route type 'reject' for export policy assignment, but got %v", routeType)
}
if err != nil {
t.Fatalf("failed to get policy assignments: %v", err)
}
policyAssignmentExists := false
for _, policyAssignment := range policyAssignments {
if policyAssignment.Name == "kube_router" {
policyAssignmentExists = true
}
}
if !policyAssignmentExists {
t.Error("export policy assignment 'kube_router' was not added")
}
statements := testcase.nrc.bgpServer.GetStatement()
for _, expectedStatement := range testcase.policyStatements {
found := false
for _, statement := range statements {
if reflect.DeepEqual(statement, expectedStatement) {
found = true
}
}
if !found {
t.Errorf("statement %v not found", expectedStatement)
}
}
checkPolicies(t, testcase, table.POLICY_DIRECTION_EXPORT, table.ROUTE_TYPE_REJECT, testcase.exportPolicyStatements)
checkPolicies(t, testcase, table.POLICY_DIRECTION_IMPORT, table.ROUTE_TYPE_ACCEPT, testcase.importPolicyStatements)
})
}
}
func checkPolicies(t *testing.T, testcase PolicyTestCase, direction table.PolicyDirection, defaultPolicy table.RouteType,
policyStatements []*config.Statement) {
policies := testcase.nrc.bgpServer.GetPolicy()
policyExists := false
for _, policy := range policies {
if policy.Name == "kube_router_"+direction.String() {
policyExists = true
break
}
}
if !policyExists {
t.Errorf("policy 'kube_router_%v' was not added", direction)
}
routeType, policyAssignments, err := testcase.nrc.bgpServer.GetPolicyAssignment("", direction)
if routeType != defaultPolicy {
t.Errorf("expected route type '%v' for %v policy assignment, but got %v", defaultPolicy, direction, routeType)
}
if err != nil {
t.Fatalf("failed to get policy assignments: %v", err)
}
policyAssignmentExists := false
for _, policyAssignment := range policyAssignments {
if policyAssignment.Name == "kube_router_"+direction.String() {
policyAssignmentExists = true
}
}
if !policyAssignmentExists {
t.Errorf("export policy assignment 'kube_router_%v' was not added", direction)
}
statements := testcase.nrc.bgpServer.GetStatement()
for _, expectedStatement := range policyStatements {
found := false
for _, statement := range statements {
if reflect.DeepEqual(statement, expectedStatement) {
found = true
}
}
if !found {
t.Errorf("statement %v not found", expectedStatement)
}
}
}
func Test_generateTunnelName(t *testing.T) {
testcases := []struct {
name string