Shorten BGP filter

cmd/metal-api/internal/metal/network.go

@@ -115,17 +115,7 @@ func (n *Network) FindPrefix(cidr string) *Prefix {
 
 // ContainsIP checks whether the given ip is included in the networks prefixes
 func (n *MachineNetwork) ContainsIP(ip string) bool {
-	pip := net.ParseIP(ip)
-	for _, p := range n.Prefixes {
-		_, n, err := net.ParseCIDR(p)
-		if err != nil {
-			continue
-		}
-		if n.Contains(pip) {
-			return true
-		}
-	}
-	return false
+	return ContainsIP(n.Prefixes, ip)
 }
 
 // SubstractPrefixes returns the prefixes of the network minus the prefixes passed in the arguments
@@ -147,6 +137,25 @@ func (n *Network) SubstractPrefixes(prefixes ...Prefix) []Prefix {
 	return result
 }
 
+// ContainsIP checks whether the given ip is included in the networks prefixes
+func (n *Network) ContainsIP(ip string) bool {
+	return ContainsIP(n.Prefixes.String(), ip)
+}
+
+func ContainsIP(prefixes []string, ip string) bool {
+	pip := net.ParseIP(ip)
+	for _, p := range prefixes {
+		_, n, err := net.ParseCIDR(p)
+		if err != nil {
+			continue
+		}
+		if n.Contains(pip) {
+			return true
+		}
+	}
+	return false
+}
+
 type NicMap map[MacAddress]*Nic
 
 // ByMac creates a indexed map from a nic list.

cmd/metal-api/internal/service/switch-service.go

@@ -628,40 +628,51 @@ func makeBGPFilterFirewall(m metal.Machine) v1.BGPFilter {
 	return v1.NewBGPFilter(vnis, cidrs)
 }
 
-func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) v1.BGPFilter {
+func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap, fwNetworks []*metal.MachineNetwork) v1.BGPFilter {
 	vnis := []string{}
 	cidrs := []string{}
 
-	var private *metal.MachineNetwork
-	var underlay *metal.MachineNetwork
-	for _, net := range m.Allocation.MachineNetworks {
-		if net.Private {
-			private = net
-		} else if net.Underlay {
-			underlay = net
+	privateNets := []string{}
+	for _, n := range m.Allocation.MachineNetworks {
+		nt, _ := n.NetworkType()
+		if nt == nil {
+			continue
 		}
-	}
 
-	// Allow all prefixes of the private network
-	if private != nil {
-		cidrs = append(cidrs, private.Prefixes...)
+		if nt.Private {
+			privateNets = append(privateNets, n.Prefixes...)
+		}
 	}
-	for _, i := range ips[m.Allocation.Project] {
-		// No need to add /32 addresses of the primary network to the whitelist.
-		if private != nil && private.ContainsIP(i.IPAddress) {
+
+	externalNets := []string{}
+	for _, fwNet := range fwNetworks {
+		nt, _ := fwNet.NetworkType()
+		if nt == nil {
 			continue
 		}
-		// Do not allow underlay addresses to be announced.
-		if underlay != nil && underlay.ContainsIP(i.IPAddress) {
-			continue
+
+		if nt.Private {
+			privateNets = append(privateNets, fwNet.Prefixes...)
+		} else if nt.Name == "external" {
+			externalNets = append(externalNets, fwNet.Prefixes...)
 		}
-		// Allow all other ip addresses allocated for the project.
-		cidrs = append(cidrs, fmt.Sprintf("%s/32", i.IPAddress))
 	}
-	return v1.NewBGPFilter(vnis, cidrs)
+
+	// Allow all prefixes of private networks attached directly to the machine
+	// or via firewall
+	cidrs = append(cidrs, privateNets...)
+
+	// Allow all prefixes of external networks allocated for the project
+	for _, i := range ips[m.Allocation.Project] {
+		if metal.ContainsIP(externalNets, i.IPAddress) {
+			cidrs = append(cidrs, fmt.Sprintf("%s/32", i.IPAddress))
+		}
+	}
+
+	return v1.NewBGPFilter(vnis, utils.UniqueSorted(cidrs))
 }
 
-func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap, iMap metal.ImageMap) v1.BGPFilter {
+func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap, iMap metal.ImageMap, fwNetworks []*metal.MachineNetwork) v1.BGPFilter {
 	var filter v1.BGPFilter
 	if m.IsFirewall(iMap) {
 		// vrf "default" means: the firewall was successfully allocated and the switch port configured
@@ -670,11 +681,41 @@ func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap, iMap metal.Ima
 			filter = makeBGPFilterFirewall(m)
 		}
 	} else {
-		filter = makeBGPFilterMachine(m, ips)
+		filter = makeBGPFilterMachine(m, ips, fwNetworks)
 	}
 	return filter
 }
 
+func findFirewallNetworks(m *metal.Machine, machines metal.Machines, iMap metal.ImageMap) (firewallNetworks []*metal.MachineNetwork) {
+	var primaryNetworkID string
+	for _, m := range m.Allocation.MachineNetworks {
+		nt, _ := m.NetworkType()
+		if nt == nil {
+			continue
+		}
+		if *nt == metal.PrivatePrimaryShared || *nt == metal.PrivatePrimaryUnshared {
+			primaryNetworkID = m.NetworkID
+		}
+	}
+
+	if primaryNetworkID == "" {
+		return
+	}
+
+	for _, n := range machines {
+		if n.Allocation == nil || !n.IsFirewall(iMap) {
+			continue
+		}
+		for _, fwn := range n.Allocation.MachineNetworks {
+			if fwn.NetworkID == primaryNetworkID {
+				firewallNetworks = n.Allocation.MachineNetworks
+				return
+			}
+		}
+	}
+	return
+}
+
 func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, iMap metal.ImageMap, machines metal.Machines) v1.SwitchNics {
 	machinesByID := map[string]*metal.Machine{}
 	for i, m := range machines {
@@ -694,7 +735,8 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, iMap metal.ImageMap, mach
 		m := machinesBySwp[n.Name]
 		var filter *v1.BGPFilter
 		if m != nil && m.Allocation != nil {
-			f := makeBGPFilter(*m, n.Vrf, ips, iMap)
+			fwNetworks := findFirewallNetworks(m, machines, iMap)
+			f := makeBGPFilter(*m, n.Vrf, ips, iMap, fwNetworks)
 			filter = &f
 		}
 		nic := v1.SwitchNic{
@@ -764,6 +806,7 @@ func makeSwitchResponseList(ss []metal.Switch, ds *datastore.RethinkStore, logge
 	if err != nil {
 		logger.Errorw("could not find machines")
 	}
+
 	for i := range ss {
 		sw := ss[i]
 		var p *metal.Partition

cmd/metal-api/internal/service/switch-service_test.go

@@ -390,9 +390,11 @@ func TestMakeBGPFilterFirewall(t *testing.T) {
 
 func TestMakeBGPFilterMachine(t *testing.T) {
 	type args struct {
-		machine metal.Machine
-		ipsMap  metal.IPsMap
+		machine    metal.Machine
+		ipsMap     metal.IPsMap
+		fwNetworks []*metal.MachineNetwork
 	}
+
 	tests := []struct {
 		name string
 		args args
@@ -414,7 +416,22 @@ func TestMakeBGPFilterMachine(t *testing.T) {
 					metal.IP{
 						IPAddress: "10.1.0.1",
 					},
+					metal.IP{
+						IPAddress: "10.1.2.3",
+					},
 				}},
+				fwNetworks: []*metal.MachineNetwork{
+					{
+						Prefixes: []string{"10.2.0.0/22", "10.1.0.0/22"},
+						Private:  true,
+					},
+					{
+						Prefixes: []string{"212.89.42.0/24"},
+					},
+					{
+						Prefixes: []string{"100.127.1.0/24"},
+					},
+				},
 				machine: metal.Machine{
 					Allocation: &metal.MachineAllocation{
 						Project: "project",
@@ -431,8 +448,9 @@ func TestMakeBGPFilterMachine(t *testing.T) {
 								Underlay: true,
 							},
 							{
-								IPs: []string{"212.89.42.2", "212.89.42.1"},
-								Vrf: 104009,
+								IPs:      []string{"212.89.42.2", "212.89.42.1"},
+								Prefixes: []string{"212.89.42.0/24"},
+								Vrf:      104009,
 							},
 						},
 					},
@@ -448,13 +466,19 @@ func TestMakeBGPFilterMachine(t *testing.T) {
 						IPAddress: "212.89.42.1",
 					},
 				}},
+				fwNetworks: []*metal.MachineNetwork{
+					{
+						Prefixes: []string{"212.89.42.0/24"},
+					},
+				},
 				machine: metal.Machine{
 					Allocation: &metal.MachineAllocation{
 						Project: "project",
 						MachineNetworks: []*metal.MachineNetwork{
 							{
-								IPs: []string{"212.89.42.2", "212.89.42.1"},
-								Vrf: 104009,
+								IPs:      []string{"212.89.42.2", "212.89.42.1"},
+								Prefixes: []string{"212.89.42.0/24"},
+								Vrf:      104009,
 							},
 						},
 					},
@@ -466,7 +490,7 @@ func TestMakeBGPFilterMachine(t *testing.T) {
 	for i := range tests {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
-			got := makeBGPFilterMachine(tt.args.machine, tt.args.ipsMap)
+			got := makeBGPFilterMachine(tt.args.machine, tt.args.ipsMap, tt.args.fwNetworks)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("makeBGPFilterMachine() = %v, want %v", got, tt.want)
 			}
@@ -524,6 +548,9 @@ func TestMakeSwitchNics(t *testing.T) {
 						metal.IP{
 							IPAddress: "212.89.1.1",
 						},
+						metal.IP{
+							IPAddress: "10.0.0.1",
+						},
 					},
 				},
 				images: metal.ImageMap{
@@ -537,6 +564,15 @@ func TestMakeSwitchNics(t *testing.T) {
 						Base: metal.Base{ID: "m1"},
 						Allocation: &metal.MachineAllocation{
 							Project: "project",
+							MachineNetworks: []*metal.MachineNetwork{
+								{
+									NetworkID:      "private",
+									Vrf:            2,
+									Private:        true,
+									PrivatePrimary: true,
+									IPs:            []string{"10.0.0.1"},
+								},
+							},
 						},
 					},
 					metal.Machine{
@@ -545,8 +581,22 @@ func TestMakeSwitchNics(t *testing.T) {
 							Project: "p",
 							ImageID: "fwimg",
 							MachineNetworks: []*metal.MachineNetwork{
-								{Vrf: 1},
-								{Vrf: 2},
+								{
+									NetworkID: "underlay",
+									Vrf:       1,
+								},
+								{
+									NetworkID:      "private",
+									Vrf:            2,
+									Private:        true,
+									PrivatePrimary: true,
+									Prefixes:       []string{"10.0.0.1/24"},
+								},
+								{
+									NetworkID: "external",
+									Vrf:       3,
+									Prefixes:  []string{"212.89.1.0/24"},
+								},
 							},
 						},
 					},
@@ -557,7 +607,7 @@ func TestMakeSwitchNics(t *testing.T) {
 					Name: "swp1",
 					Vrf:  "vrf1",
 					BGPFilter: &v1.BGPFilter{
-						CIDRs: []string{"212.89.1.1/32"},
+						CIDRs: []string{"10.0.0.1/24", "212.89.1.1/32"},
 						VNIs:  []string{},
 					},
 				},
@@ -566,7 +616,7 @@ func TestMakeSwitchNics(t *testing.T) {
 					Vrf:  "default",
 					BGPFilter: &v1.BGPFilter{
 						CIDRs: []string{},
-						VNIs:  []string{"1", "2"},
+						VNIs:  []string{"1", "2", "3"},
 					},
 				},
 			},
@@ -577,7 +627,8 @@ func TestMakeSwitchNics(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			got := makeSwitchNics(tt.args.s, tt.args.ips, tt.args.images, tt.args.machines)
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("makeSwitchNics() = %v, want %v", got, tt.want)
+
+				t.Errorf("makeSwitchNics() = %v, want %v, diff: %s", got, tt.want, cmp.Diff(got, tt.want))
 			}
 		})
 	}

cmd/metal-api/internal/tags/tags.go

@@ -1,8 +1,9 @@
 package tags
 
 import (
-	"sort"
 	"strings"
+
+	"github.com/metal-stack/metal-api/cmd/metal-api/internal/utils"
 )
 
 // Tags holds tags.
@@ -72,14 +73,5 @@ func (t *Tags) Values(prefix string) []string {
 
 // Unique returns the distinct tag values as sorted slice.
 func (t *Tags) Unique() []string {
-	tagSet := make(map[string]bool)
-	for _, t := range t.tags {
-		tagSet[t] = true
-	}
-	uniqueTags := []string{}
-	for k := range tagSet {
-		uniqueTags = append(uniqueTags, k)
-	}
-	sort.Strings(uniqueTags)
-	return uniqueTags
+	return utils.UniqueSorted(t.tags)
 }

cmd/metal-api/internal/testdata/testdata.go

@@ -799,6 +799,7 @@ func InitMockDBData(mock *r.Mock) {
 	mock.On(r.DB("mockdb").Table("network").Get("404")).Return(nil, errors.New("Test Error"))
 	mock.On(r.DB("mockdb").Table("network").Get("999")).Return(nil, nil)
 	mock.On(r.DB("mockdb").Table("network").Filter(func(var_3 r.Term) r.Term { return var_3.Field("partitionid").Eq("1") }).Filter(func(var_4 r.Term) r.Term { return var_4.Field("privatesuper").Eq(true) })).Return(Nw3, nil)
+	mock.On(r.DB("mockdb").Table("network").Filter(func(var_4 r.Term) r.Term { return var_4.Field("privatesuper").Eq(true) })).Return([]metal.Network{Partition1PrivateSuperNetwork, Partition2PrivateSuperNetwork}, nil)
 
 	mock.On(r.DB("mockdb").Table("ip").Get("1.2.3.4")).Return(IP1, nil)
 	mock.On(r.DB("mockdb").Table("ip").Get("2.3.4.5")).Return(IP2, nil)

cmd/metal-api/internal/utils/utils.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"errors"
 	"runtime"
+	"sort"
 	"strconv"
 	"strings"
 
@@ -71,3 +72,16 @@ func GetOsAndSemverFromImage(id string) (string, *semver.Version, error) {
 	}
 	return os, v, nil
 }
+
+func UniqueSorted(i []string) []string {
+	set := make(map[string]bool)
+	for _, e := range i {
+		set[e] = true
+	}
+	unique := []string{}
+	for k := range set {
+		unique = append(unique, k)
+	}
+	sort.Strings(unique)
+	return unique
+}