Skip to content

Commit bed645d

Browse files
author
Shruthi-1MN
committed
snapshot fixes
1 parent 6e45d38 commit bed645d

8 files changed

Lines changed: 237 additions & 7 deletions

File tree

‎client/fileshare_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ func TestCreateFileShareSnapshot(t *testing.T) {
170170
BaseModel: &model.BaseModel{
171171
Id: "3769855c-a102-11e7-b772-17b880d2f537",
172172
},
173-
Name: "sample-snapshot-01",
173+
Name: "samplesnapshot01",
174174
Description: "This is the first sample snapshot for testing",
175175
Status: "available",
176176
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
@@ -196,7 +196,7 @@ func TestGetFileShareSnapshot(t *testing.T) {
196196
BaseModel: &model.BaseModel{
197197
Id: "3769855c-a102-11e7-b772-17b880d2f537",
198198
},
199-
Name: "sample-snapshot-01",
199+
Name: "samplesnapshot01",
200200
Description: "This is the first sample snapshot for testing",
201201
Status: "available",
202202
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",
@@ -279,7 +279,7 @@ func TestUpdateFileShareSnapshot(t *testing.T) {
279279
BaseModel: &model.BaseModel{
280280
Id: "3769855c-a102-11e7-b772-17b880d2f537",
281281
},
282-
Name: "sample-snapshot-01",
282+
Name: "samplesnapshot01",
283283
Description: "This is the first sample snapshot for testing",
284284
Status: "available",
285285
ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c",

‎pkg/api/controllers/fileshare.go‎

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

692+
if snapshot.Name == "" {
693+
errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.")
694+
log.Error(errMsg)
695+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
696+
return
697+
}
698+
if len(snapshot.Name) > 255 {
699+
errMsg := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(snapshot.Name))
700+
log.Error(errMsg)
701+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
702+
return
703+
}
704+
reg, err := utils.Special_Character_Regex_Match_Pattern()
705+
if err != nil {
706+
log.Error(err)
707+
return
708+
}
709+
if reg.MatchString(snapshot.Name) {
710+
errMsg := fmt.Sprintf("invalid snapshot name because it has some special characters")
711+
log.Error(errMsg)
712+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
713+
return
714+
}
715+
if reg.MatchString(snapshot.Description) {
716+
errMsg := fmt.Sprintf("invalid snapshot description and it has some special characters")
717+
log.Error(errMsg)
718+
f.ErrorHandle(model.ErrorBadRequest, errMsg)
719+
return
720+
}
721+
692722
result, err := db.C.UpdateFileShareSnapshot(c.GetContext(f.Ctx), id, &snapshot)
693723
if err != nil {
694724
errMsg := fmt.Sprintf("update fileshare snapshot failed: %s", err.Error())

‎pkg/api/controllers/fileshare_test.go‎

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

353+
t.Run("Should return 400 empty snapshot name is not allowed", func(t *testing.T) {
354+
var jsonStr = []byte(`{
355+
"name":"",
356+
"description":"fake snapshot"
357+
}`)
358+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
359+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
360+
mockClient := new(dbtest.Client)
361+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
362+
Return(&expected, nil)
363+
db.C = mockClient
364+
365+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
366+
w := httptest.NewRecorder()
367+
r.Header.Set("Content-Type", "application/JSON")
368+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
369+
httpCtx.Input.SetData("context", c.NewAdminContext())
370+
})
371+
beego.BeeApp.Handlers.ServeHTTP(w, r)
372+
assertTestResult(t, w.Code, 400)
373+
})
374+
375+
t.Run("Should return 400 snapshot name length should not be more than 255 characters", func(t *testing.T) {
376+
var jsonStr = []byte(`{
377+
"name": "BpLnfgDsc2WD8F2qNfHK5a84jjJkwzDkh9h2fhfUVuS9jZ8uVbhV3vC5AWX39IVUWSP2NcHciWvqZTa2N95RxRTZHWUsaD6HEdz0ThbXfQ6pYSQ3n267l1VQKGNbSuJE9fQbzONJAAwdCxmM8BIabKERsUhPNmMmdf2eSJyYtqwcFiUILzXv2fcNIrWO7sToFgoilA0U1WxNeW1gdgUVDsEWJ77aX7tLFJ84qYU6UrN8ctecwZt5S4zjhD0tXRTmkY",
378+
"description":"fake snapshot"
379+
}`)
380+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
381+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
382+
mockClient := new(dbtest.Client)
383+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
384+
Return(&expected, nil)
385+
db.C = mockClient
386+
387+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
388+
w := httptest.NewRecorder()
389+
r.Header.Set("Content-Type", "application/JSON")
390+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
391+
httpCtx.Input.SetData("context", c.NewAdminContext())
392+
})
393+
beego.BeeApp.Handlers.ServeHTTP(w, r)
394+
assertTestResult(t, w.Code, 400)
395+
})
396+
397+
t.Run("Should return 400 invalid snapshot name because it has some special characters", func(t *testing.T) {
398+
var jsonStr = []byte(`{
399+
"name": "#Snap !$!test",
400+
"description":"fake snapshot"
401+
}`)
402+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
403+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
404+
405+
mockClient := new(dbtest.Client)
406+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
407+
Return(&expected, nil)
408+
db.C = mockClient
409+
410+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
411+
w := httptest.NewRecorder()
412+
r.Header.Set("Content-Type", "application/JSON")
413+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
414+
httpCtx.Input.SetData("context", c.NewAdminContext())
415+
})
416+
beego.BeeApp.Handlers.ServeHTTP(w, r)
417+
assertTestResult(t, w.Code, 400)
418+
})
419+
420+
t.Run("Should return 400 invalid snapshot description and it has some special characters", func(t *testing.T) {
421+
var jsonStr = []byte(`{
422+
"name":"fake snapshot",
423+
"description": "#Share !$!test"
424+
}`)
425+
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
426+
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)
427+
428+
mockClient := new(dbtest.Client)
429+
mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot).
430+
Return(&expected, nil)
431+
db.C = mockClient
432+
433+
r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr))
434+
w := httptest.NewRecorder()
435+
r.Header.Set("Content-Type", "application/JSON")
436+
beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) {
437+
httpCtx.Input.SetData("context", c.NewAdminContext())
438+
})
439+
beego.BeeApp.Handlers.ServeHTTP(w, r)
440+
assertTestResult(t, w.Code, 400)
441+
})
442+
353443
t.Run("Should return 500 if update fileshare snapshot with bad request", func(t *testing.T) {
354444
snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}}
355445
json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot)

‎pkg/api/util/db.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,27 @@ func CreateFileShareSnapshotDBEntry(ctx *c.Context, in *model.FileShareSnapshotS
288288
log.Error(errMsg)
289289
return nil, errors.New(errMsg)
290290
}
291+
if len(in.Name) > 255 {
292+
errMsg := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(in.Name))
293+
log.Error(errMsg)
294+
return nil, errors.New(errMsg)
295+
}
296+
reg, err := utils.Special_Character_Regex_Match_Pattern()
297+
if err != nil {
298+
errMsg := fmt.Sprintf("regex compilation for file share description validation failed")
299+
log.Error(errMsg)
300+
return nil, errors.New(errMsg)
301+
}
302+
if reg.MatchString(in.Name) {
303+
errMsg := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters")
304+
log.Error(errMsg)
305+
return nil, errors.New(errMsg)
306+
}
307+
if reg.MatchString(in.Description) {
308+
errMsg := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters")
309+
log.Error(errMsg)
310+
return nil, errors.New(errMsg)
311+
}
291312
if strings.HasPrefix(in.Name, "snapshot") {
292313
errMsg := fmt.Sprintf("names starting 'snapshot' are reserved. Please choose a different snapshot name.")
293314
log.Error(errMsg)

‎pkg/api/util/db_test.go‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) {
340340
BaseModel: &model.BaseModel{
341341
Id: "3769855c-a102-11e7-b772-17b880d2f537",
342342
},
343-
Name: "sample-snapshot-01",
343+
Name: "samplesnapshot01",
344344
Description: "This is the first sample snapshot for testing",
345345
Status: "available",
346346
ShareSize: int64(1),
@@ -375,6 +375,73 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) {
375375
assertTestResult(t, result, expected)
376376
})
377377

378+
t.Run("snapshot name can not be empty.", func(t *testing.T) {
379+
req.Name = ""
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 := "snapshot name can not be empty. Please give valid snapshot name"
388+
assertTestResult(t, err.Error(), expectedError)
389+
})
390+
391+
t.Run("snapshot name length should not be more than 255 characters", func(t *testing.T) {
392+
req.Name = utils.RandomString(258)
393+
mockClient := new(dbtest.Client)
394+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
395+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
396+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
397+
db.C = mockClient
398+
399+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
400+
expectedError := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(req.Name))
401+
assertTestResult(t, err.Error(), expectedError)
402+
})
403+
404+
t.Run("invalid fileshare snapshot description because it has some special characters", func(t *testing.T) {
405+
req.Name = "testsnap"
406+
req.Description = "#Snap !$!test"
407+
mockClient := new(dbtest.Client)
408+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
409+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
410+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
411+
db.C = mockClient
412+
413+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
414+
expectedError := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters")
415+
assertTestResult(t, err.Error(), expectedError)
416+
})
417+
418+
t.Run("invalid fileshare snapshot name because it has some special characters", func(t *testing.T) {
419+
req.Name = "#Snap !$!test"
420+
req.Description = "test snapshot"
421+
mockClient := new(dbtest.Client)
422+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
423+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
424+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
425+
db.C = mockClient
426+
427+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
428+
expectedError := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters")
429+
assertTestResult(t, err.Error(), expectedError)
430+
})
431+
432+
t.Run("names starting 'snapshot' are reserved", func(t *testing.T) {
433+
req.Name = "snapshotknow"
434+
req.Description = "test snapshot"
435+
mockClient := new(dbtest.Client)
436+
mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil)
437+
mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil)
438+
mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil)
439+
db.C = mockClient
440+
441+
_, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req)
442+
expectedError := fmt.Sprintf("names starting 'snapshot' are reserved. Please choose a different snapshot name.")
443+
assertTestResult(t, err.Error(), expectedError)
444+
})
378445
}
379446

380447
func TestCreateFileShareDBEntry(t *testing.T) {

‎pkg/controller/fileshare/filesharecontroller_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ func TestCreateFileShareSnapshot(t *testing.T) {
138138

139139
result, err := fc.CreateFileShareSnapshot(&pb.CreateFileShareSnapshotOpts{})
140140
if err != nil {
141-
t.Errorf("failed to create file share snapshot, err is %v\n", err)
141+
t.Errorf( "failed to create file share snapshot, err is %v\n", err)
142142
}
143143

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

‎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+
}

‎testutils/collection/data.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ var (
433433
BaseModel: &model.BaseModel{
434434
Id: "3769855c-a102-11e7-b772-17b880d2f537",
435435
},
436-
Name: "sample-snapshot-01",
436+
Name: "samplesnapshot01",
437437
Description: "This is the first sample snapshot for testing",
438438
ShareSize: int64(1),
439439
Status: "available",
@@ -737,7 +737,7 @@ var (
737737

738738
ByteFileShareSnapshot = `{
739739
"id": "3769855c-a102-11e7-b772-17b880d2f537",
740-
"name": "sample-snapshot-01",
740+
"name": "samplesnapshot01",
741741
"description": "This is the first sample snapshot for testing",
742742
"sharesize": 1,
743743
"status": "available",

0 commit comments

Comments
 (0)