[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry

2017-11-20 Thread Pranay Singh (Code Review)
Hello Bharath Vissapragada,

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

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

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

Change subject: IMPALA-4927: Impala should be able to handle invalid input from 
Sentry
..

IMPALA-4927: Impala should be able to handle invalid input from Sentry

Impala requests a list of roles from Sentry and then asks for privileges
for each role. If Sentry returns a non existent role in the first step,
then there will be a Java exception in Impala in the second step and
the communication with Sentry is aborted.

The issue is fixed by handling the exception if an invalid role is
found and continue with getting permissions for the rest of the roles.

Testing:
---
Since invalid role could not be created through impala-shell/Hue
interface the code was instrumented to have a invalid Role " " and
see how the condition is handled.

Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
1 file changed, 31 insertions(+), 24 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
Gerrit-Change-Number: 8588
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Pranay Singh


[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry

2017-11-20 Thread Pranay Singh (Code Review)
Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8588 )

Change subject: IMPALA-4927: Impala should be able to handle invalid input from 
Sentry
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@10
PS1, Line 10: non existent role
> Can you clarify what a "non existent role" is?
As per my understanding a non existent ROLE is a invalid role that is created 
by some other CLIENT, Hue in this case (which has been fixed). The role does 
not have right privileges and causes issues for other valid roles.


http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@20
PS1, Line 20: the code was instrumented to have a invalid Role " " and see how 
the condition
> nit: line overflow.
Fixed


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@140
PS1, Line 140:   // deleted from this set and we are left with the 
set of privileges that need
> nit: line overflow
Done


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@142
PS1, Line 142:   Set privilegesToRemove = 
role.getPrivilegeNames();
> Just for my understanding, which exact line is throwing the exception where
listRolePrivileges() @ line# 146 will cause issue.


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@169
PS1, Line 169: role.getName()
> Can sentry ever return a null role name?
The bug says that HUE created a  ROLE with  "  "



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
Gerrit-Change-Number: 8588
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Mon, 20 Nov 2017 22:40:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry

2017-11-18 Thread Bharath Vissapragada (Code Review)
Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8588 )

Change subject: IMPALA-4927: Impala should be able to handle invalid input from 
Sentry
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@10
PS1, Line 10: non existent role
Can you clarify what a "non existent role" is?


http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@20
PS1, Line 20: the code was instrumented to have a invalid Role " " and see how 
the condition
nit: line overflow.


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@140
PS1, Line 140:   // deleted from this set and we are left with the 
set of privileges that need
nit: line overflow


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@142
PS1, Line 142:   Set privilegesToRemove = 
role.getPrivilegeNames();
Just for my understanding, which exact line is throwing the exception where 
when rolename is empty? The jira doesn't include a stack trace.


http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@169
PS1, Line 169: role.getName()
Can sentry ever return a null role name?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
Gerrit-Change-Number: 8588
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Comment-Date: Sat, 18 Nov 2017 20:43:26 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry

2017-11-17 Thread Pranay Singh (Code Review)
Pranay Singh has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8588


Change subject: IMPALA-4927: Impala should be able to handle invalid input from 
Sentry
..

IMPALA-4927: Impala should be able to handle invalid input from Sentry

Impala requests a list of roles from Sentry and then asks for privileges
for each role. If Sentry returns a non existent role in the first step,
then there will be a Java exception in Impala in the second step and
the communication with Sentry is aborted.

The issue is fixed by handling the exception if an invalid role is
found and continue with getting permissions for the rest of the roles.

Testing:
---
Since invalid role could not be created through impala-shell/Hue interface
the code was instrumented to have a invalid Role " " and see how the condition
is handled.

Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
1 file changed, 29 insertions(+), 24 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b
Gerrit-Change-Number: 8588
Gerrit-PatchSet: 1
Gerrit-Owner: Pranay Singh