Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 12, 2016, 5:30 p.m., Alexander Rukletsov wrote: > > src/tests/master_authorization_tests.cpp, line 1030 > > > > > > Now this is not symmetrical to `SlaveAuthorizationTest`. Yes,

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132896 --- src/tests/master_authorization_tests.cpp (line 1030)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- (Updated May 12, 2016, 11:25 a.m.) Review request for mesos, Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132652 --- Fix it, then Ship it! src/master/http.cpp (lines 869 - 878)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- (Updated May 10, 2016, 6:07 p.m.) Review request for mesos, Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 10, 2016, 5:54 p.m., Alexander Rukletsov wrote: > > docs/configuration.md, line 465 > > > > > > /monitor/statistics? Ah, that isn't authorized yet! And is only available on agents! I'll remove it. - Jan

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 2846 > > > > > > There's a lot of code duplication between here and > > `Slave::Http::authorizeEndpoint()`. Can you share/reuse code

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132450 --- docs/configuration.md (line 465)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 2846 > > > > > > There's a lot of code duplication between here and > > `Slave::Http::authorizeEndpoint()`. Can you share/reuse code

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- (Updated May 10, 2016, 12:12 p.m.) Review request for mesos, Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- (Updated May 10, 2016, 11:57 a.m.) Review request for mesos, Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/master/http.cpp, line 868 > > > > > > Could you make this capture list explicit (`[this, request]`)? > > Jan Schlicht wrote: > The style guide

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 879 > > > > > > In r/46203/ you've cached flags and passed them into continuation. Here > > you call `master->flags` in the

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? > > Jan Schlicht wrote:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 9:12 a.m., Alexander Rukletsov wrote: > > src/master/http.cpp, line 2872 > > > > > > Why don't we return `MethodNotAllowed` here? Same applies to the agent, > > actually : ) > > Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:18 a.m., Alexander Rukletsov wrote: > > Also, do you want to update configuration.md? To add the "get_endpoints" action to the ACL example? Yep, makes sense to do that here, will add it. - Jan --- This is an

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 10:24 a.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? > > Jan Schlicht wrote:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 9, 2016, 11:12 a.m., Alexander Rukletsov wrote: > > src/tests/master_authorization_tests.cpp, line 1033 > > > > > > Doesn't it compile without explicit casting to string? It doesn't. See the discussion

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132232 --- Also, do you want to update configuration.md? - Alexander

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132228 --- Looks good! One more round and we can commit this. Do you want

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 869 > > > > > > Did you need to pass all ("=") the in-scope variables through to the > > lambda, or just `request`? Yes, I only need to

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132125 --- src/master/http.cpp (line 869)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review131500 --- Patch looks great! Reviews applied: [46784] Passed command:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review131491 --- Ship it! src/master/http.cpp (line 2882)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- (Updated May 3, 2016, 2:42 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/tests/master_authorization_tests.cpp, line 1024 > > > > > > This looks very similar to the `SlaveEndpointTest` suite ;) Should we > > just also make

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review131140 --- Patch looks great! Reviews applied: [46784] Passed command:

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/master/http.cpp, line 868 > > > > > > Could you make this capture list explicit (`[this, request]`)? > > Jan Schlicht wrote: > The style guide

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/master/http.cpp, line 868 > > > > > > Could you make this capture list explicit (`[this, request]`)? > > Jan Schlicht wrote: > The style guide

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote: > > src/master/http.cpp, line 868 > > > > > > Could you make this capture list explicit (`[this, request]`)? The style guide recommends to prefer default

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review131062 --- src/master/http.cpp (line 868)

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review130961 --- Patch looks great! Reviews applied: [46784] Passed command:

Review Request 46784: Added authorization of the '/flags' endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/ --- Review request for mesos, Alexander Rukletsov and Benjamin Bannier. Bugs: