Skip to content

Commit

Permalink
Merge additionalannouncablecidrs in
Browse files Browse the repository at this point in the history
  • Loading branch information
majst01 committed Aug 16, 2024
2 parents fa47c76 + 6430ea1 commit 5f3287e
Show file tree
Hide file tree
Showing 10 changed files with 120 additions and 66 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package migrations

import (
r "gopkg.in/rethinkdb/rethinkdb-go.v6"

"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
)

func init() {
datastore.MustRegisterMigration(datastore.Migration{
Name: "migrate super tenant networks to contain additionannouncablecidrs",
Version: 6,
Up: func(db *r.Term, session r.QueryExecutor, rs *datastore.RethinkStore) error {
nws, err := rs.ListNetworks()
if err != nil {
return err
}

for _, old := range nws {
if !old.PrivateSuper {
continue
}
new := old

if len(old.AdditionalAnnouncableCIDRs) == 0 {
new.AdditionalAnnouncableCIDRs = []string{
// This was the previous hard coded default in metal-core
"10.240.0.0/12",
}
}

err = rs.UpdateNetwork(&old, &new)
if err != nil {
return err
}
}
return nil
},
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func init() {
}
datastore.MustRegisterMigration(datastore.Migration{
Name: "migrate partition.childprefixlength to tenant super network",
Version: 6,
Version: 7,
Up: func(db *r.Term, session r.QueryExecutor, rs *datastore.RethinkStore) error {
nws, err := rs.ListNetworks()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/metal-api/internal/metal/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ type Network struct {
Shared bool `rethinkdb:"shared" json:"shared"`
Labels map[string]string `rethinkdb:"labels" json:"labels"`
AddressFamilies AddressFamilies `rethinkdb:"addressfamily" json:"addressfamily"`
AdditionalAnnouncableCIDRs []string `rethinkdb:"additionalannouncablecidrs" json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork"`
AdditionalAnnouncableCIDRs []string `rethinkdb:"additionalannouncablecidrs" json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork"`
}

type ChildPrefixLength map[AddressFamily]uint8
Expand Down
19 changes: 13 additions & 6 deletions cmd/metal-api/internal/service/network-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
return
}

additionalRouteMapCIDRs, err := validateAdditionalRouteMapCIDRs(requestPayload.AdditionalAnnouncableCIDRs, privateSuper)
additionalAnnouncableCIDRs, err := validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, privateSuper)
if err != nil {
r.sendError(request, response, httperrors.BadRequest(err))
return
Expand Down Expand Up @@ -406,7 +406,7 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
Vrf: vrf,
Labels: labels,
AddressFamilies: addressFamilies,
AdditionalAnnouncableCIDRs: additionalRouteMapCIDRs,
AdditionalAnnouncableCIDRs: additionalAnnouncableCIDRs,
}

ctx := request.Request.Context()
Expand All @@ -433,16 +433,16 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
r.send(request, response, http.StatusCreated, v1.NewNetworkResponse(nw, usage))
}

func validateAdditionalRouteMapCIDRs(additionalCidrs []string, privateSuper bool) ([]string, error) {
func validateAdditionalAnnouncableCIDRs(additionalCidrs []string, privateSuper bool) ([]string, error) {
var result []string
if len(additionalCidrs) > 0 {
if !privateSuper {
return nil, errors.New("additionalroutemapcidrs can only be set in a private super network")
return nil, errors.New("additionalannouncablecidrs can only be set in a private super network")
}
for _, cidr := range additionalCidrs {
_, err := netip.ParsePrefix(cidr)
if err != nil {
return nil, fmt.Errorf("given cidr:%q in additionalroutemapcidrs is malformed:%w", cidr, err)
return nil, fmt.Errorf("given cidr:%q in additionalannouncablecidrs is malformed:%w", cidr, err)
}
result = append(result, cidr)
}
Expand Down Expand Up @@ -846,13 +846,20 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
}
}

additionalRouteMapCIDRs, err := validateAdditionalRouteMapCIDRs(requestPayload.AdditionalAnnouncableCIDRs, oldNetwork.PrivateSuper)
additionalRouteMapCIDRs, err := validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, oldNetwork.PrivateSuper)
if err != nil {
r.sendError(request, response, defaultError(err))
return
}
newNetwork.AdditionalAnnouncableCIDRs = additionalRouteMapCIDRs

additionalAnnouncableCIDRs, err := validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, oldNetwork.PrivateSuper)
if err != nil {
r.sendError(request, response, httperrors.BadRequest(err))
return
}
newNetwork.AdditionalAnnouncableCIDRs = additionalAnnouncableCIDRs

err = r.ds.UpdateNetwork(oldNetwork, &newNetwork)
if err != nil {
r.sendError(request, response, defaultError(err))
Expand Down
32 changes: 17 additions & 15 deletions cmd/metal-api/internal/service/switch-service.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (r *switchResource) listSwitches(request *restful.Request, response *restfu
return
}

resp, err := r.makeSwitchResponseList(ss, r.ds)
resp, err := r.makeSwitchResponseList(ss)
if err != nil {
r.sendError(request, response, defaultError(err))
return
Expand All @@ -184,7 +184,7 @@ func (r *switchResource) findSwitches(request *restful.Request, response *restfu
return
}

resp, err := r.makeSwitchResponseList(ss, r.ds)
resp, err := r.makeSwitchResponseList(ss)
if err != nil {
r.sendError(request, response, defaultError(err))
return
Expand Down Expand Up @@ -828,18 +828,20 @@ func (r *switchResource) makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap)
cidrs = append(cidrs, private.Prefixes...)

privateNetwork, err := r.ds.FindNetworkByID(private.NetworkID)
if err != nil {
return v1.BGPFilter{}, err
}
parentNetwork, err := r.ds.FindNetworkByID(privateNetwork.ParentNetworkID)
if err != nil && !metal.IsNotFound(err) {
return v1.BGPFilter{}, err
}
// Only for private networks, additionalRouteMapCidrs are applied.
// they contain usually the pod- and service- cidrs in a kubernetes cluster
if len(parentNetwork.AdditionalAnnouncableCIDRs) > 0 {
r.log.Debug("makeBGPFilterMachine", "additional cidrs", parentNetwork.AdditionalAnnouncableCIDRs)
cidrs = append(cidrs, parentNetwork.AdditionalAnnouncableCIDRs...)
if privateNetwork != nil {
parentNetwork, err := r.ds.FindNetworkByID(privateNetwork.ParentNetworkID)
if err != nil && !metal.IsNotFound(err) {
return v1.BGPFilter{}, err
}
// Only for private networks, AdditionalAnnouncableCIDRs are applied.
// they contain usually the pod- and service- cidrs in a kubernetes cluster
if parentNetwork != nil && len(parentNetwork.AdditionalAnnouncableCIDRs) > 0 {
r.log.Debug("makeBGPFilterMachine", "additional cidrs", parentNetwork.AdditionalAnnouncableCIDRs)
cidrs = append(cidrs, parentNetwork.AdditionalAnnouncableCIDRs...)
}
}
}
for _, i := range ips[m.Allocation.Project] {
Expand Down Expand Up @@ -1041,14 +1043,14 @@ func findSwitchReferencedEntities(s *metal.Switch, ds *datastore.RethinkStore) (
return p, ips.ByProjectID(), m, ss, nil
}

func (r *switchResource) makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v1.SwitchResponse, error) {
pMap, ips, err := getSwitchReferencedEntityMaps(ds)
func (r *switchResource) makeSwitchResponseList(ss metal.Switches) ([]*v1.SwitchResponse, error) {
pMap, ips, err := getSwitchReferencedEntityMaps(r.ds)
if err != nil {
return nil, err
}

result := []*v1.SwitchResponse{}
m, err := ds.ListMachines()
m, err := r.ds.ListMachines()
if err != nil {
return nil, fmt.Errorf("could not find machines: %w", err)
}
Expand All @@ -1066,7 +1068,7 @@ func (r *switchResource) makeSwitchResponseList(ss metal.Switches, ds *datastore
return nil, err
}
cons := r.makeSwitchCons(&sw)
ss, err := ds.GetSwitchStatus(sw.ID)
ss, err := r.ds.GetSwitchStatus(sw.ID)
if err != nil && !metal.IsNotFound(err) {
return nil, err
}
Expand Down
42 changes: 23 additions & 19 deletions cmd/metal-api/internal/service/switch-service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"log/slog"
"net/http"
"net/http/httptest"
"os"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -414,7 +415,6 @@ func TestMakeBGPFilterFirewall(t *testing.T) {

func TestMakeBGPFilterMachine(t *testing.T) {
ds, mock := datastore.InitMockDB(t)
testdata.InitMockDBData(mock)

type args struct {
machine metal.Machine
Expand Down Expand Up @@ -450,29 +450,33 @@ func TestMakeBGPFilterMachine(t *testing.T) {
Project: "project",
MachineNetworks: []*metal.MachineNetwork{
{
IPs: []string{"10.1.0.1"},
Prefixes: []string{"10.2.0.0/22", "10.1.0.0/22"},
Vrf: 1234,
Private: true,
NetworkID: "1",
IPs: []string{"10.1.0.1"},
Prefixes: []string{"10.2.0.0/22", "10.1.0.0/22"},
Vrf: 1234,
Private: true,
},
{
IPs: []string{"10.0.0.2", "10.0.0.1"},
Vrf: 0,
Underlay: true,
NetworkID: "2",
IPs: []string{"10.0.0.2", "10.0.0.1"},
Vrf: 0,
Underlay: true,
},
{
IPs: []string{"212.89.42.2", "212.89.42.1"},
Vrf: 104009,
NetworkID: "3",
IPs: []string{"212.89.42.2", "212.89.42.1"},
Vrf: 104009,
},
{
IPs: []string{"2001::"},
Vrf: 104010,
NetworkID: "4",
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", "2001::1/128", "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", "10.240.0.0/12", "2001::1/128", "212.89.42.1/32", "212.89.42.2/32"}),
},
{
name: "allow only allocated ips",
Expand All @@ -487,8 +491,9 @@ func TestMakeBGPFilterMachine(t *testing.T) {
Project: "project",
MachineNetworks: []*metal.MachineNetwork{
{
IPs: []string{"212.89.42.2", "212.89.42.1"},
Vrf: 104009,
NetworkID: "5",
IPs: []string{"212.89.42.2", "212.89.42.1"},
Vrf: 104009,
},
},
},
Expand All @@ -501,13 +506,12 @@ func TestMakeBGPFilterMachine(t *testing.T) {
for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
// FIXME return super network with additionalroutemapcidrs set
mock.On(r.DB("mockdb").Table("network").Get(r.MockAnything()).Replace(r.MockAnything())).Return(testdata.EmptyResult, nil)
mock.On(r.DB("mockdb").Table("network").Get(r.MockAnything()).Replace(r.MockAnything())).Return(testdata.EmptyResult, nil)
mock.On(r.DB("mockdb").Table("network").Get(r.MockAnything())).Return(testdata.Partition1PrivateSuperNetwork, nil)

r := switchResource{webResource: webResource{ds: ds, log: slog.Default()}}
r := switchResource{webResource: webResource{ds: ds, log: slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}))}}

got, _ := r.makeBGPFilterMachine(tt.args.machine, tt.args.ipsMap)

if !reflect.DeepEqual(got, tt.want) {
t.Errorf("makeBGPFilterMachine() = %v, want %v", got, tt.want)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/metal-api/internal/service/v1/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type NetworkImmutable struct {
VrfShared *bool `json:"vrfshared" description:"if set to true, given vrf can be used by multiple networks, which is sometimes useful for network partitioning (default: false)" optional:"true"`
ParentNetworkID *string `json:"parentnetworkid" description:"the id of the parent network" optional:"true"`
AddressFamilies metal.AddressFamilies `json:"addressfamilies" description:"the addressfamilies in this network, either IPv4 or IPv6 or both"`
AdditionalAnnouncableCIDRs []string `json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork" optional:"true"`
AdditionalAnnouncableCIDRs []string `json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork" optional:"true"`
}

// NetworkUsage reports core metrics about available and used IPs or Prefixes in a Network.
Expand Down Expand Up @@ -67,7 +67,7 @@ type NetworkUpdateRequest struct {
DestinationPrefixes []string `json:"destinationprefixes" description:"the destination prefixes of this network" optional:"true"`
Labels map[string]string `json:"labels" description:"free labels that you associate with this network." optional:"true"`
Shared *bool `json:"shared" description:"marks a network as shareable." optional:"true"`
AdditionalAnnouncableCIDRs []string `json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork" optional:"true"`
AdditionalAnnouncableCIDRs []string `json:"additionalannouncablecidrs" description:"list of cidrs which are added to the route maps per tenant private network, these are typically pod- and service cidrs, can only be set in a supernetwork" optional:"true"`
}

// NetworkResponse holds all properties returned in a FindNetwork or GetNetwork request.
Expand Down
16 changes: 8 additions & 8 deletions cmd/metal-api/internal/testdata/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,14 +333,14 @@ var (
Base: metal.Base{
ID: "super-tenant-network-1",
},
Prefixes: metal.Prefixes{superPrefix},
PartitionID: Partition1.ID,
DefaultChildPrefixLength: metal.ChildPrefixLength{metal.IPv4AddressFamily: 22},
ParentNetworkID: "",
ProjectID: "",
PrivateSuper: true,
Nat: false,
Underlay: false,
Prefixes: metal.Prefixes{{IP: "10.0.0.0", Length: "16"}},
PartitionID: Partition1.ID,
ParentNetworkID: "",
ProjectID: "",
PrivateSuper: true,
Nat: false,
Underlay: false,
AdditionalAnnouncableCIDRs: []string{"10.240.0.0/12"},
}

Partition2PrivateSuperNetwork = metal.Network{
Expand Down
11 changes: 5 additions & 6 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/metal-stack/security v0.8.1
github.com/metal-stack/v v1.0.3
github.com/nsqio/go-nsq v1.1.0
github.com/prometheus/client_golang v1.19.1
github.com/prometheus/client_golang v1.20.0
github.com/samber/lo v1.47.0
github.com/spf13/cobra v1.8.1
github.com/spf13/viper v1.19.0
Expand All @@ -38,8 +38,6 @@ require (
gopkg.in/rethinkdb/rethinkdb-go.v6 v6.2.2
)

require github.com/moby/sys/userns v0.1.0 // indirect

replace (
// Newer versions do not export base entities which are used to composite other entities.
// This breaks metalctl and friends
Expand Down Expand Up @@ -143,7 +141,8 @@ require (
github.com/moby/docker-image-spec v1.3.1 // indirect
github.com/moby/patternmatcher v0.6.0 // indirect
github.com/moby/sys/sequential v0.6.0 // indirect
github.com/moby/sys/user v0.3.0 // indirect
github.com/moby/sys/user v0.2.0 // indirect
github.com/moby/sys/userns v0.1.0 // indirect
github.com/moby/term v0.5.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
Expand Down Expand Up @@ -209,8 +208,8 @@ require (
golang.org/x/text v0.17.0 // indirect
golang.org/x/time v0.6.0 // indirect; indirecct
golang.zx2c4.com/wireguard/windows v0.5.3 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240812133136-8ffd90a71988 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240812133136-8ffd90a71988 // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20240814211410-ddb44dafa142 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect
gopkg.in/cenkalti/backoff.v2 v2.2.1 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
Expand Down
Loading

0 comments on commit 5f3287e

Please sign in to comment.