HAWQ-1203. Ranger Plugin Service Implementation. (with contributions … #1092
Conversation
…by Lav Jain and Leslie Chang)
There was a problem hiding this comment.
Please run ShellCheck (https://hackage.haskell.org/package/ShellCheck) on the files contained in the ranger-plugin/scripts directory.
| <parent> | ||
| <groupId>org.apache.hawq</groupId> | ||
| <artifactId>ranger-plugin</artifactId> | ||
| <version>2.1.0.0</version> |
There was a problem hiding this comment.
Is it intentional to use "2.1.0.0" as the HAWQ Ranger Admin Plugin version? Shouldn't it be on it's own version track?
There was a problem hiding this comment.
+1 for this comment. I guess this is the version number for apache hawq? If it is, I think we don't need record this version?
There was a problem hiding this comment.
we want to version RPS (and it RPM) with the same version as HAWQ as RPS is not a standalone product. Maven requires version in pom.xml files, so we choose the one that HAWQ has and will need to update it after Apache releases.
|
|
||
| # IDEA Project | ||
| *.iml | ||
| .idea |
There was a problem hiding this comment.
Are these necessary for each one's .gitignore file?
There was a problem hiding this comment.
this removes IDEA editor artifacts from the list of potential GIT changes as those artifacts should never be committed.
| try { | ||
| if (rs != null) rs.close(); | ||
| } catch (Exception e) { | ||
| // ignore |
There was a problem hiding this comment.
What shall we do for these exceptions? Do we need log it out?
There was a problem hiding this comment.
There is nothing we can do, this is a standard practice to ignore closing errors. We can log them, but that would not be of a great value to anybody.
| "parent": "", | ||
| "mandatory": true, | ||
| "lookupSupported": true, | ||
| "recursiveSupported": false, |
There was a problem hiding this comment.
What are the meanings of recursiveSupported and excludesSupported? Why do we set the values to false and true respectively?
There was a problem hiding this comment.
@ictmalili -- these are not well documented, we copied the definition from Hive. My understanding is that:
-- recursiveSupported would allow to define a policies as recursive which is used for HDFS access policies where a policy on a given folder, if declared as recursive, will be applied to all the files and subfolders inside a given folder. This is not applicable to us, so we do not allow creation of "recursive" policies for HAWQ.
-- excludesSupported will allow a resource specified in the policy to be "excluded". Ranger Admin UI will render the box to the right of the resource to be toggled between "included" and "excluded". I am not exactly sure what effect will that have on the policy.
| <parent> | ||
| <groupId>org.apache.hawq</groupId> | ||
| <artifactId>ranger-plugin-integration</artifactId> | ||
| <version>2.1.0.0</version> |
There was a problem hiding this comment.
Again for the version number here, which version number we should keep it?
There was a problem hiding this comment.
We will keep the version the same as for the rest of HAWQ code. Perhaps a separate discussion will be useful to decide how we should manage it between releases.
| # | ||
|
|
||
| function usage() { | ||
| echo "USAGE: register_hawq.sh -r ranger_host:ranger_port -u ranger_user -p ranger_password -h hawq_host:hawq_port -w hawq_user -q hawq_password" |
There was a problem hiding this comment.
What's the meaning for this script? Will it be exposed directly out to users or is it just an internal script? If it will be directly exposed out to users, can we change it to a more meaningful name? It looks a little confusing now.
There was a problem hiding this comment.
@ictmalili -- this script must be run to register HAWQ service definition with Ranger Admin and create default instance of HAWQ service within Ranger that points to HAWQ Active master for the lookup. For now it will have to be run manually by users once, later we can think of automating it with Ambari.
What name would you suggest ?
There was a problem hiding this comment.
@denalex May we change it adding indication the meaning to register to Ranger, such as register_hawq_to_ranger.sh, or anything else? Because HAWQ now has a management tool named as hawq register
There was a problem hiding this comment.
@ictmalili -- ok, makes sense, will rename the script as part of management scripts JIRA.
| int resourceSize = resourceMap.size(); | ||
|
|
||
| // special handling for non-leaf policies | ||
| if (accessType.equals(HawqPrivilege.create.name()) && database != null && schema == null && resourceSize == 1) { |
There was a problem hiding this comment.
What's the meaning of resourceSize valued as 1 and 2? Could you add some comments in the code here? Thanks
There was a problem hiding this comment.
resourceSize reflects how many tuples of key:value were received from HAWQ in resource element of JSON request. For example: "resource" : {"database":"foo"} is only 1 entry while "resource" : {"database":"foo", "schema":"bar"} are 2 entries. We have to analyze this for our special handling of create-schema and usage-schema permissions.
|
+1 except above comments I have made. |
|
+1 |
1 similar comment
|
+1 |
|
Thank you everyone for reviewing the PR. I will commit it tomorrow morning (PST time) and will create separate tasks for:
Please create other JIRAs if needed. The work on RPS codebase will continue after this merge via further additional JIRAs. |
…by Lav Jain and Leslie Chang)