mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Schlicht <...@mesosphere.io>
Subject Re: Review Request 46203: Added authorization of the '/flags' endpoint.
Date Mon, 25 Apr 2016 10:26:49 GMT


> On April 20, 2016, 10:35 a.m., Adam B wrote:
> > src/slave/http.cpp, lines 658-660
> > <https://reviews.apache.org/r/46203/diff/9/?file=1350392#file1350392line658>
> >
> >     Where did you come up with the magic number 3? What if we reorganize the operator
endpoints in the (1.0) future? How will we know what the new value should be here?
> >     What if the user setup a reverse proxy (like in dcos) and these requests are
actually coming from a different base url than expected?
> 
> Benjamin Bannier wrote:
>     @adam: The three here is needed so that this just strips the agent part of the path,
not everything up to the last `/`. An example endpoint would be `/slave(1)/monitor/statistics`.
> 
> Jan Schlicht wrote:
>     Seems like a hard problem to fully support both requirements. Maybe reverting back
to using `std::string` instead of `http::URL` as the function parameter for `endpoint` could
resolve this.
> 
> Benjamin Bannier wrote:
>     Please use some typed entity that the usual endpoint handlers are aware of. They
currently have a `Request`, but e.g., have no idea how they are being routed.

I'll go back to using the "magic number 3". At this point `URL::path` will look like this:
"/slave(n)/name/of/endpoint". By splitting into 3 components we get rid of the "/slave(n)/".
The path is not the full URL that has been requested, hence reverse proxies shouldn't be an
issue here. I'll add a comment, explaining this.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46203/#review129696
-----------------------------------------------------------


On April 25, 2016, 10:30 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46203/
> -----------------------------------------------------------
> 
> (Updated April 25, 2016, 10:30 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5142
>     https://issues.apache.org/jira/browse/MESOS-5142
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 86ba66ac62295ca148524bcb2e57fee560ac4ac5 
>   include/mesos/authorizer/acls.proto c50deeb5565dfd5b3e5e7210283d9a36a3bfd579 
>   include/mesos/authorizer/authorizer.proto 40d93ea257d1df8d22eee8a21667db90d579a8fe

>   src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca 
>   src/authorizer/local/authorizer.cpp 0a3805fe4ce8eb89e096e8cd4326035513ba892b 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 20a4bcd0bb9dad06ea81fc4ad9b2fa462c69d2c5 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46203/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message