Alexey Serbin has posted comments on this change. Change subject: WIP [rpc] introduced per-RPC credentials policy ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/6875/3/src/kudu/rpc/reactor.cc File src/kudu/rpc/reactor.cc: PS3, Line 411: { : UserCredentials credentials; : credentials.set_real_user(conn_id.user_credentials().real_user()); : credentials.set_authn_token((policy == CredentialsPolicy::PRIMARY_CREDENTIALS) : ? boost::none : reactor()->messenger()->authn_token()); : (*conn)->set_local_user_credentials(std::move(credentials)); : } > There's a bit of a mess here, in that the Proxy object has set_user_credent Yes, I thought about this. >From one side, current approach are not using credentials properly: not using >them for hashing, etc. From another side, it's necessary to change authn >token on the fly when making a connection with PRIMARY_CREDENTIALS policy. I think passing a token as a parameter for StartConnectionNegotiation call or other way of direct passing the token is the way to resolve the inconsistency. Line 516: it = client_conns_.erase(it); > can you preserve the assertion here and ensure that we actually erase() exa The assertion is preserved, but it changed its form into CHECK(range.first != range.second). Sure, I'll additional one regarding removal of a single element. http://gerrit.cloudera.org:8080/#/c/6875/2/src/kudu/rpc/user_credentials.h File src/kudu/rpc/user_credentials.h: Line 41: void set_authn_token(const AuthnToken& token); > I thought this was created once per proxy, in which case it's set up, and t Yes -- that's when reactor creates a new connection with PRIMARY_CREDENTIALS policy. Todd mentioned that it may worth to reconsider this and remove authn token from UserCredentials. That would solve this and other consistency issues which might appear due the expiration of the token, etc. -- To view, visit http://gerrit.cloudera.org:8080/6875 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I52f806e7b6f6362f66148530124e748e199ae6c2 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
