You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2020/01/31 02:51:38 UTC

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15140


Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 170 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/15140/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 2:

(1 comment)

l

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@282
PS2, Line 282:     if (!isColumnMaskingEnabled_
If there are 100 columns, this check for isColumnMaskingEnabled_ will be done as many times until one of the columns (could be the last one) access throws an exception.  Can't we exit sooner since the flag is False anyways ?



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 04:40:07 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@228
PS4, Line 228: Also deny access of columns
             :     // containing column masking policies when column masking feature is disabled.
I would prefer to mention this in the commit message.


http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@200
PS4, Line 200: if (request.getAuthorizable().getType() == Type.TABLE)
The same condition is already checked in the previous line.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:07:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 2: Code-Review+1

> Patch Set 2:
> 
> (1 comment)
> 
> l

I see the reasoning for this now..you are reverting to the pre-IMPALA-9009 behavior which was calling the evaluateColumnMask() for each column and you have added the extra check.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 05:06:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5564/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 04:57:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15140

to look at the new patch set (#5).

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

This patch recovers some legacy codes before IMPALA-9009 (mainly inside
RangerAuthorizationChecker). When the flag is set to false, the legacy
code paths are used: access of columns containing column masking
policies will be denied.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 166 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/15140/5
-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@193
PS1, Line 193:       if (!isColumnMaskingEnabled_
authorizeColumnMask() is the legacy codes before IMPALA-9009. I just add the new flag inside this function. It doesn't belong to the new code of IMPALA-9009. It will block any access to columns with column masking policies if the flag is false.

> BTW, the check for if (!isColumnMaskingEnabled_) is being done twice ..is that needed ?

Oops, I'll remove the check here.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 04:05:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 5: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:20:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5566/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 07:56:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@285
PS3, Line 285:       throw new AuthorizationException(String.format(
> Thanks for making the change to avoid unnecessary function calls.  One mino
Yeah, I add back the check for isColumnMaskingEnabled_ anyway.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 07:13:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15140

to look at the new patch set (#4).

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

This patch recovers some legacy codes before IMPALA-9009 (mainly inside
RangerAuthorizationChecker). When the flag is set to false, the legacy
code paths are used.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 170 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/15140/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 4
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@282
PS2, Line 282:     if (!isColumnMaskingEnabled_
> Ignore this as I understand that you are reverting to the pre-IMPALA-9009 b
Thanks! But we can exit sooner if the flag is True by moving "!isColumnMaskingEnabled_" to the caller to avoid 100 function calls.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 05:13:54 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15140

to look at the new patch set (#3).

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

This patch recovers some legacy codes before IMPALA-9009 (mainly inside
RangerAuthorizationChecker). When the flag is set to false, the legacy
code paths are used.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 169 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/15140/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 5:

(2 comments)

Thanks Aman and Csaba for looking into this so quickly!

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@228
PS4, Line 228: Also deny access of columns
             :     // containing column masking policies when column masking feature is disabled.
> I would prefer to mention this in the commit message.
Done


http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@200
PS4, Line 200: authorizeRowFilter(user,
> The same condition is already checked in the previous line.
Oops, done.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:14:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

This patch recovers some legacy codes before IMPALA-9009 (mainly inside
RangerAuthorizationChecker). When the flag is set to false, the legacy
code paths are used: access of columns containing column masking
policies will be denied.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Reviewed-on: http://gerrit.cloudera.org:8080/15140
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 166 insertions(+), 6 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 7
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 15:09:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5567/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:57:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@285
PS3, Line 285:           "Column masking is disabled by --enable_column_masking flag. Can't access " +
Thanks for making the change to avoid unnecessary function calls.  One minor nit:  This error message says 'Column masking is disabled by ...'  but this assumes that the caller is always calling this function when the flag is false.  Another caller could potentially call this function even when the flag is true.  Anyways, this is ok for now since I understand the disabling is a temporary thing until the feature is fully tested.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 06:06:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@282
PS2, Line 282:     if (!isColumnMaskingEnabled_
> If there are 100 columns, this check for isColumnMaskingEnabled_ will be do
Ignore this as I understand that you are reverting to the pre-IMPALA-9009 behavior.



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 05:08:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15140/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/15140/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@193
PS1, Line 193:       if (!isColumnMaskingEnabled_
If column masking is disabled, it's not clear to me why authorizeColumnMask() is being called. Shouldn't the effect of setting this flag to False be that the new code is not called at all ?  Unless I misunderstood the intent of the flag.  BTW, the check for if (!isColumnMaskingEnabled_) is being done twice ..is that needed ?



-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 03:52:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5483/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:21:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5563/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 03:06:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Hello Aman Sinha, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/15140

to look at the new patch set (#2).

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................

IMPALA-9348: Add flag to disable column masking

Add flag --enable_column_masking which defaults to false to disable
column masking feature until this feature becomes mature.

This patch recovers some legacy codes before IMPALA-9009 (mainly inside
RangerAuthorizationChecker). When the flag is set to false, the legacy
code paths are used.

Tests:
 - Recover legacy FE tests for verifying error messages.

Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
10 files changed, 169 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/15140/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5565/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 3
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 06:00:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9348: Add flag to disable column masking

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15140 )

Change subject: IMPALA-9348: Add flag to disable column masking
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15140
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife7c98042cfb831ba00d9d2179367cba257b77a0
Gerrit-Change-Number: 15140
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 31 Jan 2020 10:21:35 +0000
Gerrit-HasComments: No