You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/01/13 00:08:27 UTC

[kudu-CR] Add tls common module

Hello Todd Lipcon, Alexey Serbin,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
5 files changed, 142 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
6 files changed, 150 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/6
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add openssl util

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Add openssl_util
......................................................................

Add openssl_util

Also migrates the existing ssl_factory static initialization to use the
new openssl_util version.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
A src/kudu/security/openssl_util.cc
A src/kudu/security/openssl_util.h
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
6 files changed, 155 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/7
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has uploaded a new patch set (#2).

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
6 files changed, 148 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add openssl util

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.cc
File src/kudu/security/tls_common.cc:

Line 99:     serr << " ";
> weird to end in a space
Done


http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.h
File src/kudu/security/tls_common.h:

Line 36: std::string GetOpenSSLErrors();
> same.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add openssl util

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.

Change subject: Add openssl_util
......................................................................


Add openssl_util

Also migrates the existing ssl_factory static initialization to use the
new openssl_util version.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Reviewed-on: http://gerrit.cloudera.org:8080/5704
Reviewed-by: Todd Lipcon <to...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/security/CMakeLists.txt
A src/kudu/security/openssl_util.cc
A src/kudu/security/openssl_util.h
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
6 files changed, 155 insertions(+), 41 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Alexey Serbin: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add openssl util

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

Line 37:   ssl_socket.cc)
> I'm planning to do it.  As part of the negotiation changes both classes wil
OK. To be clear, if we standardize on TLS, we should specifically use TLS to refer to the TLS network protocol, but still use SSL to refer to the library, cert operations, etc.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
6 files changed, 149 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/3
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add openssl util

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

Line 37:   tls_common.cc)
> I agree, and plan to standardize on TLS.  I didn't want to rename during th
I can do that in my subsequent patch, if you are not planning to have a separate one dedicated for that update.


http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.cc
File src/kudu/security/tls_common.cc:

Line 93:     char buf[256];
> is 256 always enough for the error messages we expect?
In the OpenSSL library they usually use 128.  The long things might be filenames, etc., but those are not included into the message, coming separately.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add openssl util

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add openssl util

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 7: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add openssl util

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Add openssl_util
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

Line 37:   tls_common.cc)
> I can do that in my subsequent patch, if you are not planning to have a sep
I'm planning to do it.  As part of the negotiation changes both classes will need to be changed a lot.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.cc
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
7 files changed, 151 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/5
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: Add tls_common module
......................................................................

Add tls_common module

tls_common includes common methods for working with the TLS portions of
OpenSSL.

Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
---
M src/kudu/security/CMakeLists.txt
M src/kudu/security/ssl_factory.cc
M src/kudu/security/ssl_factory.h
M src/kudu/security/ssl_socket.h
A src/kudu/security/tls_common.cc
A src/kudu/security/tls_common.h
6 files changed, 150 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/5704/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5704
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Add tls common module

Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.

Change subject: Add tls_common module
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

Line 37:   tls_common.cc)
> would be nice if we could standardize on "SSL" vs "TLS". just a note, not a
I agree, and plan to standardize on TLS.  I didn't want to rename during the move, though.


http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.h
File src/kudu/security/tls_common.h:

Line 30: // Initializes static state required by the OpenSSL library.
> I'd expect this to be called InitOpenSSL or somesuch, and be in a different
Sure, I'll rename this to openssl_util.h


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@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] Add tls common module

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Add tls_common module
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/CMakeLists.txt
File src/kudu/security/CMakeLists.txt:

Line 37:   tls_common.cc)
would be nice if we could standardize on "SSL" vs "TLS". just a note, not a blocker for this patch.


http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.cc
File src/kudu/security/tls_common.cc:

Line 93:     char buf[256];
is 256 always enough for the error messages we expect?


Line 99:     serr << " ";
weird to end in a space


http://gerrit.cloudera.org:8080/#/c/5704/6/src/kudu/security/tls_common.h
File src/kudu/security/tls_common.h:

Line 30: // Initializes static state required by the OpenSSL library.
I'd expect this to be called InitOpenSSL or somesuch, and be in a differently named .cc file. It's not TLS-only, right? ie even if we just want to play with certs, we'd need to call this.


Line 36: std::string GetOpenSSLErrors();
same.

Maybe this file should be util.h or somesuch?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fb8bc2fb4153eff7adf896e43a979783de6f4b8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes