> On May 7, 2016, 12:24 p.m., Adam B wrote: > > src/master/http.cpp, line 2846 > > <https://reviews.apache.org/r/46784/diff/3/?file=1369724#file1369724line2846> > > > > There's a lot of code duplication between here and > > `Slave::Http::authorizeEndpoint()`. Can you share/reuse code between the > > two? > > Jan Schlicht wrote: > Indeed, that's quite some code duplication. But there are also some > slight differences in both functions. Had a discussion with @arojas about how > to handle that case. On one hand it's good to remove code duplication, on the > other hand the extracted function would look a bit more complicated to > support the different requirements of master and agent. We decided that in > this particular case it's okay to stick with the two similar functions. > > Alexander Rukletsov wrote: > If we split this function in two (per suggestion below), the copy-paste > wouldn't be so drastic: we already have a lot of functions that do mapping > principal->subject (it is unfortunate we have to repeat it again and again) > and a fucntion that extracts endpoint from an URL path. Maybe we can add a > helper in `Request` for this one.
I've changed `authorizeEndpoint` to no longer extract the endpoint. I've also prepared some code that will add a function `process::http::URL::extractEndpoint`, so that this won't need to be duplicated. But as this will need some more effort (like added test cases), I'd like to introduce that as a new RR. Is that enough to consider this issue fixed or dropped? - Jan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46784/#review132125 ----------------------------------------------------------- On May 10, 2016, 12:12 p.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > 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 Rukletsov and Benjamin Bannier. > > > Bugs: MESOS-5297 > https://issues.apache.org/jira/browse/MESOS-5297 > > > Repository: mesos > > > Description > ------- > > Access to the '/flags' endpoint is authorized using > the 'GET_ENDPOINT_WITH_PATH' action. > > > Diffs > ----- > > docs/configuration.md 34271c76d10ad930e6cc586c2b820ce8989a053a > src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 > src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 > src/tests/master_authorization_tests.cpp > 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 > > Diff: https://reviews.apache.org/r/46784/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
