You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2021/08/06 20:55:06 UTC
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17759
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 307 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/17759/1
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7388/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 00:31:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9286/ : 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/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 00:52:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9264/ : 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/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 23:26:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc@907
PS1, Line 907: bool Webserver::HandleTrustedAuthHeader(
: struct sq_connection* connection, struct sq_request_info* request_info) {
: string err_msg;
: if (!GetUsernameFromAuthHeader(connection, request_info, err_msg)) {
: LOG(ERROR) << "Found trusted auth header but " << err_msg;
: return false;
: }
: return true;
: }
> nit: you can just get rid of HandleTrustedAuthHeader if you are only callin
Will remove HandleTrustedAuthHeader().
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 01:18:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7385/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 16:30:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17759/3/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:
http://gerrit.cloudera.org:8080/#/c/17759/3/be/src/rpc/authentication.cc@144
PS3, Line 144: "authentication again.");
> From a practical point of view it seems that to use a trusted header you wo
Fixed by adding a note.
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 00:34:07 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9265/ : 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/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 01:42:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 06:36:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 290 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/17759/3
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/7385/
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 22:40:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 5: Code-Review+2
Carry over +2 from Andrew.
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Aug 2021 00:30:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 291 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/17759/4
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 3:
(1 comment)
I talked to Bikram offline and we both had the same concern so I think it is worth adding to our doc on the flag.
http://gerrit.cloudera.org:8080/#/c/17759/3/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:
http://gerrit.cloudera.org:8080/#/c/17759/3/be/src/rpc/authentication.cc@144
PS3, Line 144: "authentication again.");
From a practical point of view it seems that to use a trusted header you would need some part of your system to remove the trusted header from any requests from outside the system. Do you agree? If so can you add a note to that effect here.
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 22:28:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
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/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Reviewed-on: http://gerrit.cloudera.org:8080/17759
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 291 insertions(+), 31 deletions(-)
Approvals:
Wenzhe Zhou: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9256/ : 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/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Aug 2021 21:19:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 302 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/17759/2
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@135
PS1, Line 135: // This flag defines a HTTP header which indicates a connection is trusted.
> This comment duplicates the flag description (below), probably we should ju
Fixed
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@145
PS1, Line 145: "If set as non empty string, Impala will look for this header in the connection "
> I think the header is in the http headers, rather than the connections stri
Fixed.
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@146
PS1, Line 146: "string. If the header presents, Impala will skip authentication and extract the "
> Nit: "If the header is present"
fixed
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@151
PS1, Line 151: "connections are authenticated before the requests lands on Impala.");
> Nit: land
fixed
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h
File be/src/util/webserver.h:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h@204
PS1, Line 204: /// Returns true if the connection has a valid username set in the request's the
> Nit: delete the last "the"
Fixed.
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h@222
PS1, Line 222: bool GetUsernameFromAuthHeader(struct sq_connection* connection,
> Add a simple description
Fixed
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc@887
PS1, Line 887: err_msg = Substitute("Error parsing basic authentication token from: $0 Error: $1",
> Nit: should be "error" so that if TrustedDomainCheck prints the message it
fixed
http://gerrit.cloudera.org:8080/#/c/17759/1/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java:
http://gerrit.cloudera.org:8080/#/c/17759/1/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@286
PS1, Line 286:
> What happens with the trusted auth header and a bad password? The main desc
Yes, it's ignored.
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 23:02:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 1:
(1 comment)
Looks good, Andrew's covered most of the stuff, just one more nit from my side
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc@907
PS1, Line 907: bool Webserver::HandleTrustedAuthHeader(
: struct sq_connection* connection, struct sq_request_info* request_info) {
: string err_msg;
: if (!GetUsernameFromAuthHeader(connection, request_info, err_msg)) {
: LOG(ERROR) << "Found trusted auth header but " << err_msg;
: return false;
: }
: return true;
: }
nit: you can just get rid of HandleTrustedAuthHeader if you are only calling GetUsernameFromAuthHeader.
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Aug 2021 00:30:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
IMPALA-10846: Skip Authentication for connection with trusted auth header
Adds the ability to skip authentication for connection over hs2 HTTP
endpoint or the http webserver endpoint with trusted auth header in
connection string. The trusted auth header can be specified using the
newly added "--trusted_auth_header" startup flag. Note this still
requires the client to specify a username via a basic auth header.
Testing:
- Added unit tests for both the hs2 http endpoint and the webserver
http endpoint.
- Passed core tests.
Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M be/src/util/webserver.cc
M be/src/util/webserver.h
M common/thrift/metrics.json
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
8 files changed, 291 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/17759/5
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 4: Code-Review+2
LGTM
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 16:25:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/9278/ : 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/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Aug 2021 00:56:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10846: Skip Authentication for connection with trusted auth header
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/17759 )
Change subject: IMPALA-10846: Skip Authentication for connection with trusted auth header
......................................................................
Patch Set 1:
(8 comments)
This looks good, I have a few suggestions for cleanup and clarity.
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@135
PS1, Line 135: // This flag defines a HTTP header which indicates a connection is trusted.
This comment duplicates the flag description (below), probably we should just have the flag description.
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@145
PS1, Line 145: "If set as non empty string, Impala will look for this header in the connection "
I think the header is in the http headers, rather than the connections string?
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@146
PS1, Line 146: "string. If the header presents, Impala will skip authentication and extract the "
Nit: "If the header is present"
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/rpc/authentication.cc@151
PS1, Line 151: "connections are authenticated before the requests lands on Impala.");
Nit: land
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h
File be/src/util/webserver.h:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h@204
PS1, Line 204: /// Returns true if the connection has a valid username set in the request's the
Nit: delete the last "the"
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.h@222
PS1, Line 222: bool GetUsernameFromAuthHeader(struct sq_connection* connection,
Add a simple description
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc
File be/src/util/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17759/1/be/src/util/webserver.cc@887
PS1, Line 887: err_msg = Substitute("Error parsing basic authentication token from: $0 Error: $1",
Nit: should be "error" so that if TrustedDomainCheck prints the message it will say "but error ..."
http://gerrit.cloudera.org:8080/#/c/17759/1/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
File fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java:
http://gerrit.cloudera.org:8080/#/c/17759/1/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java@286
PS1, Line 286:
What happens with the trusted auth header and a bad password? The main description of the flag says the password *may* be blank, does that imply it is ignored?
--
To view, visit http://gerrit.cloudera.org:8080/17759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7103012bc9cf9dfdc9c184af3a281526d8127a31
Gerrit-Change-Number: 17759
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Aug 2021 21:39:45 +0000
Gerrit-HasComments: Yes