You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Xiaomeng Huang <xi...@intel.com> on 2014/09/09 12:42:05 UTC
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 10:42 a.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
rebase patch
Repository: sentry
Description
-------
Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
Diff: https://reviews.apache.org/r/24963/diff/
Testing
-------
test cases are included.
Thanks,
Xiaomeng Huang
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/
-----------------------------------------------------------
(Updated Sept. 16, 2014, 12:11 p.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
add test case for CreateAsSelect and fix bug.
Repository: sentry
Description
-------
Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java 6c101ad
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
Diff: https://reviews.apache.org/r/24963/diff/
Testing
-------
test cases are included.
Thanks,
Xiaomeng Huang
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
Posted by Xiaomeng Huang <xi...@intel.com>.
> On Sept. 13, 2014, 5:28 p.m., Prasad Mujumdar wrote:
> > Couple of high level comments:
> > - I am wondering if we should support creating view with column level permissions. Today it requres select at table level. If I have access to a subset of columns and create privilege on the database, this feature should also allow me to create a view with that subset of columns. Let me know what you think.
> > - We should more end to end tests for column select access paths,
> > + INSERT OVERWRITE into Table or Directory
> > + CREATE and query views (with the current privilege model)
> > + CREATE TABLE AS query
> > -
Thx Prasad! Your comments are very valable, I have a seen to the code again.
1. It may difficult to support creating view with column level permissions. Here are the reasons:
1) If operation is CREATEVIEW, analyzeInternal in SemanticAnalyzer will return before compiler.comple, so we couldn't get accessed columns from query.
if (createVwDesc != null) {
saveViewDefinition();
// validate the create view statement
// at this point, the createVwDesc gets all the information for semantic check
validateCreateView(createVwDesc);
// Since we're only creating a view (not executing it), we
// don't need to optimize or translate the plan (and in fact, those
// procedures can interfere with the view creation). So
// skip the rest of this method.
ctx.setResDir(null);
ctx.setResFile(null);
try {
PlanUtils.addInputsForView(pCtx);
} catch (HiveException e) {
throw new SemanticException(e);
}
return;
}
...
if (!ctx.getExplainLogical()) {
// At this point we have the complete operator tree
// from which we want to create the map-reduce plan
TaskCompiler compiler = TaskCompilerFactory.getCompiler(conf, pCtx);
compiler.init(conf, console, db);
compiler.compile(pCtx, rootTasks, inputs, outputs);
fetchTask = pCtx.getFetchTask();
}
2) When we create view, we can have the columns of view with alias, e.g. “CREATE VIEW VIEW_1(a1,a2) AS SELECT id,name FROM t1”. But we can just get the true column names of the view/table from columnAccessInfo, and I also put the true columns to ReadEntity(HIVE-7730). So we don't support column level privilege of view.
2. As your suggestions, I added a test case of CreateAsSelect in TestColumnEndToEnd and found a bug. I have fixed it, thx very much! And we also has other e2e tests, e.g. TestDbPrivilegesAtColumnScope. This patch is complex and very important, for reviewing clearly by you, I updated the patch. Soon we will update the merged patch in https://reviews.apache.org/r/24973/.
> On Sept. 13, 2014, 5:28 p.m., Prasad Mujumdar wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java, line 606
> > <https://reviews.apache.org/r/24963/diff/5/?file=683931#file683931line606>
> >
> > Is it possible to have something other than table or partition ? Should we raise an error in the default case ?
In this switch, I just add column Authorizable to the end of entityHierarchy(I have put server, db to entityHierarchy before switch). Only table and partition has columns, if entity type is others, we don't add column Authorizable and just put entityHierarchy to inputHierarchy like table level security.
- Xiaomeng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/#review53269
-----------------------------------------------------------
On Sept. 9, 2014, 11:49 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24963/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 11:49 a.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
>
> Diff: https://reviews.apache.org/r/24963/diff/
>
>
> Testing
> -------
>
> test cases are included.
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/#review53269
-----------------------------------------------------------
Couple of high level comments:
- I am wondering if we should support creating view with column level permissions. Today it requres select at table level. If I have access to a subset of columns and create privilege on the database, this feature should also allow me to create a view with that subset of columns. Let me know what you think.
- We should more end to end tests for column select access paths,
+ INSERT OVERWRITE into Table or Directory
+ CREATE and query views (with the current privilege model)
+ CREATE TABLE AS query
-
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
<https://reviews.apache.org/r/24963/#comment92851>
Is it possible to have something other than table or partition ? Should we raise an error in the default case ?
- Prasad Mujumdar
On Sept. 9, 2014, 11:49 a.m., Xiaomeng Huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24963/
> -----------------------------------------------------------
>
> (Updated Sept. 9, 2014, 11:49 a.m.)
>
>
> Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
>
> Diff: https://reviews.apache.org/r/24963/diff/
>
>
> Testing
> -------
>
> test cases are included.
>
>
> Thanks,
>
> Xiaomeng Huang
>
>
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 11:49 a.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Changes
-------
fix test case failed
Repository: sentry
Description
-------
Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
Diff: https://reviews.apache.org/r/24963/diff/
Testing
-------
test cases are included.
Thanks,
Xiaomeng Huang
Re: Review Request 24963: SENTRY-392: Authorization for column level
security
Posted by Xiaomeng Huang <xi...@intel.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24963/
-----------------------------------------------------------
(Updated Sept. 9, 2014, 11:37 a.m.)
Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya Tirukkovalur.
Repository: sentry
Description
-------
Authorization for column level security. This patch is depends on HIVE-7730(https://reviews.apache.org/r/24962/)
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a760516
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzPrivilegesMap.java 2f97e30
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java 6e40a5c
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java 51d4248
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestColumnEndToEnd.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 4c66ffe
Diff: https://reviews.apache.org/r/24963/diff/
Testing
-------
test cases are included.
Thanks,
Xiaomeng Huang