You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Attila Bukor (Code Review)" <ge...@cloudera.org> on 2021/04/07 13:18:11 UTC
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Hello Alexey Serbin, Grant Henke,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/17279
to review the following change.
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_proto_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 117 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/1
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_protocol_n
> I meant not exposing this parameter at all -- that wasn't about the way to
Yea IIUC this is how the client can verify that it's talking to the correct server - it gets a service ticket for "kudu" and then it can use it to talk to a service authenticated using the "kudu" SPN. If the service is authenticated using "oryx", then the client must request a ticket for "oryx" to be able to talk to it.
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 09 Apr 2021 06:07:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_proto_name
> I wanted to be internally consistent here, as the Messenger flag we're sett
These are good points, indeed!
As for the newly introduced flag --sasl_proto_name in tool_action_common.cc: yes, I think it's better to rename it to --sasl_protocol_name.
However, I don't think it's _necessary_ to rename 'sasl_proto_name' in the Messenger class, at least right now. But if it would make things more consistent internally, why not (maybe, a separate patch would be a proper venue for that)?
I guess in the context of introducing changes in the published API, the 'external' consistency is more important than the 'internal' one. If thinking about Kudu API users, Messenger's interface is internal/private, so it's not visible from the outside. If you were using both the C++ and Java Kudu client APIs to build applications, what name would you prefer for this method? I'd also ask for extra opinion on this (Grant?), just to make sure we are on the same page.
As a side note, I think it would be great if there would be no need to expose this method in the client API at all, but I'm not sure it's possible to avoid this. BTW, did you try to play with the 3rd parameter of sasl_server_new() (called 'user_realm') in that regard?
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@462
PS3, Line 462: ASSERT_STR_CONTAINS(s.ToString(), "Server not found in Kerberos database");
> Done
Thank you!
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/tools/tool_action_common.cc@148
PS2, Line 148: sasl_proto_name
Per discussion in src/kudu/client/client.h: consider renaming into 'sasl_protocol_name'.
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@150
PS3, Line 150: Must match the "
: "servers' service principal name
> Done
Thank you!
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@421
PS3, Line 421: Status s = Mess
> Done
nit: whoops, I thought it assumed to be 2 + 4 spaces from the beginning of the line, no?
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Apr 2021 18:06:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/client/client_builder-internal.h
File src/kudu/client/client_builder-internal.h:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/client/client_builder-internal.h@45
PS3, Line 45: boost::optional<std::string>
nit: maybe, follow the suit of relying on an empty string to be a non-valid value for such SASL protocol name (like we have for authn_creds_)? What commands using introducing the boost::optional wrapper?
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@452
PS3, Line 452:
It would be nice to add a 'negative' test scenario where kerberos keytab misses credentials for appropriate service principal, e.g. having credentials for 'kudu/127.0.0.1' while the kudu-master and kudu-tserver processes are run with --principal=impala/_HOST flag. I guess it could help to better understand what to expect in case of a misconfiguration, if any.
What's the expected behavior for such a cluster itself? Would kudu-tserver and kudu-master processes be able to start at all in such case? If started, would kudu-tserver be able to connect to kudu-master?
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@453
PS3, Line 453: TEST_F(SecurityITest, TestNonDefaultPrincipal) {
Another interesting 'negative' scenario to have is when, say, kudu-master process is run with 'kudu/HOST_' principal, but kudu-tserver processes are run with 'some_other_spn/HOST_' principal. What's the expected behavior of such a cluster?
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@454
PS3, Line 454: "oryx"
nit: might define a constant and reuse it here and for setting SASL proto name
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@462
PS3, Line 462:
It would be nice to add a 'negative' test scenario when a client with wrong SASL proto name tries to connect to a Kudu cluster. The purpose of such scenario is multi-fold:
* make sure client cannot connect and isn't fruitlessly retrying for a long time before timing out
* once client failed to connect, it receives an error which is appropriate and actionable
* the server side doesn't crash :)
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool.proto@45
PS3, Line 45: Keberos
typo: Kerberos
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@150
PS3, Line 150: Must match the "
: "servers' service principal name.
nit: would be nice to clarify what 'match' means, because service principal name would be kudu/<hostname> or something
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@421
PS3, Line 421:
nit: spacing
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 07 Apr 2021 20:58:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Hongjiang Zhang (Code Review)" <ge...@cloudera.org>.
Hongjiang Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6:
Shall we replace "kudu" in https://github.com/apache/kudu/blob/master/src/kudu/rpc/messenger.cc#L76 with "FLAGS_sasl_protocol_name"?
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 09 Apr 2021 09:53:22 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17279
to look at the new patch set (#5).
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_proto_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 144 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/5
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 4:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_proto_name
> The corresponding counterpart method in the Java-side patch ( https://gerri
I wanted to be internally consistent here, as the Messenger flag we're setting is "sasl_proto_name" rather than "sasl_protocol_name" (in the underlying sasl_server_new()[1] and sasl_client_new()[2] it's simply called "service"). In Java, it's simply called "protocol"[3].
Do you think either of them should be renamed?
[1] https://linux.die.net/man/3/sasl_server_new
[2] https://linux.die.net/man/3/sasl_client_new
[3] https://docs.oracle.com/javase/7/docs/api/javax/security/sasl/Sasl.html
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/client/client_builder-internal.h
File src/kudu/client/client_builder-internal.h:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/client/client_builder-internal.h@45
PS3, Line 45: boost::optional<std::string>
> nit: maybe, follow the suit of relying on an empty string to be a non-valid
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@452
PS3, Line 452:
> It would be nice to add a 'negative' test scenario where kerberos keytab mi
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@453
PS3, Line 453: TEST_F(SecurityITest, TestNonDefaultPrincipal) {
> Another interesting 'negative' scenario to have is when, say, kudu-master p
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@454
PS3, Line 454: "oryx"
> nit: might define a constant and reuse it here and for setting SASL proto n
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/integration-tests/security-itest.cc@462
PS3, Line 462:
> It would be nice to add a 'negative' test scenario when a client with wrong
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool.proto@45
PS3, Line 45: Keberos
> typo: Kerberos
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@150
PS3, Line 150: Must match the "
: "servers' service principal name.
> nit: would be nice to clarify what 'match' means, because service principal
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@421
PS3, Line 421:
> nit: spacing
Done
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Apr 2021 11:30:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_protocol_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Reviewed-on: http://gerrit.cloudera.org:8080/17279
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 143 insertions(+), 33 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 7
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/mini-cluster/external_mini_cluster.cc@128
PS1, Line 128: ,
extra new line?
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/security/init.cc
File src/kudu/security/init.cc:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/security/init.cc@a78
PS1, Line 78:
Should this be marked as advanced and stable with this patch?
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/tools/tool.proto@55
PS1, Line 55: optional string principal = 10;
nit: add a simple doc line describing how this is used when creating the cluster. Perhaps move next to enable_kerberos.
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 1
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Apr 2021 14:08:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_proto_name
The corresponding counterpart method in the Java-side patch ( https://gerrit.cloudera.org/#/c/17280/) is called 'saslProtocolName()'. Maybe, name this 'sasl_protocol_name()' to match the name modulo the snake/camel case?
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 07 Apr 2021 21:19:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17279
to look at the new patch set (#3).
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_proto_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 122 insertions(+), 47 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/3
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 3
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 09 Apr 2021 13:49:05 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6:
> Patch Set 6:
>
> Shall we replace "kudu" in https://github.com/apache/kudu/blob/master/src/kudu/rpc/messenger.cc#L76 with "FLAGS_sasl_protocol_name"?
Thanks for the suggestion, I don't think this should be changed though. Client code, for example, is not using flags, so I believe it's better to keep this unchanged, and control it using the client API and the flags only in case of the servers and CLI tools. There's also two separate flags for this, --sasl_protocol_name is used in the CLI tools, but as servers need to be authenticated using the same SPN, it's just set to the value of the --principal flag in that case.
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hongjiang Zhang <ho...@ebay.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 09 Apr 2021 13:45:18 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17279
to look at the new patch set (#2).
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_proto_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 122 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/2
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17279
to look at the new patch set (#6).
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_protocol_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 143 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/6
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/mini-cluster/external_mini_cluster.cc@128
PS1, Line 128: ,
> extra new line?
clang-tidy "fixed" it for me when I changed the initializer list, should I manually override it?
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/security/init.cc
File src/kudu/security/init.cc:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/security/init.cc@a78
PS1, Line 78:
> Should this be marked as advanced and stable with this patch?
Done
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/tools/tool.proto
File src/kudu/tools/tool.proto:
http://gerrit.cloudera.org:8080/#/c/17279/1/src/kudu/tools/tool.proto@55
PS1, Line 55:
> nit: add a simple doc line describing how this is used when creating the cl
Done
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 2
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Apr 2021 15:47:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17279
to look at the new patch set (#4).
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
KUDU-1884 Make Kerberos principal customizable
In a Kerberized cluster, all Kudu servers service principal names (SPN)
are, by default, set to "kudu/_HOST". While this is configurable on the
server side, the SASL protocol name was hard-coded to "kudu" on the
client-side, meaning that if the SPN was anything other than "kudu", the
clients weren't able to connect to the Kudu servers. The 'principal'
flag was tagged as unsafe and experimental for this reason.
This patch allows changing the SASL protocol name in the C++ client
using the KuduClientBuilder API and the CLI tools using a new
'--sasl_proto_name' flag, and makes the servers use the configured
principal for the RPC.
Java and Python client support will be added in separate patches.
Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
---
M src/kudu/client/client.cc
M src/kudu/client/client.h
M src/kudu/client/client_builder-internal.h
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/security/init.cc
M src/kudu/server/server_base.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool.proto
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_test.cc
12 files changed, 108 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/17279/4
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 4
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_protocol_n
> These are good points, indeed!
Changed it to sasl_protocol_name. How would you like to handle it in client applications if not exposing it in the client builder? I haven't tried user_realm in sasl_server_new(), I don't see how that would be relevant tbh. Would you mind elaborating a bit on this?
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/tools/tool_action_common.cc@148
PS2, Line 148: sasl_protocol_n
> Per discussion in src/kudu/client/client.h: consider renaming into 'sasl_pr
Done
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@421
PS3, Line 421: Status s = Mess
> nit: whoops, I thought it assumed to be 2 + 4 spaces from the beginning of
I think the current one is correct, at least clang-tidy changed it to this one.
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Apr 2021 21:03:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1884 Make Kerberos principal customizable
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17279 )
Change subject: KUDU-1884 Make Kerberos principal customizable
......................................................................
Patch Set 6: Code-Review+2
(2 comments)
LGTM, but Grant might have more feedback, I guess.
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h
File src/kudu/client/client.h:
http://gerrit.cloudera.org:8080/#/c/17279/2/src/kudu/client/client.h@319
PS2, Line 319: sasl_protocol_n
> Changed it to sasl_protocol_name. How would you like to handle it in client
I meant not exposing this parameter at all -- that wasn't about the way to expose it. Being not familiar with SASL (I didn't read RFC2222/4422) I thought there might be a way around, i.e. allow to run Kudu server side with principal names independent from the SASL service names, but it seems that's not the case: https://www.iana.org/assignments/gssapi-service-names/gssapi-service-names.xhtml
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:
http://gerrit.cloudera.org:8080/#/c/17279/3/src/kudu/tools/tool_action_common.cc@421
PS3, Line 421: Status s = Mess
> I think the current one is correct, at least clang-tidy changed it to this
OK, it's just a nit after all.
--
To view, visit http://gerrit.cloudera.org:8080/17279
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c5e55714b98affe7c7dd095ce6ee37cd6bd3ddd
Gerrit-Change-Number: 17279
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 08 Apr 2021 21:36:08 +0000
Gerrit-HasComments: Yes