Skip to content

Commit 6e45d38

Browse files
kumarashitkumarashit
authored
Merge pull request #1082 from wisererik/hostvalidation
Add host api validation and filtering
2 parents 1a18fb7 + 4e16107 commit 6e45d38

19 files changed

Lines changed: 560 additions & 226 deletions

File tree

‎client/host_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func TestCreateHost(t *testing.T) {
3535
AccessMode: "agentless",
3636
HostName: "sap1",
3737
IP: "192.168.56.12",
38-
AvailabilityZones: []string{"az1", "az2"},
38+
AvailabilityZones: []string{"default", "az2"},
3939
Initiators: []*model.Initiator{
4040
&model.Initiator{
4141
PortName: "20000024ff5bb888",
@@ -71,7 +71,7 @@ func TestGetHost(t *testing.T) {
7171
AccessMode: "agentless",
7272
HostName: "sap1",
7373
IP: "192.168.56.12",
74-
AvailabilityZones: []string{"az1", "az2"},
74+
AvailabilityZones: []string{"default", "az2"},
7575
Initiators: []*model.Initiator{
7676
&model.Initiator{
7777
PortName: "20000024ff5bb888",
@@ -107,7 +107,7 @@ func TestListHosts(t *testing.T) {
107107
AccessMode: "agentless",
108108
HostName: "sap1",
109109
IP: "192.168.56.12",
110-
AvailabilityZones: []string{"az1", "az2"},
110+
AvailabilityZones: []string{"default", "az2"},
111111
Initiators: []*model.Initiator{
112112
&model.Initiator{
113113
PortName: "20000024ff5bb888",
@@ -128,7 +128,7 @@ func TestListHosts(t *testing.T) {
128128
AccessMode: "agentless",
129129
HostName: "sap2",
130130
IP: "192.168.56.13",
131-
AvailabilityZones: []string{"az1", "az2"},
131+
AvailabilityZones: []string{"default", "az2"},
132132
Initiators: []*model.Initiator{
133133
&model.Initiator{
134134
PortName: "20012324ff5ac132",
@@ -175,7 +175,7 @@ func TestUpdateHost(t *testing.T) {
175175
AccessMode: "agentless",
176176
HostName: "sap1",
177177
IP: "192.168.56.12",
178-
AvailabilityZones: []string{"az1", "az2"},
178+
AvailabilityZones: []string{"default", "az2"},
179179
Initiators: []*model.Initiator{
180180
&model.Initiator{
181181
PortName: "20000024ff5bb888",

‎client/volume_test.go‎

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ func TestCreateVolume(t *testing.T) {
3030
BaseModel: &model.BaseModel{
3131
Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
3232
},
33-
Name: "sample-volume",
34-
Description: "This is a sample volume for testing",
35-
Size: int64(1),
36-
Status: "available",
37-
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
38-
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
33+
Name: "sample-volume",
34+
Description: "This is a sample volume for testing",
35+
Size: int64(1),
36+
AvailabilityZone: "default",
37+
Status: "available",
38+
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
39+
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
3940
}
4041

4142
vol, err := fv.CreateVolume(&model.VolumeSpec{})
@@ -56,12 +57,13 @@ func TestGetVolume(t *testing.T) {
5657
BaseModel: &model.BaseModel{
5758
Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
5859
},
59-
Name: "sample-volume",
60-
Description: "This is a sample volume for testing",
61-
Size: int64(1),
62-
Status: "available",
63-
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
64-
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
60+
Name: "sample-volume",
61+
Description: "This is a sample volume for testing",
62+
Size: int64(1),
63+
AvailabilityZone: "default",
64+
Status: "available",
65+
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
66+
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
6567
}
6668

6769
vol, err := fv.GetVolume(volID)
@@ -82,12 +84,13 @@ func TestListVolumes(t *testing.T) {
8284
BaseModel: &model.BaseModel{
8385
Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
8486
},
85-
Name: "sample-volume",
86-
Description: "This is a sample volume for testing",
87-
Size: int64(1),
88-
Status: "available",
89-
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
90-
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
87+
Name: "sample-volume",
88+
Description: "This is a sample volume for testing",
89+
Size: int64(1),
90+
AvailabilityZone: "default",
91+
Status: "available",
92+
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
93+
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
9194
},
9295
}
9396

@@ -129,12 +132,13 @@ func TestUpdateVolume(t *testing.T) {
129132
BaseModel: &model.BaseModel{
130133
Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
131134
},
132-
Name: "sample-volume",
133-
Description: "This is a sample volume for testing",
134-
Size: int64(1),
135-
Status: "available",
136-
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
137-
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
135+
Name: "sample-volume",
136+
Description: "This is a sample volume for testing",
137+
Size: int64(1),
138+
AvailabilityZone: "default",
139+
Status: "available",
140+
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
141+
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
138142
}
139143

140144
if !reflect.DeepEqual(result, expected) {
@@ -159,12 +163,13 @@ func TestExtendVolume(t *testing.T) {
159163
BaseModel: &model.BaseModel{
160164
Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
161165
},
162-
Name: "sample-volume",
163-
Description: "This is a sample volume for testing",
164-
Size: int64(1),
165-
Status: "available",
166-
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
167-
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
166+
Name: "sample-volume",
167+
Description: "This is a sample volume for testing",
168+
Size: int64(1),
169+
AvailabilityZone: "default",
170+
Status: "available",
171+
PoolId: "084bf71e-a102-11e7-88a8-e31fe6d52248",
172+
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
168173
}
169174

170175
if !reflect.DeepEqual(result, expected) {

‎openapi-spec/swagger.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ paths:
383383
$ref: '#/components/responses/HTTPStatus404'
384384
'500':
385385
$ref: '#/components/responses/HTTPStatus500'
386-
'/v1beta/{tenantId}/profiles/{profileId}/customeProperties':
386+
'/v1beta/{tenantId}/profiles/{profileId}/customProperties':
387387
parameters:
388388
- $ref: '#/components/parameters/tenantId'
389389
- $ref: '#/components/parameters/profileId'

‎pkg/api/controllers/attachment.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,14 @@ func (v *VolumeAttachmentPortal) CreateVolumeAttachment() {
7979
return
8080
}
8181

82+
if !utils.Contains(host.AvailabilityZones, vol.AvailabilityZone) {
83+
errMsg := fmt.Sprintf("availability zone of volume: %s is not in the host availability zones: %v",
84+
vol.AvailabilityZone,
85+
host.AvailabilityZones)
86+
v.ErrorHandle(model.ErrorBadRequest, errMsg)
87+
return
88+
}
89+
8290
if vol.Status == model.VolumeAvailable {
8391
db.UpdateVolumeStatus(ctx, db.C, vol.Id, model.VolumeAttaching)
8492
} else if vol.Status == model.VolumeInUse {

‎pkg/api/controllers/host.go‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,15 @@ func (p *HostPortal) ListHosts() {
4141
if !policy.Authorize(p.Ctx, "host:list") {
4242
return
4343
}
44-
// TODO: handle query parameters
45-
hosts, err := db.C.ListHosts(c.GetContext(p.Ctx))
44+
45+
m, err := p.GetParameters()
46+
if err != nil {
47+
errMsg := fmt.Sprintf("get the query parameters of host failed: %s", err.Error())
48+
p.ErrorHandle(model.ErrorBadRequest, errMsg)
49+
return
50+
}
51+
52+
hosts, err := db.C.ListHosts(c.GetContext(p.Ctx), m)
4653
if err != nil {
4754
errMsg := fmt.Sprintf("list hosts failed: %s", err.Error())
4855
p.ErrorHandle(model.ErrorBadRequest, errMsg)
@@ -76,7 +83,18 @@ func (p *HostPortal) CreateHost() {
7683
return
7784
}
7885

79-
// TODO: Add prameter validation
86+
// HostName should be unique in the system
87+
hostArr, err := db.C.ListHostsByName(c.GetContext(p.Ctx), host.HostName)
88+
if err != nil {
89+
errMsg := fmt.Sprintf("check host %s failed in CreateHost method: %v", host.HostName, err)
90+
p.ErrorHandle(model.ErrorBadRequest, errMsg)
91+
return
92+
}
93+
if len(hostArr) > 0 {
94+
errMsg := fmt.Sprintf("the host with name %s already exists in the system", host.HostName)
95+
p.ErrorHandle(model.ErrorBadRequest, errMsg)
96+
return
97+
}
8098

8199
result, err := db.C.CreateHost(c.GetContext(p.Ctx), &host)
82100
if err != nil {
@@ -164,10 +182,27 @@ func (p *HostPortal) DeleteHost() {
164182
return
165183
}
166184
id := p.Ctx.Input.Param(":hostId")
185+
host, err := db.C.GetHost(c.GetContext(p.Ctx), id)
186+
if err != nil {
187+
errMsg := fmt.Sprintf("host %s not found: %s", id, err.Error())
188+
p.ErrorHandle(model.ErrorNotFound, errMsg)
189+
return
190+
}
167191

168-
// TODO: Add deletion constraition check
192+
// Check relationship with volume
193+
attachments, err := db.C.ListVolumeAttachmentsWithFilter(c.GetContext(p.Ctx), map[string][]string{"hostId": []string{id}})
194+
if err != nil {
195+
errMsg := fmt.Sprintf("list attachments failed in DeleteHost method: %v", err)
196+
p.ErrorHandle(model.ErrorBadRequest, errMsg)
197+
return
198+
}
199+
if len(attachments) > 0 {
200+
errMsg := fmt.Sprintf("some volumes are attached to host: %s, please detach them first", host.HostName)
201+
p.ErrorHandle(model.ErrorBadRequest, errMsg)
202+
return
203+
}
169204

170-
err := db.C.DeleteHost(c.GetContext(p.Ctx), id)
205+
err = db.C.DeleteHost(c.GetContext(p.Ctx), id)
171206
if err != nil {
172207
errMsg := fmt.Sprintf("delete host failed: %v", err)
173208
p.ErrorHandle(model.ErrorBadRequest, errMsg)

‎pkg/api/controllers/host_test.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ var (
4444
"hostName": "sap1",
4545
"ip": "192.168.56.12",
4646
"availabilityZones": [
47-
"az1",
47+
"default",
4848
"az2"
4949
],
5050
"initiators": [
@@ -64,7 +64,7 @@ var (
6464
AccessMode: "agentless",
6565
HostName: "sap1",
6666
IP: "192.168.56.12",
67-
AvailabilityZones: []string{"az1", "az2"},
67+
AvailabilityZones: []string{"default", "az2"},
6868
Initiators: []*model.Initiator{
6969
&model.Initiator{
7070
PortName: "20000024ff5bb888",
@@ -85,6 +85,7 @@ func TestCreateHost(t *testing.T) {
8585

8686
mockClient := new(dbtest.Client)
8787
mockClient.On("CreateHost", c.NewAdminContext(), &hostReq).Return(fakeHost, nil)
88+
mockClient.On("ListHostsByName", c.NewAdminContext(), hostReq.HostName).Return(nil, nil)
8889
db.C = mockClient
8990

9091
r, _ := http.NewRequest("POST", "/v1beta/host/hosts", bytes.NewBuffer(ByteHostReq))
@@ -108,7 +109,7 @@ func TestListHosts(t *testing.T) {
108109
t.Run("Should return 200 if everything works well", func(t *testing.T) {
109110
fakeHosts := []*model.HostSpec{&SampleHosts[0], &SampleHosts[1]}
110111
mockClient := new(dbtest.Client)
111-
mockClient.On("ListHosts", c.NewAdminContext()).Return(fakeHosts, nil)
112+
mockClient.On("ListHosts", c.NewAdminContext(), map[string][]string{}).Return(fakeHosts, nil)
112113
db.C = mockClient
113114

114115
r, _ := http.NewRequest("GET", "/v1beta/host/hosts", nil)
@@ -175,6 +176,9 @@ func TestDeleteHost(t *testing.T) {
175176
fakeHost := &SampleHosts[0]
176177
mockClient := new(dbtest.Client)
177178
mockClient.On("DeleteHost", c.NewAdminContext(), fakeHost.Id).Return(nil)
179+
mockClient.On("GetHost", c.NewAdminContext(), fakeHost.Id).Return(fakeHost, nil)
180+
mockClient.On("ListVolumeAttachmentsWithFilter", c.NewAdminContext(),
181+
map[string][]string{"hostId": []string{fakeHost.Id}}).Return(nil, nil)
178182
db.C = mockClient
179183

180184
r, _ := http.NewRequest("DELETE", "/v1beta/host/hosts/"+fakeHost.Id, nil)

‎pkg/api/controllers/volume_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ func TestExtendVolume(t *testing.T) {
243243
"name": "sample-volume",
244244
"description": "This is a sample volume for testing",
245245
"size": 1,
246+
"availabilityZone": "default",
246247
"status": "extending",
247248
"poolId": "084bf71e-a102-11e7-88a8-e31fe6d52248",
248249
"profileId": "1106b972-66ef-11e7-b172-db03f3689c9c"

‎pkg/api/util/db_test.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@ package util
1616

1717
import (
1818
"fmt"
19-
"github.com/opensds/opensds/pkg/utils"
2019
"reflect"
2120
"strconv"
2221
"testing"
2322

23+
"github.com/opensds/opensds/pkg/utils"
24+
2425
"github.com/opensds/opensds/pkg/context"
2526
"github.com/opensds/opensds/pkg/db"
2627
"github.com/opensds/opensds/pkg/model"
@@ -433,7 +434,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
433434
})
434435

435436
t.Run("File share name length equal to 0 character are not allowed", func(t *testing.T) {
436-
in.Name = utils.RandomString(0)
437+
in.Name = utils.RandSeqWithAlnum(0)
437438
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
438439
mockClient := new(dbtest.Client)
439440
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -445,7 +446,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
445446
})
446447

447448
t.Run("File share name length equal to 1 character are allowed", func(t *testing.T) {
448-
in.Name = utils.RandomString(1)
449+
in.Name = utils.RandSeqWithAlnum(1)
449450
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
450451
mockClient := new(dbtest.Client)
451452
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -460,7 +461,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
460461
})
461462

462463
t.Run("File share name length equal to 10 characters are allowed", func(t *testing.T) {
463-
in.Name = utils.RandomString(10)
464+
in.Name = utils.RandSeqWithAlnum(10)
464465
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
465466
mockClient := new(dbtest.Client)
466467
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -475,7 +476,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
475476
})
476477

477478
t.Run("File share name length equal to 254 characters are allowed", func(t *testing.T) {
478-
in.Name = utils.RandomString(254)
479+
in.Name = utils.RandSeqWithAlnum(254)
479480
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
480481
mockClient := new(dbtest.Client)
481482
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -490,7 +491,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
490491
})
491492

492493
t.Run("File share name length equal to 255 characters are allowed", func(t *testing.T) {
493-
in.Name = utils.RandomString(255)
494+
in.Name = utils.RandSeqWithAlnum(255)
494495
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
495496
mockClient := new(dbtest.Client)
496497
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -505,26 +506,26 @@ func TestCreateFileShareDBEntry(t *testing.T) {
505506
})
506507

507508
t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
508-
in.Name = utils.RandomString(256)
509+
in.Name = utils.RandSeqWithAlnum(256)
509510
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
510511
mockClient := new(dbtest.Client)
511512
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
512513
db.C = mockClient
513514

514515
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
515-
expectedError := "fileshare name length should not be more than 255 characters. input name length is : "+strconv.Itoa(len(in.Name))
516+
expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
516517
assertTestResult(t, err.Error(), expectedError)
517518
})
518519

519520
t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
520-
in.Name = utils.RandomString(257)
521+
in.Name = utils.RandSeqWithAlnum(257)
521522
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
522523
mockClient := new(dbtest.Client)
523524
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
524525
db.C = mockClient
525526

526527
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
527-
expectedError := "fileshare name length should not be more than 255 characters. input name length is : "+strconv.Itoa(len(in.Name))
528+
expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
528529
assertTestResult(t, err.Error(), expectedError)
529530
})
530531

‎pkg/db/db.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ type Client interface {
223223

224224
ListVolumeGroupsWithFilter(ctx *c.Context, m map[string][]string) ([]*model.VolumeGroupSpec, error)
225225

226-
ListHosts(ctx *c.Context) ([]*model.HostSpec, error)
226+
ListHosts(ctx *c.Context, m map[string][]string) ([]*model.HostSpec, error)
227227

228228
ListHostsByName(ctx *c.Context, hostName string) ([]*model.HostSpec, error)
229229

0 commit comments

Comments
 (0)