Skip to content

HDDS-5206. Support revoking S3 secret - #2239

Merged
smengcl merged 6 commits into
apache:masterfrom
smengcl:HDDS-5206
May 19, 2021
Merged

HDDS-5206. Support revoking S3 secret#2239
smengcl merged 6 commits into
apache:masterfrom
smengcl:HDDS-5206

Conversation

@smengcl

@smengclsmengcl commented May 12, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/HDDS-5206

Currently the CLI only support "ozone s3 getsecret", but has no user or admin command to revoke the secret, or to rotate the secret. Add a new command to do that.

@smengclsmengcl added the om label May 12, 2021
@smengclsmengcl self-assigned this May 12, 2021
xiaoyuyao
xiaoyuyao reviewed May 12, 2021
View reviewed changes

@xiaoyuyaoxiaoyuyao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @smengcl for adding this. The PR LGTM overall. I just have a few comments added inline.

Comment thread...ager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSecretRequest.java Outdated
xiaoyuyao
xiaoyuyao reviewed May 12, 2021
View reviewed changes

@xiaoyuyaoxiaoyuyao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @smengcl for adding this. The PR LGTM overall. I just have a few comments added inline.

Comment threadhadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeS3SecretHandler.java
bharatviswa504
bharatviswa504 reviewed May 12, 2021
View reviewed changes

@bharatviswa504bharatviswa504 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall LGTM.

I have few comments/questions.

Comment thread...ager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSecretRequest.java
Comment thread...ager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3RevokeSecretRequest.java Outdated
Comment threadhadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/s3/RevokeS3SecretHandler.java
@smengcl
Admins are always allowed to get/revoke secret;
1b556c8
Invalid entry in table cache immediately (Thanks Aravindan);
Impl. isAdmin in OzoneManager.
Change-Id: Ia5c8d76be0845c5530d1e84abd82217be7797141
@smengclsmengcl changed the title HDDS-5206. Support revoking S3 secret in Ozone CLIHDDS-5206. Support revoking S3 secretMay 17, 2021
smengcl added 3 commits May 17, 2021 12:56
@smengcl
Improve S3RevokeSecretResponse; checkstyle; findbugs.
fa99fcd
Change-Id: Idb7ac40b974380a43946930bb4742d44696cfcea
@smengcl
Merge remote-tracking branch 'asf/master' into HDDS-5206
90d5fd3
Conflicts:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServlet.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java
Change-Id: If923f83358898f3d084a38a70d40fa197e8b2103
@smengcl
Checkstyle.
550590b
Change-Id: I8fe03516379f9094769633b82a8a77e06e8a809b
@smengcl

smengcl commented May 17, 2021

Copy link
Copy Markdown
ContributorAuthor

@xiaoyuyao@bharatviswa504 I have addressed all comments. Please take another look. Thanks!

@swagle
swagle requested a review from vivekratnavelMay 18, 2021 18:27
vivekratnavel
vivekratnavel reviewed May 18, 2021
View reviewed changes
Comment thread...manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java Outdated
Comment thread...manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java Outdated
@smengcl
Improve check; address nits.
270e42e
Change-Id: I1d6e054010e7046299e65f8bef1d341eb60e5ea8
vivekratnavel
vivekratnavel approved these changes May 19, 2021
View reviewed changes

@vivekratnavelvivekratnavel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1 LGTM

Comment thread...manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java Outdated
@smengcl

smengcl commented May 19, 2021

Copy link
Copy Markdown
ContributorAuthor

Thanks @xiaoyuyao@bharatviswa504@vivekratnavel for reviewing this. Will merge shortly.

@smengcl
smengcl merged commit 4fd8187 into apache:masterMay 19, 2021
@smengcl

smengcl commented May 20, 2021

Copy link
Copy Markdown
ContributorAuthor

Just realized I have a typo in the permission check, should be isAdmin(user.getUserName()) instead of kerberosID. Will post an addendum in a min.

@smengclsmengcl mentioned this pull request May 20, 2021
Merged
bharatviswa504 pushed a commit to bharatviswa504/hadoop-ozone that referenced this pull request Jul 25, 2021
@smengcl
HDDS-5206. Support revoking S3 secret (apache#2239)
39489e3
(cherry picked from commit 4fd8187)
Change-Id: I97158a39ed50614706a5615d1019b07501cf45ed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiaoyuyaoxiaoyuyaoxiaoyuyao left review comments
@bharatviswa504bharatviswa504bharatviswa504 left review comments
@vivekratnavelvivekratnavelvivekratnavel approved these changes
@prashantpogdeprashantpogdeAwaiting requested review from prashantpogde

Assignees

@smengclsmengcl

Labels

om

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smengcl@vivekratnavel@xiaoyuyao@bharatviswa504