Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10510 )
Change subject: IMPALA-5552: Add support for authorized proxy groups ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/10510/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10510/5//COMMIT_MSG@11 PS5, Line 11: impalad question(s) on the design, since I've not seen this feature (or the related, user delegation support). are these flags only relevant for coordinators? why are these flags added to impalad? is there a case for different impalads having different auth lists? if there is no case, then this looks like it opens up opportunities for misconfiguration. perhaps catalogd is a better place for this? (I could be wrong on this since I didn't see how these proxies are used) http://gerrit.cloudera.org:8080/#/c/10510/5/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10510/5/be/src/service/impala-server.cc@406 PS5, Line 406: --authorized_proxy_user_config flag or --authorized_proxy_group_config can be specific to to the authorized_proxy_config for which this method was called. http://gerrit.cloudera.org:8080/#/c/10510/5/be/src/service/impala-server.cc@412 PS5, Line 412: parsed_allowed_users_or_groups if ws is included around a group or user string, e.g., " * ", will (and should) that ws be trimmed from begin/end? http://gerrit.cloudera.org:8080/#/c/10510/5/be/src/service/impala-server.cc@416 PS5, Line 416: make_pair( can use a literal pair constructor: { ... } http://gerrit.cloudera.org:8080/#/c/10510/5/be/src/service/impala-server.cc@1383 PS5, Line 1383: RETURN_IF_ERROR(s); its an error, no need to wrap. http://gerrit.cloudera.org:8080/#/c/10510/5/fe/src/main/java/org/apache/impala/service/JniFrontend.java File fe/src/main/java/org/apache/impala/service/JniFrontend.java: http://gerrit.cloudera.org:8080/#/c/10510/5/fe/src/main/java/org/apache/impala/service/JniFrontend.java@635 PS5, Line 635: startsWith This makes the error messages in the security package part of the api. Is there any existing alternative to avoid this? If not, is there a JIRA filed to change that? -- To view, visit http://gerrit.cloudera.org:8080/10510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6953f89c293b06b72f523b11802232133d9d6cbb Gerrit-Change-Number: 10510 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Greg Rahn <gr...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Wed, 30 May 2018 17:47:06 +0000 Gerrit-HasComments: Yes
