Skip to content

Commit aafbb7a

Browse files
author
Shruthi-1MN
committed
snapshot fixes
1 parent f8581c3 commit aafbb7a

5 files changed

Lines changed: 140 additions & 77 deletions

File tree

‎pkg/api/controllers/fileshare.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,24 @@ func (f *FileShareSnapshotPortal) UpdateFileShareSnapshot() {
689689
}
690690
snapshot.Id = id
691691

692+
reg, err := utils.Special_Character_Regex_Match_Pattern()
693+
if err != nil {
694+
log.Error(err)
695+
return
696+
}
697+
if reg.MatchString(snapshot.Name) {
698+
errMsg := fmt.Sprintf("invalid snapshot name because it has some special characters")
699+
log.Error(errMsg)
700+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
701+
return
702+
}
703+
if reg.MatchString(snapshot.Description) {
704+
errMsg := fmt.Sprintf("invalid snapshot description and it has some special characters")
705+
log.Error(errMsg)
706+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
707+
return
708+
}
709+
692710
result, err := db.C.UpdateFileShareSnapshot(c.GetContext(f.Ctx), id, &snapshot)
693711
if err != nil {
694712
errMsg := fmt.Sprintf("update fileshare snapshot failed: %s", err.Error())

‎pkg/api/controllers/fileshare_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,52 @@ func TestUpdateFileShareSnapshot(t *testing.T) {
350350
assertTestResult(t, &output, &expected)
351351
})
352352

353+
t.Run("Should return 400 invalid snapshot name because it has some special characters", func(t *testing.T) {
354+
var jsonStr = []byte(`{
355+
"name": "#Snap !$!test",
356+
"description":"fake snapshot"
357+
}`)
358+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
359+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
360+
361+
mockClient := new(dbtest.Client)
362+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
363+
Return(&expected, nil)
364+
db.C = mockClient
365+
366+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
367+
w := httptest.NewRecorder()
368+
r.Header.Set("Content-Type", "application/JSON")
369+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
370+
httpCtx.Input.SetData("context", c.NewAdminContext())
371+
})
372+
beego.BeeApp.Handlers.ServeHTTP(w, r)
373+
assertTestResult(t, w.Code, 400)
374+
})
375+
376+
t.Run("Should return 400 invalid snapshot description and it has some special characters", func(t *testing.T) {
377+
var jsonStr = []byte(`{
378+
"name":"fake snapshot",
379+
"description": "#Share !$!test"
380+
}`)
381+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
382+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
383+
384+
mockClient := new(dbtest.Client)
385+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
386+
Return(&expected, nil)
387+
db.C = mockClient
388+
389+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
390+
w := httptest.NewRecorder()
391+
r.Header.Set("Content-Type", "application/JSON")
392+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
393+
httpCtx.Input.SetData("context", c.NewAdminContext())
394+
})
395+
beego.BeeApp.Handlers.ServeHTTP(w, r)
396+
assertTestResult(t, w.Code, 400)
397+
})
398+
353399
t.Run("Should return 500 if update fileshare snapshot with bad request", func(t *testing.T) {
354400
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
355401
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)

‎pkg/api/util/db.go‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,6 @@ func CreateFileShareDBEntry(ctx *c.Context, in *model.FileShareSpec) (*model.Fil
170170
if in.UpdatedAt == "" {
171171
in.UpdatedAt = time.Now().Format(constants.TimeFormat)
172172
}
173-
//validate the name
174-
if in.Name == "" {
175-
errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.")
176-
log.Error(errMsg)
177-
return nil, errors.New(errMsg)
178-
}
179-
if len(in.Name) > 255 {
180-
errMsg := fmt.Sprintf("fileshare name length should not be more than 255 characters. input name length is : %d", len(in.Name))
181-
log.Error(errMsg)
182-
return nil, errors.New(errMsg)
183-
}
184173

185174
reg, err := regexp.Compile("^[a-zA-Z0-9_-]+$")
186175
if err != nil {
@@ -281,9 +270,19 @@ func CreateFileShareSnapshotDBEntry(ctx *c.Context, in *model.FileShareSnapshotS
281270
in.CreatedAt = time.Now().Format(constants.TimeFormat)
282271
}
283272

284-
//validate the snapshot name
285-
if in.Name == "" {
286-
errMsg := fmt.Sprintf("snapshot name can not be empty. Please give valid snapshot name")
273+
reg, err := utils.Special_Character_Regex_Match_Pattern()
274+
if err != nil {
275+
errMsg := fmt.Sprintf("regex compilation for file share description validation failed")
276+
log.Error(errMsg)
277+
return nil, errors.New(errMsg)
278+
}
279+
if reg.MatchString(in.Name) {
280+
errMsg := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters")
281+
log.Error(errMsg)
282+
return nil, errors.New(errMsg)
283+
}
284+
if reg.MatchString(in.Description) {
285+
errMsg := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters")
287286
log.Error(errMsg)
288287
return nil, errors.New(errMsg)
289288
}

‎pkg/api/util/db_test.go‎

Lines changed: 41 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package util
1717
import (
1818
"fmt"
1919
"reflect"
20-
"strconv"
2120
"testing"
2221

2322
"github.com/opensds/opensds/pkg/utils"
@@ -375,6 +374,47 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) {
375374
assertTestResult(t, result, expected)
376375
})
377376

377+
t.Run("invalid fileshare snapshot description because it has some special characters", func(t *testing.T) {
378+
req.Name = "testsnap"
379+
req.Description = "#Snap !$!test"
380+
mockClient := new(dbtest.Client)
381+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
382+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
383+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
384+
db.C = mockClient
385+
386+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
387+
expectedError := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters")
388+
assertTestResult(t, err.Error(), expectedError)
389+
})
390+
391+
t.Run("invalid fileshare snapshot name because it has some special characters", func(t *testing.T) {
392+
req.Name = "#Snap !$!test"
393+
req.Description = "test snapshot"
394+
mockClient := new(dbtest.Client)
395+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
396+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
397+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
398+
db.C = mockClient
399+
400+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
401+
expectedError := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters")
402+
assertTestResult(t, err.Error(), expectedError)
403+
})
404+
405+
t.Run("names starting 'snapshot' are reserved", func(t *testing.T) {
406+
req.Name = "snapshotknow"
407+
req.Description = "test snapshot"
408+
mockClient := new(dbtest.Client)
409+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
410+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
411+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
412+
db.C = mockClient
413+
414+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
415+
expectedError := fmt.Sprintf("names starting 'snapshot' are reserved. Please choose a different snapshot name.")
416+
assertTestResult(t, err.Error(), expectedError)
417+
})
378418
}
379419

380420
func TestCreateFileShareDBEntry(t *testing.T) {
@@ -422,44 +462,6 @@ func TestCreateFileShareDBEntry(t *testing.T) {
422462
assertTestResult(t, err.Error(), expectedError)
423463
})
424464

425-
t.Run("Empty file share name is allowed", func(t *testing.T) {
426-
in.Size, in.Name, in.ProfileId = int64(1), "", "b3585ebe-c42c-120g-b28e-f373746a71ca"
427-
mockClient := new(dbtest.Client)
428-
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
429-
db.C = mockClient
430-
431-
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
432-
expectedError := "empty fileshare name is not allowed. Please give valid name."
433-
assertTestResult(t, err.Error(), expectedError)
434-
})
435-
436-
t.Run("File share name length equal to 0 character are not allowed", func(t *testing.T) {
437-
in.Name = utils.RandSeqWithAlnum(0)
438-
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
439-
mockClient := new(dbtest.Client)
440-
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
441-
db.C = mockClient
442-
443-
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
444-
expectedError := "empty fileshare name is not allowed. Please give valid name."
445-
assertTestResult(t, err.Error(), expectedError)
446-
})
447-
448-
t.Run("File share name length equal to 1 character are allowed", func(t *testing.T) {
449-
in.Name = utils.RandSeqWithAlnum(1)
450-
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
451-
mockClient := new(dbtest.Client)
452-
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
453-
db.C = mockClient
454-
455-
var expected = &SampleFileShares[0]
456-
result, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
457-
if err != nil {
458-
t.Errorf("failed to create fileshare err is %v\n", err)
459-
}
460-
assertTestResult(t, result, expected)
461-
})
462-
463465
t.Run("File share name length equal to 10 characters are allowed", func(t *testing.T) {
464466
in.Name = utils.RandSeqWithAlnum(10)
465467
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
@@ -504,30 +506,6 @@ func TestCreateFileShareDBEntry(t *testing.T) {
504506
}
505507
assertTestResult(t, result, expected)
506508
})
507-
508-
t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
509-
in.Name = utils.RandSeqWithAlnum(256)
510-
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
511-
mockClient := new(dbtest.Client)
512-
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
513-
db.C = mockClient
514-
515-
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
516-
expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
517-
assertTestResult(t, err.Error(), expectedError)
518-
})
519-
520-
t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
521-
in.Name = utils.RandSeqWithAlnum(257)
522-
in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
523-
mockClient := new(dbtest.Client)
524-
mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
525-
db.C = mockClient
526-
527-
_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
528-
expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
529-
assertTestResult(t, err.Error(), expectedError)
530-
})
531509
}
532510

533511
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"
@@ -336,3 +337,24 @@ func ContainsIgnoreCase(a []string, x string) bool {
336337
}
337338
return false
338339
}
340+
341+
func RandomString(n int) string {
342+
var letter = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")
343+
344+
b := make([]rune, n)
345+
for i := range b {
346+
b[i] = letter[rand.Intn(len(letter))]
347+
}
348+
return string(b)
349+
}
350+
351+
func Special_Character_Regex_Match_Pattern() (*regexp.Regexp, error) {
352+
reg, err := regexp.Compile("[^a-zA-Z0-9 _-]+")
353+
if err != nil {
354+
errMsg := fmt.Sprintf("regex compilation validation failed")
355+
log.Error(errMsg)
356+
return nil, errors.New(errMsg)
357+
} else {
358+
return reg, nil
359+
}
360+
}

0 commit comments

Comments
 (0)