Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15394 )
Change subject: KUDU-3050: recover from corrupt kerberos ccache ...................................................................... Patch Set 4: (4 comments) Looks good overall; Alexey should also take a look. http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/integration-tests/security-itest.cc File src/kudu/integration-tests/security-itest.cc: http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/integration-tests/security-itest.cc@410 PS4, Line 410: // KinitContext is generally a global singleton so does not clean up memory. Just leak : // it for this test. Not needed anymore? http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/security/init.cc File src/kudu/security/init.cc: http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/security/init.cc@181 PS4, Line 181: // Free memory associated with these objects. Will these functions no-op if the values are null? If Kinit() fails mid-way, will the remaining values be null, or garbage? http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/security/init.cc@245 PS4, Line 245: << Krb5CallToStatus(g_krb5_ctx, code).ToString(); Could also wrap krb5_cc_start_seq_get with Krb5CallToStatus and then do the error check via the more idiomatic "if !status.ok()". Not a big deal either way. http://gerrit.cloudera.org:8080/#/c/15394/4/src/kudu/security/init.cc@299 PS4, Line 299: LOG(WARNING) << "Could not find kerberos principal in credential cache"; Can we add more useful information to this warning? Such as the location of the cache, if it's on disk? -- To view, visit http://gerrit.cloudera.org:8080/15394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d6e06c3ea65708896a6bf0134cc84838b3f1b58 Gerrit-Change-Number: 15394 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 10 Mar 2020 21:35:07 +0000 Gerrit-HasComments: Yes
