Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15538 )
Change subject: IMPALA-9537: Add LDAP auth to the webui ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/15538/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15538/1//COMMIT_MSG@9 PS1, Line 9: This patch adds a startup flag --webserver_require_ldap, which if set > I think we might also need some kind of authorisation solution too? I.e. on Done http://gerrit.cloudera.org:8080/#/c/15538/1/be/src/util/webserver.cc File be/src/util/webserver.cc: http://gerrit.cloudera.org:8080/#/c/15538/1/be/src/util/webserver.cc@122 PS1, Line 122: DEFINE_bool(webserver_require_ldap, false, > Maybe in the help briefly explain the interaction between the different kin Yeah, allowing both while getting the return headers right is a little tricky and would require some code restructuring. I don't think its an important use case, so I just disallowed it. I'll note that in the description of the flags. http://gerrit.cloudera.org:8080/#/c/15538/1/be/src/util/webserver.cc@581 PS1, Line 581: total_cookie_auth_failure_->Increment(1); > It feels a little weird that we don't set authenticated = true here. The co Done http://gerrit.cloudera.org:8080/#/c/15538/1/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java File fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java: http://gerrit.cloudera.org:8080/#/c/15538/1/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java@59 PS1, Line 59: // Print out the output of the process, for debugging. We only need to print stdout, > I guess this is good to aid debugging. Maybe merits a one line comment to e Done -- To view, visit http://gerrit.cloudera.org:8080/15538 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e92481929f2f06898b8496233ab4134792c9f10 Gerrit-Change-Number: 15538 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 31 Mar 2020 21:20:31 +0000 Gerrit-HasComments: Yes
