Validate Prefixes and DestinationPrefixes on create and update network

metal-stack · Jul 7, 2024 · e8571c9 · e8571c9
1 parent 22d916e
commit e8571c9
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 43 deletions.
diff --git a/cmd/metal-api/internal/datastore/migrations/06_childprefixlength.go b/cmd/metal-api/internal/datastore/migrations/06_childprefixlength.go
@@ -34,6 +34,7 @@ func init() {
 					return err
 				}
 
+				// TODO: does not work somehow
 				new := old
 				new.ChildPrefixLength = &partition.PrivateNetworkPrefixLength
 				err = rs.UpdateNetwork(&old, &new)

diff --git a/cmd/metal-api/internal/service/network-service.go b/cmd/metal-api/internal/service/network-service.go
@@ -262,35 +262,16 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
 		return
 	}
 
-	prefixes := metal.Prefixes{}
-	addressFamilies := make(map[string]bool)
-	var addressFamily v1.AddressFamily
 	// all Prefixes must be valid and from the same addressfamily
-	for i := range requestPayload.Prefixes {
-		p := requestPayload.Prefixes[i]
-		prefix, err := metal.NewPrefixFromCIDR(p)
-		if err != nil {
-			r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("given prefix %v is not a valid ip with mask: %w", p, err)))
-			return
-		}
-		ipprefix, err := netip.ParsePrefix(p)
-		if err != nil {
-			r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("given prefix %v is not a valid ip with mask: %w", p, err)))
-			return
-		}
-		if ipprefix.Addr().Is4() {
-			addressFamilies["ipv4"] = true
-			addressFamily = v1.IPv4AddressFamily
-		}
-		if ipprefix.Addr().Is6() {
-			addressFamilies["ipv6"] = true
-			addressFamily = v1.IPv6AddressFamily
-		}
-		prefixes = append(prefixes, *prefix)
+	prefixes, addressFamily, err := validatePrefixes(requestPayload.Prefixes)
+	if err != nil {
+		r.sendError(request, response, httperrors.BadRequest(err))
+		return
 	}
-
-	if len(addressFamilies) > 1 {
-		r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("given prefixes have different addressfamilies")))
+	// all DestinationPrefixes must be valid and from the same addressfamily
+	_, _, err = validatePrefixes(requestPayload.DestinationPrefixes)
+	if err != nil {
+		r.sendError(request, response, httperrors.BadRequest(err))
 		return
 	}
 
@@ -480,6 +461,37 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
 	r.send(request, response, http.StatusCreated, v1.NewNetworkResponse(nw, usage))
 }
 
+func validatePrefixes(prefixes []string) (metal.Prefixes, v1.AddressFamily, error) {
+	var (
+		result          metal.Prefixes
+		addressFamilies = make(map[string]bool)
+		addressFamily   v1.AddressFamily
+	)
+	for _, p := range prefixes {
+		prefix, err := metal.NewPrefixFromCIDR(p)
+		if err != nil {
+			return nil, v1.IPv4AddressFamily, fmt.Errorf("given prefix %v is not a valid ip with mask: %w", p, err)
+		}
+		ipprefix, err := netip.ParsePrefix(p)
+		if err != nil {
+			return nil, v1.IPv4AddressFamily, fmt.Errorf("given prefix %v is not a valid ip with mask: %w", p, err)
+		}
+		if ipprefix.Addr().Is4() {
+			addressFamilies["ipv4"] = true
+			addressFamily = v1.IPv4AddressFamily
+		}
+		if ipprefix.Addr().Is6() {
+			addressFamilies["ipv6"] = true
+			addressFamily = v1.IPv6AddressFamily
+		}
+		result = append(result, *prefix)
+	}
+	if len(addressFamilies) > 1 {
+		return nil, v1.IPv4AddressFamily, fmt.Errorf("given prefixes have different addressfamilies")
+	}
+	return result, addressFamily, nil
+}
+
 // TODO add possibility to allocate from a non super network if given in the AllocateRequest and super has childprefixlength
 func (r *networkResource) allocateNetwork(request *restful.Request, response *restful.Response) {
 	var requestPayload v1.NetworkAllocateRequest
@@ -736,12 +748,15 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
 	var prefixesToBeAdded metal.Prefixes
 
 	if len(requestPayload.Prefixes) > 0 {
-		newNetwork.Prefixes, err = prefixesFromCidr(requestPayload.Prefixes)
+		// all Prefixes must be valid and from the same addressfamily
+		prefixes, _, err := validatePrefixes(requestPayload.Prefixes)
 		if err != nil {
-			r.sendError(request, response, defaultError(err))
+			r.sendError(request, response, httperrors.BadRequest(err))
 			return
 		}
 
+		newNetwork.Prefixes = prefixes
+
 		prefixesToBeRemoved = oldNetwork.SubtractPrefixes(newNetwork.Prefixes...)
 
 		// now validate if there are ips which have a prefix to be removed as a parent
@@ -779,11 +794,14 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
 	}
 
 	if len(requestPayload.DestinationPrefixes) > 0 {
-		newNetwork.DestinationPrefixes, err = prefixesFromCidr(requestPayload.DestinationPrefixes)
+		// all Prefixes must be valid and from the same addressfamily
+		prefixes, _, err := validatePrefixes(requestPayload.Prefixes)
 		if err != nil {
-			r.sendError(request, response, defaultError(err))
+			r.sendError(request, response, httperrors.BadRequest(err))
 			return
 		}
+
+		newNetwork.DestinationPrefixes = prefixes
 	}
 
 	err = r.ds.UpdateNetwork(oldNetwork, &newNetwork)
@@ -797,18 +815,6 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
 	r.send(request, response, http.StatusOK, v1.NewNetworkResponse(&newNetwork, usage))
 }
 
-func prefixesFromCidr(PrefixesCidr []string) (metal.Prefixes, error) {
-	var prefixes metal.Prefixes
-	for _, prefixCidr := range PrefixesCidr {
-		Prefix, err := metal.NewPrefixFromCIDR(prefixCidr)
-		if err != nil {
-			return nil, err
-		}
-		prefixes = append(prefixes, *Prefix)
-	}
-	return prefixes, nil
-}
-
 func (r *networkResource) deleteNetwork(request *restful.Request, response *restful.Response) {
 	id := request.PathParameter("id")