You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Gregory Chanan <gc...@cloudera.com> on 2016/01/29 02:32:03 UTC

Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/
-----------------------------------------------------------

Review request for sentry.


Repository: sentry


Description
-------

>From the jira:
Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.

So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"


Diffs
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 

Diff: https://reviews.apache.org/r/42933/diff/


Testing
-------

Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.


Thanks,

Gregory Chanan


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review117057
-----------------------------------------------------------


Ship it!




Ship It!

- Sravya Tirukkovalur


On Jan. 29, 2016, 9:22 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 9:22 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Gregory Chanan <gc...@cloudera.com>.

> On Jan. 29, 2016, 11:33 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java, line 99
> > <https://reviews.apache.org/r/42933/diff/2/?file=1225945#file1225945line99>
> >
> >     Looks unrelated to the patch. Is this intended?

Well, I was originally fixing a few issues in the shell, but we moved all those to the client so this looks out of place now :).  I'm happy to commit it separately if you want.


- Gregory


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review117050
-----------------------------------------------------------


On Jan. 29, 2016, 9:22 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 9:22 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On Jan. 29, 2016, 11:33 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java, line 99
> > <https://reviews.apache.org/r/42933/diff/2/?file=1225945#file1225945line99>
> >
> >     Looks unrelated to the patch. Is this intended?
> 
> Gregory Chanan wrote:
>     Well, I was originally fixing a few issues in the shell, but we moved all those to the client so this looks out of place now :).  I'm happy to commit it separately if you want.

Thats fine. Just wanted to make sure it was not an intended change :-)


- Sravya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review117050
-----------------------------------------------------------


On Jan. 29, 2016, 9:22 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 9:22 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On Jan. 29, 2016, 11:33 p.m., Sravya Tirukkovalur wrote:
> > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java, line 99
> > <https://reviews.apache.org/r/42933/diff/2/?file=1225945#file1225945line99>
> >
> >     Looks unrelated to the patch. Is this intended?
> 
> Gregory Chanan wrote:
>     Well, I was originally fixing a few issues in the shell, but we moved all those to the client so this looks out of place now :).  I'm happy to commit it separately if you want.
> 
> Sravya Tirukkovalur wrote:
>     Thats fine. Just wanted to make sure it was not an intended change :-)

Correction: unintended.


- Sravya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review117050
-----------------------------------------------------------


On Jan. 29, 2016, 9:22 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 9:22 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review117050
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java (line 99)
<https://reviews.apache.org/r/42933/#comment178116>

    Looks unrelated to the patch. Is this intended?


- Sravya Tirukkovalur


On Jan. 29, 2016, 9:22 p.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 9:22 p.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Gregory Chanan <gc...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/
-----------------------------------------------------------

(Updated Jan. 29, 2016, 9:22 p.m.)


Review request for sentry.


Changes
-------

Moved Configuration handling from Shell to Client, as suggested by Sravya.


Repository: sentry


Description
-------

>From the jira:
Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.

So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java 761b0a469bcbe978ad262c517434f1d9346aab84 
  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 

Diff: https://reviews.apache.org/r/42933/diff/


Testing
-------

Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.


Thanks,

Gregory Chanan


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Gregory Chanan <gc...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?

Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83

That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.

The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.


- Gregory


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?
> 
> Gregory Chanan wrote:
>     Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83
>     
>     That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.
>     
>     The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.
> 
> Sravya Tirukkovalur wrote:
>     Thanks for the explanation Greg! So it seems like it is still safe to just bring in 
>     new Configuration() and setting hadoop.security.authentication followed by setting UseGroupInformation.setconf to the SentryGenericServiceClientDefaultImpl just before the UGI.getloginUser(). That way, future services would not have to do the workaround themselves?
> 
> Gregory Chanan wrote:
>     You are correct except I would change "still safe" to "still safe as long as the client process is not doing other hadoop-auth operations"
> 
> Sravya Tirukkovalur wrote:
>     Even if they are doing other operations, as we are loading the configurations via new Configurations() anyways isnt it? (Otherwise they would be loaded inside getloginUser())
> 
> Gregory Chanan wrote:
>     the issue is that you set the global configuration in UserGroupInformation.setConfiguration(...).  So imagine this set of events in the same process:
>     - Using generic client, set hadoop.security.authentication to kerberos via UGI.setConfiguration(...)
>     - Use some other client, using some other authentication method.  The UGI's configuration will still be using kerberos, so could have issues.  Could call UGI.setConfiguration again, although I don't know how that will affect the on going generic client operations.  In any case, I don't know if this is even fixable, just wanted to bring it up.

Yes, the problem is only with setting the authentication menthod, which might overwrite a different value of this paramter in the configuration. And hence we should set this only if "sentry.service.security.mode" is kerberos. That way it always works for secure kereberos clients. And for other clients, login fails as expected.


- Sravya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?
> 
> Gregory Chanan wrote:
>     Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83
>     
>     That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.
>     
>     The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.
> 
> Sravya Tirukkovalur wrote:
>     Thanks for the explanation Greg! So it seems like it is still safe to just bring in 
>     new Configuration() and setting hadoop.security.authentication followed by setting UseGroupInformation.setconf to the SentryGenericServiceClientDefaultImpl just before the UGI.getloginUser(). That way, future services would not have to do the workaround themselves?
> 
> Gregory Chanan wrote:
>     You are correct except I would change "still safe" to "still safe as long as the client process is not doing other hadoop-auth operations"

Even if they are doing other operations, as we are loading the configurations via new Configurations() anyways isnt it? (Otherwise they would be loaded inside getloginUser())


- Sravya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Gregory Chanan <gc...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?
> 
> Gregory Chanan wrote:
>     Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83
>     
>     That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.
>     
>     The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.
> 
> Sravya Tirukkovalur wrote:
>     Thanks for the explanation Greg! So it seems like it is still safe to just bring in 
>     new Configuration() and setting hadoop.security.authentication followed by setting UseGroupInformation.setconf to the SentryGenericServiceClientDefaultImpl just before the UGI.getloginUser(). That way, future services would not have to do the workaround themselves?

You are correct except I would change "still safe" to "still safe as long as the client process is not doing other hadoop-auth operations"


- Gregory


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?
> 
> Gregory Chanan wrote:
>     Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83
>     
>     That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.
>     
>     The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.

Thanks for the explanation Greg! So it seems like it is still safe to just bring in 
new Configuration() and setting hadoop.security.authentication followed by setting UseGroupInformation.setconf to the SentryGenericServiceClientDefaultImpl just before the UGI.getloginUser(). That way, future services would not have to do the workaround themselves?


- Sravya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Gregory Chanan <gc...@cloudera.com>.

> On Jan. 29, 2016, 6:56 p.m., Sravya Tirukkovalur wrote:
> > From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?
> 
> Gregory Chanan wrote:
>     Good question.  The issue is initializing hadoop's UserGroupInformation, which we are using for authentication.  See: https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L83
>     
>     That sets the authentication mechanism based on "hadoop.security.authentication".  By default that is retrieved from "new Configuration()" (default: SIMPLE).  So, if you have hadoop configuration on the classpath that sets "hadoop.security.authentication" to "kerberos" everything works, otherwise it doesn't.  I think it makes sense for us to set hadoop.security.authentication in the client generally because we have our own property for authentication; just because there is some hadoop configuration on the classpath that uses kerberos (for hadoop!) doesn't mean we should be using that for anything.
>     
>     The trickiness comes in setting the configuration statically (UserGroupInformation.setConfiguration).  We need to do this otherwise the configuration doesn't get picked up.  This sets the configuration for the entire process, though, so if your process is doing other hadoop-related things (e.g. reading off hdfs), it can affect it.  That was the nice thing about setting it in the shell; we know the shell is essentially a standalone process that isn't going to be doing other hadoop related things.  Given the limitations of hadoop-auth, though, we should probably just set the configuration there.
> 
> Sravya Tirukkovalur wrote:
>     Thanks for the explanation Greg! So it seems like it is still safe to just bring in 
>     new Configuration() and setting hadoop.security.authentication followed by setting UseGroupInformation.setconf to the SentryGenericServiceClientDefaultImpl just before the UGI.getloginUser(). That way, future services would not have to do the workaround themselves?
> 
> Gregory Chanan wrote:
>     You are correct except I would change "still safe" to "still safe as long as the client process is not doing other hadoop-auth operations"
> 
> Sravya Tirukkovalur wrote:
>     Even if they are doing other operations, as we are loading the configurations via new Configurations() anyways isnt it? (Otherwise they would be loaded inside getloginUser())

the issue is that you set the global configuration in UserGroupInformation.setConfiguration(...).  So imagine this set of events in the same process:
- Using generic client, set hadoop.security.authentication to kerberos via UGI.setConfiguration(...)
- Use some other client, using some other authentication method.  The UGI's configuration will still be using kerberos, so could have issues.  Could call UGI.setConfiguration again, although I don't know how that will affect the on going generic client operations.  In any case, I don't know if this is even fixable, just wanted to bring it up.


- Gregory


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>


Re: Review Request 42933: SENTRY-1037: Handle solr shell configuration in the same manner as sentry-binding

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42933/#review116984
-----------------------------------------------------------



>From this (https://github.com/apache/incubator-sentry/blob/95b1e40e7062343f05e131afaea0a97bbf71ba4f/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java#L127) , it looks like the client is using property "sentry.service.security.mode" to get the authentication mechanism. I dont think clients should be required to set this property. We should fix this in Sentry service/client if possible. Where in the client did you find looking up the hadoop property?

- Sravya Tirukkovalur


On Jan. 29, 2016, 1:32 a.m., Gregory Chanan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42933/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2016, 1:32 a.m.)
> 
> 
> Review request for sentry.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> From the jira:
> Today, the SentryShellSolr follows the same pattern as SentryShellHive, which is just getting a "new Configuration()". In order to connect to a service requiring kerberos, the Configuration must have "hadoop.security.authentication" set to "kerberos" since the generic client uses hadoop-auth to do the authentication. But this will often not be set in the context of Solr, which may not even have a hadoop-related configuration around.
> 
> So, we should handle the configuration in the same way as we do for the binding; namely, if the client intends to use kerberos, we set "hadoop.security.authentication" to "kerberos"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ec786a546b9def25e5b4c9d22ae5b49b79982c88 
> 
> Diff: https://reviews.apache.org/r/42933/diff/
> 
> 
> Testing
> -------
> 
> Tested on a cluster where sentry-site.xml does not specify hadoop.security.authentication and there are no other Configuration files on the classpath.
> 
> 
> Thanks,
> 
> Gregory Chanan
> 
>