You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by Ramesh Mani <rm...@hortonworks.com> on 2019/08/20 00:28:07 UTC
Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/
-----------------------------------------------------------
Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
Bugs: RANGER-2539
https://issues.apache.org/jira/browse/RANGER-2539
Repository: ranger
Description
-------
RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
Diffs
-----
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
Diff: https://reviews.apache.org/r/71322/diff/1/
Testing
-------
Testing in local vm by create a hive service.
Thanks,
Ramesh Mani
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217321
-----------------------------------------------------------
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 194 (patched)
<https://reviews.apache.org/r/71322/#comment304624>
Is use of RangerServiceDefHelper necessary here? Wouldn't it be simpler to add following 2 policies?
- policy #1:
name: default database
resource: {database=default; table=* }
allowItem: { group=public, access=create }
- policy #2:
name: information_schema database
resource: { database=information_schema; table=*; column=* }
allowItem: { group=public; access=select }
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 313 (patched)
<https://reviews.apache.org/r/71322/#comment304623>
Replacing user={USER} with group=public might be easier to read/understand the policy.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 319 (patched)
<https://reviews.apache.org/r/71322/#comment304625>
Why is addition of ACCESS_TYPE_ALL necessary? There is already a policy in place for table '{OWNEER}'s to have all permissions.
- Madhan Neethiraj
On Aug. 20, 2019, 5:37 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 5:37 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2539
> https://issues.apache.org/jira/browse/RANGER-2539
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
>
>
> Diffs
> -----
>
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
>
>
> Diff: https://reviews.apache.org/r/71322/diff/2/
>
>
> Testing
> -------
>
> Testing in local vm by create a hive service.
>
>
> Thanks,
>
> Ramesh Mani
>
>
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/
-----------------------------------------------------------
(Updated Aug. 20, 2019, 3:51 p.m.)
Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
Changes
-------
Review comment addressed
Bugs: RANGER-2539
https://issues.apache.org/jira/browse/RANGER-2539
Repository: ranger
Description
-------
RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
Diffs (updated)
-----
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
Diff: https://reviews.apache.org/r/71322/diff/4/
Changes: https://reviews.apache.org/r/71322/diff/3-4/
Testing
-------
Testing in local vm by create a hive service.
Thanks,
Ramesh Mani
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217331
-----------------------------------------------------------
Fix it, then Ship it!
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 196 (patched)
<https://reviews.apache.org/r/71322/#comment304630>
Consider replacing '*' with WILDCARD_ASTERISK - in line #196, #197, #228, #229.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 206 (patched)
<https://reviews.apache.org/r/71322/#comment304631>
Is it necessary to grant 'select' access to 'public' on all tables in default db? This will allow everyone to be able to perform 'select' on every table in default db. If line #206 is removed, only owner of the table will be allowed to perform 'select' (and all other operations).
- Madhan Neethiraj
On Aug. 20, 2019, 8:23 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 8:23 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2539
> https://issues.apache.org/jira/browse/RANGER-2539
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
>
>
> Diffs
> -----
>
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
>
>
> Diff: https://reviews.apache.org/r/71322/diff/3/
>
>
> Testing
> -------
>
> Testing in local vm by create a hive service.
>
>
> Thanks,
>
> Ramesh Mani
>
>
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/
-----------------------------------------------------------
(Updated Aug. 20, 2019, 8:23 a.m.)
Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
Changes
-------
Fixed review comments
Bugs: RANGER-2539
https://issues.apache.org/jira/browse/RANGER-2539
Repository: ranger
Description
-------
RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
Diffs (updated)
-----
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
Diff: https://reviews.apache.org/r/71322/diff/3/
Changes: https://reviews.apache.org/r/71322/diff/2-3/
Testing
-------
Testing in local vm by create a hive service.
Thanks,
Ramesh Mani
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217323
-----------------------------------------------------------
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 304 (patched)
<https://reviews.apache.org/r/71322/#comment304627>
What would the returned policy name look like?
- default - database
- default - database, table
- default - database, table, column
- default - database, udf
- information_schema - database
- information_schema - database, table
- information_schema - database, table, column
- information_schema - database, udf
These names may not be intutive. Also, I think we really need following 2 policies to be created; if yes, I suggest the names given below:
- default database tables
- information_schema database tables
- Madhan Neethiraj
On Aug. 20, 2019, 5:37 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 5:37 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2539
> https://issues.apache.org/jira/browse/RANGER-2539
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
>
>
> Diffs
> -----
>
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
>
>
> Diff: https://reviews.apache.org/r/71322/diff/2/
>
>
> Testing
> -------
>
> Testing in local vm by create a hive service.
>
>
> Thanks,
>
> Ramesh Mani
>
>
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Ramesh Mani <rm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/
-----------------------------------------------------------
(Updated Aug. 20, 2019, 5:37 a.m.)
Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
Changes
-------
Fixed review comments
Bugs: RANGER-2539
https://issues.apache.org/jira/browse/RANGER-2539
Repository: ranger
Description
-------
RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
Diffs (updated)
-----
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
Diff: https://reviews.apache.org/r/71322/diff/2/
Changes: https://reviews.apache.org/r/71322/diff/1-2/
Testing
-------
Testing in local vm by create a hive service.
Thanks,
Ramesh Mani
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Ramesh Mani <rm...@hortonworks.com>.
> On Aug. 20, 2019, 1:24 a.m., Abhay Kulkarni wrote:
> > hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
> > Lines 212 (patched)
> > <https://reviews.apache.org/r/71322/diff/1/?file=2161882#file2161882line212>
> >
> > Please consider skipping hierarchies that do not contain ResourceDef with name "database". In future, if more top-level resources get added to hive service-def, such code will not require any change.
hierarchies skipping is done. the naming of the variable is wrong here, so fixed that.
- Ramesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217310
-----------------------------------------------------------
On Aug. 20, 2019, 5:37 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 5:37 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2539
> https://issues.apache.org/jira/browse/RANGER-2539
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
>
>
> Diffs
> -----
>
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
>
>
> Diff: https://reviews.apache.org/r/71322/diff/2/
>
>
> Testing
> -------
>
> Testing in local vm by create a hive service.
>
>
> Thanks,
>
> Ramesh Mani
>
>
Re: Review Request 71322: RANGER-2539:Create Default Policies for Hive
Databases -default, Information_schema
Posted by Abhay Kulkarni <ak...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71322/#review217310
-----------------------------------------------------------
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 157 (patched)
<https://reviews.apache.org/r/71322/#comment304601>
Please consider simplifying this by directly providing the database name as an argument to createPoliciesForHiveDB() function (which is private and hence visible only in this class).
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 180 (patched)
<https://reviews.apache.org/r/71322/#comment304602>
Please consider renaming this function as RangerPolicy createDatabaseDefaultPolicies(String databaseName);
with return value as newly created policy. THe caller can accumulate returned policy into a list.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 183 (patched)
<https://reviews.apache.org/r/71322/#comment304603>
Please review text of the function entry message.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 188 (patched)
<https://reviews.apache.org/r/71322/#comment304604>
I assume that Integer.valueOf(0) refers to RangerPolicy.POLICY_TYPE_ACCESS. If so, please use the it instead of hard-coding value 0.
Also consider the type of PROP_POLICY_NAME_SUFFIX to be Iterator of correct type, such as Iterator<List<RangerServiceDef.RangerResourceDef>>.
Then the type casting on the next line will not be required.
Please consider renaming PROP_POLICY_NAME_PREFIX and PROP_POLICY_NAME_SUFFIX to more descriptive names.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 191 (patched)
<https://reviews.apache.org/r/71322/#comment304605>
policyIndexes->resourceDefList
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 201 (patched)
<https://reviews.apache.org/r/71322/#comment304606>
Please review error message text. Database name is hardcoded there as DEFAULT.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 212 (patched)
<https://reviews.apache.org/r/71322/#comment304607>
Please consider skipping hierarchies that do not contain ResourceDef with name "database". In future, if more top-level resources get added to hive service-def, such code will not require any change.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 223 (patched)
<https://reviews.apache.org/r/71322/#comment304608>
Pleare review name of the function.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 225 (patched)
<https://reviews.apache.org/r/71322/#comment304609>
Please review text of log message to have correct method name.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 288 (patched)
<https://reviews.apache.org/r/71322/#comment304610>
If the database name is passed to this function, then this code block will not be required. Please review.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 319 (patched)
<https://reviews.apache.org/r/71322/#comment304611>
Why is this code block needed? hiveDatabaseName is not used in the body of switch statement anywhere.
hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java
Lines 340 (patched)
<https://reviews.apache.org/r/71322/#comment304612>
This may not be needed if the database name passed in to private functions in this class.
- Abhay Kulkarni
On Aug. 20, 2019, 12:28 a.m., Ramesh Mani wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71322/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2019, 12:28 a.m.)
>
>
> Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, Pradeep Agrawal, Selvamohan Neethiraj, Sailaja Polavarapu, and Velmurugan Periasamy.
>
>
> Bugs: RANGER-2539
> https://issues.apache.org/jira/browse/RANGER-2539
>
>
> Repository: ranger
>
>
> Description
> -------
>
> RANGER-2539:Create Default Policies for Hive Databases -default, Information_schema
>
>
> Diffs
> -----
>
> hive-agent/src/main/java/org/apache/ranger/services/hive/RangerServiceHive.java 89b8100
>
>
> Diff: https://reviews.apache.org/r/71322/diff/1/
>
>
> Testing
> -------
>
> Testing in local vm by create a hive service.
>
>
> Thanks,
>
> Ramesh Mani
>
>