Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update cloud migrate group backup access denied message #1258

Conversation

ggriffiths
Copy link
Contributor

Signed-off-by: Grant Griffiths [email protected]

What this PR does / why we need it:
Updates the error message for cloud backup group create when a user does not have access to a volume in a group.

Which issue(s) this PR fixes (optional)
Closes #1257

Special notes for your reviewer:

@ggriffiths ggriffiths requested review from lpabon and talakad October 1, 2019 21:44
@@ -133,7 +134,11 @@ func (s *CloudBackupServer) GroupCreate(

// Check ownership for this intersection
if err := checkAccessFromDriverForVolumeIds(ctx, s.driver(ctx), volumesToCheck, api.Ownership_Read); err != nil {
return nil, err
if err == kvdb.ErrNotFound {
Copy link
Contributor

@talakad talakad Oct 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkAccessFromDriverForVolume returns "codes.PermissionDenied" for access errors. Perhaps we should check for that and return other errors as is?
Actually that function returns "codes.NotFound" instead of kvdb.ErrNotFound.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkAccessFromDriverForVolumeIds is used by other functions, so I don't want to return the error about GroupIDs there.

But yeah I'll change the caller to check for codes.NotFound instead.

@ggriffiths ggriffiths force-pushed the errormsg_cloudbackup_create_group branch from 8e92f91 to 6f7831f Compare October 1, 2019 23:31
@ggriffiths ggriffiths force-pushed the errormsg_cloudbackup_create_group branch from 6f7831f to 3b1354a Compare October 1, 2019 23:47
Copy link
Contributor

@talakad talakad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloud backup group create access denied error
2 participants