----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71859/#review218916 -----------------------------------------------------------
Sorry, one more thing that I originally missed in this one and noticed when looking at the next reviews, but it's most relevant to mention in this initial one: src/master/authorization.cpp Lines 38 (patched) <https://reviews.apache.org/r/71859/#comment306830> Actually, come to think of it, this construction style seems quite odd? We're mixing construction and mutation, which means we have a intermediate state where the action object is not valid. I guess the struct-like approach doesn't work because we don't want to default initialize the action field: ``` ActionObject actionObject; actionObject.action = authorization::RUN_TASK; actionObject.object = Object(); *actionObject.object->mutable_task_info() = task; ``` So it seems we should go with something like: ``` Object object = ...; ... ActionObject actionObject( actionObject.action = authorization::RUN_TASK, std::move(object)); ``` Also, might want to make this constructor private for only the helpers to use? Or do we want callers using it? - Benjamin Mahler On Dec. 3, 2019, 3:02 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71859/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2019, 3:02 p.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Mahler. > > > Bugs: MESOS-10023 and MESOS-10056 > https://issues.apache.org/jira/browse/MESOS-10023 > https://issues.apache.org/jira/browse/MESOS-10056 > > > Repository: mesos > > > Description > ------- > > This is the first patch in the chain that extract Master code > generating Action-Object pairs into a dedicated ActionObject class, > thus seperating authz Object creation from feeding them into authorizer. > > This is a prerequisite to using ObjectApprover interface for > synchronous authorization of Scheduler API calls. > > > Diffs > ----- > > src/CMakeLists.txt ef9382dc77d37fed344b7267119f3251acd3088a > src/Makefile.am 111c156c8a7abc5ece04779e8ac8879a30c22dbf > src/common/authorization.hpp 565d5ca6620442803fa80be652ab7382102347f5 > src/common/authorization.cpp fa71b0e8e8b9541376a9fd199f4d7b9db56a3f0f > src/master/authorization.hpp PRE-CREATION > src/master/authorization.cpp PRE-CREATION > src/master/master.hpp 93630421d58e6fd26566e81a23cd910957795665 > src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869 > > > Diff: https://reviews.apache.org/r/71859/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
