Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9373 )
Change subject: KUDU-2319 follower masters should be able to verify authn tokens ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/9373/5/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/9373/5/src/kudu/integration-tests/security-itest.cc@408 PS5, Line 408: { "required", "required", }, > Do we need to test the case encryption is disabled? It does not make sense to test when encryption is disabled, because in that case client would send authn token even if it had it. http://gerrit.cloudera.org:8080/#/c/9373/5/src/kudu/integration-tests/security-itest.cc@467 PS5, Line 467: ASSERT_EVENTUALLY([&] { > Can you add a comment what is this block of trying to test? Sure, will do. http://gerrit.cloudera.org:8080/#/c/9373/5/src/kudu/integration-tests/security-itest.cc@497 PS5, Line 497: ASSERT_OK(client->ListTables(&tables)); > Can we somehow ensure that client is not getting any exceptions from the fo I don't think we need it here: the ASSERT_EVENTUALLY above verifies that the client is able to authenticate against both masters using its authn token. Here we are just verifying that client is able to connect and perform basic operations once it's using correct set of masters' endpoints. -- To view, visit http://gerrit.cloudera.org:8080/9373 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idcc92dd4fae3d555af563d86634c07d3d06147a7 Gerrit-Change-Number: 9373 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Fri, 23 Feb 2018 07:34:06 +0000 Gerrit-HasComments: Yes
