Skip to content

Commit c5a3c2a

Browse files
author
Shruthi-1MN
committed
File share name fixes
1 parent 6e45d38 commit c5a3c2a

File tree

7 files changed

+221
-10
lines changed

7 files changed

+221
-10
lines changed

‎pkg/api/controllers/fileshare.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,35 @@ func (f *FileSharePortal) UpdateFileShare() {
371371
return
372372
}
373373

374+
if fshare.Name == "" {
375+
errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.")
376+
log.Error(errMsg)
377+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
378+
return
379+
}
380+
if len(fshare.Name) > 255 {
381+
errMsg := fmt.Sprintf("fileshare name length should not be more than 255 characters. input name length is : %d", len(fshare.Name))
382+
log.Error(errMsg)
383+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
384+
return
385+
}
386+
reg, err := utils.Special_Character_Regex_Match_Pattern()
387+
if err != nil {
388+
log.Error(err)
389+
return
390+
}
391+
if reg.MatchString(fshare.Name) {
392+
errMsg := fmt.Sprintf("invalid fileshare name because it has some special characters")
393+
log.Error(errMsg)
394+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
395+
return
396+
}
397+
if reg.MatchString(fshare.Description) {
398+
errMsg := fmt.Sprintf("invalid fileshare description and it has some special characters")
399+
log.Error(errMsg)
400+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
401+
return
402+
}
374403
fshare.Id = id
375404
result, err := db.C.UpdateFileShare(c.GetContext(f.Ctx), &fshare)
376405
if err != nil {

‎pkg/api/controllers/fileshare_test.go‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/opensds/opensds/pkg/db"
3030
"github.com/opensds/opensds/pkg/model"
3131
pb "github.com/opensds/opensds/pkg/model/proto"
32+
"github.com/opensds/opensds/pkg/utils"
3233
. "github.com/opensds/opensds/testutils/collection"
3334
ctrtest "github.com/opensds/opensds/testutils/controller/testing"
3435
dbtest "github.com/opensds/opensds/testutils/db/testing"
@@ -208,6 +209,94 @@ func TestUpdateFileShare(t *testing.T) {
208209
assertTestResult(t, &output, &expected)
209210
})
210211

212+
t.Run("Should return 400 empty fileshare name is not allowed", func(t *testing.T) {
213+
var jsonStr = []byte(`{
214+
"name":"",
215+
"description":"fake fileshare"
216+
}`)
217+
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
218+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
219+
mockClient := new(dbtest.Client)
220+
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
221+
Return(&expected, nil)
222+
db.C = mockClient
223+
224+
r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
225+
w := httptest.NewRecorder()
226+
r.Header.Set("Content-Type", "application/JSON")
227+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
228+
httpCtx.Input.SetData("context", c.NewAdminContext())
229+
})
230+
beego.BeeApp.Handlers.ServeHTTP(w, r)
231+
assertTestResult(t, w.Code, 400)
232+
})
233+
234+
t.Run("Should return 400 fileshare name length should not be more than 255 characters", func(t *testing.T) {
235+
var jsonStr = []byte(`{
236+
"description":"fake fileshare"
237+
}`)
238+
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
239+
share.Name = utils.RandomString(258)
240+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
241+
mockClient := new(dbtest.Client)
242+
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
243+
Return(&expected, nil)
244+
db.C = mockClient
245+
246+
r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
247+
w := httptest.NewRecorder()
248+
r.Header.Set("Content-Type", "application/JSON")
249+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
250+
httpCtx.Input.SetData("context", c.NewAdminContext())
251+
})
252+
beego.BeeApp.Handlers.ServeHTTP(w, r)
253+
assertTestResult(t, w.Code, 400)
254+
})
255+
256+
t.Run("Should return 400 invalid fileshare name because it has some special characters", func(t *testing.T) {
257+
var jsonStr = []byte(`{
258+
"description":"fake fileshare"
259+
}`)
260+
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
261+
share.Name = "#Share !$!test"
262+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
263+
mockClient := new(dbtest.Client)
264+
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
265+
Return(&expected, nil)
266+
db.C = mockClient
267+
268+
r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
269+
w := httptest.NewRecorder()
270+
r.Header.Set("Content-Type", "application/JSON")
271+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
272+
httpCtx.Input.SetData("context", c.NewAdminContext())
273+
})
274+
beego.BeeApp.Handlers.ServeHTTP(w, r)
275+
assertTestResult(t, w.Code, 400)
276+
})
277+
278+
t.Run("Should return 400 invalid fileshare description and it has some special characters", func(t *testing.T) {
279+
var jsonStr = []byte(`{
280+
"name": "sampletestfileshare01",
281+
"description":"#Share !$!test"
282+
}`)
283+
share := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
284+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&share)
285+
mockClient := new(dbtest.Client)
286+
mockClient.On("UpdateFileShare", c.NewAdminContext(), share.Id, &share).
287+
Return(&expected, nil)
288+
db.C = mockClient
289+
290+
r, _ := http.NewRequest("PUT", "/v1beta/file/shares/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
291+
w := httptest.NewRecorder()
292+
r.Header.Set("Content-Type", "application/JSON")
293+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
294+
httpCtx.Input.SetData("context", c.NewAdminContext())
295+
})
296+
beego.BeeApp.Handlers.ServeHTTP(w, r)
297+
assertTestResult(t, w.Code, 400)
298+
})
299+
211300
t.Run("Should return 500 if update file share with bad request", func(t *testing.T) {
212301
fileshare := model.FileShareSpec{BaseModel: &model.BaseModel{}}
213302
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&fileshare)

‎pkg/api/util/db.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"errors"
2323
"fmt"
2424
"net"
25-
"regexp"
2625
"strings"
2726
"time"
2827

@@ -182,10 +181,32 @@ func CreateFileShareDBEntry(ctx *c.Context, in *model.FileShareSpec) (*model.Fil
182181
return nil, errors.New(errMsg)
183182
}
184183

184+
shares, err := db.C.ListFileShares(ctx)
185+
if err != nil {
186+
return nil, err
187+
} else {
188+
for _, fshare := range shares {
189+
if fshare.Name == in.Name {
190+
errMsg := fmt.Sprintf("file share name already exists")
191+
log.Error(errMsg)
192+
return nil, errors.New(errMsg)
193+
}
194+
if fshare.Id == in.Id {
195+
errMsg := fmt.Sprintf("file share id already exists")
196+
log.Error(errMsg)
197+
return nil, errors.New(errMsg)
198+
}
199+
}
200+
}
201+
185202
// validate the description
186-
reg, err := regexp.Compile("[^a-zA-Z0-9 ]+")
203+
reg, err := utils.Special_Character_Regex_Match_Pattern()
187204
if err != nil {
188-
errMsg := fmt.Sprintf("regex compilation for file share description validation failed")
205+
log.Error(err)
206+
return nil, err
207+
}
208+
if reg.MatchString(in.Name) {
209+
errMsg := fmt.Sprintf("invalid fileshare name because it has some special characters")
189210
log.Error(errMsg)
190211
return nil, errors.New(errMsg)
191212
}

‎pkg/api/util/db_test.go‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) {
380380
func TestCreateFileShareDBEntry(t *testing.T) {
381381
var in = &model.FileShareSpec{
382382
BaseModel: &model.BaseModel{},
383-
Name: "sample-fileshare-01",
383+
Name: "samplefileshare01",
384384
Description: "This is a sample fileshare for testing",
385385
Size: int64(1),
386386
ProfileId: "b3585ebe-c42c-120g-b28e-f373746a71ca",
@@ -390,6 +390,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
390390
t.Run("Everything should work well", func(t *testing.T) {
391391
mockClient := new(dbtest.Client)
392392
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
393+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
393394
db.C = mockClient
394395

395396
var expected = &SampleFileShares[0]
@@ -404,6 +405,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
404405
in.Size = int64(-2)
405406
mockClient := new(dbtest.Client)
406407
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
408+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
407409
db.C = mockClient
408410

409411
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
@@ -415,6 +417,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
415417
in.ProfileId = ""
416418
mockClient := new(dbtest.Client)
417419
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
420+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
418421
db.C = mockClient
419422

420423
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
@@ -426,6 +429,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
426429
in.Size, in.Name, in.ProfileId = int64(1), "", "b3585ebe-c42c-120g-b28e-f373746a71ca"
427430
mockClient := new(dbtest.Client)
428431
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
432+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
429433
db.C = mockClient
430434

431435
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
@@ -438,6 +442,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
438442
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
439443
mockClient := new(dbtest.Client)
440444
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
445+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
441446
db.C = mockClient
442447

443448
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
@@ -450,6 +455,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
450455
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
451456
mockClient := new(dbtest.Client)
452457
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
458+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
453459
db.C = mockClient
454460

455461
var expected = &SampleFileShares[0]
@@ -465,6 +471,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
465471
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
466472
mockClient := new(dbtest.Client)
467473
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
474+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
468475
db.C = mockClient
469476

470477
var expected = &SampleFileShares[0]
@@ -480,6 +487,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
480487
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
481488
mockClient := new(dbtest.Client)
482489
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
490+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
483491
db.C = mockClient
484492

485493
var expected = &SampleFileShares[0]
@@ -495,6 +503,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
495503
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
496504
mockClient := new(dbtest.Client)
497505
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
506+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
498507
db.C = mockClient
499508

500509
var expected = &SampleFileShares[0]
@@ -510,6 +519,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
510519
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
511520
mockClient := new(dbtest.Client)
512521
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
522+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
513523
db.C = mockClient
514524

515525
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
@@ -522,23 +532,63 @@ func TestCreateFileShareDBEntry(t *testing.T) {
522532
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
523533
mockClient := new(dbtest.Client)
524534
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
535+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
525536
db.C = mockClient
526537

527538
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
528539
expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
529540
assertTestResult(t, err.Error(), expectedError)
530541
})
531542

543+
t.Run("Special characters in file share name are not allowed", func(t *testing.T) {
544+
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "#Share !$!test", "b3585ebe-c42c-120g-b28e-f373746a71ca", "File share test"
545+
mockClient := new(dbtest.Client)
546+
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
547+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
548+
db.C = mockClient
549+
550+
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
551+
expectedError := "invalid fileshare name because it has some special characters"
552+
assertTestResult(t, err.Error(), expectedError)
553+
})
554+
532555
t.Run("Special characters in file share description are not allowed", func(t *testing.T) {
533-
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "sample-fileshare-01", "b3585ebe-c42c-120g-b28e-f373746a71ca", "#FileShare Code!$! test"
556+
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "test tmp", "b3585ebe-c42c-120g-b28e-f373746a71ca", "#Share !$!test"
534557
mockClient := new(dbtest.Client)
535558
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
559+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(nil, nil)
536560
db.C = mockClient
537561

538562
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
539563
expectedError := "invalid fileshare description and it has some special characters"
540564
assertTestResult(t, err.Error(), expectedError)
541565
})
566+
567+
t.Run("Duplicate file share name are not allowed", func(t *testing.T) {
568+
var sampleshares = []*model.FileShareSpec{&SampleFileShares[0]}
569+
in.Size, in.Name, in.ProfileId, in.Description = int64(1), "samplefileshare01", "b3585ebe-c42c-120g-b28e-f373746a71ca", "File share test"
570+
mockClient := new(dbtest.Client)
571+
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
572+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(sampleshares, nil)
573+
db.C = mockClient
574+
575+
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
576+
expectedError := "file share name already exists"
577+
assertTestResult(t, err.Error(), expectedError)
578+
})
579+
580+
t.Run("Overwritting existing file share resource are not allowed", func(t *testing.T) {
581+
var sampleshares = []*model.FileShareSpec{&SampleFileShares[0]}
582+
in.Size, in.Name, in.ProfileId, in.Description, in.Id = int64(1), "samplefileshare05", "b3585ebe-c42c-120g-b28e-f373746a71ca", "File share test", "d2975ebe-d82c-430f-b28e-f373746a71ca"
583+
mockClient := new(dbtest.Client)
584+
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
585+
mockClient.On("ListFileShares", context.NewAdminContext()).Return(sampleshares, nil)
586+
db.C = mockClient
587+
588+
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
589+
expectedError := "file share id already exists"
590+
assertTestResult(t, err.Error(), expectedError)
591+
})
542592
}
543593

544594
func TestDeleteFileShareDBEntry(t *testing.T) {

‎pkg/utils/utils.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"reflect"
2424
"sort"
2525
"strings"
26+
"regexp"
2627
"time"
2728

2829
"github.com/opensds/opensds/pkg/utils/constants"
@@ -327,3 +328,24 @@ func WaitForCondition(f func() (bool, error), interval, timeout time.Duration) e
327328
}
328329
return fmt.Errorf("wait for condition timeout")
329330
}
331+
332+
func RandomString(n int) string {
333+
var letter = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")
334+
335+
b := make([]rune, n)
336+
for i := range b {
337+
b[i] = letter[rand.Intn(len(letter))]
338+
}
339+
return string(b)
340+
}
341+
342+
func Special_Character_Regex_Match_Pattern() (*regexp.Regexp, error) {
343+
reg, err := regexp.Compile(`[^a-zA-Z0-9 ]+`)
344+
if err != nil {
345+
errMsg := fmt.Sprintf("regex compilation validation failed")
346+
log.Error(errMsg)
347+
return nil, errors.New(errMsg)
348+
} else {
349+
return reg, nil
350+
}
351+
}

‎test/e2e/e2e_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ func TestUpdateVolumeGroup(t *testing.T) {
714714
func TestCreateFileShare(t *testing.T) {
715715
t.Log("Start creating file share...")
716716
var body = &model.FileShareSpec{
717-
Name: "share_test",
717+
Name: "sharetest",
718718
Description: "This is a test",
719719
Size: int64(1),
720720
}
@@ -743,7 +743,7 @@ func prepareGetFileShare(t *testing.T) (string, error) {
743743
}
744744

745745
for _, item := range filesharelst {
746-
if item.Name == "share_test" {
746+
if item.Name == "sharetest" {
747747
fileshare_id = item.Id
748748
}
749749
}
@@ -792,7 +792,7 @@ func prepareGetShare(t *testing.T) (string, error) {
792792
}
793793

794794
for _, item := range filesharelst {
795-
if item.Name == "share_test" {
795+
if item.Name == "sharetest" {
796796
fileshare_id = item.Id
797797
}
798798
}

0 commit comments

Comments
 (0)