diff --git a/cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go b/cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go index c42b553ab..48820f295 100644 --- a/cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go +++ b/cmd/metal-api/internal/datastore/migrations_integration/migrate_integration_test.go @@ -5,6 +5,7 @@ package migrations_integration import ( "context" + "fmt" "log/slog" "os" "time" @@ -15,6 +16,7 @@ import ( _ "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore/migrations" "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" "github.com/metal-stack/metal-api/test" + r "gopkg.in/rethinkdb/rethinkdb-go.v6" "testing" @@ -22,7 +24,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_Migration(t *testing.T) { +func Test_MigrationProvisioningEventContainer(t *testing.T) { container, c, err := test.StartRethink(t) require.NoError(t, err) defer func() { @@ -125,3 +127,98 @@ func Test_Migration(t *testing.T) { assert.Equal(t, ec.Events[0].Time.Unix(), lastEventTime.Unix()) assert.Equal(t, ec.Events[1].Time.Unix(), now.Unix()) } + +func Test_MigrationChildPrefixLength(t *testing.T) { + type tmpPartition struct { + ID string `rethinkdb:"id"` + PrivateNetworkPrefixLength uint8 `rethinkdb:"privatenetworkprefixlength"` + } + + 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.LevelDebug})) + + rs := datastore.New(log, c.IP+":"+c.Port, c.DB, c.User, c.Password) + // limit poolsize to speed up initialization + rs.VRFPoolRangeMin = 10000 + rs.VRFPoolRangeMax = 10010 + rs.ASNPoolRangeMin = 10000 + rs.ASNPoolRangeMax = 10010 + + err = rs.Connect() + require.NoError(t, err) + err = rs.Initialize() + require.NoError(t, err) + + var ( + p1 = &tmpPartition{ + ID: "p1", + PrivateNetworkPrefixLength: 22, + } + p2 = &tmpPartition{ + ID: "p2", + PrivateNetworkPrefixLength: 24, + } + n1 = &metal.Network{ + Base: metal.Base{ + ID: "n1", + }, + PartitionID: "p1", + PrivateSuper: true, + } + n2 = &metal.Network{ + Base: metal.Base{ + ID: "n2", + }, + PartitionID: "p2", + PrivateSuper: true, + } + n3 = &metal.Network{ + Base: metal.Base{ + ID: "n3", + }, + PartitionID: "p2", + PrivateSuper: false, + } + ) + _, err = r.DB("metal").Table("partition").Insert(p1).RunWrite(rs.Session()) + require.NoError(t, err) + _, err = r.DB("metal").Table("partition").Insert(p2).RunWrite(rs.Session()) + require.NoError(t, err) + + err = rs.CreateNetwork(n1) + require.NoError(t, err) + err = rs.CreateNetwork(n2) + require.NoError(t, err) + err = rs.CreateNetwork(n3) + require.NoError(t, err) + + err = rs.Migrate(nil, false) + require.NoError(t, err) + + p, err := rs.FindPartition(p1.ID) + require.NoError(t, err) + require.NotNil(t, p) + p, err = rs.FindPartition(p2.ID) + require.NoError(t, err) + require.NotNil(t, p) + + n1fetched, err := rs.FindNetworkByID(n1.ID) + require.NoError(t, err) + require.NotNil(t, n1fetched) + require.Equal(t, p1.PrivateNetworkPrefixLength, *n1fetched.ChildPrefixLength, fmt.Sprintf("childprefixlength:%d", *n1fetched.ChildPrefixLength)) + + n2fetched, err := rs.FindNetworkByID(n2.ID) + require.NoError(t, err) + require.NotNil(t, n2fetched) + require.Equal(t, p2.PrivateNetworkPrefixLength, *n2fetched.ChildPrefixLength, fmt.Sprintf("childprefixlength:%d", *n2fetched.ChildPrefixLength)) + + n3fetched, err := rs.FindNetworkByID(n3.ID) + require.NoError(t, err) + require.NotNil(t, n3fetched) + require.Nil(t, n3fetched.ChildPrefixLength) +} diff --git a/cmd/metal-api/internal/datastore/rethinkdb.go b/cmd/metal-api/internal/datastore/rethinkdb.go index 38cbc07ca..6564278a7 100644 --- a/cmd/metal-api/internal/datastore/rethinkdb.go +++ b/cmd/metal-api/internal/datastore/rethinkdb.go @@ -78,6 +78,11 @@ func New(log *slog.Logger, dbhost string, dbname string, dbuser string, dbpass s } } +// Session exported for migration unit test +func (rs *RethinkStore) Session() r.QueryExecutor { + return rs.session +} + func multi(session r.QueryExecutor, tt ...r.Term) error { for _, t := range tt { if err := t.Exec(session); err != nil { diff --git a/cmd/metal-api/internal/service/network-service.go b/cmd/metal-api/internal/service/network-service.go index 43089d7a0..fd8e37949 100644 --- a/cmd/metal-api/internal/service/network-service.go +++ b/cmd/metal-api/internal/service/network-service.go @@ -23,6 +23,11 @@ import ( "github.com/metal-stack/metal-lib/pkg/pointer" ) +const ( + ipv4DefaultChildPrefixLength = uint8(22) + ipv6DefaultChildPrefixLength = uint8(64) +) + type networkResource struct { webResource ipamer ipam.IPAMer @@ -278,10 +283,10 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest if privateSuper && requestPayload.ChildPrefixLength == nil { var childprefixlength *uint8 if addressFamily == v1.IPv4AddressFamily { - childprefixlength = pointer.Pointer(uint8(22)) + childprefixlength = pointer.Pointer(ipv4DefaultChildPrefixLength) } if addressFamily == v1.IPv6AddressFamily { - childprefixlength = pointer.Pointer(uint8(64)) + childprefixlength = pointer.Pointer(ipv6DefaultChildPrefixLength) } r.log.Info("createnetwork childprefixlength not set for private super network, using default", "addressfamily", addressFamily, "childprefixlength", childprefixlength) } @@ -461,6 +466,24 @@ func (r *networkResource) createNetwork(request *restful.Request, response *rest r.send(request, response, http.StatusCreated, v1.NewNetworkResponse(nw, usage)) } +func getAddressFamily(prefixes metal.Prefixes) (*v1.AddressFamily, error) { + if len(prefixes) == 0 { + return nil, nil + } + + parsed, err := netip.ParsePrefix(prefixes[0].String()) + if err != nil { + return nil, err + } + if parsed.Addr().Is4() { + return pointer.Pointer(v1.IPv4AddressFamily), nil + } + if parsed.Addr().Is6() { + return pointer.Pointer(v1.IPv6AddressFamily), nil + } + return nil, fmt.Errorf("unable to detect addressfamily from prefixes:%v", prefixes.String()) +} + func validatePrefixes(prefixes []string) (metal.Prefixes, v1.AddressFamily, error) { var ( result metal.Prefixes @@ -724,6 +747,12 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest return } + addressFamily, err := getAddressFamily(oldNetwork.Prefixes) + if err != nil { + r.sendError(request, response, defaultError(err)) + return + } + newNetwork := *oldNetwork if requestPayload.Name != nil { @@ -744,17 +773,24 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest return } - var prefixesToBeRemoved metal.Prefixes - var prefixesToBeAdded metal.Prefixes + var ( + prefixesToBeRemoved metal.Prefixes + prefixesToBeAdded metal.Prefixes + ) if len(requestPayload.Prefixes) > 0 { // all Prefixes must be valid and from the same addressfamily - prefixes, _, err := validatePrefixes(requestPayload.Prefixes) + prefixes, af, err := validatePrefixes(requestPayload.Prefixes) if err != nil { r.sendError(request, response, httperrors.BadRequest(err)) return } + if af != *addressFamily { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("new prefixes have different addressfamily %q then existing prefixes %q", af, *addressFamily))) + return + } + newNetwork.Prefixes = prefixes prefixesToBeRemoved = oldNetwork.SubtractPrefixes(newNetwork.Prefixes...) @@ -795,12 +831,17 @@ func (r *networkResource) updateNetwork(request *restful.Request, response *rest if len(requestPayload.DestinationPrefixes) > 0 { // all Prefixes must be valid and from the same addressfamily - prefixes, _, err := validatePrefixes(requestPayload.Prefixes) + prefixes, af, err := validatePrefixes(requestPayload.Prefixes) if err != nil { r.sendError(request, response, httperrors.BadRequest(err)) return } + if af != *addressFamily { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("new destination prefixes have different addressfamily %q then existing destination prefixes %q", af, *addressFamily))) + return + } + newNetwork.DestinationPrefixes = prefixes } diff --git a/cmd/metal-api/internal/service/network-service_test.go b/cmd/metal-api/internal/service/network-service_test.go index 3f1bb9771..9a7d8238b 100644 --- a/cmd/metal-api/internal/service/network-service_test.go +++ b/cmd/metal-api/internal/service/network-service_test.go @@ -8,25 +8,23 @@ import ( "net/http" "net/http/httptest" "net/netip" + "reflect" "testing" + restful "github.com/emicklei/go-restful/v3" mdmv1 "github.com/metal-stack/masterdata-api/api/v1" mdmv1mock "github.com/metal-stack/masterdata-api/api/v1/mocks" mdm "github.com/metal-stack/masterdata-api/pkg/client" - "github.com/metal-stack/metal-lib/httperrors" - "github.com/metal-stack/metal-lib/pkg/pointer" - r "gopkg.in/rethinkdb/rethinkdb-go.v6" - "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore" "github.com/metal-stack/metal-api/cmd/metal-api/internal/ipam" - "github.com/metal-stack/metal-api/cmd/metal-api/internal/metal" v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" - - restful "github.com/emicklei/go-restful/v3" "github.com/metal-stack/metal-api/cmd/metal-api/internal/testdata" + "github.com/metal-stack/metal-lib/httperrors" + "github.com/metal-stack/metal-lib/pkg/pointer" testifymock "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + r "gopkg.in/rethinkdb/rethinkdb-go.v6" ) func TestGetNetworks(t *testing.T) { @@ -550,3 +548,53 @@ func Test_networkResource_allocateNetwork(t *testing.T) { } } } + +func Test_getAddressFamily(t *testing.T) { + tests := []struct { + name string + prefixes metal.Prefixes + want *v1.AddressFamily + wantErr bool + }{ + { + name: "ipv4", + prefixes: metal.Prefixes{ + metal.Prefix{IP: "10.0.0.0", Length: "8"}, + }, + want: pointer.Pointer(v1.IPv4AddressFamily), + }, + { + name: "ipv6", + prefixes: metal.Prefixes{ + metal.Prefix{IP: "2001::", Length: "64"}, + }, + want: pointer.Pointer(v1.IPv6AddressFamily), + }, + { + name: "empty prefixes", + prefixes: metal.Prefixes{}, + want: nil, + wantErr: false, + }, + { + name: "malformed ipv4", + prefixes: metal.Prefixes{ + metal.Prefix{IP: "10.0.0.0.0", Length: "6"}, + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := getAddressFamily(tt.prefixes) + if (err != nil) != tt.wantErr { + t.Errorf("getAddressFamily() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("getAddressFamily() = %v, want %v", got, tt.want) + } + }) + } +}