Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On Jan. 28, 2016, 12:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 12:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Jan. 28, 2016, 5:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 28, 2016, 5:59 p.m.)


Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.


Changes
---

Added new comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 5:59 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 5:59 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 5:49 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 1118-1119
> > 
> >
> > I advocate the practice when each block wrapped in curly braces is 
> > prepended with a comment. Effectively, each such block is a test case 
> > inside a test, so since we strive to provide a comment for each test we 
> > should do the same for each block. I think you had great comments that you 
> > could reused here. The most important bit is to drag people's attention to 
> > the absence of credentials. Does it make sense?
> 
> Greg Mann wrote:
> Yep, it makes sense. Anand advocated removing the bit about not including 
> authentication headers because it was self-explanatory, but I do think that 
> it's better to call it out explicitly, since if you don't know the function 
> signature of `createBasicAuthHeaders`, it's not immediately obvious. Thanks, 
> will update.

Ah sorry, my mistake. Looking at Anand's comment above, he was suggesting 
something similar :-)


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 5:49 p.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, lines 1118-1119
> > 
> >
> > I advocate the practice when each block wrapped in curly braces is 
> > prepended with a comment. Effectively, each such block is a test case 
> > inside a test, so since we strive to provide a comment for each test we 
> > should do the same for each block. I think you had great comments that you 
> > could reused here. The most important bit is to drag people's attention to 
> > the absence of credentials. Does it make sense?

Yep, it makes sense. Anand advocated removing the bit about not including 
authentication headers because it was self-explanatory, but I do think that 
it's better to call it out explicitly, since if you don't know the function 
signature of `createBasicAuthHeaders`, it's not immediately obvious. Thanks, 
will update.


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

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




src/tests/persistent_volume_endpoints_tests.cpp (lines 1118 - 1119)


I advocate the practice when each block wrapped in curly braces is 
prepended with a comment. Effectively, each such block is a test case inside a 
test, so since we strive to provide a comment for each test we should do the 
same for each block. I think you had great comments that you could reused here. 
The most important bit is to drag people's attention to the absence of 
credentials. Does it make sense?


- Alexander Rukletsov


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 28, 2016, 11:05 a.m., Alexander Rukletsov wrote:
> >

Thanks Alex! :-)


> On Jan. 28, 2016, 11:05 a.m., Alexander Rukletsov wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1087
> > 
> >
> > Does it make sense to add `masterFlags.credentials = None();`?

I don't think it makes sense in this case. Setting the `authenticate_http` flag 
to false will disable authentication regardless of whether or not we explicitly 
set the credentials to `None()`.


- Greg


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


On Jan. 28, 2016, 4:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 28, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 28, 2016, 4:43 p.m.)


Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/tests/persistent_volume_endpoints_tests.cpp (line 1085)


I think this comment should say something about disabling authn rather then 
creating a master. Moreover, though it's not the way it's done in legacy tests, 
I would omit a self explanatory comment about creating a master here.



src/tests/persistent_volume_endpoints_tests.cpp (line 1087)


Does it make sense to add `masterFlags.credentials = None();`?



src/tests/persistent_volume_endpoints_tests.cpp (line 1094)


s/a slave/an agent



src/tests/persistent_volume_endpoints_tests.cpp (line 1099)


s/slaveId/agentId

and everywhere if you decide to abolish slavery in this test : ).



src/tests/persistent_volume_endpoints_tests.cpp (line 1108)


Let's extract "role1" into a constant in the beginning of the test.



src/tests/persistent_volume_endpoints_tests.cpp (line 1120)


To be 100% correct, you should `ASSERT_READY(slaveId);` before.


- Alexander Rukletsov


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

> On Jan. 27, 2016, 11:20 p.m., Anand Mazumdar wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1112
> > 
> >
> > Nit: Can we omit the part about not setting the authentication headers 
> > since it's self-explanatory. 
> > 
> > How about just:
> > 
> > ```
> > // Try a request to create a volume without authentication headers. 
> > Additionally, authorization is not enabled, so any principal, including 
> > `None()`, can create and destroy volumes.
> > ```
> > 
> > What do you think?

I think this comment should prepend the test in some form. It's an important 
bit that we can't do authz if authn is disabled (i.e. no "honor system"). Here 
we just need to drag reader's attention that there are no authn headers 
created. How about "Send a create volume request with absent credentials."?


- Alexander


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


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Anand Mazumdar]

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


Fix it, then Ship it!




LGTM.


src/tests/persistent_volume_endpoints_tests.cpp (line 1112)


Nit: Can we omit the part about not setting the authentication headers 
since it's self-explanatory. 

How about just:

```
// Try a request to create a volume without authentication headers. 
Additionally, authorization is not enabled, so any principal, including 
`None()`, can create and destroy volumes.
```

What do you think?



src/tests/persistent_volume_endpoints_tests.cpp (line 1116)


Can we wrap both of these in blocks i.e.

```
{
  Future response = blah blah;
  ...
}

{
  Future response = blah blah part 2;
  ...
}
```

Also,
s/createResponse/response
s/destroyResponse/response thereafter.


- Anand Mazumdar


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 27, 2016, 11:03 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


Changes
---

Edited comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

> On Jan. 27, 2016, 11:30 a.m., Alexander Rukletsov wrote:
> > I think the description is a bit outdated, you introduce one test instead 
> > of two : ).

Whoops! Thanks Alex :-)


- Greg


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


On Jan. 27, 2016, 6:24 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 6:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 27, 2016, 6:24 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


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


Repository: mesos


Description (updated)
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing (updated)
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Alexander Rukletsov]

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



I think the description is a bit outdated, you introduce one test instead of 
two : ).

- Alexander Rukletsov


On Jan. 22, 2016, 5:03 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 22, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled, 
> both with and without authorization enabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> without authorization and without a principal; this patch introduces test 
> cases for this scenario: 
> `PersistentVolumeEndpointsTest.NoAuthenticationNoAuthorization` and 
> `PersistentVolumeEndpointsTest.NoAuthenticationWithAuthorization`. NOTE: due 
> to the current behavior of the HTTP authentication code, when HTTP 
> authentication is disabled, the endpoint callbacks always receive a principal 
> of `None()`, which means that authorization cannot be properly used when HTTP 
> authentication is disabled.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthorization`, was added to the 
> persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 22, 2016, 5:03 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 22, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled, 
> both with and without authorization enabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> without authorization and without a principal; this patch introduces test 
> cases for this scenario: 
> `PersistentVolumeEndpointsTest.NoAuthenticationNoAuthorization` and 
> `PersistentVolumeEndpointsTest.NoAuthenticationWithAuthorization`. NOTE: due 
> to the current behavior of the HTTP authentication code, when HTTP 
> authentication is disabled, the endpoint callbacks always receive a principal 
> of `None()`, which means that authorization cannot be properly used when HTTP 
> authentication is disabled.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthorization`, was added to the 
> persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 22, 2016, 5:03 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


Changes
---

Removed extraneous test cases.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled, both 
with and without authorization enabled.

The persistent volume endpoint tests allow volume creation and destruction 
without authorization and without a principal; this patch introduces test cases 
for this scenario: 
`PersistentVolumeEndpointsTest.NoAuthenticationNoAuthorization` and 
`PersistentVolumeEndpointsTest.NoAuthenticationWithAuthorization`. NOTE: due to 
the current behavior of the HTTP authentication code, when HTTP authentication 
is disabled, the endpoint callbacks always receive a principal of `None()`, 
which means that authorization cannot be properly used when HTTP authentication 
is disabled.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthorization`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 21, 2016, 9:03 p.m.)


Review request for mesos, Michael Park and Neil Conway.


Changes
---

Added another test with authorization enabled.


Summary (updated)
-

Added persistent volume endpoint test without authentication.


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


Repository: mesos


Description (updated)
---

Added persistent volume endpoint tests with HTTP authentication disabled, both 
with and without authorization enabled.

The persistent volume endpoint tests allow volume creation and destruction 
without authorization and without a principal; this patch introduces test cases 
for this scenario: 
`PersistentVolumeEndpointsTest.NoAuthenticationNoAuthorization` and 
`PersistentVolumeEndpointsTest.NoAuthenticationWithAuthorization`. NOTE: due to 
the current behavior of the HTTP authentication code, when HTTP authentication 
is disabled, the endpoint callbacks always receive a principal of `None()`, 
which means that authorization cannot be properly used when HTTP authentication 
is disabled.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthorization`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann