New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIFI-1733 Adding a Ranger implementation of NiFi's Authorizer API #574
Conversation
8c845fd
to
51a79e9
Compare
e9b00b2
to
53f8e53
Compare
4da3418
to
2416d17
Compare
@bbende really nice job on the licensing stuff and thanks for finding/fixing the lgpl findbugs annotations issue! |
@bbende happy to take this on for review. Please update with the latest to resolve conflicts. |
final String rangerKerberosEnabledValue = getConfigValue(configurationContext, RANGER_KERBEROS_ENABLED_PROP, Boolean.FALSE.toString()); | ||
rangerKerberosEnabled = rangerKerberosEnabledValue.equals(Boolean.TRUE.toString()) ? true : false; | ||
|
||
if (rangerKerberosEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could not see testing coverage in this area, especially in the case where kerberos is enabled but no kerberos properties (keytab/principal) exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, working on some unit tests now for these scenarios
@bbende overall looks great ! I executed unit tests for coverage evaluation (hence my notes above) and did some integration testing with a remote Ranger server which also worked well. In looking at the RangerBasePluginWithPolicies it appeared to me to be a convenience class for extracting resources and storing a copy of the resource value for lookup later. One thought I had was whether the RangerPolicyEngine could be interrogated (via methods on the RangerBasePlugin) to check for existing resources to avoid the copy? |
@YolandaMDavis thanks for reviewing! Working on some changes based on your feedback. Regarding the need for RangerBasePluginWithPolicies... unfortunately the PolicyEngine is a private member variable of RangerBasePlugin and there is no getter for it, so no way to access it. I think most other plugins would never need to, but for NiFi we need to know if the reason for denying access was because no policy exists for the resource, or because a specific policy exists that doesn't match the incoming request. So the best I could come up with was to intercept when the policies are refreshed and store the resource ids so that when RangerAccessResult getIsAllowed() returns false we can then do a second check to see if there was even a policy for the given resource, and if not then return resource not found, rather than denied. |
- Adding Authorizer implementation for Ranger - Adding build profile and assembly that controls the inclusion of Ranger in the final assembly - Add properties to specify ranger admin identity and a flag to indicate if ranger is using kerberos, plugin is updated to perform a UGI login if ranger is using kerberos
…rBasePluginWithPolicies, cleaning up code to use Java 8 features
@YolandaMDavis rebased against master to resolve conflicts and pushed a new commit that addresses your feedback |
@bbende all looks good on this end. Thanks Bryan! |
This PR adds an Authorizer implementation that uses Apache Ranger and also modifies the build so that Ranger related artifacts are only included when using -Pinclude-ranger, this way the normal build does not need to include anything related to Ranger, and those that want it can also easily build it themselves.
When using NiFi with Ranger you would declare an Authorizer like the following in authorizers.xml:
For anyone interested in playing around with this, I created a Vagrant VM that can run a build of Ranger:
https://github.com/bbende/apache-ranger-vagrant