Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21925 )
Change subject: IMPALA-11298: Allow users to share hs2 sessions from multiple hosts ...................................................................... Patch Set 3: (4 comments) The code looks good to me, but the feature may lead to some strange behavior as Impala generally assumes that the host + user are the same during the lifetime of a session. http://gerrit.cloudera.org:8080/#/c/21925/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21925/3//COMMIT_MSG@9 PS3, Line 9: hs2 sessions are not necessarily bound to a host but to a user A case where this may be a bit weird is reporting the connected user like the profile's "Connected User:". My understanding is that the original full Kerberos username will be printed (the one that started the session), while the given query may be initiated from a different host. The same is true for FunctionContext::user() that can be called by UDFs. Not sure what would be the best way to handle these situation. http://gerrit.cloudera.org:8080/#/c/21925/3//COMMIT_MSG@11 PS3, Line 11: This is because the username could : include the hostname such as 'user/instance@REALM'. Instead we compare : the short user name of connected user with session user This means that REALM difference is also ignore, right? I don't think that this is a problem, but could be mentioned explicitly. http://gerrit.cloudera.org:8080/#/c/21925/3/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/21925/3/be/src/service/impala-server.h@1384 PS3, Line 1384: WithSession unrelated to the current change, but this function looks too complex for the .h file to me, it would be nice to move it to .cc http://gerrit.cloudera.org:8080/#/c/21925/3/be/src/service/impala-server.h@1401 PS3, Line 1401: connected_user_short Other comments mention Hue as a motivation for this. Hue acts as a proxy user, so while what we are comparing here is the connected user (Hue), there should be a do_as_user too which will be used as the actual initiator of queries (e.g. for authorization). Maybe it is enough to allow different host (and realm) for the case when there is a do_as_user? Impala already considers only the short name for proxy users: https://github.com/apache/impala/blob/2535e79491078a0353dbeed1a094e91366906149/be/src/service/impala-server.cc#L2118 So I think that in the proxy user case, it is safe to ignore host/realm, as it is not relevant when deciding whether the proxy user is allowed to impersonate do_as_user. I don't know if this is true for normal users - isn't it possible that the different host/realm would lead to different privileges or at least different user name in audits? -- To view, visit http://gerrit.cloudera.org:8080/21925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9c539cda8c760c8667a2e8cbb6d5c7902888de9 Gerrit-Change-Number: 21925 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Tue, 15 Oct 2024 12:28:10 +0000 Gerrit-HasComments: Yes
