> On Nov. 8, 2017, 10:28 a.m., Jie Yu wrote: > > src/slave/http.cpp > > Line 2359 (original), 2420-2423 (patched) > > <https://reviews.apache.org/r/62145/diff/4/?file=1879171#file1879171line2420> > > > > 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. > > } > > ```
As discussed offline, since we're making the AuthZ into ANY or NONE, some of this can go away. > On Nov. 8, 2017, 10:28 a.m., Jie Yu wrote: > > src/slave/http.cpp > > Lines 2510 (patched) > > <https://reviews.apache.org/r/62145/diff/4/?file=1879171#file1879171line2512> > > > > maybe try to remove the sandbox created above and return a > > InternalServerError()? This is mirroring the logic of sandbox creation for normal top-level containers: https://github.com/apache/mesos/blob/a595bcbf7afd9783e15f3e32cd9e70fa979531df/src/slave/paths.cpp#L613-L617 We don't fail the launch if the sandbox chown fails. However, since this API is more user facing, it makes sense to fail here. - Joseph ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62145/#review190413 ----------------------------------------------------------- On Nov. 2, 2017, 8:57 a.m., Joseph Wu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62145/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 8:57 a.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 > >
