Skip to content

Commit 59c7511

Browse files
majst01majst01
committed
Constify defaultChildPrefixlength
1 parent e8571c9 commit 59c7511

File tree

6 files changed

+212
-17
lines changed

6 files changed

+212
-17
lines changed

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

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package migrations_integration
55

66
import (
77
"context"
8+
"fmt"
89
"log/slog"
910
"os"
1011
"time"
@@ -15,14 +16,15 @@ import (
1516
_ "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore/migrations"
1617
"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
1718
"github.com/metal-stack/metal-api/test"
19+
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
1820

1921
"testing"
2022

2123
"github.com/stretchr/testify/assert"
2224
"github.com/stretchr/testify/require"
2325
)
2426

25-
func Test_Migration(t *testing.T) {
27+
func Test_MigrationProvisioningEventContainer(t *testing.T) {
2628
container, c, err := test.StartRethink(t)
2729
require.NoError(t, err)
2830
defer func() {
@@ -125,3 +127,98 @@ func Test_Migration(t *testing.T) {
125127
assert.Equal(t, ec.Events[0].Time.Unix(), lastEventTime.Unix())
126128
assert.Equal(t, ec.Events[1].Time.Unix(), now.Unix())
127129
}
130+
131+
func Test_MigrationChildPrefixLength(t *testing.T) {
132+
type tmpPartition struct {
133+
ID string `rethinkdb:"id"`
134+
PrivateNetworkPrefixLength uint8 `rethinkdb:"privatenetworkprefixlength"`
135+
}
136+
137+
container, c, err := test.StartRethink(t)
138+
require.NoError(t, err)
139+
defer func() {
140+
_ = container.Terminate(context.Background())
141+
}()
142+
143+
log := slog.New(slog.NewJSONHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug}))
144+
145+
rs := datastore.New(log, c.IP+":"+c.Port, c.DB, c.User, c.Password)
146+
// limit poolsize to speed up initialization
147+
rs.VRFPoolRangeMin = 10000
148+
rs.VRFPoolRangeMax = 10010
149+
rs.ASNPoolRangeMin = 10000
150+
rs.ASNPoolRangeMax = 10010
151+
152+
err = rs.Connect()
153+
require.NoError(t, err)
154+
err = rs.Initialize()
155+
require.NoError(t, err)
156+
157+
var (
158+
p1 = &tmpPartition{
159+
ID: "p1",
160+
PrivateNetworkPrefixLength: 22,
161+
}
162+
p2 = &tmpPartition{
163+
ID: "p2",
164+
PrivateNetworkPrefixLength: 24,
165+
}
166+
n1 = &metal.Network{
167+
Base: metal.Base{
168+
ID: "n1",
169+
},
170+
PartitionID: "p1",
171+
PrivateSuper: true,
172+
}
173+
n2 = &metal.Network{
174+
Base: metal.Base{
175+
ID: "n2",
176+
},
177+
PartitionID: "p2",
178+
PrivateSuper: true,
179+
}
180+
n3 = &metal.Network{
181+
Base: metal.Base{
182+
ID: "n3",
183+
},
184+
PartitionID: "p2",
185+
PrivateSuper: false,
186+
}
187+
)
188+
_, err = r.DB("metal").Table("partition").Insert(p1).RunWrite(rs.Session())
189+
require.NoError(t, err)
190+
_, err = r.DB("metal").Table("partition").Insert(p2).RunWrite(rs.Session())
191+
require.NoError(t, err)
192+
193+
err = rs.CreateNetwork(n1)
194+
require.NoError(t, err)
195+
err = rs.CreateNetwork(n2)
196+
require.NoError(t, err)
197+
err = rs.CreateNetwork(n3)
198+
require.NoError(t, err)
199+
200+
err = rs.Migrate(nil, false)
201+
require.NoError(t, err)
202+
203+
p, err := rs.FindPartition(p1.ID)
204+
require.NoError(t, err)
205+
require.NotNil(t, p)
206+
p, err = rs.FindPartition(p2.ID)
207+
require.NoError(t, err)
208+
require.NotNil(t, p)
209+
210+
n1fetched, err := rs.FindNetworkByID(n1.ID)
211+
require.NoError(t, err)
212+
require.NotNil(t, n1fetched)
213+
require.Equal(t, p1.PrivateNetworkPrefixLength, *n1fetched.ChildPrefixLength, fmt.Sprintf("childprefixlength:%d", *n1fetched.ChildPrefixLength))
214+
215+
n2fetched, err := rs.FindNetworkByID(n2.ID)
216+
require.NoError(t, err)
217+
require.NotNil(t, n2fetched)
218+
require.Equal(t, p2.PrivateNetworkPrefixLength, *n2fetched.ChildPrefixLength, fmt.Sprintf("childprefixlength:%d", *n2fetched.ChildPrefixLength))
219+
220+
n3fetched, err := rs.FindNetworkByID(n3.ID)
221+
require.NoError(t, err)
222+
require.NotNil(t, n3fetched)
223+
require.Nil(t, n3fetched.ChildPrefixLength)
224+
}

‎cmd/metal-api/internal/datastore/rethinkdb.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ func New(log *slog.Logger, dbhost string, dbname string, dbuser string, dbpass s
7878
}
7979
}
8080

81+
// Session exported for migration unit test
82+
func (rs *RethinkStore) Session() r.QueryExecutor {
83+
return rs.session
84+
}
85+
8186
func multi(session r.QueryExecutor, tt ...r.Term) error {
8287
for _, t := range tt {
8388
if err := t.Exec(session); err != nil {

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

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ import (
2323
"github.com/metal-stack/metal-lib/pkg/pointer"
2424
)
2525

26+
const (
27+
ipv4DefaultChildPrefixLength = uint8(22)
28+
ipv6DefaultChildPrefixLength = uint8(64)
29+
)
30+
2631
type networkResource struct {
2732
webResource
2833
ipamer ipam.IPAMer
@@ -278,10 +283,10 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
278283
if privateSuper && requestPayload.ChildPrefixLength == nil {
279284
var childprefixlength *uint8
280285
if addressFamily == v1.IPv4AddressFamily {
281-
childprefixlength = pointer.Pointer(uint8(22))
286+
childprefixlength = pointer.Pointer(ipv4DefaultChildPrefixLength)
282287
}
283288
if addressFamily == v1.IPv6AddressFamily {
284-
childprefixlength = pointer.Pointer(uint8(64))
289+
childprefixlength = pointer.Pointer(ipv6DefaultChildPrefixLength)
285290
}
286291
r.log.Info("createnetwork childprefixlength not set for private super network, using default", "addressfamily", addressFamily, "childprefixlength", childprefixlength)
287292
}
@@ -461,6 +466,24 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest
461466
r.send(request, response, http.StatusCreated, v1.NewNetworkResponse(nw, usage))
462467
}
463468

469+
func getAddressFamily(prefixes metal.Prefixes) (*v1.AddressFamily, error) {
470+
if len(prefixes) == 0 {
471+
return nil, nil
472+
}
473+
474+
parsed, err := netip.ParsePrefix(prefixes[0].String())
475+
if err != nil {
476+
return nil, err
477+
}
478+
if parsed.Addr().Is4() {
479+
return pointer.Pointer(v1.IPv4AddressFamily), nil
480+
}
481+
if parsed.Addr().Is6() {
482+
return pointer.Pointer(v1.IPv6AddressFamily), nil
483+
}
484+
return nil, fmt.Errorf("unable to detect addressfamily from prefixes:%v", prefixes.String())
485+
}
486+
464487
func validatePrefixes(prefixes []string) (metal.Prefixes, v1.AddressFamily, error) {
465488
var (
466489
result metal.Prefixes
@@ -724,6 +747,12 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
724747
return
725748
}
726749

750+
addressFamily, err := getAddressFamily(oldNetwork.Prefixes)
751+
if err != nil {
752+
r.sendError(request, response, defaultError(err))
753+
return
754+
}
755+
727756
newNetwork := *oldNetwork
728757

729758
if requestPayload.Name != nil {
@@ -744,17 +773,24 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
744773
return
745774
}
746775

747-
var prefixesToBeRemoved metal.Prefixes
748-
var prefixesToBeAdded metal.Prefixes
776+
var (
777+
prefixesToBeRemoved metal.Prefixes
778+
prefixesToBeAdded metal.Prefixes
779+
)
749780

750781
if len(requestPayload.Prefixes) > 0 {
751782
// all Prefixes must be valid and from the same addressfamily
752-
prefixes, _, err := validatePrefixes(requestPayload.Prefixes)
783+
prefixes, af, err := validatePrefixes(requestPayload.Prefixes)
753784
if err != nil {
754785
r.sendError(request, response, httperrors.BadRequest(err))
755786
return
756787
}
757788

789+
if af != *addressFamily {
790+
r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("new prefixes have different addressfamily %q then existing prefixes %q", af, *addressFamily)))
791+
return
792+
}
793+
758794
newNetwork.Prefixes = prefixes
759795

760796
prefixesToBeRemoved = oldNetwork.SubtractPrefixes(newNetwork.Prefixes...)
@@ -795,12 +831,17 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest
795831

796832
if len(requestPayload.DestinationPrefixes) > 0 {
797833
// all Prefixes must be valid and from the same addressfamily
798-
prefixes, _, err := validatePrefixes(requestPayload.Prefixes)
834+
prefixes, af, err := validatePrefixes(requestPayload.Prefixes)
799835
if err != nil {
800836
r.sendError(request, response, httperrors.BadRequest(err))
801837
return
802838
}
803839

840+
if af != *addressFamily {
841+
r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("new destination prefixes have different addressfamily %q then existing destination prefixes %q", af, *addressFamily)))
842+
return
843+
}
844+
804845
newNetwork.DestinationPrefixes = prefixes
805846
}
806847

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

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,23 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"net/netip"
11+
"reflect"
1112
"testing"
1213

14+
restful "github.com/emicklei/go-restful/v3"
1315
mdmv1 "github.com/metal-stack/masterdata-api/api/v1"
1416
mdmv1mock "github.com/metal-stack/masterdata-api/api/v1/mocks"
1517
mdm "github.com/metal-stack/masterdata-api/pkg/client"
16-
"github.com/metal-stack/metal-lib/httperrors"
17-
"github.com/metal-stack/metal-lib/pkg/pointer"
18-
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
19-
2018
"github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore"
2119
"github.com/metal-stack/metal-api/cmd/metal-api/internal/ipam"
22-
2320
"github.com/metal-stack/metal-api/cmd/metal-api/internal/metal"
2421
v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1"
25-
26-
restful "github.com/emicklei/go-restful/v3"
2722
"github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata"
23+
"github.com/metal-stack/metal-lib/httperrors"
24+
"github.com/metal-stack/metal-lib/pkg/pointer"
2825
testifymock "github.com/stretchr/testify/mock"
2926
"github.com/stretchr/testify/require"
27+
r "gopkg.in/rethinkdb/rethinkdb-go.v6"
3028
)
3129

3230
func TestGetNetworks(t *testing.T) {
@@ -550,3 +548,53 @@ func Test_networkResource_allocateNetwork(t *testing.T) {
550548
}
551549
}
552550
}
551+
552+
func Test_getAddressFamily(t *testing.T) {
553+
tests := []struct {
554+
name string
555+
prefixes metal.Prefixes
556+
want *v1.AddressFamily
557+
wantErr bool
558+
}{
559+
{
560+
name: "ipv4",
561+
prefixes: metal.Prefixes{
562+
metal.Prefix{IP: "10.0.0.0", Length: "8"},
563+
},
564+
want: pointer.Pointer(v1.IPv4AddressFamily),
565+
},
566+
{
567+
name: "ipv6",
568+
prefixes: metal.Prefixes{
569+
metal.Prefix{IP: "2001::", Length: "64"},
570+
},
571+
want: pointer.Pointer(v1.IPv6AddressFamily),
572+
},
573+
{
574+
name: "empty prefixes",
575+
prefixes: metal.Prefixes{},
576+
want: nil,
577+
wantErr: false,
578+
},
579+
{
580+
name: "malformed ipv4",
581+
prefixes: metal.Prefixes{
582+
metal.Prefix{IP: "10.0.0.0.0", Length: "6"},
583+
},
584+
want: nil,
585+
wantErr: true,
586+
},
587+
}
588+
for _, tt := range tests {
589+
t.Run(tt.name, func(t *testing.T) {
590+
got, err := getAddressFamily(tt.prefixes)
591+
if (err != nil) != tt.wantErr {
592+
t.Errorf("getAddressFamily() error = %v, wantErr %v", err, tt.wantErr)
593+
return
594+
}
595+
if !reflect.DeepEqual(got, tt.want) {
596+
t.Errorf("getAddressFamily() = %v, want %v", got, tt.want)
597+
}
598+
})
599+
}
600+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ type NetworkAllocateRequest struct {
4848
NetworkBase
4949
DestinationPrefixes []string `json:"destinationprefixes" description:"the destination prefixes of this network" optional:"true"`
5050
Nat *bool `json:"nat" description:"if set to true, packets leaving this network get masqueraded behind interface ip" optional:"true"`
51-
AddressFamily *string `json:"address_family" description:"can be ipv4 or ipv6, defaults to ipv4" optional:"true"`
52-
Length *uint8 `json:"length" description:"the bitlen of the prefix to allocate, defaults to childprefixlength of super prefix" optional:"true"`
51+
AddressFamily *string `json:"address_family" description:"can be ipv4 or ipv6, defaults to ipv4"`
52+
Length *uint8 `json:"length" description:"the bitlen of the prefix to allocate, defaults to childprefixlength of super prefix"`
5353
}
5454

5555
// AddressFamily identifies IPv4/IPv6

‎spec/metal-api.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3612,7 +3612,11 @@
36123612
"description": "marks a network as shareable.",
36133613
"type": "boolean"
36143614
}
3615-
}
3615+
},
3616+
"required": [
3617+
"address_family",
3618+
"length"
3619+
]
36163620
},
36173621
"v1.NetworkBase": {
36183622
"properties": {

0 commit comments

Comments
 (0)