From bed645d45d791cd8a1c0cfab90be41421b87bf91 Mon Sep 17 00:00:00 2001 From: Shruthi-1MN Date: Fri, 22 Nov 2019 16:22:50 +0530 Subject: [PATCH] snapshot fixes --- client/fileshare_test.go | 6 +- pkg/api/controllers/fileshare.go | 30 +++++++ pkg/api/controllers/fileshare_test.go | 90 +++++++++++++++++++ pkg/api/util/db.go | 21 +++++ pkg/api/util/db_test.go | 69 +++++++++++++- .../fileshare/filesharecontroller_test.go | 2 +- pkg/utils/utils.go | 22 +++++ testutils/collection/data.go | 4 +- 8 files changed, 237 insertions(+), 7 deletions(-) diff --git a/client/fileshare_test.go b/client/fileshare_test.go index 69e86628e..109833bcd 100644 --- a/client/fileshare_test.go +++ b/client/fileshare_test.go @@ -170,7 +170,7 @@ func TestCreateFileShareSnapshot(t *testing.T) { BaseModel: &model.BaseModel{ Id: "3769855c-a102-11e7-b772-17b880d2f537", }, - Name: "sample-snapshot-01", + Name: "samplesnapshot01", Description: "This is the first sample snapshot for testing", Status: "available", ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c", @@ -196,7 +196,7 @@ func TestGetFileShareSnapshot(t *testing.T) { BaseModel: &model.BaseModel{ Id: "3769855c-a102-11e7-b772-17b880d2f537", }, - Name: "sample-snapshot-01", + Name: "samplesnapshot01", Description: "This is the first sample snapshot for testing", Status: "available", ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c", @@ -279,7 +279,7 @@ func TestUpdateFileShareSnapshot(t *testing.T) { BaseModel: &model.BaseModel{ Id: "3769855c-a102-11e7-b772-17b880d2f537", }, - Name: "sample-snapshot-01", + Name: "samplesnapshot01", Description: "This is the first sample snapshot for testing", Status: "available", ProfileId: "1106b972-66ef-11e7-b172-db03f3689c9c", diff --git a/pkg/api/controllers/fileshare.go b/pkg/api/controllers/fileshare.go index f55843ef2..60827598d 100644 --- a/pkg/api/controllers/fileshare.go +++ b/pkg/api/controllers/fileshare.go @@ -689,6 +689,36 @@ func (f *FileShareSnapshotPortal) UpdateFileShareSnapshot() { } snapshot.Id = id + if snapshot.Name == "" { + errMsg := fmt.Sprintf("empty fileshare name is not allowed. Please give valid name.") + log.Error(errMsg) + f.ErrorHandle(model.ErrorBadRequest, errMsg) + return + } + if len(snapshot.Name) > 255 { + errMsg := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(snapshot.Name)) + log.Error(errMsg) + f.ErrorHandle(model.ErrorBadRequest, errMsg) + return + } + reg, err := utils.Special_Character_Regex_Match_Pattern() + if err != nil { + log.Error(err) + return + } + if reg.MatchString(snapshot.Name) { + errMsg := fmt.Sprintf("invalid snapshot name because it has some special characters") + log.Error(errMsg) + f.ErrorHandle(model.ErrorBadRequest, errMsg) + return + } + if reg.MatchString(snapshot.Description) { + errMsg := fmt.Sprintf("invalid snapshot description and it has some special characters") + log.Error(errMsg) + f.ErrorHandle(model.ErrorBadRequest, errMsg) + return + } + result, err := db.C.UpdateFileShareSnapshot(c.GetContext(f.Ctx), id, &snapshot) if err != nil { errMsg := fmt.Sprintf("update fileshare snapshot failed: %s", err.Error()) diff --git a/pkg/api/controllers/fileshare_test.go b/pkg/api/controllers/fileshare_test.go index 03cd8fbf6..5a5d9c2dc 100644 --- a/pkg/api/controllers/fileshare_test.go +++ b/pkg/api/controllers/fileshare_test.go @@ -350,6 +350,96 @@ func TestUpdateFileShareSnapshot(t *testing.T) { assertTestResult(t, &output, &expected) }) + t.Run("Should return 400 empty snapshot name is not allowed", func(t *testing.T) { + var jsonStr = []byte(`{ + "name":"", + "description":"fake snapshot" + }`) + snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}} + json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot) + mockClient := new(dbtest.Client) + mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot). + Return(&expected, nil) + db.C = mockClient + + r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr)) + w := httptest.NewRecorder() + r.Header.Set("Content-Type", "application/JSON") + beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) { + httpCtx.Input.SetData("context", c.NewAdminContext()) + }) + beego.BeeApp.Handlers.ServeHTTP(w, r) + assertTestResult(t, w.Code, 400) + }) + + t.Run("Should return 400 snapshot name length should not be more than 255 characters", func(t *testing.T) { + var jsonStr = []byte(`{ + "name": "BpLnfgDsc2WD8F2qNfHK5a84jjJkwzDkh9h2fhfUVuS9jZ8uVbhV3vC5AWX39IVUWSP2NcHciWvqZTa2N95RxRTZHWUsaD6HEdz0ThbXfQ6pYSQ3n267l1VQKGNbSuJE9fQbzONJAAwdCxmM8BIabKERsUhPNmMmdf2eSJyYtqwcFiUILzXv2fcNIrWO7sToFgoilA0U1WxNeW1gdgUVDsEWJ77aX7tLFJ84qYU6UrN8ctecwZt5S4zjhD0tXRTmkY", + "description":"fake snapshot" + }`) + snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}} + json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot) + mockClient := new(dbtest.Client) + mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot). + Return(&expected, nil) + db.C = mockClient + + r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr)) + w := httptest.NewRecorder() + r.Header.Set("Content-Type", "application/JSON") + beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) { + httpCtx.Input.SetData("context", c.NewAdminContext()) + }) + beego.BeeApp.Handlers.ServeHTTP(w, r) + assertTestResult(t, w.Code, 400) + }) + + t.Run("Should return 400 invalid snapshot name because it has some special characters", func(t *testing.T) { + var jsonStr = []byte(`{ + "name": "#Snap !$!test", + "description":"fake snapshot" + }`) + snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}} + json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot) + + mockClient := new(dbtest.Client) + mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot). + Return(&expected, nil) + db.C = mockClient + + r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr)) + w := httptest.NewRecorder() + r.Header.Set("Content-Type", "application/JSON") + beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) { + httpCtx.Input.SetData("context", c.NewAdminContext()) + }) + beego.BeeApp.Handlers.ServeHTTP(w, r) + assertTestResult(t, w.Code, 400) + }) + + t.Run("Should return 400 invalid snapshot description and it has some special characters", func(t *testing.T) { + var jsonStr = []byte(`{ + "name":"fake snapshot", + "description": "#Share !$!test" + }`) + snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}} + json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot) + + mockClient := new(dbtest.Client) + mockClient.On("UpdateFileShareSnapshot", c.NewAdminContext(), snapshot.Id, &snapshot). + Return(&expected, nil) + db.C = mockClient + + r, _ := http.NewRequest("PUT", "/v1beta/file/snapshots/3769855c-a102-11e7-b772-17b880d2f537", bytes.NewBuffer(jsonStr)) + w := httptest.NewRecorder() + r.Header.Set("Content-Type", "application/JSON") + beego.InsertFilter("*", beego.BeforeExec, func(httpCtx *context.Context) { + httpCtx.Input.SetData("context", c.NewAdminContext()) + }) + beego.BeeApp.Handlers.ServeHTTP(w, r) + assertTestResult(t, w.Code, 400) + }) + t.Run("Should return 500 if update fileshare snapshot with bad request", func(t *testing.T) { snapshot := model.FileShareSnapshotSpec{BaseModel: &model.BaseModel{}} json.NewDecoder(bytes.NewBuffer(jsonStr)).Decode(&snapshot) diff --git a/pkg/api/util/db.go b/pkg/api/util/db.go index ddec18623..496e6eac0 100644 --- a/pkg/api/util/db.go +++ b/pkg/api/util/db.go @@ -288,6 +288,27 @@ func CreateFileShareSnapshotDBEntry(ctx *c.Context, in *model.FileShareSnapshotS log.Error(errMsg) return nil, errors.New(errMsg) } + if len(in.Name) > 255 { + errMsg := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(in.Name)) + log.Error(errMsg) + return nil, errors.New(errMsg) + } + reg, err := utils.Special_Character_Regex_Match_Pattern() + if err != nil { + errMsg := fmt.Sprintf("regex compilation for file share description validation failed") + log.Error(errMsg) + return nil, errors.New(errMsg) + } + if reg.MatchString(in.Name) { + errMsg := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters") + log.Error(errMsg) + return nil, errors.New(errMsg) + } + if reg.MatchString(in.Description) { + errMsg := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters") + log.Error(errMsg) + return nil, errors.New(errMsg) + } if strings.HasPrefix(in.Name, "snapshot") { errMsg := fmt.Sprintf("names starting 'snapshot' are reserved. Please choose a different snapshot name.") log.Error(errMsg) diff --git a/pkg/api/util/db_test.go b/pkg/api/util/db_test.go index 22ac5a054..533d40d56 100644 --- a/pkg/api/util/db_test.go +++ b/pkg/api/util/db_test.go @@ -340,7 +340,7 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) { BaseModel: &model.BaseModel{ Id: "3769855c-a102-11e7-b772-17b880d2f537", }, - Name: "sample-snapshot-01", + Name: "samplesnapshot01", Description: "This is the first sample snapshot for testing", Status: "available", ShareSize: int64(1), @@ -375,6 +375,73 @@ func TestCreateFileShareSnapshotDBEntry(t *testing.T) { assertTestResult(t, result, expected) }) + t.Run("snapshot name can not be empty.", func(t *testing.T) { + req.Name = "" + mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil) + mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil) + mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil) + db.C = mockClient + + _, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req) + expectedError := "snapshot name can not be empty. Please give valid snapshot name" + assertTestResult(t, err.Error(), expectedError) + }) + + t.Run("snapshot name length should not be more than 255 characters", func(t *testing.T) { + req.Name = utils.RandomString(258) + mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil) + mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil) + mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil) + db.C = mockClient + + _, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req) + expectedError := fmt.Sprintf("snapshot name length should not be more than 255 characters. input name length is : %d", len(req.Name)) + assertTestResult(t, err.Error(), expectedError) + }) + + t.Run("invalid fileshare snapshot description because it has some special characters", func(t *testing.T) { + req.Name = "testsnap" + req.Description = "#Snap !$!test" + mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil) + mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil) + mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil) + db.C = mockClient + + _, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req) + expectedError := fmt.Sprintf("invalid fileshare snapshot description because it has some special characters") + assertTestResult(t, err.Error(), expectedError) + }) + + t.Run("invalid fileshare snapshot name because it has some special characters", func(t *testing.T) { + req.Name = "#Snap !$!test" + req.Description = "test snapshot" + mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil) + mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil) + mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil) + db.C = mockClient + + _, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req) + expectedError := fmt.Sprintf("invalid fileshare snapshot name because it has some special characters") + assertTestResult(t, err.Error(), expectedError) + }) + + t.Run("names starting 'snapshot' are reserved", func(t *testing.T) { + req.Name = "snapshotknow" + req.Description = "test snapshot" + mockClient := new(dbtest.Client) + mockClient.On("GetFileShare", context.NewAdminContext(), "bd5b12a8-a101-11e7-941e-d77981b584d8").Return(fileshare, nil) + mockClient.On("ListFileShareSnapshots", context.NewAdminContext()).Return(nil, nil) + mockClient.On("CreateFileShareSnapshot", context.NewAdminContext(), req).Return(&SampleShareSnapshots[0], nil) + db.C = mockClient + + _, err := CreateFileShareSnapshotDBEntry(context.NewAdminContext(), req) + expectedError := fmt.Sprintf("names starting 'snapshot' are reserved. Please choose a different snapshot name.") + assertTestResult(t, err.Error(), expectedError) + }) } func TestCreateFileShareDBEntry(t *testing.T) { diff --git a/pkg/controller/fileshare/filesharecontroller_test.go b/pkg/controller/fileshare/filesharecontroller_test.go index abe688a68..532c7cfd1 100644 --- a/pkg/controller/fileshare/filesharecontroller_test.go +++ b/pkg/controller/fileshare/filesharecontroller_test.go @@ -138,7 +138,7 @@ func TestCreateFileShareSnapshot(t *testing.T) { result, err := fc.CreateFileShareSnapshot(&pb.CreateFileShareSnapshotOpts{}) if err != nil { - t.Errorf("failed to create file share snapshot, err is %v\n", err) + t.Errorf( "failed to create file share snapshot, err is %v\n", err) } if !reflect.DeepEqual(result, expected) { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 970bfe443..f0751350a 100755 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -23,6 +23,7 @@ import ( "reflect" "sort" "strings" + "regexp" "time" "github.com/opensds/opensds/pkg/utils/constants" @@ -327,3 +328,24 @@ func WaitForCondition(f func() (bool, error), interval, timeout time.Duration) e } return fmt.Errorf("wait for condition timeout") } + +func RandomString(n int) string { + var letter = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789") + + b := make([]rune, n) + for i := range b { + b[i] = letter[rand.Intn(len(letter))] + } + return string(b) +} + +func Special_Character_Regex_Match_Pattern() (*regexp.Regexp, error) { + reg, err := regexp.Compile("[^a-zA-Z0-9 ]+") + if err != nil { + errMsg := fmt.Sprintf("regex compilation validation failed") + log.Error(errMsg) + return nil, errors.New(errMsg) + } else { + return reg, nil + } +} \ No newline at end of file diff --git a/testutils/collection/data.go b/testutils/collection/data.go index 89aa998f2..63e7ff7c3 100644 --- a/testutils/collection/data.go +++ b/testutils/collection/data.go @@ -433,7 +433,7 @@ var ( BaseModel: &model.BaseModel{ Id: "3769855c-a102-11e7-b772-17b880d2f537", }, - Name: "sample-snapshot-01", + Name: "samplesnapshot01", Description: "This is the first sample snapshot for testing", ShareSize: int64(1), Status: "available", @@ -737,7 +737,7 @@ var ( ByteFileShareSnapshot = `{ "id": "3769855c-a102-11e7-b772-17b880d2f537", - "name": "sample-snapshot-01", + "name": "samplesnapshot01", "description": "This is the first sample snapshot for testing", "sharesize": 1, "status": "available",