diff --git a/cmd/metal-api/internal/service/switch-service.go b/cmd/metal-api/internal/service/switch-service.go index cd4d9c40..092096c1 100644 --- a/cmd/metal-api/internal/service/switch-service.go +++ b/cmd/metal-api/internal/service/switch-service.go @@ -81,11 +81,10 @@ 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.PathParameter("force", "if true switch is deleted with no validation").DataType("boolean").DefaultValue("false")). + 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{}). - Returns(http.StatusBadRequest, "Bad input data", httperrors.HTTPErrorResponse{}). DefaultReturns("Error", httperrors.HTTPErrorResponse{})) ws.Route(ws.POST("/register"). @@ -194,9 +193,15 @@ func (r *switchResource) findSwitches(request *restful.Request, response *restfu func (r *switchResource) deleteSwitch(request *restful.Request, response *restful.Response) { id := request.PathParameter("id") - force, err := strconv.ParseBool(request.PathParameter("force")) + 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) diff --git a/cmd/metal-api/internal/service/switch-service_test.go b/cmd/metal-api/internal/service/switch-service_test.go index 81e7e738..46bbd031 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 99eaf221..85c214c0 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/spec/metal-api.json b/spec/metal-api.json index aaa73418..065bccfb 100644 --- a/spec/metal-api.json +++ b/spec/metal-api.json @@ -9371,9 +9371,8 @@ { "default": false, "description": "if true switch is deleted with no validation", - "in": "path", + "in": "query", "name": "force", - "required": true, "type": "boolean" } ], @@ -9387,12 +9386,6 @@ "$ref": "#/definitions/v1.SwitchResponse" } }, - "400": { - "description": "Bad input data", - "schema": { - "$ref": "#/definitions/httperrors.HTTPErrorResponse" - } - }, "default": { "description": "Error", "schema": {