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.

Sign up for GitHub

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

Jump to bottom

backupccl: when running SHOW BACKUP with specified s3 credentials, expect to be able to do it if non-admin #58190

Closed
mwang1026 opened this issue Dec 22, 2020 · 0 comments · Fixed by #58318
Closed

backupccl: when running SHOW BACKUP with specified s3 credentials, expect to be able to do it if non-admin #58190

mwang1026 opened this issue Dec 22, 2020 · 0 comments · Fixed by #58318
Assignees
@adityamaru
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. T-disaster-recovery

Comments

@mwang1026
Copy link

If running SHOW BACKUP 's3://bucket?AWS_SECRET_ACCESS_KEY=key1&AWS_ACCESS_KEY_ID=key2 it says that I must be admin.

But if I just add AUTH=specified then it works (SHOW BACKUP 's3://bucket?AWS_SECRET_ACCESS_KEY=key1&AWS_ACCESS_KEY_ID=key2&AUTH=specified)

I expect it to work with the first command since the default is specified, which is a UX surprise.
@mwang1026 mwang1026 added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. A-disaster-recovery T-disaster-recovery labels Dec 22, 2020
@adityamaru adityamaru mentioned this issue Dec 29, 2020
Merged
craig bot pushed a commit that referenced this issue Feb 9, 2021
@adityamaru
Merge #58318
2a5aaa6 
58318: cloudimpl: fix SHOW BACKUP when AUTH="" for s3 r=miretskiy a=adityamaru

When the AUTH param for an s3 URI is left unset, it defaults to
specified. This case was missed when checking if an operation such as
show backup required a user with admin role.

It is important to note that when AUTH is left empty for GS URIs it
defaults to reading from the cluster settings or env variables and so we
consider it implicit authentication.

Fixes: #58190

Release note: None

Co-authored-by: Aditya Maru <[email protected]>
@craig craig bot closed this as completed in ad9b0c7 Feb 9, 2021
dankinder pushed a commit to dankinder/cockroach that referenced this issue May 5, 2021
@adityamaru @dankinder
cloudimpl: fix SHOW BACKUP when AUTH="" for s3
fbcd8fc 
When the AUTH param for an s3 URI is left unset, it defaults to
specified. This case was missed when checking if an operation such as
show backup required a user with admin role.

It is important to note that when AUTH is left empty for GS URIs it
defaults to reading from the cluster settings or env variables and so we
consider it implicit authentication.

Fixes: cockroachdb#58190

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adityamaru adityamaru

Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. T-disaster-recovery
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

cloudimpl: fix SHOW BACKUP when AUTH="" for s3 adityamaru/cockroach
2 participants
@adityamaru @mwang1026