----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62145/#review190413 -----------------------------------------------------------
src/slave/http.cpp Line 2359 (original), 2420-2423 (patched) <https://reviews.apache.org/r/62145/#comment267741> I think we should only try to set 'user' if agent `switch_user` flag is set. ``` if (slave->flags.switch_user) { if (commandInfo.has_user()) { containerUser = commandInfo.user(); } // If 'user' is not specified for standalone container, // it'll be the same as not switching user in containerizer. // No need to call os::user() here. } ``` src/slave/http.cpp Lines 2436 (patched) <https://reviews.apache.org/r/62145/#comment267846> ``` containerUser = executor->user; if (slave->flags.switch_user && commandInfo.has_user()) { containerUser = commandInfo.user(); } ``` src/slave/http.cpp Lines 2461 (patched) <https://reviews.apache.org/r/62145/#comment267742> Instead of passing a default user, i'd calculate that in the caller (`_launchContainer`) so that the contaienr user calculation is done in one place. src/slave/http.cpp Lines 2510 (patched) <https://reviews.apache.org/r/62145/#comment267743> maybe try to remove the sandbox created above and return a InternalServerError()? src/slave/http.cpp Lines 2641 (patched) <https://reviews.apache.org/r/62145/#comment267850> I am not sure if this is going to work. Looks like the way we do authorization for wait_nested_contianer is very special. It basically check if the top level container id matches or not. For standalone container, that means somehow we need to use the same JWT token based authentication and set `cid` claim properly? is that what we plan to do? I am not super familiar with the code around that. You may want to check with Greg on this? - Jie Yu On Nov. 2, 2017, 3:57 p.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62145/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 3:57 p.m.) > > > Review request for mesos, Gilbert Song and Jie Yu. > > > Bugs: MESOS-7305 > https://issues.apache.org/jira/browse/MESOS-7305 > > > Repository: mesos > > > Description > ------- > > The Standalone and Nested Container APIs are very similar. > This commit combines the two API implementations by adding a > translation function (i.e. `launchNestedContainer` and > `launchContainer`) which unpacks the V1 protobuf into fields > which can be passed into a common function (i.e. `_launchContainer`). > > The common functions authorize based on the type of container being > launched and it is possible to use both Standalone and Nested > Container APIs interchangably for nested containers. > > This approach is somewhat messy for for the `WAIT_(NESTED_)CONTAINER` > calls, as these methods require different return protobufs based on > the original call. > > > Diffs > ----- > > src/slave/http.hpp 44a95dec4c9b8bb65d712c5538bbd7afffe2cf7b > src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b > > > Diff: https://reviews.apache.org/r/62145/diff/4/ > > > Testing > ------- > > See later in chain. > > > Thanks, > > Joseph Wu > >
