smengcl commented on PR #4291: URL: https://github.com/apache/ozone/pull/4291#issuecomment-1438742336
Thanks @djordje-mijatovic for the patch. I have several questions and comments: 1. Since the change is mostly done in `RequestContext`, it should be easy to add support for Ozone native ACL in `OzoneNativeAuthorizer` as well. 2. As the PR itself does not update `OzoneNativeAuthorizer` (in Ozone code base) or `RangerOzoneAuthorizer` (in Ranger code base), and serves to lays the groundwork for those upcoming authorizer changes, IMO it would be more accurate for the JIRA/PR title to be: `[Snapshot] Add snapshot field in Ozone ACL RequestContext`. 3. A new `snapshotName` field alone may or may not suffice. Currently we only support the snapshot scope of a single bucket, but we would at least expand the snapshot scope to a volume. I'd like to know the intended usage of this new ACL resource field. Could you give an example or two? 4. Does this change imply we would have a new 4th field (e.g. `Snapshot`) other than what we currently have (volume, bucket and key) in the UI? Thus some UI changes could be required on the Ranger side as well. For reference, currently the Ranger Ozone policy editor Web UI looks like this: <img width="1200" alt="ranger-ozone-policy-ui" src="https://user-images.githubusercontent.com/50227127/220395512-199ff5d8-ed7e-41dd-8aa4-19f49a43f279.png"> cc @GeorgeJahad @neils-dev -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
