From 06aabd9ead8088cab248738502502224e15c4bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20Pab=C3=B3n?= Date: Thu, 14 Mar 2019 06:19:31 -0700 Subject: [PATCH 1/2] Fix getting ownership from cred params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #941 Signed-off-by: Luis Pabón --- api/server/sdk/credentials.go | 3 +++ api/server/sdk/credentials_test.go | 34 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/api/server/sdk/credentials.go b/api/server/sdk/credentials.go index 15e5a966a..f73ca476c 100644 --- a/api/server/sdk/credentials.go +++ b/api/server/sdk/credentials.go @@ -446,6 +446,9 @@ func (s *CredentialServer) getOwnershipFromCred(cred interface{}) (*api.Ownershi var ownership *api.Ownership ownershipString, ok := info[api.OptCredOwnership].(string) if ok { + if len(ownershipString) == 0 { + return nil, nil + } ownership = &api.Ownership{} err := jsonpb.UnmarshalString(ownershipString, ownership) if err != nil { diff --git a/api/server/sdk/credentials_test.go b/api/server/sdk/credentials_test.go index 22bc82865..4fab9e4d1 100644 --- a/api/server/sdk/credentials_test.go +++ b/api/server/sdk/credentials_test.go @@ -996,3 +996,37 @@ func TestSdkCredentialOwnership(t *testing.T) { }) assert.NoError(t, err) } + +func TestSdkCredentialGetOwnershipFromCred(t *testing.T) { + s := &CredentialServer{} + + o, err := s.getOwnershipFromCred(map[string]interface{}{ + api.OptCredOwnership: "", + }) + assert.NoError(t, err) + assert.Nil(t, o) + + o, err = s.getOwnershipFromCred(map[string]interface{}{ + "hello": "world", + }) + assert.NoError(t, err) + assert.Nil(t, o) + + ownership := &api.Ownership{ + Owner: "user1", + Acls: &api.Ownership_AccessControl{ + Collaborators: map[string]api.Ownership_AccessType{ + "collabread": api.Ownership_Read, + "collabadmin": api.Ownership_Admin, + }, + }, + } + m := jsonpb.Marshaler{OrigName: true} + oStr, err := m.MarshalToString(ownership) + assert.NoError(t, err) + o, err = s.getOwnershipFromCred(map[string]interface{}{ + api.OptCredOwnership: oStr, + }) + assert.NoError(t, err) + assert.NotNil(t, o) +} From d01bcebce0f51a337a61b4a5d4ba8df4cdde8652 Mon Sep 17 00:00:00 2001 From: veda Date: Thu, 14 Mar 2019 15:33:21 -0700 Subject: [PATCH 2/2] Default credentials for cloud backup requests. Signed-off-by: veda --- api/server/sdk/cloud_backup.go | 136 ++++++++++++++++++++-------- api/server/sdk/cloud_backup_test.go | 73 ++++++++++++--- 2 files changed, 162 insertions(+), 47 deletions(-) diff --git a/api/server/sdk/cloud_backup.go b/api/server/sdk/cloud_backup.go index ed573cb14..019296a90 100644 --- a/api/server/sdk/cloud_backup.go +++ b/api/server/sdk/cloud_backup.go @@ -43,24 +43,32 @@ func (s *CloudBackupServer) Create( if s.driver(ctx) == nil { return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } - + credId := req.GetCredentialId() + var err error if len(req.GetVolumeId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must supply a volume id") - } else if len(req.GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must supply credential uuid") + } + if len(req.GetCredentialId()) == 0 { + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } } // Check ownership if err := checkAccessFromDriverForVolumeId(ctx, s.driver(ctx), req.GetVolumeId(), api.Ownership_Read); err != nil { return nil, err } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + + if len(req.GetCredentialId()) != 0 { + if err := s.checkAccessToCredential(ctx, credId); err != nil { + return nil, err + } } r, err := s.driver(ctx).CloudBackupCreate(&api.CloudBackupCreateRequest{ VolumeID: req.GetVolumeId(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, Full: req.GetFull(), Name: req.GetTaskId(), Labels: req.GetLabels(), @@ -82,20 +90,27 @@ func (s *CloudBackupServer) Restore( if s.driver(ctx) == nil { return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } - + credId := req.GetCredentialId() + var err error if len(req.GetBackupId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must provide backup id") } else if len(req.GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must provide credential uuid") + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + + if len(req.GetCredentialId()) != 0 { + if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { + return nil, err + } } r, err := s.driver(ctx).CloudBackupRestore(&api.CloudBackupRestoreRequest{ ID: req.GetBackupId(), RestoreVolumeName: req.GetRestoreVolumeName(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, NodeID: req.GetNodeId(), Name: req.GetTaskId(), }) @@ -118,19 +133,25 @@ func (s *CloudBackupServer) Delete( if s.driver(ctx) == nil { return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } - + credId := req.GetCredentialId() + var err error if len(req.GetBackupId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must provide backup id") } else if len(req.GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must provide credential uuid") + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + if len(req.GetCredentialId()) != 0 { + if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { + return nil, err + } } if err := s.driver(ctx).CloudBackupDelete(&api.CloudBackupDeleteRequest{ ID: req.GetBackupId(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, Force: req.GetForce(), }); err != nil { return nil, status.Errorf(codes.Internal, "Failed to delete backup: %v", err) @@ -147,20 +168,25 @@ func (s *CloudBackupServer) DeleteAll( if s.driver(ctx) == nil { return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } - + credId := req.GetCredentialId() + var err error if len(req.GetSrcVolumeId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must provide source volume id") } else if len(req.GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must provide credential uuid") + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + if len(req.GetCredentialId()) != 0 { + if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { + return nil, err + } } - if err := s.driver(ctx).CloudBackupDeleteAll(&api.CloudBackupDeleteAllRequest{ CloudBackupGenericRequest: api.CloudBackupGenericRequest{ SrcVolumeID: req.GetSrcVolumeId(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, }, }); err != nil { return nil, status.Errorf(codes.Internal, "Failed to delete backup: %v", err) @@ -177,19 +203,25 @@ func (s *CloudBackupServer) EnumerateWithFilters( if s.driver(ctx) == nil { return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } - + credId := req.GetCredentialId() + var err error if len(req.GetCredentialId()) == 0 { + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } return nil, status.Error(codes.InvalidArgument, "Must provide credential uuid") - } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + } else { + if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { + return nil, err + } } r, err := s.driver(ctx).CloudBackupEnumerate(&api.CloudBackupEnumerateRequest{ CloudBackupGenericRequest: api.CloudBackupGenericRequest{ SrcVolumeID: req.GetSrcVolumeId(), ClusterID: req.GetClusterId(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, All: req.GetAll(), }, }) @@ -236,16 +268,23 @@ func (s *CloudBackupServer) Catalog( if len(req.GetBackupId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must provide backup id") - } else if len(req.GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must provide credential uuid") } - if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { - return nil, err + credId := req.GetCredentialId() + var err error + if len(req.GetCredentialId()) == 0 { + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } + } else { + if err := s.checkAccessToCredential(ctx, req.GetCredentialId()); err != nil { + return nil, err + } } r, err := s.driver(ctx).CloudBackupCatalog(&api.CloudBackupCatalogRequest{ ID: req.GetBackupId(), - CredentialUUID: req.GetCredentialId(), + CredentialUUID: credId, }) if err != nil { return nil, status.Errorf(codes.Internal, "Failed to get catalog: %v", err) @@ -334,12 +373,17 @@ func (s *CloudBackupServer) SchedCreate( return nil, status.Error(codes.Unavailable, "Resource has not been initialized") } + credId := req.GetCloudSchedInfo().GetCredentialId() + var err error if req.GetCloudSchedInfo() == nil { return nil, status.Error(codes.InvalidArgument, "BackupSchedule object cannot be nil") } else if len(req.GetCloudSchedInfo().GetSrcVolumeId()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must supply source volume id") } else if len(req.GetCloudSchedInfo().GetCredentialId()) == 0 { - return nil, status.Error(codes.InvalidArgument, "Must supply credential uuid") + credId, err = s.defaultCloudBackupCreds(ctx) + if err != nil { + return nil, err + } } else if req.GetCloudSchedInfo().GetSchedules() == nil || len(req.GetCloudSchedInfo().GetSchedules()) == 0 { return nil, status.Error(codes.InvalidArgument, "Must supply Schedule") @@ -349,8 +393,10 @@ func (s *CloudBackupServer) SchedCreate( if err := checkAccessFromDriverForVolumeId(ctx, s.driver(ctx), req.GetCloudSchedInfo().GetSrcVolumeId(), api.Ownership_Read); err != nil { return nil, err } - if err := s.checkAccessToCredential(ctx, req.GetCloudSchedInfo().GetCredentialId()); err != nil { - return nil, err + if len(req.GetCloudSchedInfo().GetCredentialId()) != 0 { + if err := s.checkAccessToCredential(ctx, req.GetCloudSchedInfo().GetCredentialId()); err != nil { + return nil, err + } } sched, err := sdkSchedToRetainInternalSpecYamlByte(req.GetCloudSchedInfo().GetSchedules()) @@ -360,7 +406,7 @@ func (s *CloudBackupServer) SchedCreate( bkpRequest := api.CloudBackupSchedCreateRequest{} bkpRequest.SrcVolumeID = req.GetCloudSchedInfo().GetSrcVolumeId() - bkpRequest.CredentialUUID = req.GetCloudSchedInfo().GetCredentialId() + bkpRequest.CredentialUUID = credId bkpRequest.Schedule = string(sched) bkpRequest.MaxBackups = uint(req.GetCloudSchedInfo().GetMaxBackups()) bkpRequest.Full = req.GetCloudSchedInfo().GetFull() @@ -462,3 +508,21 @@ func ToSdkCloudBackupdScheduleInfo(s api.CloudBackupScheduleInfo) *api.SdkCloudB } return cloudSched } + +func (s *CloudBackupServer) defaultCloudBackupCreds( + ctx context.Context, +) (string, error) { + + req := &api.SdkCredentialEnumerateRequest{} + cs := &CredentialServer{server: s.server} + credList, err := cs.Enumerate(ctx, req) + if err != nil { + return "", err + } + + if len(credList.CredentialIds) > 1 || len(credList.CredentialIds) == 0 { + return "", status.Error(codes.InvalidArgument, "Either no credential or more than one configured,"+ + "please specify a credential name or uuid to use") + } + return credList.CredentialIds[0], nil +} diff --git a/api/server/sdk/cloud_backup_test.go b/api/server/sdk/cloud_backup_test.go index e08d4f3ad..f4196ef9b 100644 --- a/api/server/sdk/cloud_backup_test.go +++ b/api/server/sdk/cloud_backup_test.go @@ -45,6 +45,34 @@ func setupExpectedCredentialsPassing(s *testServer, credid string) { Return(creds, nil) } +func setupExpectedCredentialsNotPassing(s *testServer) { + s.MockDriver(). + EXPECT(). + CredsEnumerate(). + Return(nil, nil) +} + +func setupExpectedCredentialsNotPassingMoreThanOne(s *testServer) { + enumAzure1 := map[string]interface{}{ + api.OptCredType: "azure", + api.OptCredAzureAccountName: "test-azure-account-1", + api.OptCredAzureAccountKey: "test-azure-account-1", + } + enumAzure2 := map[string]interface{}{ + api.OptCredType: "azure", + api.OptCredAzureAccountName: "test-azure-account-2", + api.OptCredAzureAccountKey: "test-azure-account-2", + } + creds := map[string]interface{}{ + "uuid-1": enumAzure1, + "uuid-2": enumAzure2, + } + s.MockDriver(). + EXPECT(). + CredsEnumerate(). + Return(creds, nil) +} + func TestSdkCloudBackupCreate(t *testing.T) { // Create server and client connection @@ -73,7 +101,7 @@ func TestSdkCloudBackupCreate(t *testing.T) { Id: id, }, }, nil). - Times(1) + Times(2) s.MockDriver(). EXPECT(). CloudBackupCreate(&api.CloudBackupCreateRequest{ @@ -84,7 +112,7 @@ func TestSdkCloudBackupCreate(t *testing.T) { Labels: labels, }). Return(&api.CloudBackupCreateResponse{Name: "good-backup-name"}, nil). - Times(1) + Times(2) setupExpectedCredentialsPassing(s, uuid) // Setup client @@ -93,6 +121,13 @@ func TestSdkCloudBackupCreate(t *testing.T) { // Get info _, err := c.Create(context.Background(), req) assert.NoError(t, err) + + setupExpectedCredentialsPassing(s, uuid) + // default credentials + req.CredentialId = "" + _, err = c.Create(context.Background(), req) + assert.NoError(t, err) + } func TestSdkCloudBackupCreateBadArguments(t *testing.T) { @@ -114,13 +149,23 @@ func TestSdkCloudBackupCreateBadArguments(t *testing.T) { assert.Equal(t, serverError.Code(), codes.InvalidArgument) assert.Contains(t, serverError.Message(), "volume id") - // Missing credential uuid - req.VolumeId = "id" + // cred id missing + req.VolumeId = "myvol" + setupExpectedCredentialsNotPassing(s) + _, err = c.Create(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid to use") + + // more than 1 default creds + setupExpectedCredentialsNotPassingMoreThanOne(s) + _, err = c.Create(context.Background(), req) + serverError, ok = status.FromError(err) + assert.True(t, ok) + assert.Equal(t, serverError.Code(), codes.InvalidArgument) + assert.Contains(t, serverError.Message(), "credential name or uuid to use") } func TestSdkCloudRestoreCreate(t *testing.T) { @@ -182,12 +227,13 @@ func TestSdkCloudBackupRestoreBadArguments(t *testing.T) { assert.Contains(t, serverError.Message(), "backup id") // Missing credential uuid + setupExpectedCredentialsNotPassing(s) req.BackupId = "id" _, err = c.Restore(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudDeleteCreate(t *testing.T) { @@ -243,11 +289,12 @@ func TestSdkCloudBackupDeleteBadArguments(t *testing.T) { // Missing credential uuid req.BackupId = "id" + setupExpectedCredentialsNotPassing(s) _, err = c.Delete(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudDeleteAllCreate(t *testing.T) { @@ -303,12 +350,13 @@ func TestSdkCloudBackupDeleteAllBadArguments(t *testing.T) { assert.Contains(t, serverError.Message(), "volume id") // Missing credential uuid + setupExpectedCredentialsNotPassing(s) req.SrcVolumeId = "id" _, err = c.DeleteAll(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudBackupEnumerateWithFilters(t *testing.T) { @@ -398,11 +446,12 @@ func TestSdkCloudBackupEnumerateWithFiltersBadArguments(t *testing.T) { // Missing credential uuid req.SrcVolumeId = "id" + setupExpectedCredentialsNotPassing(s) _, err := c.EnumerateWithFilters(context.Background(), req) serverError, ok := status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudBackupStatus(t *testing.T) { @@ -551,11 +600,12 @@ func TestSdkCloudBackupCatalogBadArguments(t *testing.T) { // Missing credential uuid req.BackupId = "id" + setupExpectedCredentialsNotPassing(s) _, err = c.Catalog(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudBackupHistory(t *testing.T) { @@ -809,12 +859,13 @@ func TestSdkCloudBackupSchedCreateBadArguments(t *testing.T) { // Missing credential uuid req.VolumeId = "id" + setupExpectedCredentialsNotPassing(s) req.TaskId = "backup-task" _, err = c.Create(context.Background(), req) serverError, ok = status.FromError(err) assert.True(t, ok) assert.Equal(t, serverError.Code(), codes.InvalidArgument) - assert.Contains(t, serverError.Message(), "credential uuid") + assert.Contains(t, serverError.Message(), "credential name or uuid") } func TestSdkCloudBackupSchedEnumerate(t *testing.T) {