You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/03/15 16:59:13 UTC

[kudu-CR] [security] added 'failed KDC' integration test

Alexey Serbin has uploaded a new change for review.

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

Change subject: [security] added 'failed KDC' integration test
......................................................................

[security] added 'failed KDC' integration test

Change-Id: Ib9f403d4d1458dbb3380377fa95794c05ac69af5
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/security_components_faults-itest.cc
M src/kudu/security/test/mini_kdc-test.cc
M src/kudu/security/test/mini_kdc.cc
M src/kudu/util/subprocess.h
5 files changed, 298 insertions(+), 24 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9f403d4d1458dbb3380377fa95794c05ac69af5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [security] added 'failed KDC' integration test

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

Dan Burkert has posted comments on this change.

Change subject: [security] added 'failed KDC' integration test
......................................................................

Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6405/1/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:

Line 80: ADD_KUDU_TEST(security_components_faults-itest)
Couple of nits on the name:

'components' isn't very descriptive in this context, so I would drop it.

I think our convention is to use dashes over underscores when the name isn't a code module, so this should be 'security-faults-itest'.

http://gerrit.cloudera.org:8080/#/c/6405/1/src/kudu/integration-tests/security_components_faults-itest.cc
File src/kudu/integration-tests/security_components_faults-itest.cc:

Line 61: krb_lifetime_seconds_(8) {
Does this test get flaky with a smaller lifetime? This test takes about 25s on my machine, which is pretty long

Line 85: /*
consider removing

Line 188: FLAGS_rpc_trace_negotiation = true;
FYI failing negotiations are always traced. This is fine if you want successful ones as well.

Line 206: // until it's clarified.
I think this is just the crappy heimdal error message. Instead of ideffing this out, could you just add that as a valid match in the regex below?

http://gerrit.cloudera.org:8080/#/c/6405/1/src/kudu/security/test/mini_kdc.cc
File src/kudu/security/test/mini_kdc.cc:

Line 178: const bool need_config_update = (options_.port == 0);
Is pulling this out to a local necessary? Looks like it might be leftover from an intermediate revision.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9f403d4d1458dbb3380377fa95794c05ac69af5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: 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