You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Mano Kovacs via Review Board <no...@reviews.apache.org> on 2018/01/04 13:51:40 UTC
Review Request 64949: SENTRY-641 Add binding for lily hbase indexer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/
-----------------------------------------------------------
Review request for sentry.
Bugs: SENTRY-641
https://issues.apache.org/jira/browse/SENTRY-641
Repository: sentry
Description
-------
Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
Diffs
-----
pom.xml c797181c8f123bc914ece27c13b70540978091bc
sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
Diff: https://reviews.apache.org/r/64949/diff/1/
Testing
-------
Thanks,
Mano Kovacs
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
> On Jan. 4, 2018, 8:23 p.m., Steve Moist wrote:
> > sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
> > Lines 92 (patched)
> > <https://reviews.apache.org/r/64949/diff/1/?file=1930458#file1930458line92>
> >
> > What if they are empty strings?
Well, if you define hadoop.security.authentication=kerberos then:
- if you define hbase.regionserver.keytab.file empty string, you probably get error for invalid file;
- if you define hbase.regionserver.kerberos.principal empty string, you will have InvalidArgumentException at line 182.
- Mano
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review194783
-----------------------------------------------------------
On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 1:51 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Steve Moist via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review194783
-----------------------------------------------------------
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 92 (patched)
<https://reviews.apache.org/r/64949/#comment273781>
What if they are empty strings?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 134 (patched)
<https://reviews.apache.org/r/64949/#comment273787>
Change to String.format for readability and not having to concat everything.
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java
Lines 23 (patched)
<https://reviews.apache.org/r/64949/#comment273791>
change to HBASE_INDEXER
- Steve Moist
On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 1:51 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review194874
-----------------------------------------------------------
Created new patch, added to jira. If test-patch passes, uploading here.
- Mano Kovacs
On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 1:51 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
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/64949/#review195233
-----------------------------------------------------------
Ship it!
Ship It!
- Sergio Pena
On Jan. 10, 2018, 11:37 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 10, 2018, 11:37 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/pom.xml b082aa66b4324c0195ea4e7c6bdd794dbcbc18d3
> sentry-policy/sentry-policy-indexer/pom.xml 0696b1dec5313b91d7423f9ebf4685029f2e6db1
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java 932690c6ddad18e3c3b02b513fb80dec02a12d93
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/AbstractTestIndexerPolicyEngine.java 66455e8665786b48d3abf8f6659ccd22c992f130
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/IndexPolicyTestUtil.java 45f100e655be17748c1955f8b9925a7cbd18a81a
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestCommonPrivilegeForIndexer.java aa8aa5f96b1f621a8a77ef957d5469a1feb0f583
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerAuthorizationProviderGeneralCases.java 29b377e9d0f560d8f38cd30ab12e81f75ea6233e
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerAuthorizationProviderSpecialCases.java 1717c42f831d116e3389b3a827f946128b2fd74f
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerModelAuthorizables.java 1d8ca539083b5bae6547aab1d7350cdb4f8ef0cd
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyEngineDFS.java d166f0f981ff231cf4b323d137232fba2df677ab
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyEngineLocalFS.java 19ddefbb2b4af738ff9626c56a8c264b6a45c752
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyNegative.java e1a0dcc55bc48adb5ad527f92d68ef6e8ace1cb6
> sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerRequiredInRole.java ff20d0388df9cdbb4885ac1e74c9f8fd53ee0876
> sentry-policy/sentry-policy-indexer/src/test/resources/log4j.properties
> sentry-policy/sentry-policy-indexer/src/test/resources/test-authz-provider.ini 0e4051157c31996da251c368af2dbafe5ed665b5
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/
-----------------------------------------------------------
(Updated Jan. 10, 2018, 11:37 p.m.)
Review request for sentry.
Changes
-------
Thank you for the additional review, Sergio. I uploaded the changes accordingly.
Bugs: SENTRY-641
https://issues.apache.org/jira/browse/SENTRY-641
Repository: sentry
Description
-------
Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
Diffs (updated)
-----
pom.xml c797181c8f123bc914ece27c13b70540978091bc
sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
sentry-policy/pom.xml b082aa66b4324c0195ea4e7c6bdd794dbcbc18d3
sentry-policy/sentry-policy-indexer/pom.xml 0696b1dec5313b91d7423f9ebf4685029f2e6db1
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/SimpleIndexerPolicyEngine.java 932690c6ddad18e3c3b02b513fb80dec02a12d93
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/AbstractTestIndexerPolicyEngine.java 66455e8665786b48d3abf8f6659ccd22c992f130
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/IndexPolicyTestUtil.java 45f100e655be17748c1955f8b9925a7cbd18a81a
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestCommonPrivilegeForIndexer.java aa8aa5f96b1f621a8a77ef957d5469a1feb0f583
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerAuthorizationProviderGeneralCases.java 29b377e9d0f560d8f38cd30ab12e81f75ea6233e
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerAuthorizationProviderSpecialCases.java 1717c42f831d116e3389b3a827f946128b2fd74f
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerModelAuthorizables.java 1d8ca539083b5bae6547aab1d7350cdb4f8ef0cd
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyEngineDFS.java d166f0f981ff231cf4b323d137232fba2df677ab
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyEngineLocalFS.java 19ddefbb2b4af738ff9626c56a8c264b6a45c752
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerPolicyNegative.java e1a0dcc55bc48adb5ad527f92d68ef6e8ace1cb6
sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestIndexerRequiredInRole.java ff20d0388df9cdbb4885ac1e74c9f8fd53ee0876
sentry-policy/sentry-policy-indexer/src/test/resources/log4j.properties
sentry-policy/sentry-policy-indexer/src/test/resources/test-authz-provider.ini 0e4051157c31996da251c368af2dbafe5ed665b5
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
Diff: https://reviews.apache.org/r/64949/diff/3/
Changes: https://reviews.apache.org/r/64949/diff/2-3/
Testing
-------
Thanks,
Mano Kovacs
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
> On Jan. 9, 2018, 11:10 p.m., Sergio Pena wrote:
> > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java
> > Line 108 (original), 28 (patched)
> > <https://reviews.apache.org/r/64949/diff/2/?file=1934158#file1934158line108>
> >
> > What's the difference between using IndexerWildcardPrivilege or CommonPrivilege directly? I see there is only one static class, but can we have a similar class in CommonPrivilege? or there is some configuration that I am missing?
Removing the module altogether.
- Mano
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review195088
-----------------------------------------------------------
On Jan. 6, 2018, 9:31 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2018, 9:31 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
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/64949/#review195088
-----------------------------------------------------------
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 212 (patched)
<https://reviews.apache.org/r/64949/#comment274232>
If an exception occurs, the kerberosInit will stayed as 'true' and won't attempt to initialize kerberos later, right? should we set it back to false?
sentry-dist/src/license/THIRD-PARTY.properties
Line 4 (original)
<https://reviews.apache.org/r/64949/#comment274234>
This change should not be part of the patch.
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java
Line 108 (original), 28 (patched)
<https://reviews.apache.org/r/64949/#comment274233>
What's the difference between using IndexerWildcardPrivilege or CommonPrivilege directly? I see there is only one static class, but can we have a similar class in CommonPrivilege? or there is some configuration that I am missing?
- Sergio Pena
On Jan. 6, 2018, 9:31 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2018, 9:31 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/
-----------------------------------------------------------
(Updated Jan. 6, 2018, 9:31 p.m.)
Review request for sentry.
Changes
-------
Thank you for the reviews. I am uploading the changes as requested.
Bugs: SENTRY-641
https://issues.apache.org/jira/browse/SENTRY-641
Repository: sentry
Description
-------
Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
Diffs (updated)
-----
pom.xml c797181c8f123bc914ece27c13b70540978091bc
sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
Diff: https://reviews.apache.org/r/64949/diff/2/
Changes: https://reviews.apache.org/r/64949/diff/1-2/
Testing
-------
Thanks,
Mano Kovacs
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
> On Jan. 4, 2018, 10:19 p.m., Sergio Pena wrote:
> > sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
> > Lines 199 (patched)
> > <https://reviews.apache.org/r/64949/diff/1/?file=1930458#file1930458line199>
> >
> > Shouldn't we throw a checked exception instead of RuntimeException? Also, can we add an error message about what the user should do in case a login error happens? perhaps specify that they need to use X and Y configuration variables for the keryba and principal?
Well, I assume the original intent of this wrapping was to hide the internal errors from the upstream APIs. I think the proper stacktrace would be more the sufficient here as it would complain about the exact file the user was configuring.
- Mano
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review194781
-----------------------------------------------------------
On Jan. 6, 2018, 9:31 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2018, 9:31 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
Posted by Mano Kovacs via Review Board <no...@reviews.apache.org>.
> On Jan. 4, 2018, 10:19 p.m., Sergio Pena wrote:
> > sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
> > Lines 187-189 (patched)
> > <https://reviews.apache.org/r/64949/diff/1/?file=1930458#file1930458line187>
> >
> > I think we can avoid the synchronized() block by using an AtomicBoolean for the kerberosInit instead? Seems once kerberosInit is true, then there's no way the rest of the syncrhonized block will be executed, right?
Changed to AtomicBoolean. Please, take a look.
- Mano
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64949/#review194781
-----------------------------------------------------------
On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 1:51 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>
Re: Review Request 64949: SENTRY-641 Add binding for lily hbase
indexer
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/64949/#review194781
-----------------------------------------------------------
pom.xml
Lines 513 (patched)
<https://reviews.apache.org/r/64949/#comment273775>
sentry-binding-hive-v2 is not used anymore by Sentry. It will be removed later in a future release. Is there something there that should be moved to sentry-binding-hive?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 48 (patched)
<https://reviews.apache.org/r/64949/#comment273807>
Could you add javadoc on this class?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 51 (patched)
<https://reviews.apache.org/r/64949/#comment273779>
Is this used somewhere?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 54 (patched)
<https://reviews.apache.org/r/64949/#comment273780>
Shouldn't we use AuthorizationComponent.HBASEINDEXER instead of this variable?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 57 (patched)
<https://reviews.apache.org/r/64949/#comment273829>
Can we use 'boolean' instead of 'Boolean'?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 79 (patched)
<https://reviews.apache.org/r/64949/#comment273820>
Can you mention this is an 'HBase indexer authorization provider' to help the developer know about it?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 130 (patched)
<https://reviews.apache.org/r/64949/#comment273835>
I see that kafka and solr uses 'authorize' as a method name. Should we stay consistent with them?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 134-137 (patched)
<https://reviews.apache.org/r/64949/#comment273837>
How should we handle the access denied exception here?
Sentry has SentryAccessDeniedException class already. Is that useful?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 159 (patched)
<https://reviews.apache.org/r/64949/#comment273816>
Can you add more detailed to the error message about what the user should do to fix the problem?
Here's an example:
The HBase indexer resource '%s' does not exist. Use the 'hbaseindxer.hdfs.confdir' configuration variable to specify an existing resource directory.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 162 (patched)
<https://reviews.apache.org/r/64949/#comment273817>
Can you add more detailed to the error message about what the user should do to fix the problem?
Here's an example:
The HBase indexer resource '%s' is not a directory. Use the 'hbaseindxer.hdfs.confdir' configuration variable to specify an existing resource directory.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 165 (patched)
<https://reviews.apache.org/r/64949/#comment273818>
Can you add more detailed to the error message about what the user should do to fix the problem?
Here's an example:
The HBase indexer resource '%s' does not have read permissions. Change the directory
permissions to allow the HBase indexer process to read or use the 'hbaseindxer.hdfs.confdir' configuration variable to specify an existing resource directory with read permissions.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 176-180 (patched)
<https://reviews.apache.org/r/64949/#comment273822>
Can you make this comment as a javadoc comment? Adding @param and @throw comments?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 182 (patched)
<https://reviews.apache.org/r/64949/#comment273827>
Could you add an error message that tells the user how to fix this issue if happens?
I think we should get the kerberos info from the authzConf in this method so that you avoid throwing an exception if the kerberos credential are not specified on the configuration file.
Btw, does this method need to be public? I don't see uded anywhere else.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 185 (patched)
<https://reviews.apache.org/r/64949/#comment273828>
Same comment from the keytabFile.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 187-189 (patched)
<https://reviews.apache.org/r/64949/#comment273830>
I think we can avoid the synchronized() block by using an AtomicBoolean for the kerberosInit instead? Seems once kerberosInit is true, then there's no way the rest of the syncrhonized block will be executed, right?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 194 (patched)
<https://reviews.apache.org/r/64949/#comment273831>
Can you mention that your attempting to acquire the kerberos ticket for the HBase indexer binding or something like that? Just to help for debugging purposes.
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java
Lines 199 (patched)
<https://reviews.apache.org/r/64949/#comment273833>
Shouldn't we throw a checked exception instead of RuntimeException? Also, can we add an error message about what the user should do in case a login error happens? perhaps specify that they need to use X and Y configuration variables for the keryba and principal?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java
Lines 26 (patched)
<https://reviews.apache.org/r/64949/#comment273839>
Can you add javadoc in this class and methods?
sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java
Lines 72 (patched)
<https://reviews.apache.org/r/64949/#comment273782>
Is this variable used somewhere?
sentry-dist/src/license/THIRD-PARTY.properties
Lines 1-21 (original), 1-18 (patched)
<https://reviews.apache.org/r/64949/#comment273776>
These THRID-PARTY.properties changes are generated everytime we compile our code. We will remove this dependency later, but can you remove these change from this patch?
sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java
Line 124 (original), 124 (patched)
<https://reviews.apache.org/r/64949/#comment273777>
We have a CommonPrivilege.java class which handles these capital and lower cases scenarios. I think the CommonPrivilege was created to have a more generic API. Solr, Kafka and Hive use this privilege, should we replace the IndexerWildcardPrivilege for the CommonePrivilege instead?
- Sergio Pena
On Jan. 4, 2018, 1:51 p.m., Mano Kovacs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64949/
> -----------------------------------------------------------
>
> (Updated Jan. 4, 2018, 1:51 p.m.)
>
>
> Review request for sentry.
>
>
> Bugs: SENTRY-641
> https://issues.apache.org/jira/browse/SENTRY-641
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Binding for Lily Hbase Indexer, based on the implementation Gregory Chanan created for CDH version of Sentry. The patch contains implementation that has no dependency to Lily.
>
>
> Diffs
> -----
>
> pom.xml c797181c8f123bc914ece27c13b70540978091bc
> sentry-binding/pom.xml b12683c1c742c989ac118e968c71df50366a8557
> sentry-binding/sentry-binding-hbase-indexer/pom.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/HBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/authz/SentryHBaseIndexerAuthorizationException.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/main/java/org/apache/sentry/binding/hbaseindexer/conf/HBaseIndexerAuthzConf.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/java/org/apache/sentry/binding/hbaseindexer/TestHBaseIndexerAuthzBinding.java PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/log4j.properties PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site-service.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/sentry-site.xml PRE-CREATION
> sentry-binding/sentry-binding-hbase-indexer/src/test/resources/test-authz-provider.ini PRE-CREATION
> sentry-dist/pom.xml 63eb81c87172dc3237039b6d7641881b2831079f
> sentry-dist/src/license/THIRD-PARTY.properties 22429fc3d644ea961d59260b4a08eb380d7a4325
> sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java 9e9a0d2b3afc22098980b5a54b6a9989ad035b6f
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationComponent.java 5dc2b5592bdfbb281b083bdc835d455d07df0d86
>
>
> Diff: https://reviews.apache.org/r/64949/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Mano Kovacs
>
>