Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 

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

(Updated Jan. 7, 2016, 11:21 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben Mahler, 
Jan Schlicht, and Vinod Kone.


Changes
---

Addressed Adam's comments.


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


Repository: mesos


Description
---

Adds new `authenticate_http` flag to the master.
Also updates some tests that were using the credentials
for de/activating HTTP authentication.


Diffs (updated)
-

  docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
  src/master/master.cpp 145c5932610bd019eb090947569b608df6815c3a 
  src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
  src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 

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


Testing
---

make check 

functional testing by playing with the framework shutdown endpoint (teardown).

---

./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
-> fails with "No credentials provided for the default 'basic' HTTP 
authenticator."



./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
--credentials=file://some_valid_file
./src/test-framework --master=127.0.0.1:5050

curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
-> fails with "HTTP/1.1 401 Unauthorized"

curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> fails with "HTTP/1.1 401 Unauthorized"

curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"



./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
./src/test-framework --master=127.0.0.1:5050

curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"

curl --user some:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"


Thanks,

Till Toenshoff

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 


> On Jan. 7, 2016, 10:04 p.m., Adam B wrote:
> > src/tests/master_quota_tests.cpp, line 1103
> > 
> >
> > Does it matter that credentials != None()? Should you maybe clear out 
> > credentials as well?

It should not matter but seems somewhat cleaner to me, agreed.


> On Jan. 7, 2016, 10:04 p.m., Adam B wrote:
> > src/tests/master_quota_tests.cpp, line 1359
> > 
> >
> > Ditto

Ditto.


> On Jan. 7, 2016, 10:04 p.m., Adam B wrote:
> > src/master/master.cpp, line 544
> > 
> >
> > This no longer needs to be an `else if`, right? It can just be an 
> > `else` now.

Doh, thanks!


> On Jan. 7, 2016, 10:04 p.m., Adam B wrote:
> > src/master/master.cpp, lines 530-534
> > 
> >
> > We don't usually put punctuation at the end of log/exit lines.

Dang, thanks!


- Till


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


On Jan. 7, 2016, 8:29 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 


> On Jan. 7, 2016, 2:38 p.m., Joerg Schad wrote:
> > src/master/flags.cpp, line 210
> > 
> >
> > Please add to configuration.md

Thanks!


- Till


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


On Jan. 7, 2016, 2:38 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 2:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Joerg Schad 

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



src/master/flags.cpp (line 210)


Please add to configuration.md


- Joerg Schad


On Jan. 7, 2016, 2:38 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 2:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Joerg Schad 


> On Jan. 7, 2016, 7:39 p.m., Joerg Schad wrote:
> > src/master/master.cpp, line 527
> > 
> >
> > Does it make sense to document this behavior (the master not starting 
> > if flag and no credentials) to either some documentation and/or flags?
> 
> Till Toenshoff wrote:
> Yeah, good point. We should add this to the existing authentication 
> documentation. Opened https://issues.apache.org/jira/browse/MESOS-4309 for 
> that. Dropping the issue here as it is now tracked by the JIRA.

Sorry Proposal below, forgot to save the suggestion ;-).


- Joerg


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


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 


> On Jan. 7, 2016, 7:46 p.m., Joerg Schad wrote:
> > docs/configuration.md, line 344
> > 
> >
> > How about: 
> > 
> > If true only authenticated requests for HTTP endpoint 
> > requiring authentification are allowed.
> > 
> > 'If false also unauthenticated requests for HTTP endpoint 
> > requiring authentification are also allowed.'
> 
> Till Toenshoff wrote:
> aye!
> 
> Till Toenshoff wrote:
> "If 'true' only authenticated requests for HTTP endpoints supporting\n"
>   "authentification are allowed.\n"
>   "If 'false' also unauthenticated requests for HTTP endpoints 
> supporting\n"
>   "authentification are allowed.",

Actually, the second part should be set back towards the original as that is 
identical and less complicated :D
 "If 'false' unauthenticated HTTP endpoint requests are also allowed.\n"


- Till


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


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 

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

(Updated Jan. 7, 2016, 8:29 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben Mahler, 
Jan Schlicht, and Vinod Kone.


Changes
---

Addressed Joergs comments.


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


Repository: mesos


Description
---

Adds new `authenticate_http` flag to the master.
Also updates some tests that were using the credentials
for de/activating HTTP authentication.


Diffs (updated)
-

  docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
  src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
  src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
  src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
  src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
  src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 

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


Testing
---

make check 

functional testing by playing with the framework shutdown endpoint (teardown).

---

./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
-> fails with "No credentials provided for the default 'basic' HTTP 
authenticator."



./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
--credentials=file://some_valid_file
./src/test-framework --master=127.0.0.1:5050

curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
-> fails with "HTTP/1.1 401 Unauthorized"

curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> fails with "HTTP/1.1 401 Unauthorized"

curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"



./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
./src/test-framework --master=127.0.0.1:5050

curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"

curl --user some:credentials  --data "frameworkId=valid_framework_id" 
127.0.0.1:5050/teardown -v
-> succeeds with "HTTP/1.1 200 OK"


Thanks,

Till Toenshoff

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Joerg Schad 

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

Ship it!


- Joerg Schad


On Jan. 7, 2016, 8:29 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 


> On Jan. 7, 2016, 7:46 p.m., Joerg Schad wrote:
> > docs/configuration.md, line 344
> > 
> >
> > How about: 
> > 
> > If true only authenticated requests for HTTP endpoint 
> > requiring authentification are allowed.
> > 
> > 'If false also unauthenticated requests for HTTP endpoint 
> > requiring authentification are also allowed.'

aye!


- Till


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


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Till Toenshoff 


> On Jan. 7, 2016, 7:39 p.m., Joerg Schad wrote:
> > src/master/master.cpp, line 527
> > 
> >
> > Does it make sense to document this behavior (the master not starting 
> > if flag and no credentials) to either some documentation and/or flags?

Yeah, good point. We should add this to the existing authentication 
documentation. Opened https://issues.apache.org/jira/browse/MESOS-4309 for 
that. Dropping the issue here as it is now tracked by the JIRA.


> On Jan. 7, 2016, 7:39 p.m., Joerg Schad wrote:
> > src/master/flags.cpp, line 212
> > 
> >
> > See proposal above...

Indeed seems more of a factual correct description.


- Till


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


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Joerg Schad 

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



src/master/flags.cpp (line 212)


See proposal above...



src/master/master.cpp (line 527)


Does it make sense to document this behavior (the master not starting if 
flag and no credentials) to either some documentation and/or flags?


- Joerg Schad


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Joerg Schad 

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



docs/configuration.md (line 344)


How about: 

If true only authenticated requests for HTTP endpoint 
requiring authentification are allowed.

'If false also unauthenticated requests for HTTP endpoint 
requiring authentification are also allowed.'


- Joerg Schad


On Jan. 7, 2016, 7:12 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 7:12 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 42025: Added flag for enabling HTTP authentication.

2016-01-07 Thread Adam B 

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

Ship it!


Minor suggestions, but otherwise looks great Thanks!


src/master/master.cpp (lines 530 - 534)


We don't usually put punctuation at the end of log/exit lines.



src/master/master.cpp (line 544)


This no longer needs to be an `else if`, right? It can just be an `else` 
now.



src/tests/master_quota_tests.cpp (line 1102)


Does it matter that credentials != None()? Should you maybe clear out 
credentials as well?



src/tests/master_quota_tests.cpp (line 1357)


Ditto


- Adam B


On Jan. 7, 2016, 12:29 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42025/
> ---
> 
> (Updated Jan. 7, 2016, 12:29 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Bernd Mathiske, Ben 
> Mahler, Jan Schlicht, and Vinod Kone.
> 
> 
> Bugs: MESOS-3024
> https://issues.apache.org/jira/browse/MESOS-3024
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds new `authenticate_http` flag to the master.
> Also updates some tests that were using the credentials
> for de/activating HTTP authentication.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md fb6f6784e5d11850ba0bafaeafa3213a1038e6b4 
>   src/master/flags.hpp 9af6c68eef6bcf39d5776809fab6c66dc95da6b2 
>   src/master/flags.cpp f864419a6276356c97a0a4fe29e5cfce6c4660c4 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
>   src/tests/master_quota_tests.cpp bc8a117982b994279e0df7639b21abaeb308b76d 
>   src/tests/mesos.cpp 3867ed2fcb197f98f0a19ca37ed81392db27e8a0 
> 
> Diff: https://reviews.apache.org/r/42025/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> functional testing by playing with the framework shutdown endpoint (teardown).
> 
> ---
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http
> -> fails with "No credentials provided for the default 'basic' HTTP 
> authenticator."
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --authenticate_http 
> --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user invalid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> fails with "HTTP/1.1 401 Unauthorized"
> 
> curl --user valid:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> 
> ./bin/mesos-master.sh --work_dir=/tmp --credentials=file://some_valid_file
> ./src/test-framework --master=127.0.0.1:5050
> 
> curl --data "frameworkId=valid_framework_id" 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> curl --user some:credentials  --data "frameworkId=valid_framework_id" 
> 127.0.0.1:5050/teardown -v
> -> succeeds with "HTTP/1.1 200 OK"
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>