Conversation
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @xiaoyuyao for working on this. LGTM, some typos noted.
Key and prefix ACL CLI code is mostly the same. Would it make sense to add --prefix flag for existing key *acl operations to let them work on prefixes, instead of introducing separate prefix *acl commands?
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Add ACL to bucket. |
There was a problem hiding this comment.
| * Add ACL to bucket. | |
| * Add ACL to prefix. |
There was a problem hiding this comment.
I had the similar thought at the beginning. But Moving forward, we want to remove the support of individual key level acl SET/ADD/REMOVE with native authorizer. The prefix acl CLI is intend to make it different from the deprecated key level acl support.
| Assert.assertEquals("vol1", address.getVolumeName()); | ||
| Assert.assertEquals("bucket", address.getBucketName()); | ||
| Assert.assertEquals("prefix", address.getKeyName()); | ||
| Assert.assertTrue("this not be a prefix", |
There was a problem hiding this comment.
| Assert.assertTrue("this not be a prefix", | |
| Assert.assertTrue("this should be a prefix", |
|
Thanks @adoroszlai for the review. I've fixed the typos in the new commit. |
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @xiaoyuyao for updating the patch.
|
Thanks @adoroszlai for the review. PR has been merged. |
2 participants
What changes were proposed in this pull request?
Draft patch that adds the Prefix ACL CLI.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4905
Please replace this section with the link to the Apache JIRA)
How was this patch tested?
New acceptance tests added.