Merge additionalannouncablecidrs in

cmd/metal-api/internal/datastore/migrations/06_additional_announcable_cidrs.go

@@ -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
+		},
+	})
+}

cmd/metal-api/internal/datastore/migrations/06_childprefixlength.go → cmd/metal-api/internal/datastore/migrations/07_childprefixlength.go

@@ -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 {

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

@@ -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

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

@@ -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
@@ -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()
@@ -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)
 		}
@@ -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))

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

@@ -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
@@ -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
@@ -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] {
@@ -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)
 	}
@@ -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
 		}

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"
@@ -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
@@ -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",
@@ -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,
 							},
 						},
 					},
@@ -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)
 			}

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

@@ -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.
@@ -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.

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

@@ -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{

go.mod

@@ -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
@@ -143,7 +143,7 @@ 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/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
@@ -209,8 +209,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