> On June 23, 2016, 2:14 p.m., Adam B wrote: > > src/slave/http.cpp, line 150 > > <https://reviews.apache.org/r/49082/diff/3/?file=1428527#file1428527line150> > > > > Is it safe to pass `this` through here? Could you instead just pass > > immutable copies of `executor_->launchedTasks` and `framework_->info` and > > `taskApprover_`?
`this` is safe here since the code is executed synchronously. I don't know what you mean by immutable copies since they don't exist in lambdas, but if what you are suggesting is using `[&variable]`, that is not more safe than using `this` since it hast exactly the same drawbacks. The main problem is that the `framework->info` and `executor_->launchedTasks` are potentially very big objects very expensive to copy. - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49082/#review139234 ----------------------------------------------------------- On June 23, 2016, 12:01 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49082/ > ----------------------------------------------------------- > > (Updated June 23, 2016, 12:01 p.m.) > > > Review request for mesos, Adam B, Joerg Schad, and Till Toenshoff. > > > Repository: mesos > > > Description > ------- > > While information about frameworks, executors and tasks were well > protected in the master, this information was not protected in the > agents, which enabled unauthorized users to verify the data by getting > the agent `/state` endpoint. This was particularly pressing since the > Mesos UI would work as a proxy for agents endpoints so even being > behind a firewall would not had been enough. > > > Diffs > ----- > > src/common/http.hpp 588a1eb18e2665e71cc4361835d873652c4e100d > src/common/http.cpp 4b35736ef53baeb8eeefe5aadee3964905469e6d > src/master/http.cpp 70f084b84db90fde20c05d2354be190f28e72996 > src/slave/http.cpp 22b5930a7af336a749af4b6b50254f60433ecea5 > src/tests/mesos.hpp b9de9b6dfe0e62df91d5b82006b799345b49fd3c > src/tests/mesos.cpp 15aaaa7979f227083580afc1349373f3e3364c65 > src/tests/slave_authorization_tests.cpp > 98fde4914a5fddda4401d366e75322458bc9104c > > Diff: https://reviews.apache.org/r/49082/diff/ > > > Testing > ------- > > `make check`, some manual testing in the Mesos UI and the following script: > > ```sh > #! /usr/bin/env bash > > rm -rf /tmp/mesos/* > > cat <<EOF > /tmp/credentials.txt > foo bar > baz bar > EOF > > cat <<EOF > /tmp/acls.json > { > "permissive": false, > "access_mesos_logs" : [ > { > "principals" : { "values" : ["foo"] }, > "logs" : { "type" : "ANY" } > } > ], > "access_sandboxes" : [ > { > "principals" : { "values" : ["foo"] }, > "users" : { "values" : ["$USER"] } > } > ], > "view_tasks" : [ > { > "principals" : { "values" : ["foo"] }, > "users" : { "values" : ["$USER"] } > } > ], > "view_executors" : [ > { > "principals" : { "values" : ["foo"] }, > "users" : { "values" : ["$USER"] } > } > ], > "view_frameworks" : [ > { > "principals" : { "values" : ["foo"] }, > "users" : { "values" : ["$USER"] } > } > ], > "register_frameworks" : [ > { > "principals" : { "values" : ["foo"] }, > "roles" : { "values" : ["test"] } > } > ], > "run_tasks" : [ > { > "principals" : { "values" : ["foo"] }, > "users" : { "values" : ["$USER"] } > } > ], > "get_endpoints" : [ > { > "principals" : { "values" : ["foo"] }, > "paths" : { "type" : "ANY" } > } > ] > } > EOF > > ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \ > --authenticate_http \ > --log_dir=/tmp/mesos/logs/master \ > --http_credentials=file:///tmp/credentials.txt \ > --acls=file:///tmp/acls.json & > ./bin/mesos-slave.sh --work_dir=/tmp/mesos/slave \ > --master=127.0.0.1:5050 \ > --authenticate_http \ > --http_credentials=file:///tmp/credentials.txt \ > --acls=file:///tmp/acls.json & > ./src/mesos-execute \ > --command='while true; do echo "Hello world"; sleep 3; done' \ > --role=test \ > --master=127.0.0.1:5050 \ > --name=echoer \ > --principal=foo & > > # This should show complete information about frameworks, > # executors and tasks. > http GET http://127.0.0.1:5051/state -a foo:bar > > # This should show partial elements of the state but none > # related to the running framework. > http GET http://127.0.0.1:5051/state -a baz:bar > > # This should yield a 401 Unauthorized response. > http GET http://127.0.0.1:5051/state -a bar:bar > ``` > > > Thanks, > > Alexander Rojas > >
