diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index 54e1c51c5..092096c1e 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -6,6 +6,7 @@ import ( "log/slog" "net/http" "sort" + "strconv" "strings" "time" @@ -80,6 +81,7 @@ func (r *switchResource) webService() *restful.WebService { Operation("deleteSwitch"). Doc("deletes an switch and returns the deleted entity"). Param(ws.PathParameter("id", "identifier of the switch").DataType("string")). + Param(ws.QueryParameter("force", "if true switch is deleted with no validation").DataType("boolean").DefaultValue("false")). Metadata(restfulspec.KeyOpenAPITags, tags). Writes(v1.SwitchResponse{}). Returns(http.StatusOK, "OK", v1.SwitchResponse{}). @@ -191,6 +193,16 @@ func (r *switchResource) findSwitches(request *restful.Request, response *restfu func (r *switchResource) deleteSwitch(request *restful.Request, response *restful.Response) { id := request.PathParameter("id") + forceParam := request.QueryParameter("force") + if forceParam == "" { + forceParam = "false" + } + + force, err := strconv.ParseBool(forceParam) + if err != nil { + r.sendError(request, response, httperrors.BadRequest(err)) + return + } s, err := r.ds.FindSwitch(id) if err != nil { @@ -198,6 +210,11 @@ func (r *switchResource) deleteSwitch(request *restful.Request, response *restfu return } + if !force && len(s.MachineConnections) > 0 { + r.sendError(request, response, httperrors.BadRequest(fmt.Errorf("cannot delete switch %s while it still has machines connected to it", id))) + return + } + err = r.ds.DeleteSwitch(s) if err != nil { r.sendError(request, response, defaultError(err)) diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 81e7e738a..46bbd031f 100644 --- a/cmd/metal-api/internal/service/switch-service_test.go +++ b/cmd/metal-api/internal/service/switch-service_test.go @@ -13,7 +13,9 @@ import ( restful "github.com/emicklei/go-restful/v3" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/rethinkdb/rethinkdb-go.v6" r "gopkg.in/rethinkdb/rethinkdb-go.v6" "github.com/metal-stack/metal-api/cmd/metal-api/internal/datastore" @@ -21,6 +23,7 @@ import ( v1 "github.com/metal-stack/metal-api/cmd/metal-api/internal/service/v1" "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" ) func TestRegisterSwitch(t *testing.T) { @@ -1418,3 +1421,128 @@ func TestToggleSwitchNicWithoutMachine(t *testing.T) { require.NoError(t, err) require.Equal(t, result.Message, fmt.Sprintf("switch %q does not have a connected machine at port %q", testdata.Switch1.ID, testdata.Switch1.Nics[1].Name)) } + +func Test_SwitchDelete(t *testing.T) { + tests := []struct { + name string + mockFn func(mock *rethinkdb.Mock) + want *v1.SwitchResponse + wantErr error + wantStatus int + force bool + }{ + { + name: "delete switch", + mockFn: func(mock *rethinkdb.Mock) { + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{ + Base: metal.Base{ + ID: "switch-1", + }, + }, nil) + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil) + mock.On(rethinkdb.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil) + mock.On(rethinkdb.DB("mockdb").Table("ip")).Return(nil, nil) + }, + want: &v1.SwitchResponse{ + Common: v1.Common{ + Identifiable: v1.Identifiable{ + ID: "switch-1", + }, + Describable: v1.Describable{ + Name: pointer.Pointer(""), + Description: pointer.Pointer(""), + }, + }, + Nics: v1.SwitchNics{}, + Connections: []v1.SwitchConnection{}, + }, + wantStatus: http.StatusOK, + }, + { + name: "delete switch does not work due to machine connections", + mockFn: func(mock *rethinkdb.Mock) { + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{ + Base: metal.Base{ + ID: "switch-1", + }, + MachineConnections: metal.ConnectionMap{ + "port-a": metal.Connections{}, + }, + }, nil) + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil) + }, + wantErr: &httperrors.HTTPErrorResponse{ + StatusCode: http.StatusBadRequest, + Message: "cannot delete switch switch-1 while it still has machines connected to it", + }, + wantStatus: http.StatusBadRequest, + }, + { + name: "delete switch with force", + mockFn: func(mock *rethinkdb.Mock) { + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1")).Return(&metal.Switch{ + Base: metal.Base{ + ID: "switch-1", + }, + MachineConnections: metal.ConnectionMap{ + "port-a": metal.Connections{}, + }, + }, nil) + mock.On(rethinkdb.DB("mockdb").Table("switch").Get("switch-1").Delete()).Return(testdata.EmptyResult, nil) + mock.On(rethinkdb.DB("mockdb").Table("switchstatus").Get("switch-1")).Return(nil, nil) + mock.On(rethinkdb.DB("mockdb").Table("ip")).Return(nil, nil) + }, + force: true, + want: &v1.SwitchResponse{ + Common: v1.Common{ + Identifiable: v1.Identifiable{ + ID: "switch-1", + }, + Describable: v1.Describable{ + Name: pointer.Pointer(""), + Description: pointer.Pointer(""), + }, + }, + Nics: v1.SwitchNics{}, + Connections: []v1.SwitchConnection{}, + }, + wantStatus: http.StatusOK, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + var ( + ds, mock = datastore.InitMockDB(t) + ws = NewSwitch(slog.Default(), ds) + ) + + if tt.mockFn != nil { + tt.mockFn(mock) + } + + if tt.wantErr != nil { + code, got := genericWebRequest[*httperrors.HTTPErrorResponse](t, ws, testAdminUser, nil, "DELETE", "/v1/switch/switch-1") + assert.Equal(t, tt.wantStatus, code) + + if diff := cmp.Diff(tt.wantErr, got); diff != "" { + t.Errorf("diff (-want +got):\n%s", diff) + } + + return + } + + force := "" + if tt.force { + force = "?force=true" + } + + code, got := genericWebRequest[*v1.SwitchResponse](t, ws, testAdminUser, nil, "DELETE", "/v1/switch/switch-1"+force) + assert.Equal(t, tt.wantStatus, code) + + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("diff (-want +got):\n%s", diff) + } + }) + } +} diff --git a/cmd/metal-api/internal/service/v1/switch.go b/cmd/metal-api/internal/service/v1/switch.go index 99eaf221d..85c214c0d 100644 --- a/cmd/metal-api/internal/service/v1/switch.go +++ b/cmd/metal-api/internal/service/v1/switch.go @@ -138,6 +138,11 @@ func NewSwitchResponse(s *metal.Switch, ss *metal.SwitchStatus, p *metal.Partiti } } + var partition PartitionResponse + if partitionResp := NewPartitionResponse(p); partitionResp != nil { + partition = *partitionResp + } + return &SwitchResponse{ Common: Common{ Identifiable: Identifiable{ @@ -157,7 +162,7 @@ func NewSwitchResponse(s *metal.Switch, ss *metal.SwitchStatus, p *metal.Partiti ConsoleCommand: s.ConsoleCommand, }, Nics: nics, - Partition: *NewPartitionResponse(p), + Partition: partition, Connections: cons, LastSync: snr.LastSync, LastSyncError: snr.LastSyncError, diff --git a/cmd/metal-api/main.go b/cmd/metal-api/main.go index 03bb9224c..c5ea61907 100644 --- a/cmd/metal-api/main.go +++ b/cmd/metal-api/main.go @@ -33,6 +33,7 @@ import ( "github.com/metal-stack/metal-lib/jwt/grp" "github.com/metal-stack/metal-lib/jwt/sec" + "github.com/metal-stack/metal-lib/pkg/pointer" "connectrpc.com/connect" restfulspec "github.com/emicklei/go-restful-openapi/v2" @@ -296,6 +297,7 @@ func init() { rootCmd.Flags().String("headscale-api-key", "", "initial api key to connect to headscale server") rootCmd.Flags().StringP("minimum-client-version", "", "v0.0.1", "the minimum metalctl version required to talk to this version of metal-api") + rootCmd.Flags().String("release-version", "", "the metal-stack release version") must(viper.BindPFlags(rootCmd.Flags())) must(viper.BindPFlags(rootCmd.PersistentFlags())) @@ -749,6 +751,11 @@ func initRestServices(audit auditing.Auditing, withauth bool, ipmiSuperUser meta log.Fatalf("given minimum client version is not semver parsable: %s", err) } + var releaseVersion *string + if viper.IsSet("release-version") { + releaseVersion = pointer.Pointer(viper.GetString("release-version")) + } + restful.DefaultContainer.Add(service.NewAudit(logger.WithGroup("audit-service"), audit)) restful.DefaultContainer.Add(service.NewPartition(logger.WithGroup("partition-service"), ds, nsqer)) restful.DefaultContainer.Add(service.NewImage(logger.WithGroup("image-service"), ds)) @@ -766,7 +773,11 @@ func initRestServices(audit auditing.Auditing, withauth bool, ipmiSuperUser meta restful.DefaultContainer.Add(service.NewSwitch(logger.WithGroup("switch-service"), ds)) restful.DefaultContainer.Add(healthService) restful.DefaultContainer.Add(service.NewVPN(logger.WithGroup("vpn-service"), headscaleClient)) - restful.DefaultContainer.Add(rest.NewVersion(moduleName, service.BasePath, minClientVersion.Original())) + restful.DefaultContainer.Add(rest.NewVersion(moduleName, &rest.VersionOpts{ + BasePath: service.BasePath, + MinClientVersion: minClientVersion.Original(), + ReleaseVersion: releaseVersion, + })) restful.DefaultContainer.Filter(rest.RequestLoggerFilter(logger)) // FIXME restful.DefaultContainer.Filter(metrics.RestfulMetrics) diff --git a/go.mod b/go.mod index 0d2a34bea..09fc15553 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/looplab/fsm v1.0.2 github.com/metal-stack/go-ipam v1.14.1 github.com/metal-stack/masterdata-api v0.11.4 - github.com/metal-stack/metal-lib v0.17.0 + github.com/metal-stack/metal-lib v0.17.1 github.com/metal-stack/security v0.8.0 github.com/metal-stack/v v1.0.3 github.com/nsqio/go-nsq v1.1.0 diff --git a/go.sum b/go.sum index e0c9729f8..c67327fe7 100644 --- a/go.sum +++ b/go.sum @@ -299,8 +299,8 @@ github.com/metal-stack/go-ipam v1.14.1 h1:kTe4GUvLTNd27sad2uzxZUS5jD7pZfgfr077xo github.com/metal-stack/go-ipam v1.14.1/go.mod h1:0KqCwF1nTB3SZGED+Z+bxdZwXQjP7CiLPcjGDLMKn3s= github.com/metal-stack/masterdata-api v0.11.4 h1:bgRk7PbD5BjYbmAReaV7gTKKKrW5x/ZzCwj98VSWoJk= github.com/metal-stack/masterdata-api v0.11.4/go.mod h1:fD0AtsoNNaOLqRMBeZzDFljiQW9RlrOnxeZ20Pqhxas= -github.com/metal-stack/metal-lib v0.17.0 h1:0fCRUtYweJ5wbUwiEalFGiHkEz0mZwTWQUIIo3Npzkw= -github.com/metal-stack/metal-lib v0.17.0/go.mod h1:nyNGI4DZFOcWbSoq2Y6V3SHpFxuXBIqYBZHTb6cy//s= +github.com/metal-stack/metal-lib v0.17.1 h1:JLa4wJ62dgxtY9UOLF+QDk10/i/W5vhzrv8RsundDUY= +github.com/metal-stack/metal-lib v0.17.1/go.mod h1:nyNGI4DZFOcWbSoq2Y6V3SHpFxuXBIqYBZHTb6cy//s= github.com/metal-stack/security v0.8.0 h1:tVaSDB9m5clwYrnLyaXfPy7mQlJTnmeoHscG+RUy/xo= github.com/metal-stack/security v0.8.0/go.mod h1:7GAcQb+pOgflW30ohJygxpqc3i0dQ2ahGJK1CU5tqa0= github.com/metal-stack/v v1.0.3 h1:Sh2oBlnxrCUD+mVpzfC8HiqL045YWkxs0gpTvkjppqs= diff --git a/spec/metal-api.json b/spec/metal-api.json index 897b54824..5efe1b755 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -402,6 +402,9 @@ "name": { "type": "string" }, + "release_version": { + "type": "string" + }, "revision": { "type": "string" }, @@ -9371,6 +9374,13 @@ "name": "id", "required": true, "type": "string" + }, + { + "default": false, + "description": "if true switch is deleted with no validation", + "in": "query", + "name": "force", + "type": "boolean" } ], "produces": [