Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16956 )
Change subject: KUDU-3230: Fix the issue of hardcode sasl_proto_name ...................................................................... Patch Set 16: (2 comments) http://gerrit.cloudera.org:8080/#/c/16956/16/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/16956/16/src/kudu/integration-tests/security-itest.cc@227 PS16, Line 227: Test customized SASL protocol name. It would be nice to outline the sequence of the actions and the expected outcome. I.e., are Kudu server components supposed to run with the customized Kerberos credentials? What Kerberos credentials the client is going to use to access the cluster? Basically, outline the essence of the scenario here pointing to the important preconditions and outline the expected outcome. In the code below, you could add comments to outline some particular details, as needed. http://gerrit.cloudera.org:8080/#/c/16956/16/src/kudu/integration-tests/security-itest.cc@230 PS16, Line 230: FLAGS_sasl_krb5_principal_name I'm not sure I understand what's the purpose of setting this flag right here: is it just to have custom principal name for the client or the servers are supposed to use the custom SPN as well? If the latter, the proper way to do so is adding custom flags for kudu-master process run as a part of the ExternalMiniCluster. An example can be found in the SaslPlainFallback test scenario. -- To view, visit http://gerrit.cloudera.org:8080/16956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia1c8f9d9f772d000d9a588e4e9d6028711a62915 Gerrit-Change-Number: 16956 Gerrit-PatchSet: 16 Gerrit-Owner: Hongjiang Zhang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 27 Jan 2021 18:42:20 +0000 Gerrit-HasComments: Yes
