Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove hardcoded /32 bitmask for some switch entities #555

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Remove hardcoded /32 bitmask for some switch entities #555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 45 additions & 16 deletions cmd/metal-api/internal/service/switch-service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"log/slog"
"net/http"
"net/netip"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -776,31 +777,38 @@ func makeSwitchResponse(s *metal.Switch, ds *datastore.RethinkStore) (*v1.Switch
return nil, err
}

nics := makeSwitchNics(s, ips, machines)
nics, err := makeSwitchNics(s, ips, machines)
if err != nil {
return nil, err
}
cons := makeSwitchCons(s)

return v1.NewSwitchResponse(s, ss, p, nics, cons), nil
}

func makeBGPFilterFirewall(m metal.Machine) v1.BGPFilter {
func makeBGPFilterFirewall(m metal.Machine) (v1.BGPFilter, error) {
vnis := []string{}
cidrs := []string{}

for _, net := range m.Allocation.MachineNetworks {
if net.Underlay {
for _, ip := range net.IPs {
cidrs = append(cidrs, fmt.Sprintf("%s/32", ip))
ipwithMask, err := ipWithMask(ip)
if err != nil {
return v1.BGPFilter{}, err
}
cidrs = append(cidrs, ipwithMask)
}
} else {
vnis = append(vnis, fmt.Sprintf("%d", net.Vrf))
// filter for "project" addresses / cidrs is not possible since EVPN Type-5 routes can not be filtered by prefixes
}
}

return v1.NewBGPFilter(vnis, cidrs)
return v1.NewBGPFilter(vnis, cidrs), nil
}

func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) v1.BGPFilter {
func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, error) {
vnis := []string{}
cidrs := []string{}

Expand Down Expand Up @@ -828,29 +836,44 @@ func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) v1.BGPFilter {
continue
}
// Allow all other ip addresses allocated for the project.
cidrs = append(cidrs, fmt.Sprintf("%s/32", i.IPAddress))
ipwithMask, err := ipWithMask(i.IPAddress)
if err != nil {
return v1.BGPFilter{}, err
}
cidrs = append(cidrs, ipwithMask)
}

return v1.NewBGPFilter(vnis, cidrs)
return v1.NewBGPFilter(vnis, cidrs), nil
}

func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) v1.BGPFilter {
var filter v1.BGPFilter
func ipWithMask(ip string) (string, error) {
parsed, err := netip.ParseAddr(ip)
if err != nil {
return "", err
}
return fmt.Sprintf("%s/%d", ip, parsed.BitLen()), nil
}

func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, error) {
var (
filter v1.BGPFilter
err error
)

if m.IsFirewall() {
// vrf "default" means: the firewall was successfully allocated and the switch port configured
// otherwise the port is still not configured yet (pxe-setup) and a BGPFilter would break the install routine
if vrf == "default" {
filter = makeBGPFilterFirewall(m)
filter, err = makeBGPFilterFirewall(m)
}
} else {
filter = makeBGPFilterMachine(m, ips)
filter, err = makeBGPFilterMachine(m, ips)
}

return filter
return filter, err
}

func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) v1.SwitchNics {
func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) (v1.SwitchNics, error) {
machinesByID := map[string]*metal.Machine{}
for i, m := range machines {
machinesByID[m.ID] = &machines[i]
Expand All @@ -871,7 +894,10 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines)
m := machinesBySwp[n.Name]
var filter *v1.BGPFilter
if m != nil && m.Allocation != nil {
f := makeBGPFilter(*m, n.Vrf, ips)
f, err := makeBGPFilter(*m, n.Vrf, ips)
if err != nil {
return nil, err
}
filter = &f
}
nic := v1.SwitchNic{
Expand All @@ -896,7 +922,7 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines)
return nics[i].Name < nics[j].Name
})

return nics
return nics, nil
}

func makeSwitchCons(s *metal.Switch) []v1.SwitchConnection {
Expand Down Expand Up @@ -990,7 +1016,10 @@ func makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v
p = &partitionEntity
}

nics := makeSwitchNics(&sw, ips, m)
nics, err := makeSwitchNics(&sw, ips, m)
if err != nil {
return nil, err
}
cons := makeSwitchCons(&sw)
ss, err := ds.GetSwitchStatus(sw.ID)
if err != nil && !metal.IsNotFound(err) {
Expand Down
55 changes: 35 additions & 20 deletions cmd/metal-api/internal/service/switch-service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/rethinkdb/rethinkdb-go.v6"
r "gopkg.in/rethinkdb/rethinkdb-go.v6"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
Expand Down Expand Up @@ -351,11 +350,20 @@ func TestMakeBGPFilterFirewall(t *testing.T) {
IPs: []string{"212.89.42.1", "212.89.42.2"},
Vrf: 104009,
},
{
IPs: []string{"2001::", "fe80::"},
Vrf: 104011,
},
{
IPs: []string{"2002::", "fe81::"},
Underlay: true,
Vrf: 104012,
},
},
},
},
},
want: v1.NewBGPFilter([]string{"104009", "104010"}, []string{"10.0.0.1/32", "10.0.0.2/32"}),
want: v1.NewBGPFilter([]string{"104009", "104010", "104011"}, []string{"10.0.0.1/32", "10.0.0.2/32", "2002::/128", "fe81::/128"}),
},
{
name: "no underlay firewall networks",
Expand Down Expand Up @@ -395,7 +403,7 @@ func TestMakeBGPFilterFirewall(t *testing.T) {
for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
got := makeBGPFilterFirewall(tt.args.machine)
got, _ := makeBGPFilterFirewall(tt.args.machine)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("makeBGPFilterFirewall() = %v, want %v", got, tt.want)
}
Expand Down Expand Up @@ -429,6 +437,9 @@ func TestMakeBGPFilterMachine(t *testing.T) {
metal.IP{
IPAddress: "10.1.0.1",
},
metal.IP{
IPAddress: "2001::1",
},
}},
machine: metal.Machine{
Allocation: &metal.MachineAllocation{
Expand All @@ -449,11 +460,15 @@ func TestMakeBGPFilterMachine(t *testing.T) {
IPs: []string{"212.89.42.2", "212.89.42.1"},
Vrf: 104009,
},
{
IPs: []string{"2001::"},
Vrf: 104010,
},
},
},
},
},
want: v1.NewBGPFilter([]string{}, []string{"10.1.0.0/22", "10.2.0.0/22", "100.127.1.1/32", "212.89.42.1/32", "212.89.42.2/32"}),
want: v1.NewBGPFilter([]string{}, []string{"10.1.0.0/22", "10.2.0.0/22", "100.127.1.1/32", "2001::1/128", "212.89.42.1/32", "212.89.42.2/32"}),
},
{
name: "allow only allocated ips",
Expand Down Expand Up @@ -481,7 +496,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)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("makeBGPFilterMachine() = %v, want %v", got, tt.want)
}
Expand Down Expand Up @@ -588,7 +603,7 @@ func TestMakeSwitchNics(t *testing.T) {
for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
got := makeSwitchNics(tt.args.s, tt.args.ips, tt.args.machines)
got, _ := makeSwitchNics(tt.args.s, tt.args.ips, tt.args.machines)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("makeSwitchNics() = %v, want %v", got, tt.want)
}
Expand Down Expand Up @@ -1425,23 +1440,23 @@ func TestToggleSwitchNicWithoutMachine(t *testing.T) {
func Test_SwitchDelete(t *testing.T) {
tests := []struct {
name string
mockFn func(mock *rethinkdb.Mock)
mockFn func(mock *r.Mock)
want *v1.SwitchResponse
wantErr error
wantStatus int
force bool
}{
{
name: "delete switch",
mockFn: func(mock *rethinkdb.Mock) {
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
mockFn: func(mock *r.Mock) {
mock.On(r.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
Base: metal.Base{
ID: "switch-1",
},
}, nil)
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
mock.On(rethinkdb.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil)
mock.On(rethinkdb.DB("mockdb").Table("ip")).Return(nil, nil)
mock.On(r.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
mock.On(r.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil)
mock.On(r.DB("mockdb").Table("ip")).Return(nil, nil)
},
want: &v1.SwitchResponse{
Common: v1.Common{
Expand All @@ -1460,16 +1475,16 @@ func Test_SwitchDelete(t *testing.T) {
},
{
name: "delete switch does not work due to machine connections",
mockFn: func(mock *rethinkdb.Mock) {
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
mockFn: func(mock *r.Mock) {
mock.On(r.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
Base: metal.Base{
ID: "switch-1",
},
MachineConnections: metal.ConnectionMap{
"port-a": metal.Connections{},
},
}, nil)
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
mock.On(r.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
},
wantErr: &httperrors.HTTPErrorResponse{
StatusCode: http.StatusBadRequest,
Expand All @@ -1479,18 +1494,18 @@ func Test_SwitchDelete(t *testing.T) {
},
{
name: "delete switch with force",
mockFn: func(mock *rethinkdb.Mock) {
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
mockFn: func(mock *r.Mock) {
mock.On(r.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{
Base: metal.Base{
ID: "switch-1",
},
MachineConnections: metal.ConnectionMap{
"port-a": metal.Connections{},
},
}, nil)
mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
mock.On(rethinkdb.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil)
mock.On(rethinkdb.DB("mockdb").Table("ip")).Return(nil, nil)
mock.On(r.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil)
mock.On(r.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil)
mock.On(r.DB("mockdb").Table("ip")).Return(nil, nil)
},
force: true,
want: &v1.SwitchResponse{
Expand Down