Merge pull request #1082 from wisererik/hostvalidation

Add host api validation and filtering

client/host_test.go

@@ -35,7 +35,7 @@ func TestCreateHost(t *testing.T) {
 		AccessMode:        "agentless",
 		HostName:          "sap1",
 		IP:                "192.168.56.12",
-		AvailabilityZones: []string{"az1", "az2"},
+		AvailabilityZones: []string{"default", "az2"},
 		Initiators: []*model.Initiator{
 			&model.Initiator{
 				PortName: "20000024ff5bb888",
@@ -71,7 +71,7 @@ func TestGetHost(t *testing.T) {
 		AccessMode:        "agentless",
 		HostName:          "sap1",
 		IP:                "192.168.56.12",
-		AvailabilityZones: []string{"az1", "az2"},
+		AvailabilityZones: []string{"default", "az2"},
 		Initiators: []*model.Initiator{
 			&model.Initiator{
 				PortName: "20000024ff5bb888",
@@ -107,7 +107,7 @@ func TestListHosts(t *testing.T) {
 			AccessMode:        "agentless",
 			HostName:          "sap1",
 			IP:                "192.168.56.12",
-			AvailabilityZones: []string{"az1", "az2"},
+			AvailabilityZones: []string{"default", "az2"},
 			Initiators: []*model.Initiator{
 				&model.Initiator{
 					PortName: "20000024ff5bb888",
@@ -128,7 +128,7 @@ func TestListHosts(t *testing.T) {
 			AccessMode:        "agentless",
 			HostName:          "sap2",
 			IP:                "192.168.56.13",
-			AvailabilityZones: []string{"az1", "az2"},
+			AvailabilityZones: []string{"default", "az2"},
 			Initiators: []*model.Initiator{
 				&model.Initiator{
 					PortName: "20012324ff5ac132",
@@ -175,7 +175,7 @@ func TestUpdateHost(t *testing.T) {
 		AccessMode:        "agentless",
 		HostName:          "sap1",
 		IP:                "192.168.56.12",
-		AvailabilityZones: []string{"az1", "az2"},
+		AvailabilityZones: []string{"default", "az2"},
 		Initiators: []*model.Initiator{
 			&model.Initiator{
 				PortName: "20000024ff5bb888",

client/volume_test.go

@@ -30,12 +30,13 @@ func TestCreateVolume(t *testing.T) {
 		BaseModel: &model.BaseModel{
 			Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
 		},
-		Name:        "sample-volume",
-		Description: "This is a sample volume for testing",
-		Size:        int64(1),
-		Status:      "available",
-		PoolId:      "084bf71e-a102-11e7-88a8-e31fe6d52248",
-		ProfileId:   "1106b972-66ef-11e7-b172-db03f3689c9c",
+		Name:             "sample-volume",
+		Description:      "This is a sample volume for testing",
+		Size:             int64(1),
+		AvailabilityZone: "default",
+		Status:           "available",
+		PoolId:           "084bf71e-a102-11e7-88a8-e31fe6d52248",
+		ProfileId:        "1106b972-66ef-11e7-b172-db03f3689c9c",
 	}
 
 	vol, err := fv.CreateVolume(&model.VolumeSpec{})
@@ -56,12 +57,13 @@ func TestGetVolume(t *testing.T) {
 		BaseModel: &model.BaseModel{
 			Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
 		},
-		Name:        "sample-volume",
-		Description: "This is a sample volume for testing",
-		Size:        int64(1),
-		Status:      "available",
-		PoolId:      "084bf71e-a102-11e7-88a8-e31fe6d52248",
-		ProfileId:   "1106b972-66ef-11e7-b172-db03f3689c9c",
+		Name:             "sample-volume",
+		Description:      "This is a sample volume for testing",
+		Size:             int64(1),
+		AvailabilityZone: "default",
+		Status:           "available",
+		PoolId:           "084bf71e-a102-11e7-88a8-e31fe6d52248",
+		ProfileId:        "1106b972-66ef-11e7-b172-db03f3689c9c",
 	}
 
 	vol, err := fv.GetVolume(volID)
@@ -82,12 +84,13 @@ func TestListVolumes(t *testing.T) {
 			BaseModel: &model.BaseModel{
 				Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
 			},
-			Name:        "sample-volume",
-			Description: "This is a sample volume for testing",
-			Size:        int64(1),
-			Status:      "available",
-			PoolId:      "084bf71e-a102-11e7-88a8-e31fe6d52248",
-			ProfileId:   "1106b972-66ef-11e7-b172-db03f3689c9c",
+			Name:             "sample-volume",
+			Description:      "This is a sample volume for testing",
+			Size:             int64(1),
+			AvailabilityZone: "default",
+			Status:           "available",
+			PoolId:           "084bf71e-a102-11e7-88a8-e31fe6d52248",
+			ProfileId:        "1106b972-66ef-11e7-b172-db03f3689c9c",
 		},
 	}
 
@@ -129,12 +132,13 @@ func TestUpdateVolume(t *testing.T) {
 		BaseModel: &model.BaseModel{
 			Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
 		},
-		Name:        "sample-volume",
-		Description: "This is a sample volume for testing",
-		Size:        int64(1),
-		Status:      "available",
-		PoolId:      "084bf71e-a102-11e7-88a8-e31fe6d52248",
-		ProfileId:   "1106b972-66ef-11e7-b172-db03f3689c9c",
+		Name:             "sample-volume",
+		Description:      "This is a sample volume for testing",
+		Size:             int64(1),
+		AvailabilityZone: "default",
+		Status:           "available",
+		PoolId:           "084bf71e-a102-11e7-88a8-e31fe6d52248",
+		ProfileId:        "1106b972-66ef-11e7-b172-db03f3689c9c",
 	}
 
 	if !reflect.DeepEqual(result, expected) {
@@ -159,12 +163,13 @@ func TestExtendVolume(t *testing.T) {
 		BaseModel: &model.BaseModel{
 			Id: "bd5b12a8-a101-11e7-941e-d77981b584d8",
 		},
-		Name:        "sample-volume",
-		Description: "This is a sample volume for testing",
-		Size:        int64(1),
-		Status:      "available",
-		PoolId:      "084bf71e-a102-11e7-88a8-e31fe6d52248",
-		ProfileId:   "1106b972-66ef-11e7-b172-db03f3689c9c",
+		Name:             "sample-volume",
+		Description:      "This is a sample volume for testing",
+		Size:             int64(1),
+		AvailabilityZone: "default",
+		Status:           "available",
+		PoolId:           "084bf71e-a102-11e7-88a8-e31fe6d52248",
+		ProfileId:        "1106b972-66ef-11e7-b172-db03f3689c9c",
 	}
 
 	if !reflect.DeepEqual(result, expected) {

openapi-spec/swagger.yaml

@@ -383,7 +383,7 @@ paths:
           $ref: '#/components/responses/HTTPStatus404'
         '500':
           $ref: '#/components/responses/HTTPStatus500'
-  '/v1beta/{tenantId}/profiles/{profileId}/customeProperties':
+  '/v1beta/{tenantId}/profiles/{profileId}/customProperties':
     parameters:
       - $ref: '#/components/parameters/tenantId'
       - $ref: '#/components/parameters/profileId'

pkg/api/controllers/attachment.go

@@ -79,6 +79,14 @@ func (v *VolumeAttachmentPortal) CreateVolumeAttachment() {
 		return
 	}
 
+	if !utils.Contains(host.AvailabilityZones, vol.AvailabilityZone) {
+		errMsg := fmt.Sprintf("availability zone of volume: %s is not in the host availability zones: %v",
+			vol.AvailabilityZone,
+			host.AvailabilityZones)
+		v.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
+
 	if vol.Status == model.VolumeAvailable {
 		db.UpdateVolumeStatus(ctx, db.C, vol.Id, model.VolumeAttaching)
 	} else if vol.Status == model.VolumeInUse {

pkg/api/controllers/host.go

@@ -41,8 +41,15 @@ func (p *HostPortal) ListHosts() {
 	if !policy.Authorize(p.Ctx, "host:list") {
 		return
 	}
-	// TODO: handle query parameters
-	hosts, err := db.C.ListHosts(c.GetContext(p.Ctx))
+
+	m, err := p.GetParameters()
+	if err != nil {
+		errMsg := fmt.Sprintf("get the query parameters of host failed: %s", err.Error())
+		p.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
+
+	hosts, err := db.C.ListHosts(c.GetContext(p.Ctx), m)
 	if err != nil {
 		errMsg := fmt.Sprintf("list hosts failed: %s", err.Error())
 		p.ErrorHandle(model.ErrorBadRequest, errMsg)
@@ -76,7 +83,18 @@ func (p *HostPortal) CreateHost() {
 		return
 	}
 
-	// TODO: Add prameter validation
+	// HostName should be unique in the system
+	hostArr, err := db.C.ListHostsByName(c.GetContext(p.Ctx), host.HostName)
+	if err != nil {
+		errMsg := fmt.Sprintf("check host %s failed in CreateHost method: %v", host.HostName, err)
+		p.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
+	if len(hostArr) > 0 {
+		errMsg := fmt.Sprintf("the host with name %s already exists in the system", host.HostName)
+		p.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
 
 	result, err := db.C.CreateHost(c.GetContext(p.Ctx), &host)
 	if err != nil {
@@ -164,10 +182,27 @@ func (p *HostPortal) DeleteHost() {
 		return
 	}
 	id := p.Ctx.Input.Param(":hostId")
+	host, err := db.C.GetHost(c.GetContext(p.Ctx), id)
+	if err != nil {
+		errMsg := fmt.Sprintf("host %s not found: %s", id, err.Error())
+		p.ErrorHandle(model.ErrorNotFound, errMsg)
+		return
+	}
 
-	// TODO: Add deletion constraition check
+	// Check relationship with volume
+	attachments, err := db.C.ListVolumeAttachmentsWithFilter(c.GetContext(p.Ctx), map[string][]string{"hostId": []string{id}})
+	if err != nil {
+		errMsg := fmt.Sprintf("list attachments failed in DeleteHost method: %v", err)
+		p.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
+	if len(attachments) > 0 {
+		errMsg := fmt.Sprintf("some volumes are attached to host: %s, please detach them first", host.HostName)
+		p.ErrorHandle(model.ErrorBadRequest, errMsg)
+		return
+	}
 
-	err := db.C.DeleteHost(c.GetContext(p.Ctx), id)
+	err = db.C.DeleteHost(c.GetContext(p.Ctx), id)
 	if err != nil {
 		errMsg := fmt.Sprintf("delete host failed: %v", err)
 		p.ErrorHandle(model.ErrorBadRequest, errMsg)

pkg/api/controllers/host_test.go

@@ -44,7 +44,7 @@ var (
             "hostName": "sap1",
             "ip": "192.168.56.12",
             "availabilityZones": [
-                "az1",
+                "default",
                 "az2"
             ],
             "initiators": [
@@ -64,7 +64,7 @@ var (
 		AccessMode:        "agentless",
 		HostName:          "sap1",
 		IP:                "192.168.56.12",
-		AvailabilityZones: []string{"az1", "az2"},
+		AvailabilityZones: []string{"default", "az2"},
 		Initiators: []*model.Initiator{
 			&model.Initiator{
 				PortName: "20000024ff5bb888",
@@ -85,6 +85,7 @@ func TestCreateHost(t *testing.T) {
 
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateHost", c.NewAdminContext(), &hostReq).Return(fakeHost, nil)
+		mockClient.On("ListHostsByName", c.NewAdminContext(), hostReq.HostName).Return(nil, nil)
 		db.C = mockClient
 
 		r, _ := http.NewRequest("POST", "/v1beta/host/hosts", bytes.NewBuffer(ByteHostReq))
@@ -108,7 +109,7 @@ func TestListHosts(t *testing.T) {
 	t.Run("Should return 200 if everything works well", func(t *testing.T) {
 		fakeHosts := []*model.HostSpec{&SampleHosts[0], &SampleHosts[1]}
 		mockClient := new(dbtest.Client)
-		mockClient.On("ListHosts", c.NewAdminContext()).Return(fakeHosts, nil)
+		mockClient.On("ListHosts", c.NewAdminContext(), map[string][]string{}).Return(fakeHosts, nil)
 		db.C = mockClient
 
 		r, _ := http.NewRequest("GET", "/v1beta/host/hosts", nil)
@@ -175,6 +176,9 @@ func TestDeleteHost(t *testing.T) {
 		fakeHost := &SampleHosts[0]
 		mockClient := new(dbtest.Client)
 		mockClient.On("DeleteHost", c.NewAdminContext(), fakeHost.Id).Return(nil)
+		mockClient.On("GetHost", c.NewAdminContext(), fakeHost.Id).Return(fakeHost, nil)
+		mockClient.On("ListVolumeAttachmentsWithFilter", c.NewAdminContext(),
+			map[string][]string{"hostId": []string{fakeHost.Id}}).Return(nil, nil)
 		db.C = mockClient
 
 		r, _ := http.NewRequest("DELETE", "/v1beta/host/hosts/"+fakeHost.Id, nil)

pkg/api/controllers/volume_test.go

@@ -243,6 +243,7 @@ func TestExtendVolume(t *testing.T) {
 		"name": "sample-volume",
 		"description": "This is a sample volume for testing",
 		"size": 1,
+        "availabilityZone": "default",
 		"status": "extending",
 		"poolId": "084bf71e-a102-11e7-88a8-e31fe6d52248",
 		"profileId": "1106b972-66ef-11e7-b172-db03f3689c9c"

pkg/api/util/db_test.go

@@ -16,11 +16,12 @@ package util
 
 import (
 	"fmt"
-	"github.com/opensds/opensds/pkg/utils"
 	"reflect"
 	"strconv"
 	"testing"
 
+	"github.com/opensds/opensds/pkg/utils"
+
 	"github.com/opensds/opensds/pkg/context"
 	"github.com/opensds/opensds/pkg/db"
 	"github.com/opensds/opensds/pkg/model"
@@ -433,7 +434,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length equal to 0 character are not allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(0)
+		in.Name = utils.RandSeqWithAlnum(0)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -445,7 +446,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length equal to 1 character are allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(1)
+		in.Name = utils.RandSeqWithAlnum(1)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -460,7 +461,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length equal to 10 characters are allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(10)
+		in.Name = utils.RandSeqWithAlnum(10)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -475,7 +476,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length equal to 254 characters are allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(254)
+		in.Name = utils.RandSeqWithAlnum(254)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -490,7 +491,7 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length equal to 255 characters are allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(255)
+		in.Name = utils.RandSeqWithAlnum(255)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
@@ -505,26 +506,26 @@ func TestCreateFileShareDBEntry(t *testing.T) {
 	})
 
 	t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(256)
+		in.Name = utils.RandSeqWithAlnum(256)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
 		db.C = mockClient
 
 		_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
-		expectedError := "fileshare name length should not be more than 255 characters. input name length is : "+strconv.Itoa(len(in.Name))
+		expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
 		assertTestResult(t, err.Error(), expectedError)
 	})
 
 	t.Run("File share name length more than 255 characters are not allowed", func(t *testing.T) {
-		in.Name = utils.RandomString(257)
+		in.Name = utils.RandSeqWithAlnum(257)
 		in.Size, in.ProfileId = int64(1), "b3585ebe-c42c-120g-b28e-f373746a71ca"
 		mockClient := new(dbtest.Client)
 		mockClient.On("CreateFileShare", context.NewAdminContext(), in).Return(&SampleFileShares[0], nil)
 		db.C = mockClient
 
 		_, err := CreateFileShareDBEntry(context.NewAdminContext(), in)
-		expectedError := "fileshare name length should not be more than 255 characters. input name length is : "+strconv.Itoa(len(in.Name))
+		expectedError := "fileshare name length should not be more than 255 characters. input name length is : " + strconv.Itoa(len(in.Name))
 		assertTestResult(t, err.Error(), expectedError)
 	})
 

pkg/db/db.go

@@ -223,7 +223,7 @@ type Client interface {
 
 	ListVolumeGroupsWithFilter(ctx *c.Context, m map[string][]string) ([]*model.VolumeGroupSpec, error)
 
-	ListHosts(ctx *c.Context) ([]*model.HostSpec, error)
+	ListHosts(ctx *c.Context, m map[string][]string) ([]*model.HostSpec, error)
 
 	ListHostsByName(ctx *c.Context, hostName string) ([]*model.HostSpec, error)