You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/10/28 01:18:44 UTC
[kudu-CR] Remove unused code for checking PLAIN authentication
Hello Dan Burkert, Alexey Serbin,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4874
to review the following change.
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Remove unused code for checking PLAIN authentication
This removes the 'AuthStore' abstraction which theoretically supported
checking username/password authentication. In practice, we have no plans
to implement our own username/password authentication any time soon, so
this extra code just confuses the matter.
Additionally, this deprecates the user information that's passed as part
of the 'Connection Context'. The client still sends it, to satisfy older
servers, but the server ignores it and instead determines user
information from the SASL handshake.
Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
---
M src/kudu/rpc/CMakeLists.txt
D src/kudu/rpc/auth_store.cc
D src/kudu/rpc/auth_store.h
M src/kudu/rpc/connection.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
14 files changed, 45 insertions(+), 261 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/4874/1
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4874/1/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:
Line 223: void WhoAmI(const WhoAmIRequestPB* req, WhoAmIResponsePB* resp, RpcContext* context) override {
> warning: parameter 'req' is unused [misc-unused-parameters]
Done
http://gerrit.cloudera.org:8080/#/c/4874/1/src/kudu/rpc/sasl_common.h
File src/kudu/rpc/sasl_common.h:
Line 107: static const char* name_of(Type val);
> warning: function 'kudu::rpc::SaslMechanism::name_of' has a definition with
Done
http://gerrit.cloudera.org:8080/#/c/4874/1/src/kudu/rpc/sasl_server.cc
File src/kudu/rpc/sasl_server.cc:
Line 462: int SaslServer::PlainAuthCb(sasl_conn_t *conn, const char *user, const char *pass,
> warning: parameter 'pass' is unused [misc-unused-parameters]
Done
Line 463: unsigned passlen, struct propctx *propctx) {
> warning: parameter 'propctx' is unused [misc-unused-parameters]
Done
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4874/2/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:
Line 223: void WhoAmI(const WhoAmIRequestPB* /*req*/, WhoAmIResponsePB* resp, RpcContext* context) override {
nit: it seems the lint build was unhappy with this long line (more than 100 characters in length).
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4874
to look at the new patch set (#2).
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Remove unused code for checking PLAIN authentication
This removes the 'AuthStore' abstraction which theoretically supported
checking username/password authentication. In practice, we have no plans
to implement our own username/password authentication any time soon, so
this extra code just confuses the matter.
Additionally, this deprecates the user information that's passed as part
of the 'Connection Context'. The client still sends it, to satisfy older
servers, but the server ignores it and instead determines user
information from the SASL handshake.
Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M src/kudu/rpc/CMakeLists.txt
D src/kudu/rpc/auth_store.cc
D src/kudu/rpc/auth_store.h
M src/kudu/rpc/connection.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
15 files changed, 52 insertions(+), 266 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/4874/2
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4874
to look at the new patch set (#3).
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Remove unused code for checking PLAIN authentication
This removes the 'AuthStore' abstraction which theoretically supported
checking username/password authentication. In practice, we have no plans
to implement our own username/password authentication any time soon, so
this extra code just confuses the matter.
Additionally, this deprecates the user information that's passed as part
of the 'Connection Context'. The client still sends it, to satisfy older
servers, but the server ignores it and instead determines user
information from the SASL handshake.
Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
---
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M src/kudu/rpc/CMakeLists.txt
D src/kudu/rpc/auth_store.cc
D src/kudu/rpc/auth_store.h
M src/kudu/rpc/connection.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
15 files changed, 54 insertions(+), 266 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/74/4874/3
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Remove unused code for checking PLAIN authentication
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: Remove unused code for checking PLAIN authentication
......................................................................
Remove unused code for checking PLAIN authentication
This removes the 'AuthStore' abstraction which theoretically supported
checking username/password authentication. In practice, we have no plans
to implement our own username/password authentication any time soon, so
this extra code just confuses the matter.
Additionally, this deprecates the user information that's passed as part
of the 'Connection Context'. The client still sends it, to satisfy older
servers, but the server ignores it and instead determines user
information from the SASL handshake.
Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Reviewed-on: http://gerrit.cloudera.org:8080/4874
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M src/kudu/rpc/CMakeLists.txt
D src/kudu/rpc/auth_store.cc
D src/kudu/rpc/auth_store.h
M src/kudu/rpc/connection.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/outbound_call.cc
M src/kudu/rpc/outbound_call.h
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_header.proto
M src/kudu/rpc/sasl_common.cc
M src/kudu/rpc/sasl_common.h
M src/kudu/rpc/sasl_rpc-test.cc
M src/kudu/rpc/sasl_server.cc
M src/kudu/rpc/sasl_server.h
15 files changed, 54 insertions(+), 266 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie960fae30fe573b859f7ef0e27d656faac50d4c2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>