From 28523097d3f8e81bc0adb2ee91b27b6803de597d Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Wed, 14 Aug 2024 12:48:51 +0200 Subject: [PATCH] Make additional announcable cidrs configurable per tenant super network --- .../datastore/network_integration_test.go | 3 + cmd/metal-api/internal/metal/network.go | 23 +++---- .../internal/service/switch-service.go | 62 ++++++++++++------- .../internal/service/switch-service_test.go | 50 ++++++++++----- cmd/metal-api/internal/service/v1/network.go | 41 ++++++------ cmd/metal-api/internal/testdata/testdata.go | 15 ++--- spec/metal-api.json | 28 +++++++++ 7 files changed, 146 insertions(+), 76 deletions(-) diff --git a/cmd/metal-api/internal/datastore/network_integration_test.go b/cmd/metal-api/internal/datastore/network_integration_test.go index a756df90..78261299 100644 --- a/cmd/metal-api/internal/datastore/network_integration_test.go +++ b/cmd/metal-api/internal/datastore/network_integration_test.go @@ -65,6 +65,9 @@ func (_ *networkTestable) defaultBody(n *metal.Network) *metal.Network { if n.DestinationPrefixes == nil { n.DestinationPrefixes = metal.Prefixes{} } + if n.AdditionalAnnouncableCIDRs == nil { + n.AdditionalAnnouncableCIDRs = []string{} + } return n } diff --git a/cmd/metal-api/internal/metal/network.go b/cmd/metal-api/internal/metal/network.go index 9f2477d7..22d344bf 100644 --- a/cmd/metal-api/internal/metal/network.go +++ b/cmd/metal-api/internal/metal/network.go @@ -207,17 +207,18 @@ func (p *Prefix) equals(other *Prefix) bool { // TODO specify rethinkdb restrictions. type Network struct { Base - Prefixes Prefixes `rethinkdb:"prefixes" json:"prefixes"` - DestinationPrefixes Prefixes `rethinkdb:"destinationprefixes" json:"destinationprefixes"` - PartitionID string `rethinkdb:"partitionid" json:"partitionid"` - ProjectID string `rethinkdb:"projectid" json:"projectid"` - ParentNetworkID string `rethinkdb:"parentnetworkid" json:"parentnetworkid"` - Vrf uint `rethinkdb:"vrf" json:"vrf"` - PrivateSuper bool `rethinkdb:"privatesuper" json:"privatesuper"` - Nat bool `rethinkdb:"nat" json:"nat"` - Underlay bool `rethinkdb:"underlay" json:"underlay"` - Shared bool `rethinkdb:"shared" json:"shared"` - Labels map[string]string `rethinkdb:"labels" json:"labels"` + Prefixes Prefixes `rethinkdb:"prefixes" json:"prefixes"` + DestinationPrefixes Prefixes `rethinkdb:"destinationprefixes" json:"destinationprefixes"` + PartitionID string `rethinkdb:"partitionid" json:"partitionid"` + ProjectID string `rethinkdb:"projectid" json:"projectid"` + ParentNetworkID string `rethinkdb:"parentnetworkid" json:"parentnetworkid"` + Vrf uint `rethinkdb:"vrf" json:"vrf"` + PrivateSuper bool `rethinkdb:"privatesuper" json:"privatesuper"` + Nat bool `rethinkdb:"nat" json:"nat"` + Underlay bool `rethinkdb:"underlay" json:"underlay"` + Shared bool `rethinkdb:"shared" json:"shared"` + Labels map[string]string `rethinkdb:"labels" json:"labels"` + 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"` } // Networks is a list of networks. diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 8fea0f96..cf601314 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -144,7 +144,7 @@ func (r *switchResource) findSwitch(request *restful.Request, response *restful. return } - resp, err := makeSwitchResponse(s, r.ds) + resp, err := r.makeSwitchResponse(s) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -160,7 +160,7 @@ func (r *switchResource) listSwitches(request *restful.Request, response *restfu return } - resp, err := makeSwitchResponseList(ss, r.ds) + resp, err := r.makeSwitchResponseList(ss, r.ds) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -184,7 +184,7 @@ func (r *switchResource) findSwitches(request *restful.Request, response *restfu return } - resp, err := makeSwitchResponseList(ss, r.ds) + resp, err := r.makeSwitchResponseList(ss, r.ds) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -223,7 +223,7 @@ func (r *switchResource) deleteSwitch(request *restful.Request, response *restfu return } - resp, err := makeSwitchResponse(s, r.ds) + resp, err := r.makeSwitchResponse(s) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -388,7 +388,7 @@ func (r *switchResource) toggleSwitchPort(request *restful.Request, response *re } } - resp, err := makeSwitchResponse(&newSwitch, r.ds) + resp, err := r.makeSwitchResponse(&newSwitch) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -437,7 +437,7 @@ func (r *switchResource) updateSwitch(request *restful.Request, response *restfu return } - resp, err := makeSwitchResponse(&newSwitch, r.ds) + resp, err := r.makeSwitchResponse(&newSwitch) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -553,7 +553,7 @@ func (r *switchResource) registerSwitch(request *restful.Request, response *rest } - resp, err := makeSwitchResponse(s, r.ds) + resp, err := r.makeSwitchResponse(s) if err != nil { r.sendError(request, response, defaultError(err)) return @@ -772,22 +772,22 @@ func updateSwitchNics(oldNics, newNics map[string]*metal.Nic, currentConnections return finalNics, nil } -func makeSwitchResponse(s *metal.Switch, ds *datastore.RethinkStore) (*v1.SwitchResponse, error) { - p, ips, machines, ss, err := findSwitchReferencedEntities(s, ds) +func (r *switchResource) makeSwitchResponse(s *metal.Switch) (*v1.SwitchResponse, error) { + p, ips, machines, ss, err := findSwitchReferencedEntities(s, r.ds) if err != nil { return nil, err } - nics, err := makeSwitchNics(s, ips, machines) + nics, err := r.makeSwitchNics(s, ips, machines) if err != nil { return nil, err } - cons := makeSwitchCons(s) + cons := r.makeSwitchCons(s) return v1.NewSwitchResponse(s, ss, p, nics, cons), nil } -func makeBGPFilterFirewall(m metal.Machine) (v1.BGPFilter, error) { +func (r *switchResource) makeBGPFilterFirewall(m metal.Machine) (v1.BGPFilter, error) { vnis := []string{} cidrs := []string{} @@ -809,7 +809,7 @@ func makeBGPFilterFirewall(m metal.Machine) (v1.BGPFilter, error) { return v1.NewBGPFilter(vnis, cidrs), nil } -func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, error) { +func (r *switchResource) makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, error) { vnis := []string{} cidrs := []string{} @@ -826,6 +826,24 @@ func makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, erro // Allow all prefixes of the private network if private != nil { cidrs = append(cidrs, private.Prefixes...) + + privateNetwork, err := r.ds.FindNetworkByID(private.NetworkID) + if err != nil && !metal.IsNotFound(err) { + return v1.BGPFilter{}, err + } + if privateNetwork != nil { + parentNetwork, err := r.ds.FindNetworkByID(privateNetwork.ParentNetworkID) + if err != nil && !metal.IsNotFound(err) { + return v1.BGPFilter{}, err + } + r.log.Info("makeBGPFilterMachine", "parent", parentNetwork) + // Only for private networks, AdditionalAnnouncableCIDRs are applied. + // they contain usually the pod- and service- cidrs in a kubernetes cluster + if len(parentNetwork.AdditionalAnnouncableCIDRs) > 0 { + r.log.Info("makeBGPFilterMachine", "additional cidrs", parentNetwork.AdditionalAnnouncableCIDRs) + cidrs = append(cidrs, parentNetwork.AdditionalAnnouncableCIDRs...) + } + } } for _, i := range ips[m.Allocation.Project] { // No need to add /32 addresses of the primary network to the whitelist. @@ -884,7 +902,7 @@ func compactCidrs(cidrs []string) ([]string, error) { return compactedCidrs, nil } -func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, error) { +func (r *switchResource) makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, error) { var ( filter v1.BGPFilter err error @@ -894,16 +912,16 @@ func makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, // 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, err = makeBGPFilterFirewall(m) + filter, err = r.makeBGPFilterFirewall(m) } } else { - filter, err = makeBGPFilterMachine(m, ips) + filter, err = r.makeBGPFilterMachine(m, ips) } return filter, err } -func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) (v1.SwitchNics, error) { +func (r *switchResource) 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] @@ -924,7 +942,7 @@ 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, err := makeBGPFilter(*m, n.Vrf, ips) + f, err := r.makeBGPFilter(*m, n.Vrf, ips) if err != nil { return nil, err } @@ -955,7 +973,7 @@ func makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) return nics, nil } -func makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { +func (r *switchResource) makeSwitchCons(s *metal.Switch) []v1.SwitchConnection { cons := []v1.SwitchConnection{} nicMap := s.Nics.ByName() @@ -1026,7 +1044,7 @@ func findSwitchReferencedEntities(s *metal.Switch, ds *datastore.RethinkStore) ( return p, ips.ByProjectID(), m, ss, nil } -func makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v1.SwitchResponse, error) { +func (r *switchResource) makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v1.SwitchResponse, error) { pMap, ips, err := getSwitchReferencedEntityMaps(ds) if err != nil { return nil, err @@ -1046,11 +1064,11 @@ func makeSwitchResponseList(ss metal.Switches, ds *datastore.RethinkStore) ([]*v p = &partitionEntity } - nics, err := makeSwitchNics(&sw, ips, m) + nics, err := r.makeSwitchNics(&sw, ips, m) if err != nil { return nil, err } - cons := makeSwitchCons(&sw) + cons := r.makeSwitchCons(&sw) ss, err := ds.GetSwitchStatus(sw.ID) if err != nil && !metal.IsNotFound(err) { return nil, err diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 8d003d12..05477052 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "os" "reflect" "testing" "time" @@ -403,7 +404,8 @@ 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) + r := switchResource{} + got, _ := r.makeBGPFilterFirewall(tt.args.machine) if !reflect.DeepEqual(got, tt.want) { t.Errorf("makeBGPFilterFirewall() = %v, want %v", got, tt.want) } @@ -412,6 +414,8 @@ func TestMakeBGPFilterFirewall(t *testing.T) { } func TestMakeBGPFilterMachine(t *testing.T) { + ds, mock := datastore.InitMockDB(t) + type args struct { machine metal.Machine ipsMap metal.IPsMap @@ -446,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", @@ -483,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, }, }, }, @@ -493,10 +502,16 @@ func TestMakeBGPFilterMachine(t *testing.T) { want: v1.NewBGPFilter([]string{}, []string{"212.89.42.1/32"}), }, } + for i := range tests { tt := tests[i] t.Run(tt.name, func(t *testing.T) { - got, _ := makeBGPFilterMachine(tt.args.machine, tt.args.ipsMap) + mock.On(r.DB("mockdb").Table("network").Get(r.MockAnything())).Return(testdata.Partition1PrivateSuperNetwork, nil) + + 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) } @@ -603,7 +618,8 @@ 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) + r := switchResource{} + got, _ := r.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) } diff --git a/cmd/metal-api/internal/service/v1/network.go b/cmd/metal-api/internal/service/v1/network.go index d1bb9cc7..911e3e54 100644 --- a/cmd/metal-api/internal/service/v1/network.go +++ b/cmd/metal-api/internal/service/v1/network.go @@ -15,14 +15,15 @@ type NetworkBase struct { // NetworkImmutable defines the properties which are immutable in the Network. type NetworkImmutable struct { - Prefixes []string `json:"prefixes" modelDescription:"a network which contains prefixes from which IP addresses can be allocated" description:"the prefixes of this network"` - DestinationPrefixes []string `json:"destinationprefixes" modelDescription:"prefixes that are reachable within this network" description:"the destination prefixes of this network"` - Nat bool `json:"nat" description:"if set to true, packets leaving this network get masqueraded behind interface ip"` - PrivateSuper bool `json:"privatesuper" description:"if set to true, this network will serve as a partition's super network for the internal machine networks,there can only be one privatesuper network per partition"` - Underlay bool `json:"underlay" description:"if set to true, this network can be used for underlay communication"` - Vrf *uint `json:"vrf" description:"the vrf this network is associated with" optional:"true"` - 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"` + Prefixes []string `json:"prefixes" modelDescription:"a network which contains prefixes from which IP addresses can be allocated" description:"the prefixes of this network"` + DestinationPrefixes []string `json:"destinationprefixes" modelDescription:"prefixes that are reachable within this network" description:"the destination prefixes of this network"` + Nat bool `json:"nat" description:"if set to true, packets leaving this network get masqueraded behind interface ip"` + PrivateSuper bool `json:"privatesuper" description:"if set to true, this network will serve as a partition's super network for the internal machine networks,there can only be one privatesuper network per partition"` + Underlay bool `json:"underlay" description:"if set to true, this network can be used for underlay communication"` + Vrf *uint `json:"vrf" description:"the vrf this network is associated with" optional:"true"` + 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"` + 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. @@ -57,10 +58,11 @@ type NetworkFindRequest struct { // NetworkUpdateRequest defines the properties of a Network which can be updated. type NetworkUpdateRequest struct { Common - Prefixes []string `json:"prefixes" description:"the prefixes of this network" optional:"true"` - 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"` + Prefixes []string `json:"prefixes" description:"the prefixes of this network" optional:"true"` + 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"` } // NetworkResponse holds all properties returned in a FindNetwork or GetNetwork request. @@ -104,13 +106,14 @@ func NewNetworkResponse(network *metal.Network, usage *metal.NetworkUsage) *Netw Shared: &network.Shared, }, NetworkImmutable: NetworkImmutable{ - Prefixes: network.Prefixes.String(), - DestinationPrefixes: network.DestinationPrefixes.String(), - Nat: network.Nat, - PrivateSuper: network.PrivateSuper, - Underlay: network.Underlay, - Vrf: &network.Vrf, - ParentNetworkID: parentNetworkID, + Prefixes: network.Prefixes.String(), + DestinationPrefixes: network.DestinationPrefixes.String(), + Nat: network.Nat, + PrivateSuper: network.PrivateSuper, + Underlay: network.Underlay, + Vrf: &network.Vrf, + ParentNetworkID: parentNetworkID, + AdditionalAnnouncableCIDRs: network.AdditionalAnnouncableCIDRs, }, Usage: NetworkUsage{ AvailableIPs: usage.AvailableIPs, diff --git a/cmd/metal-api/internal/testdata/testdata.go b/cmd/metal-api/internal/testdata/testdata.go index 273f5bfb..3002ac73 100644 --- a/cmd/metal-api/internal/testdata/testdata.go +++ b/cmd/metal-api/internal/testdata/testdata.go @@ -323,13 +323,14 @@ var ( Base: metal.Base{ ID: "super-tenant-network-1", }, - Prefixes: metal.Prefixes{{IP: "10.0.0.0", Length: "16"}}, - PartitionID: Partition1.ID, - 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{ diff --git a/spec/metal-api.json b/spec/metal-api.json index deac2690..d05e69e1 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -3613,6 +3613,13 @@ }, "v1.NetworkCreateRequest": { "properties": { + "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", + "items": { + "type": "string" + }, + "type": "array" + }, "description": { "description": "a description for this entity", "type": "string" @@ -3746,6 +3753,13 @@ "v1.NetworkImmutable": { "description": "a network which contains prefixes from which IP addresses can be allocated\nprefixes that are reachable within this network", "properties": { + "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", + "items": { + "type": "string" + }, + "type": "array" + }, "destinationprefixes": { "description": "the destination prefixes of this network", "items": { @@ -3796,6 +3810,13 @@ }, "v1.NetworkResponse": { "properties": { + "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", + "items": { + "type": "string" + }, + "type": "array" + }, "changed": { "description": "the last changed timestamp of this entity", "format": "date-time", @@ -3895,6 +3916,13 @@ }, "v1.NetworkUpdateRequest": { "properties": { + "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", + "items": { + "type": "string" + }, + "type": "array" + }, "description": { "description": "a description for this entity", "type": "string"