[
https://issues.apache.org/jira/browse/HBASE-20185?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16398583#comment-16398583
]
Appy commented on HBASE-20185:
------------------------------
bq. Leave a TODO comment around the "pre" CP hooks to mirror ....
Makes sense. done.
bq. Some unrelated whitespace/formatting changes make this appear bigger than
it is
IDEs. :( I thought of removing them, but them thought that it wasn't not too
much and isolated to just one file.
bq. Is the object visibility change on AccessChecker in RSRpcServices actually
necessary?
accessChecker declared in RsRpcServices but now also used in
MasterRpcServices#execProcedure.
There is already a test for that in TestAccessController-
https://github.com/apache/hbase/blob/master/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java#L809,
- however, those tests are hacked to not run the operations but the AC hooks
directly. for eg. instead of triggering flush, it just runs
ACCESS_CONTROLLER.preTableFlush(). It's not possible/good to change just a few
tests there given that will break the overall design and make things harder to
maintain.
The new test should go in TestAdminOnlyOperations (and the test class name
should probably be changed to TestRpcAccessChecks).
Let me commit what we have for now.
> Fix ACL check for MasterRpcServices#execProcedure
> -------------------------------------------------
>
> Key: HBASE-20185
> URL: https://issues.apache.org/jira/browse/HBASE-20185
> Project: HBase
> Issue Type: Bug
> Reporter: Appy
> Assignee: Appy
> Priority: Major
> Fix For: 2.0.0
>
> Attachments: HBASE-20185.master.001.patch
>
>
> Mailing thread ref:
> [http://mail-archives.apache.org/mod_mbox/hbase-dev/201803.mbox/%3CCAAjhxrriGy_UXpC4iHCSyBB18iAbjU3Y2%2BnjQ-66i9kPPCrPRQ%40mail.gmail.com%3E]
> TLDR; HBASE-19400 messed up perms required for flushing a table.
> ----
> Looks like flush and snapshot procedures are already doing permissions check
> as part of preTableFlush/preSnapshot hooks. However,
> LogRollMasterProcedureManager is missing access checks ([~elserj], can
> someone look at it?)
>
> With that, it makes no sense to put an ADMIN perm requirement which was added
> by me in HBASE-19400. Removing it.
> However, to make things better for future, i have made few design changes
> which will ensure 1) perm checks don't slip by mistake, 2) a suitable
> placeholder for checks for flush & snapshot when we remove AccessController
> for good.
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
