Merge master

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

@@ -28,18 +28,27 @@ func init() {
 				if err != nil {
 					return err
 				}
+				if cursor.IsNil() {
+					_ = cursor.Close()
+					continue
+				}
 				var partition tmpPartition
 				err = cursor.One(&partition)
+				if err != nil {
+					_ = cursor.Close()
+					return err
+				}
+				err = cursor.Close()
 				if err != nil {
 					return err
 				}
-
 				new := old
 
 				var (
 					af                       metal.AddressFamily
 					defaultChildPrefixLength = metal.ChildPrefixLength{}
 				)
+				// FIXME check all prefixes
 				parsed, err := netip.ParsePrefix(new.Prefixes[0].String())
 				if err != nil {
 					return err
@@ -73,10 +82,6 @@ func init() {
 				if err != nil {
 					return err
 				}
-				err = cursor.Close()
-				if err != nil {
-					return err
-				}
 			}
 
 			_, err = db.Table("partition").Replace(r.Row.Without("privatenetworkprefixlength")).RunWrite(session)

cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go

@@ -23,14 +23,14 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func Test_MigrationProvisioningEventContainer(t *testing.T) {
+func Test_Migration(t *testing.T) {
 	container, c, err := test.StartRethink(t)
 	require.NoError(t, err)
 	defer func() {
 		_ = container.Terminate(context.Background())
 	}()
 
-	log := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelError}))
+	log := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelInfo}))
 
 	rs := datastore.New(log, c.IP+":"+c.Port, c.DB, c.User, c.Password)
 	rs.VRFPoolRangeMin = 10000

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

@@ -7,6 +7,8 @@ import (
 	"log/slog"
 	"net/http"
 	"net/netip"
+	"slices"
+	"strconv"
 
 	"connectrpc.com/connect"
 	mdmv1 "github.com/metal-stack/masterdata-api/api/v1"
@@ -370,7 +372,7 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
 		return
 	}
 
-	additionalAnnouncableCIDRs, err := validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, privateSuper)
+	err = validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, privateSuper)
 	if err != nil {
 		r.sendError(request, response, httperrors.BadRequest(err))
 		return
@@ -407,7 +409,7 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
 		Vrf:                        vrf,
 		Labels:                     labels,
 		AddressFamilies:            addressFamilies,
-		AdditionalAnnouncableCIDRs: additionalAnnouncableCIDRs,
+		AdditionalAnnouncableCIDRs: requestPayload.AdditionalAnnouncableCIDRs,
 	}
 
 	ctx := request.Request.Context()
@@ -434,21 +436,23 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
 	r.send(request, response, http.StatusCreated, v1.NewNetworkResponse(nw, usage))
 }
 
-func validateAdditionalAnnouncableCIDRs(additionalCidrs []string, privateSuper bool) ([]string, error) {
-	var result []string
-	if len(additionalCidrs) > 0 {
-		if !privateSuper {
-			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 additionalannouncablecidrs is malformed:%w", cidr, err)
-			}
-			result = append(result, cidr)
+func validateAdditionalAnnouncableCIDRs(additionalCidrs []string, privateSuper bool) error {
+	if len(additionalCidrs) == 0 {
+		return nil
+	}
+
+	if !privateSuper {
+		return errors.New("additionalannouncablecidrs can only be set in a private super network")
+	}
+
+	for _, cidr := range additionalCidrs {
+		_, err := netip.ParsePrefix(cidr)
+		if err != nil {
+			return fmt.Errorf("given cidr:%q in additionalannouncablecidrs is malformed:%w", cidr, err)
 		}
 	}
-	return result, nil
+
+	return nil
 }
 
 func validatePrefixesAndAddressFamilies(prefixes, destinationPrefixes []string, defaultChildPrefixLength metal.ChildPrefixLength, privateSuper bool) (metal.Prefixes, metal.Prefixes, metal.AddressFamilies, error) {
@@ -859,19 +863,12 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
 		}
 	}
 
-	additionalRouteMapCIDRs, err := validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, oldNetwork.PrivateSuper)
+	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
+	newNetwork.AdditionalAnnouncableCIDRs = requestPayload.AdditionalAnnouncableCIDRs
 
 	err = validateAdditionalAnnouncableCIDRs(requestPayload.AdditionalAnnouncableCIDRs, oldNetwork.PrivateSuper)
 	if err != nil {

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

@@ -773,12 +773,12 @@ func updateSwitchNics(oldNics, newNics map[string]*metal.Nic, currentConnections
 }
 
 func (r *switchResource) makeSwitchResponse(s *metal.Switch) (*v1.SwitchResponse, error) {
-	p, ips, machines, ss, err := findSwitchReferencedEntities(s, r.ds)
+	p, nws, ips, machines, ss, err := r.findSwitchReferencedEntities(s)
 	if err != nil {
 		return nil, err
 	}
 
-	nics, err := r.makeSwitchNics(s, ips, machines)
+	nics, err := r.makeSwitchNics(s, nws, ips, machines)
 	if err != nil {
 		return nil, err
 	}
@@ -809,7 +809,7 @@ func (r *switchResource) makeBGPFilterFirewall(m metal.Machine) (v1.BGPFilter, e
 	return v1.NewBGPFilter(vnis, cidrs), nil
 }
 
-func (r *switchResource) makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap) (v1.BGPFilter, error) {
+func (r *switchResource) makeBGPFilterMachine(m metal.Machine, nws metal.NetworkMap, ips metal.IPsMap) (v1.BGPFilter, error) {
 	vnis := []string{}
 	cidrs := []string{}
 
@@ -827,21 +827,19 @@ func (r *switchResource) makeBGPFilterMachine(m metal.Machine, ips metal.IPsMap)
 	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
+		privateNetwork, ok := nws[private.NetworkID]
+		if !ok {
+			return v1.BGPFilter{}, fmt.Errorf("no private network found for ID:%s", private.NetworkID)
 		}
-		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...)
-			}
+		parentNetwork, ok := nws[privateNetwork.ParentNetworkID]
+		if !ok {
+			return v1.BGPFilter{}, fmt.Errorf("no parent network found for ID:%s", privateNetwork.ParentNetworkID)
+		}
+		// 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.Debug("makeBGPFilterMachine", "additional cidrs", parentNetwork.AdditionalAnnouncableCIDRs)
+			cidrs = append(cidrs, parentNetwork.AdditionalAnnouncableCIDRs...)
 		}
 	}
 	for _, i := range ips[m.Allocation.Project] {
@@ -901,7 +899,7 @@ func compactCidrs(cidrs []string) ([]string, error) {
 	return compactedCidrs, nil
 }
 
-func (r *switchResource) makeBGPFilter(m metal.Machine, vrf string, ips metal.IPsMap) (v1.BGPFilter, error) {
+func (r *switchResource) makeBGPFilter(m metal.Machine, vrf string, nws metal.NetworkMap, ips metal.IPsMap) (v1.BGPFilter, error) {
 	var (
 		filter v1.BGPFilter
 		err    error
@@ -914,13 +912,13 @@ func (r *switchResource) makeBGPFilter(m metal.Machine, vrf string, ips metal.IP
 			filter, err = r.makeBGPFilterFirewall(m)
 		}
 	} else {
-		filter, err = r.makeBGPFilterMachine(m, ips)
+		filter, err = r.makeBGPFilterMachine(m, nws, ips)
 	}
 
 	return filter, err
 }
 
-func (r *switchResource) makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machines metal.Machines) (v1.SwitchNics, error) {
+func (r *switchResource) makeSwitchNics(s *metal.Switch, nws metal.NetworkMap, ips metal.IPsMap, machines metal.Machines) (v1.SwitchNics, error) {
 	machinesByID := map[string]*metal.Machine{}
 	for i, m := range machines {
 		machinesByID[m.ID] = &machines[i]
@@ -941,7 +939,7 @@ func (r *switchResource) makeSwitchNics(s *metal.Switch, ips metal.IPsMap, machi
 		m := machinesBySwp[n.Name]
 		var filter *v1.BGPFilter
 		if m != nil && m.Allocation != nil {
-			f, err := r.makeBGPFilter(*m, n.Vrf, ips)
+			f, err := r.makeBGPFilter(*m, n.Vrf, nws, ips)
 			if err != nil {
 				return nil, err
 			}
@@ -1051,7 +1049,7 @@ func (r *switchResource) findSwitchReferencedEntities(s *metal.Switch) (*metal.P
 }
 
 func (r *switchResource) makeSwitchResponseList(ss metal.Switches) ([]*v1.SwitchResponse, error) {
-	pMap, ips, err := getSwitchReferencedEntityMaps(r.ds)
+	pMap, nws, ips, err := getSwitchReferencedEntityMaps(r.ds)
 	if err != nil {
 		return nil, err
 	}
@@ -1070,7 +1068,7 @@ func (r *switchResource) makeSwitchResponseList(ss metal.Switches) ([]*v1.Switch
 			p = &partitionEntity
 		}
 
-		nics, err := r.makeSwitchNics(&sw, ips, m)
+		nics, err := r.makeSwitchNics(&sw, nws, ips, m)
 		if err != nil {
 			return nil, err
 		}

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

@@ -525,7 +525,7 @@ func TestMakeBGPFilterMachine(t *testing.T) {
 
 			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)
+			got, _ := r.makeBGPFilterMachine(tt.args.machine, tt.args.nws, tt.args.ipsMap)
 
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("makeBGPFilterMachine() = %v, want %v", got, tt.want)
@@ -635,7 +635,7 @@ func TestMakeSwitchNics(t *testing.T) {
 		tt := tests[i]
 		t.Run(tt.name, func(t *testing.T) {
 			r := switchResource{}
-			got, _ := r.makeSwitchNics(tt.args.s, tt.args.ips, tt.args.machines)
+			got, _ := r.makeSwitchNics(tt.args.s, tt.args.nws, tt.args.ips, tt.args.machines)
 			if !reflect.DeepEqual(got, tt.want) {
 				t.Errorf("makeSwitchNics() = %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,omitempty" 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 for private super networks"`
 }
 
 // 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 for private super networks" optional:"true"`
 }
 
 // NetworkResponse holds all properties returned in a FindNetwork or GetNetwork request.