You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2019/08/02 22:59:16 UTC

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13994


Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................

IMPALA-8828: Support impersonation via http paths

This patch allows clients that connect over the HTTP server to specify
the 'doAs' parameter in the provided path in order to perform
impersonation.

The existing rules for impersonation are applied, i.e.
authorized_proxy_user_config or authorized_proxy_group_config must be
set with the appropriate values for impersonation to be successful.

Testing:
- Added a FE test that verifies impersonation works as expected with
  impala-shell and ldap.
- Manually tested with Apache Knox.

Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
7 files changed, 157 insertions(+), 58 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc@591
PS1, Line 591:         connection_context->do_as_user = key_value[1];
Should we UrlDecode() the username? I was comparing this code to the code in Webserver::BuildArgumentMap(). I don't know how common special characters in usernames are, but it would be good to handle it correctly.


http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc@1091
PS1, Line 1091:     case ThriftServer::BINARY:
Maybe comment to indicate why it was left blank?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 17:21:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4138/ : 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/13994
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 20:20:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:53:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4129/ : 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/13994
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Aug 2019 23:39:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 04:14:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4145/ : 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/13994
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 00:19:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13994/2/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/13994/2/be/src/rpc/authentication.cc@594
PS2, Line 594:           LOG(WARNING) << "Could not decode 'doAs' parameter from HTTP request with "
Is this a safe fallback? 

If we ignore the doAs user, couldn't the client execute operations as the delegating user, who might be more privileged? Seems like we might actually have to set an error flag on connection_context and bubble back an error to avoid this.

There's an argument that this is not our problem - Apache Knox or whatever other system that is doing the delegation needs to get this security-critical stuff right. But it still seems better to me to fail safe in this case.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 20:42:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13994 )

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc@591
PS1, Line 591:       if (key_value.size() == 2 && key_value[0] == "doAs") {
> Should we UrlDecode() the username? I was comparing this code to the code i
Done


http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/rpc/authentication.cc@1091
PS1, Line 1091: 
> Maybe comment to indicate why it was left blank?
Done


http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/service/impala-hs2-server.cc@312
PS1, Line 312:   const ThriftServer::ConnectionContext* connection_context =
> line too long (105 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 19:39:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/4142/ : 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/13994
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:51:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................

IMPALA-8828: Support impersonation via http paths

This patch allows clients that connect over the HTTP server to specify
the 'doAs' parameter in the provided path in order to perform
impersonation.

The existing rules for impersonation are applied, i.e.
authorized_proxy_user_config or authorized_proxy_group_config must be
set with the appropriate values for impersonation to be successful.

Testing:
- Added a FE test that verifies impersonation works as expected with
  impala-shell and ldap.
- Manually tested with Apache Knox.

Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
7 files changed, 174 insertions(+), 58 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13994 )

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 4:

> Can we add a test for the error path?

Done


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:37:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................

IMPALA-8828: Support impersonation via http paths

This patch allows clients that connect over the HTTP server to specify
the 'doAs' parameter in the provided path in order to perform
impersonation.

The existing rules for impersonation are applied, i.e.
authorized_proxy_user_config or authorized_proxy_group_config must be
set with the appropriate values for impersonation to be successful.

Testing:
- Added a FE test that verifies impersonation works as expected with
  impala-shell and ldap.
- Manually tested with Apache Knox.

Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
7 files changed, 168 insertions(+), 58 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................

IMPALA-8828: Support impersonation via http paths

This patch allows clients that connect over the HTTP server to specify
the 'doAs' parameter in the provided path in order to perform
impersonation.

The existing rules for impersonation are applied, i.e.
authorized_proxy_user_config or authorized_proxy_group_config must be
set with the appropriate values for impersonation to be successful.

Testing:
- Added a FE test that verifies impersonation works as expected with
  impala-shell and ldap.
- Manually tested with Apache Knox.

Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
7 files changed, 180 insertions(+), 58 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:52:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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/13994 )

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................

IMPALA-8828: Support impersonation via http paths

This patch allows clients that connect over the HTTP server to specify
the 'doAs' parameter in the provided path in order to perform
impersonation.

The existing rules for impersonation are applied, i.e.
authorized_proxy_user_config or authorized_proxy_group_config must be
set with the appropriate values for impersonation to be successful.

Testing:
- Added a FE test that verifies impersonation works as expected with
  impala-shell and ldap.
- Manually tested with Apache Knox.

Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Reviewed-on: http://gerrit.cloudera.org:8080/13994
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/auth-provider.h
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
7 files changed, 180 insertions(+), 58 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 6
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13994 )

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13994/2/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/13994/2/be/src/rpc/authentication.cc@594
PS2, Line 594:           *err_msg = Substitute(
> Is this a safe fallback? 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:10:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/13994/1/be/src/service/impala-hs2-server.cc@312
PS1, Line 312:   const ThriftServer::ConnectionContext* connection_context = ThriftServer::GetThreadConnectionContext();
line too long (105 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 02 Aug 2019 22:59:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 5:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:53:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8828: Support impersonation via http paths

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

Change subject: IMPALA-8828: Support impersonation via http paths
......................................................................


Patch Set 3:

Can we add a test for the error path?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I20b9c2e2d106530732f1c52f8d3d1ecc24ae4bd6
Gerrit-Change-Number: 13994
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Aug 2019 22:21:50 +0000
Gerrit-HasComments: No