Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19503 )
Change subject: IMPALA-11922 Verify JWKS URL server TLS certificate by default. ...................................................................... Patch Set 7: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/19503/1/be/src/kudu/util/curl_util.cc File be/src/kudu/util/curl_util.cc: http://gerrit.cloudera.org:8080/#/c/19503/1/be/src/kudu/util/curl_util.cc@122 PS1, Line 122: CHECK_EQ > I copied this code from line 92-93. I'm very new to Impala, so I don't hav You could check the definition of those two macros -- it could make it clearer. CHECK_OK() would lead to a crash (due to SIGABRT) if the parameter of the macro isn't Status::OK, and that was the way to handle such a situation in the constructor. Since EasyCurl::DoRequest() returns Status, now there is a way to handle a situation when curl_easy_setopt() fails gracefully, the same way as it's done for other curl_easy_setopt() invocations in this method. http://gerrit.cloudera.org:8080/#/c/19503/1/be/src/util/jwt-util-internal.h File be/src/util/jwt-util-internal.h: http://gerrit.cloudera.org:8080/#/c/19503/1/be/src/util/jwt-util-internal.h@374 PS1, Line 374: of certs > I modified this comment a little bit in an attempt to clarify. It's only u OK, great. So, maybe then update the comment to make it more specific what the certs in the bundle are used for? http://gerrit.cloudera.org:8080/#/c/19503/7/fe/src/test/java/org/apache/impala/testutil/X509CertChain.java File fe/src/test/java/org/apache/impala/testutil/X509CertChain.java: PS7: Just curious: any specific reason for bringing in BouncyCastle in addition to java.security/javax.security? -- To view, visit http://gerrit.cloudera.org:8080/19503 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5f1e887fae39b5fb82fa9a40352e4b507b7d8d35 Gerrit-Change-Number: 19503 Gerrit-PatchSet: 7 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Wed, 22 Feb 2023 16:50:30 +0000 Gerrit-HasComments: Yes
