fix clusteripprefixset import policy (#771)

pkg/controllers/routing/bgp_policies.go

@@ -53,10 +53,10 @@ func (nrc *NetworkRoutingController) AddPolicies() error {
 		nrc.bgpServer.AddDefinedSet(clusterIPPrefixSet)
 	}
 
+	iBGPPeers := make([]string, 0)
 	if nrc.bgpEnableInternal {
 		// Get the current list of the nodes from the local cache
 		nodes := nrc.nodeLister.List()
-		iBGPPeers := make([]string, 0)
 		for _, node := range nodes {
 			nodeObj := node.(*v1core.Node)
 			nodeIP, err := utils.GetNodeIP(nodeObj)
@@ -97,6 +97,17 @@ func (nrc *NetworkRoutingController) AddPolicies() error {
 		}
 	}
 
+	// a slice of all peers is used as a match condition for reject statement of clusteripprefixset import polcy
+	allBgpPeers := append(externalBgpPeers, iBGPPeers...)
+	ns, _ := table.NewNeighborSet(config.NeighborSet{
+		NeighborSetName:  "allpeerset",
+		NeighborInfoList: allBgpPeers,
+	})
+	err = nrc.bgpServer.ReplaceDefinedSet(ns)
+	if err != nil {
+		nrc.bgpServer.AddDefinedSet(ns)
+	}
+
 	err = nrc.addExportPolicies()
 	if err != nil {
 		return err
@@ -258,7 +269,7 @@ func (nrc *NetworkRoutingController) addExportPolicies() error {
 }
 
 // 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
+// - do not import Service VIPs advertised from any peers, instead each kube-router originates and injects Service VIPs into local rib.
 func (nrc *NetworkRoutingController) addImportPolicies() error {
 	statements := make([]config.Statement, 0)
 
@@ -267,6 +278,9 @@ func (nrc *NetworkRoutingController) addImportPolicies() error {
 			MatchPrefixSet: config.MatchPrefixSet{
 				PrefixSet: "clusteripprefixset",
 			},
+			MatchNeighborSet: config.MatchNeighborSet{
+				NeighborSet: "allpeerset",
+			},
 		},
 		Actions: config.Actions{
 			RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,

pkg/controllers/routing/network_routes_controller_test.go

@@ -1490,6 +1490,7 @@ type PolicyTestCase struct {
 	podDefinedSet          *config.DefinedSets
 	clusterIPDefinedSet    *config.DefinedSets
 	externalPeerDefinedSet *config.DefinedSets
+	allPeerDefinedSet      *config.DefinedSets
 	exportPolicyStatements []*config.Statement
 	importPolicyStatements []*config.Statement
 	err                    error
@@ -1578,6 +1579,17 @@ func Test_AddPolicies(t *testing.T) {
 				BgpDefinedSets: config.BgpDefinedSets{},
 			},
 			&config.DefinedSets{},
+			&config.DefinedSets{
+				PrefixSets: []config.PrefixSet{},
+				NeighborSets: []config.NeighborSet{
+					{
+						NeighborSetName:  "allpeerset",
+						NeighborInfoList: []string{},
+					},
+				},
+				TagSets:        []config.TagSet{},
+				BgpDefinedSets: config.BgpDefinedSets{},
+			},
 			[]*config.Statement{
 				{
 					Name: "kube_router_export_stmt0",
@@ -1604,6 +1616,10 @@ func Test_AddPolicies(t *testing.T) {
 							PrefixSet:       "clusteripprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "allpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
@@ -1711,6 +1727,17 @@ func Test_AddPolicies(t *testing.T) {
 				TagSets:        []config.TagSet{},
 				BgpDefinedSets: config.BgpDefinedSets{},
 			},
+			&config.DefinedSets{
+				PrefixSets: []config.PrefixSet{},
+				NeighborSets: []config.NeighborSet{
+					{
+						NeighborSetName:  "allpeerset",
+						NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
+					},
+				},
+				TagSets:        []config.TagSet{},
+				BgpDefinedSets: config.BgpDefinedSets{},
+			},
 			[]*config.Statement{
 				{
 					Name: "kube_router_export_stmt0",
@@ -1753,6 +1780,10 @@ func Test_AddPolicies(t *testing.T) {
 							PrefixSet:       "clusteripprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "allpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
@@ -1860,6 +1891,17 @@ func Test_AddPolicies(t *testing.T) {
 				TagSets:        []config.TagSet{},
 				BgpDefinedSets: config.BgpDefinedSets{},
 			},
+			&config.DefinedSets{
+				PrefixSets: []config.PrefixSet{},
+				NeighborSets: []config.NeighborSet{
+					{
+						NeighborSetName:  "allpeerset",
+						NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
+					},
+				},
+				TagSets:        []config.TagSet{},
+				BgpDefinedSets: config.BgpDefinedSets{},
+			},
 			[]*config.Statement{
 				{
 					Name: "kube_router_export_stmt0",
@@ -1886,6 +1928,10 @@ func Test_AddPolicies(t *testing.T) {
 							PrefixSet:       "clusteripprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "allpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
@@ -1996,6 +2042,17 @@ func Test_AddPolicies(t *testing.T) {
 				TagSets:        []config.TagSet{},
 				BgpDefinedSets: config.BgpDefinedSets{},
 			},
+			&config.DefinedSets{
+				PrefixSets: []config.PrefixSet{},
+				NeighborSets: []config.NeighborSet{
+					{
+						NeighborSetName:  "allpeerset",
+						NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
+					},
+				},
+				TagSets:        []config.TagSet{},
+				BgpDefinedSets: config.BgpDefinedSets{},
+			},
 			[]*config.Statement{
 				{
 					Name: "kube_router_export_stmt0",
@@ -2044,6 +2101,10 @@ func Test_AddPolicies(t *testing.T) {
 							PrefixSet:       "clusteripprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "allpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
@@ -2153,6 +2214,17 @@ func Test_AddPolicies(t *testing.T) {
 				TagSets:        []config.TagSet{},
 				BgpDefinedSets: config.BgpDefinedSets{},
 			},
+			&config.DefinedSets{
+				PrefixSets: []config.PrefixSet{},
+				NeighborSets: []config.NeighborSet{
+					{
+						NeighborSetName:  "allpeerset",
+						NeighborInfoList: []string{"10.10.0.1/32", "10.10.0.2/32"},
+					},
+				},
+				TagSets:        []config.TagSet{},
+				BgpDefinedSets: config.BgpDefinedSets{},
+			},
 			[]*config.Statement{
 				{
 					Name: "kube_router_export_stmt0",
@@ -2195,6 +2267,10 @@ func Test_AddPolicies(t *testing.T) {
 							PrefixSet:       "clusteripprefixset",
 							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
 						},
+						MatchNeighborSet: config.MatchNeighborSet{
+							NeighborSet:     "allpeerset",
+							MatchSetOptions: config.MATCH_SET_OPTIONS_RESTRICTED_TYPE_ANY,
+						},
 					},
 					Actions: config.Actions{
 						RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE,
@@ -2280,6 +2356,17 @@ func Test_AddPolicies(t *testing.T) {
 				t.Error("unexpected external peer defined set")
 			}
 
+			allPeerDefinedSet, err := testcase.nrc.bgpServer.GetDefinedSet(table.DEFINED_TYPE_NEIGHBOR, "allpeerset")
+			if err != nil {
+				t.Fatalf("error validating defined sets: %v", err)
+			}
+
+			if !allPeerDefinedSet.Equal(testcase.allPeerDefinedSet) {
+				t.Logf("expected all peer defined set: %+v", testcase.allPeerDefinedSet.NeighborSets)
+				t.Logf("actual all peer defined set: %+v", allPeerDefinedSet.NeighborSets)
+				t.Error("unexpected all peer defined set")
+			}
+
 			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)
 		})