Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/23237 )
Change subject: IMPALA-14285: Add SAML2 authentication support for Impala Web UI ...................................................................... Patch Set 11: (5 comments) http://gerrit.cloudera.org:8080/#/c/23237/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23237/11//COMMIT_MSG@7 PS11, Line 7: Impala Would this work for other components than coordinator? If not than it could be mentioned that this is only for coordinator. Actually I don't see why this couldn't work in catalog or the executor - as these are untested, it could make sense to check during startup if this is a coordinator. http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/rpc/authentication-util.cc File be/src/rpc/authentication-util.cc: http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/rpc/authentication-util.cc@300 PS11, Line 300: delete response; It seems clear to me to use and return a unique_ptr<TWrappedHttpResponse>. The original code was passing ownership with connection_context->response.reset(response); immediately - the hs2 code could release the unique_ptr while the webserver already uses it. http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/rpc/authentication-util.cc@307 PS11, Line 307: TWrappedHttpResponse* response = new TWrappedHttpResponse(); Same as line 300 http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/util/backend-gflag-util.cc File be/src/util/backend-gflag-util.cc: http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/util/backend-gflag-util.cc@173 PS11, Line 173: hs2_saml2_idp_metadata There was some discussion offline and it may be better to keep the old flag name for the old configs to avoid breaking environments that set them. So my preference would be saml2_idp_metadata + webserver_saml2_idp_metadata http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/23237/11/be/src/util/webserver.cc@1018 PS11, Line 1018: susccess typo -- To view, visit http://gerrit.cloudera.org:8080/23237 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I12540300529f9c240abf7196141ecb0ae6e37995 Gerrit-Change-Number: 23237 Gerrit-PatchSet: 11 Gerrit-Owner: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Tue, 13 Jan 2026 16:07:06 +0000 Gerrit-HasComments: Yes
