You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Arjun Mishra via Review Board <no...@reviews.apache.org> on 2018/10/15 18:41:47 UTC

Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

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

Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Repository: sentry


Description
-------

Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well


Diffs
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 


Diff: https://reviews.apache.org/r/69030/diff/1/


Testing
-------


Thanks,

Arjun Mishra

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/#review209560
-----------------------------------------------------------




sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java
Lines 70 (patched)
<https://reviews.apache.org/r/69030/#comment294086>

    should we have a unit test to show how it is used?


- Na Li


On Oct. 15, 2018, 6:42 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2018, 6:42 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/#review209658
-----------------------------------------------------------


Ship it!




Ship It!

- Na Li


On Oct. 16, 2018, 4:17 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2018, 4:17 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Sergio Pena via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/#review209718
-----------------------------------------------------------


Ship it!




Ship It!

- Sergio Pena


On Oct. 16, 2018, 4:17 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2018, 4:17 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/
-----------------------------------------------------------

(Updated Oct. 16, 2018, 4:17 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
-------

Some hadoop versions don't throw NoMatchingRuleException


Bugs: SENTRY-2427
    https://issues.apache.org/jira/browse/SENTRY-2427


Repository: sentry


Description
-------

Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well


Diffs (updated)
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 


Diff: https://reviews.apache.org/r/69030/diff/4/

Changes: https://reviews.apache.org/r/69030/diff/3-4/


Testing
-------


Thanks,

Arjun Mishra

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.

> On Oct. 15, 2018, 10:38 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java
> > Lines 70 (patched)
> > <https://reviews.apache.org/r/69030/diff/3/?file=2098163#file2098163line70>
> >
> >     from this rule, it seems "user2@TEST.REALM.COM" should be allowed. What's the reason it is not valid?

No it shouldn't. Look at the setUp() method where the only user allowed to connect is hive. Here "user2@TEST.REALM.COM" maps to "solr" which is not "hive" so allowConnect returns false


- Arjun


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


On Oct. 15, 2018, 9:56 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2018, 9:56 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.

> On Oct. 15, 2018, 10:38 p.m., Na Li wrote:
> > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java
> > Lines 52 (patched)
> > <https://reviews.apache.org/r/69030/diff/3/?file=2098163#file2098163line52>
> >
> >     You never set HADOOP_SECURITY_AUTH_TO_LOCAL in conf. What is different from following code
> >     
> >     KerberosName.setRules(defaultRule);
> >     
> >     instead of 
> >         String defaultRule = "DEFAULT";
> >         String ruleString = conf.get(HADOOP_SECURITY_AUTH_TO_LOCAL, defaultRule);
> >         KerberosName.setRules(ruleString);

Lina, I simplified the config setting for the sake of testing. In reality the HADOOP_SECURITY_AUTH_TO_LOCAL mappign will be done in the UserGroupInformation#initialize() method which is invoked when sentry service is started. 

Here I am invoking the configuration by calling KerberosName.setRules(ruleString);


- Arjun


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


On Oct. 15, 2018, 9:56 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2018, 9:56 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/#review209572
-----------------------------------------------------------




sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java
Lines 52 (patched)
<https://reviews.apache.org/r/69030/#comment294100>

    You never set HADOOP_SECURITY_AUTH_TO_LOCAL in conf. What is different from following code
    
    KerberosName.setRules(defaultRule);
    
    instead of 
        String defaultRule = "DEFAULT";
        String ruleString = conf.get(HADOOP_SECURITY_AUTH_TO_LOCAL, defaultRule);
        KerberosName.setRules(ruleString);



sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java
Lines 70 (patched)
<https://reviews.apache.org/r/69030/#comment294099>

    from this rule, it seems "user2@TEST.REALM.COM" should be allowed. What's the reason it is not valid?


- Na Li


On Oct. 15, 2018, 9:56 p.m., Arjun Mishra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69030/
> -----------------------------------------------------------
> 
> (Updated Oct. 15, 2018, 9:56 p.m.)
> 
> 
> Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Bugs: SENTRY-2427
>     https://issues.apache.org/jira/browse/SENTRY-2427
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
>   sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69030/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Arjun Mishra
> 
>

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/
-----------------------------------------------------------

(Updated Oct. 15, 2018, 9:56 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Bugs: SENTRY-2427
    https://issues.apache.org/jira/browse/SENTRY-2427


Repository: sentry


Description
-------

Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well


Diffs (updated)
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 


Diff: https://reviews.apache.org/r/69030/diff/3/

Changes: https://reviews.apache.org/r/69030/diff/2-3/


Testing
-------


Thanks,

Arjun Mishra

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/
-----------------------------------------------------------

(Updated Oct. 15, 2018, 9:55 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
-------

Added test cases


Bugs: SENTRY-2427
    https://issues.apache.org/jira/browse/SENTRY-2427


Repository: sentry


Description
-------

Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well


Diffs (updated)
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 
  sentry-service/sentry-service-server/src/test/java/org/apache/sentry/service/thrift/TestGSSCallback.java PRE-CREATION 


Diff: https://reviews.apache.org/r/69030/diff/2/

Changes: https://reviews.apache.org/r/69030/diff/1-2/


Testing
-------


Thanks,

Arjun Mishra

Re: Review Request 69030: SENTRY-2427: Use Hadoop KerberosName class to derive shortName

Posted by Arjun Mishra via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69030/
-----------------------------------------------------------

(Updated Oct. 15, 2018, 6:42 p.m.)


Review request for sentry, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Bugs: SENTRY-2427
    https://issues.apache.org/jira/browse/SENTRY-2427


Repository: sentry


Description
-------

Sentry doesn't use auth to local group mapping hadoop configuration. We may have a use case for cross realm users to have access to sentry service and in which case Sentry needs to have access to those configurations. Switching to using KerberosName will handle that case and other cases as well


Diffs
-----

  sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/GSSCallback.java d2d85d3a2 


Diff: https://reviews.apache.org/r/69030/diff/1/


Testing
-------


Thanks,

Arjun Mishra