Re: Review Request 36049: Added support for modularized Authorizer

2015-08-13 Thread Bernd Mathiske

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

Ship it!


Ship It!

- Bernd Mathiske


On Aug. 12, 2015, 6:52 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 12, 2015, 6:52 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 942003149b071a322933e2c085d9122903f69713 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-13 Thread Till Toenshoff

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

Ship it!



src/local/local.cpp (line 219)
https://reviews.apache.org/r/36049/#comment150242

s/authenticator/authorizer/  - will fix while committing.



src/local/local.cpp (line 222)
https://reviews.apache.org/r/36049/#comment150243

See above.



src/master/main.cpp (line 349)
https://reviews.apache.org/r/36049/#comment150244

See above.



src/master/main.cpp (line 352)
https://reviews.apache.org/r/36049/#comment150245

See above.


- Till Toenshoff


On Aug. 12, 2015, 1:52 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 12, 2015, 1:52 p.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 942003149b071a322933e2c085d9122903f69713 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-12 Thread Alexander Rojas

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

(Updated Aug. 12, 2015, 3:52 p.m.)


Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
Toenshoff.


Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs
-

  docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff 
  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am 942003149b071a322933e2c085d9122903f69713 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
  src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-08-10 Thread Alexander Rojas

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

(Updated Aug. 10, 2015, 5:29 p.m.)


Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
Toenshoff.


Changes
---

mcpark review.


Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs (updated)
-

  docs/configuration.md 506bab15a452f8b1b22328d0c5e9c46af6dfebff 
  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am 942003149b071a322933e2c085d9122903f69713 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
  src/tests/cluster.hpp 422ebd17da045002294e67027e9ebe1377bc53cd 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-08-09 Thread Michael Park

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



src/local/local.cpp (lines 217 - 219)
https://reviews.apache.org/r/36049/#comment149269

Does it make sense to log a warning for this?

```
if (flags.authorizers != master::DEFAULT_AUTHORIZER  flags.acls.isSome()) 
{
  LOG(WARNNING)  /* acls are being ignored! */;
}
```



src/local/local.cpp (lines 238 - 248)
https://reviews.apache.org/r/36049/#comment149268

This behavior is noticeably different, since we used to simply 
`EXIT(EXIT_FAILURE)` on this error. What's the rationale here?

My thoughts: if an authorizer fails to initialize given some ACLs, it's 
seems likely that the ACLs are ill-formed. In which case, the intent from the 
user is clear that they __want__ authorization, but they made a mistake in 
their ACL construction. Therefore, it seems more reasonable for us to exit with 
an error rather than ignoring their intent and skipping authorization 
altogether.



src/master/flags.cpp (line 417)
https://reviews.apache.org/r/36049/#comment149270

Can we expand a little upon `Authorizer implementation to use.`?

Maybe something like: `Authorizer implementation to use when authorizating 
actions that required it.`?



src/master/flags.cpp (line 422)
https://reviews.apache.org/r/36049/#comment149271

`s/than default/than the default`
`s/contents//`



src/master/flags.cpp (line 425)
https://reviews.apache.org/r/36049/#comment149272

I think we can follow what we do for `authenticators` in 
`src/master/master.cpp` to indicate the lack of support for multiple 
authorizers.

(1) split `flags.authorizers` by `,`
(2) if empty - no authorizers specified
(3) if size  1, multiple authorizers not supported
(4) proceed with the first name.



src/master/main.cpp (lines 346 - 379)
https://reviews.apache.org/r/36049/#comment149273

Same comments as `src/local/local.cpp`


- Michael Park


On Aug. 5, 2015, 9:15 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 5, 2015, 9:15 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-09 Thread Joerg Schad

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



src/master/flags.cpp (line 415)
https://reviews.apache.org/r/36049/#comment149277

Could we please add this flag to the documentation i.e. configuration.md


- Joerg Schad


On Aug. 5, 2015, 9:15 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 5, 2015, 9:15 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-05 Thread Alexander Rojas


 On Aug. 3, 2015, 4:34 p.m., Bernd Mathiske wrote:
  src/local/local.cpp, line 221
  https://reviews.apache.org/r/36049/diff/8/?file=1017149#file1017149line221
 
  That's a bit too subtle for me. Proposals, either:
  - pass the acls to the custom authorizer and let it decide what to do 
  with them
  - or disallow the presence of both flags, terminate the slave with an 
  error.
  
  The latter would also greatly simplify the control flow here.

The ideal would be to remove the whole `Authorizer::initialize()`, however we 
cannot disallow the presence of both flags since `--authorizers` is always set 
(with a default value). True, the code ain't pretty, but instead of giving it a 
make up, I would rather push for a long term solution (removing the initialize 
method at least with that signature), and then entirely remove this code.


- Alexander


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


On Aug. 4, 2015, 6:26 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 4, 2015, 6:26 p.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-05 Thread Jan Schlicht

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

Ship it!



src/local/local.cpp (line 226)
https://reviews.apache.org/r/36049/#comment148761

Wrong spacing.


- Jan Schlicht


On Aug. 4, 2015, 6:26 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 4, 2015, 6:26 p.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-04 Thread Jan Schlicht

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

Ship it!


Ship It!

- Jan Schlicht


On Aug. 3, 2015, 11:47 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 3, 2015, 11:47 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-04 Thread Alexander Rojas

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

(Updated Aug. 4, 2015, 6:26 p.m.)


Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
Toenshoff.


Changes
---

Rebase, changes from reviewers.


Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am 54eaf205eecb6bf1a9a5c4b5ddad55f46ad635ec 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp e05a472b86170eb26df26aaa4b65437fcdd413ce 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
  src/tests/cluster.hpp ba17c0c74a9dc36c595c4ad77fe68be94c5c7c0b 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-08-03 Thread Bernd Mathiske

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



src/local/local.cpp (line 221)
https://reviews.apache.org/r/36049/#comment148332

That's a bit too subtle for me. Proposals, either:
- pass the acls to the custom authorizer and let it decide what to do with 
them
- or disallow the presence of both flags, terminate the slave with an error.

The latter would also greatly simplify the control flow here.



src/master/flags.cpp (line 230)
https://reviews.apache.org/r/36049/#comment148333

See above. IMHO better to disallow having both flags simultaneously. If the 
user gives contradictory instructions, the slave should not start.



src/master/flags.cpp (line 421)
https://reviews.apache.org/r/36049/#comment148335

See above.



src/master/main.cpp (line 305)
https://reviews.apache.org/r/36049/#comment148336

See above.


- Bernd Mathiske


On Aug. 3, 2015, 2:47 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 3, 2015, 2:47 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-08-03 Thread Till Toenshoff

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

Ship it!


Ship It!

- Till Toenshoff


On Aug. 3, 2015, 9:47 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated Aug. 3, 2015, 9:47 a.m.)
 
 
 Review request for mesos, Adam B, Bernd Mathiske, Jan Schlicht, and Till 
 Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-20 Thread Alexander Rojas

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

(Updated July 20, 2015, 11:01 p.m.)


Review request for mesos, Adam B and Till Toenshoff.


Changes
---

Till's review.


Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am e2cbd1524f25b3e3a9419af6bdf8cc6e5022a784 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp fd4de4d0d9c3e9617408022d10b5e161bdc911e1 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-17 Thread Alexander Rojas


 On July 8, 2015, 12:12 a.m., Till Toenshoff wrote:
  src/local/local.cpp, line 241
  https://reviews.apache.org/r/36049/diff/7/?file=1000249#file1000249line241
 
  I am assuming that the `LocalAuthorizer` should be considered unusable 
  should its initialize function ever fail.
  
  My most favored solution here would be to log the failure and make sure 
  that `authorizer` remains unset so that we can operate without any 
  authorization. That would be following the approach of the authenticator 
  `initialize` failure handling.
  
  ```
   TryNothing initialize = 
  authorizer.get()-initialize(flags.acls.get());
   
   if (initialize.isError()) {
// A failure to initialize the authorizer does lead to unusable 
  authorization
// but allows actions to skip authorization.
LOG(WARNING)  Authorization is disabled: Failed to initialize '
  flags.authorizers  ' authorizer:   
  initialize.error();
delete authorizer.get();
authorizer = None();
  }
  ```
  
  Inherited from  
  https://github.com/apache/mesos/blob/master/src/master/master.cpp#L484

As a note, please don't use links to the master branch, use a specific review 
since any update on the master invalidates the line you want to show. e.g. 
https://github.com/apache/mesos/blob/bfe6c07b79550bb3d1f2ab6f5344d740e6eb6f60/src/master/master.cpp#L484


- Alexander


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


On July 7, 2015, 9:34 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 7, 2015, 9:34 a.m.)
 
 
 Review request for mesos, Adam B and Till Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-07 Thread Alexander Rojas

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

(Updated July 7, 2015, 9:34 a.m.)


Review request for mesos, Adam B and Till Toenshoff.


Changes
---

Update JIRA entry.


Bugs: MESOS-2947
https://issues.apache.org/jira/browse/MESOS-2947


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
  src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-07 Thread Till Toenshoff

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


There are some nits and slight inconsistencies but overall I think we are in 
good shape here.


src/local/local.cpp (line 217)
https://reviews.apache.org/r/36049/#comment143899

Capital The please.



src/local/local.cpp (line 220)
https://reviews.apache.org/r/36049/#comment143898

Please start with a capital Add after that colon.



src/local/local.cpp (lines 227 - 228)
https://reviews.apache.org/r/36049/#comment143900

I think we should rephrase the message here;

```
Could not create '  flags.authorizers  ' authorizer:   
create.error()
```



src/local/local.cpp (line 232)
https://reviews.apache.org/r/36049/#comment143903

For validating the configuration, I always found it very helpful that we 
were showing the activated authenticator name/s in the master log -- hence I 
would like to suggest to do the same here as well;

```
LOG(INFO)  Using '  flags.authorizers  ' authorizer;
```



src/local/local.cpp (line 234)
https://reviews.apache.org/r/36049/#comment143909

I am assuming that the `LocalAuthorizer` should be considered unusable 
should its initialize function ever fail.

My most favored solution here would be to log the failure and make sure 
that `authorizer` remains unset so that we can operate without any 
authorization. That would be following the approach of the authenticator 
`initialize` failure handling.

```
 TryNothing initialize = authorizer.get()-initialize(flags.acls.get());
 
 if (initialize.isError()) {
  // A failure to initialize the authorizer does lead to unusable 
authorization
  // but allows actions to skip authorization.
  LOG(WARNING)  Authorization is disabled: Failed to initialize '
flags.authorizers  ' authorizer:   
initialize.error();
  delete authorizer.get();
  authorizer = None();
}
```

Inherited from  
https://github.com/apache/mesos/blob/master/src/master/master.cpp#L484



src/master/flags.cpp (line 230)
https://reviews.apache.org/r/36049/#comment143910

s/authorizer/authorizers/



src/master/flags.cpp (line 231)
https://reviews.apache.org/r/36049/#comment143911

Lets make sure we match the flag name and also replace that default by 
the actual  implementation name.

```
  Note that if the flag --authorizers is provided with a value different\n
  than ' + DEFAULT_AUTHORIZER + ', the ACLs contents will be ignored.\n
  \n
```



src/master/flags.cpp (line 421)
https://reviews.apache.org/r/36049/#comment143912

s/authorizer/authorizers/

Please sure you check if you properly renamed that flag in all references. 
Thanks Alexander :)



src/master/flags.cpp (lines 423 - 424)
https://reviews.apache.org/r/36049/#comment143913

That looks like weird wrapping to me.



src/master/main.cpp (lines 301 - 317)
https://reviews.apache.org/r/36049/#comment143916

See my comments on local.cpp starting at line 217 ff. regarding this entire 
block.


- Till Toenshoff


On July 7, 2015, 7:34 a.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 7, 2015, 7:34 a.m.)
 
 
 Review request for mesos, Adam B and Till Toenshoff.
 
 
 Bugs: MESOS-2947
 https://issues.apache.org/jira/browse/MESOS-2947
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
   src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-06 Thread Alexander Rojas

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

(Updated July 6, 2015, 5:42 p.m.)


Review request for mesos, Adam B and Till Toenshoff.


Changes
---

Proper use of factory. 
Includes cleanup.


Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945


Repository: mesos


Description
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
  src/tests/cluster.hpp cfe7ef0c7a6dc62cddc3e5f5b5b28c8bcb2bed26 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Alexander Rojas

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

(Updated July 3, 2015, 3:17 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

No longer assumes that `Authorizer::create()` can return a null pointer.

Adds documentation on the usage of the `--authorizers` flag.

Adds a **TODO** for support of multiple authorizers.


Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945


Repository: mesos


Description (updated)
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.

It also adds a flag which allows to select the module name.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Alexander Rojas


 On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
  src/authorizer/authorizer.cpp, lines 17-28
  https://reviews.apache.org/r/36049/diff/5/?file=998603#file998603line17
 
  Great update! But it seems the comment is not applying, no?
  
  The built-in authorizer factory `TryAuthorizer* 
  LocalAuthorizer::create()` does not seem to test for `nullptr`. You seem to 
  do those checks in master.cpp and local.cpp instead. Or am I missing the 
  point here?

`LocalAuthorizer::create()` now returns an error if new fails, i.e. returns a 
null pointer. Still not sure if I should remove the check in main.


 On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
  src/local/local.cpp, lines 227-228
  https://reviews.apache.org/r/36049/diff/5/?file=998604#file998604line227
 
  You may reach this without an error-message, no?
  
  How about:
  ```
  if (create.isError() || create.get() == nullptr) {
EXIT(EXIT_FAILURE)  Could not create authorizer module '
flags.authorizer  '
(create.isError() ? :  + create.error() : );
;
  }
  
  ```
  
  Or actually, that factory can not return a `nullptr` anymore, can it?

Now the comment from the previos issue is true, so create never returns a 
nullptr but instead sets the `Try` to an `Error`.


 On July 3, 2015, 10:29 a.m., Till Toenshoff wrote:
  src/master/flags.cpp, lines 415-424
  https://reviews.apache.org/r/36049/diff/5/?file=998608#file998608line415
 
  We may want to follow the pattern used for the authenticator - as in, 
  call the flag authorizers and prepare the CLI interface for allowing the 
  activation of multiple, concurrently active authorizers.
  
  What do you think?

as bespoken.


- Alexander


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


On July 3, 2015, 3:17 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 3, 2015, 3:17 p.m.)
 
 
 Review request for mesos and Till Toenshoff.
 
 
 Bugs: MESOS-2945
 https://issues.apache.org/jira/browse/MESOS-2945
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds and integrates helper classes needed to support an `Authorizer` module. 
 Also adds a flag to the master, allowing the selection of an `Authorizer` 
 module.
 
 It also adds a flag which allows to select the module name.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Alexander Rojas

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

(Updated July 3, 2015, 3:40 p.m.)


Review request for mesos and Till Toenshoff.


Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945


Repository: mesos


Description (updated)
---

Adds and integrates helper classes needed to support an `Authorizer` module. 
Also adds a flag to the master, allowing the selection of an `Authorizer` 
module.


Diffs
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Till Toenshoff


 On July 3, 2015, 8:29 a.m., Till Toenshoff wrote:
 

Let's also update the description of this RR;
```
Adds and integrates helper classes needed to support an Authorizer
module. Also adds a flag to the master, allowing the selection of
an Authorizer module.
```

or maybe better `.. Authorizer modules.`


- Till


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


On July 2, 2015, 11:34 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 2, 2015, 11:34 p.m.)
 
 
 Review request for mesos and Till Toenshoff.
 
 
 Bugs: MESOS-2945
 https://issues.apache.org/jira/browse/MESOS-2945
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds all the helper classes needed to support an Authorizer module.
 
 It also adds a flag which allows to select the module name.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Till Toenshoff

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



src/authorizer/authorizer.cpp (lines 17 - 28)
https://reviews.apache.org/r/36049/#comment143338

Great update! But it seems the comment is not applying, no?

The built-in authorizer factory `TryAuthorizer* 
LocalAuthorizer::create()` does not seem to test for `nullptr`. You seem to do 
those checks in master.cpp and local.cpp instead. Or am I missing the point 
here?



src/local/local.cpp (lines 227 - 228)
https://reviews.apache.org/r/36049/#comment143336

You may reach this without an error-message, no?

How about:
```
if (create.isError() || create.get() == nullptr) {
  EXIT(EXIT_FAILURE)  Could not create authorizer module '
  flags.authorizer  '
  (create.isError() ? :  + create.error() : );
  ;
}

```

Or actually, that factory can not return a `nullptr` anymore, can it?



src/master/flags.cpp (lines 415 - 424)
https://reviews.apache.org/r/36049/#comment143339

We may want to follow the pattern used for the authenticator - as in, call 
the flag authorizers and prepare the CLI interface for allowing the 
activation of multiple, concurrently active authorizers.

What do you think?



src/master/main.cpp (line 311)
https://reviews.apache.org/r/36049/#comment143337

See local.cpp


- Till Toenshoff


On July 2, 2015, 11:34 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 2, 2015, 11:34 p.m.)
 
 
 Review request for mesos and Till Toenshoff.
 
 
 Bugs: MESOS-2945
 https://issues.apache.org/jira/browse/MESOS-2945
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds all the helper classes needed to support an Authorizer module.
 
 It also adds a flag which allows to select the module name.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-03 Thread Till Toenshoff

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



src/master/flags.cpp (line 422)
https://reviews.apache.org/r/36049/#comment143353

s/default/ + DEFAULT_AUTHORIZER + /


- Till Toenshoff


On July 2, 2015, 11:34 p.m., Alexander Rojas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36049/
 ---
 
 (Updated July 2, 2015, 11:34 p.m.)
 
 
 Review request for mesos and Till Toenshoff.
 
 
 Bugs: MESOS-2945
 https://issues.apache.org/jira/browse/MESOS-2945
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Adds all the helper classes needed to support an Authorizer module.
 
 It also adds a flag which allows to select the module name.
 
 
 Diffs
 -
 
   include/mesos/authorizer/authorizer.hpp PRE-CREATION 
   include/mesos/module/authorizer.hpp PRE-CREATION 
   src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
   src/authorizer/authorizer.cpp PRE-CREATION 
   src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
   src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
   src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
   src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
   src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
   src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
   src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 
 
 Diff: https://reviews.apache.org/r/36049/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Alexander Rojas
 




Re: Review Request 36049: Added support for modularized Authorizer

2015-07-02 Thread Alexander Rojas

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

(Updated July 2, 2015, 12:25 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Fixes include path.


Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945


Repository: mesos


Description
---

Adds all the helper classes needed to support an Authorizer module.

It also adds a flag which allows to select the module name.


Diffs (updated)
-

  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas



Re: Review Request 36049: Added support for modularized Authorizer

2015-07-02 Thread Alexander Rojas

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

(Updated July 3, 2015, 1:34 a.m.)


Review request for mesos and Till Toenshoff.


Bugs: MESOS-2945
https://issues.apache.org/jira/browse/MESOS-2945


Repository: mesos


Description
---

Adds all the helper classes needed to support an Authorizer module.

It also adds a flag which allows to select the module name.


Diffs
-

  include/mesos/authorizer/authorizer.hpp PRE-CREATION 
  include/mesos/module/authorizer.hpp PRE-CREATION 
  src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 
  src/authorizer/authorizer.cpp PRE-CREATION 
  src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff 
  src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce 
  src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 
  src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f 
  src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 
  src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 
  src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 

Diff: https://reviews.apache.org/r/36049/diff/


Testing
---

make check


Thanks,

Alexander Rojas