You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2019/06/06 23:24:30 UTC
Review Request 70795: WIP: Updated SSL docs to include new libprocess
flag.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------
Review request for mesos, Alexander Rukletsov and Joseph Wu.
Repository: mesos
Description
-------
WIP: Updated SSL docs to include new libprocess flag.
Diffs
-----
docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
Diff: https://reviews.apache.org/r/70795/diff/1/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
> On June 20, 2019, 1:05 p.m., Jan-Philip Gehrcke wrote:
> > docs/ssl.md
> > Lines 145 (patched)
> > <https://reviews.apache.org/r/70795/diff/2/?file=2151355#file2151355line145>
> >
> > "via DNS name" instead of "via hostname"?
I know that "DNS name" is more common in the context of x509 certification, but here I think it may be confusing for readers: It's the first only time a "DNS name" is mentioned in this document, and also there's no requirement that the DNS name must be resolved via DNS or stored in any DNS server.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review215998
-----------------------------------------------------------
On June 20, 2019, 5:27 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:27 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Jan-Philip Gehrcke via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review215998
-----------------------------------------------------------
Fix it, then Ship it!
Great work, left a few comments. Thanks!
docs/ssl.md
Lines 132 (patched)
<https://reviews.apache.org/r/70795/#comment302930>
Let's maybe call this VALIDATION_SCHEME instead of VALIDATION_ALGORITHM
docs/ssl.md
Lines 142 (patched)
<https://reviews.apache.org/r/70795/#comment302934>
backticks around `openssl`?
docs/ssl.md
Lines 145 (patched)
<https://reviews.apache.org/r/70795/#comment302931>
"via DNS name" instead of "via hostname"?
docs/ssl.md
Lines 147 (patched)
<https://reviews.apache.org/r/70795/#comment302932>
Even if this consumes a little more space let's make clear what happens when `LIBPROCESS_SSL_VERIFY_IPADD` is false
(IIUC then the TLS handshake will fail when we use IP addresses for opening the connection)
docs/ssl.md
Lines 150 (patched)
<https://reviews.apache.org/r/70795/#comment302933>
"Don't"
or
"Do not"
docs/ssl.md
Lines 154 (patched)
<https://reviews.apache.org/r/70795/#comment302935>
backticks?
docs/ssl.md
Lines 177 (patched)
<https://reviews.apache.org/r/70795/#comment302936>
Great!
- Jan-Philip Gehrcke
On June 19, 2019, 2:46 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 19, 2019, 2:46 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> > I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> > - announcing that `openssl` will be standard soon on compatible boxes
> > - announcing it to be gone at some point
> >
> > Or am I too eager for unification here?
>
> Benno Evers wrote:
> It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.
>
> So I actually agree we should do this deprecation, but I'm not sure about the timeline.
>
> Benno Evers wrote:
> Created https://issues.apache.org/jira/browse/MESOS-9857 to track the change.
Great - next we would update all relevant documentation with a deprecation note and a reference of that ticket.
Right now I am contemplating doing this in a single run, right away, instead of multiple phases. Multiple phases would which would allow us to have that `libprocess` default without having to warn about it.
What do you think?
We would ...
- add a comment note here
- add an SSL flags description note
- possibly have the flags validation output a deprecation warning
- anything I forgot here?
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------
On June 21, 2019, 3:05 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 3:05 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_scheme` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> > I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> > - announcing that `openssl` will be standard soon on compatible boxes
> > - announcing it to be gone at some point
> >
> > Or am I too eager for unification here?
>
> Benno Evers wrote:
> It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.
>
> So I actually agree we should do this deprecation, but I'm not sure about the timeline.
Created https://issues.apache.org/jira/browse/MESOS-9857 to track the change.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------
On June 21, 2019, 3:05 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 21, 2019, 3:05 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_scheme` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
> On June 21, 2019, 1:34 a.m., Till Toenshoff wrote:
> > docs/ssl.md
> > Lines 194 (patched)
> > <https://reviews.apache.org/r/70795/diff/3/?file=2151430#file2151430line194>
> >
> > I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
> > - announcing that `openssl` will be standard soon on compatible boxes
> > - announcing it to be gone at some point
> >
> > Or am I too eager for unification here?
It's actually a pretty big change - the 'libprocess' behaviour was built, I assume, to "magically" work with normal certificates w/o IP addresses despite libprocess only knowing about IP addresses. In DC/OS we don't notice most of it, since there all our certificates *do* contain the correct IP address, but at least quite a few unit tests will break by switching the default.
So I actually agree we should do this deprecation, but I'm not sure about the timeline.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------
On June 20, 2019, 5:27 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:27 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Till Toenshoff via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/#review216020
-----------------------------------------------------------
Fix it, then Ship it!
Great!
Please don't forget to update the review description to match the changed naming.
docs/ssl.md
Lines 176 (patched)
<https://reviews.apache.org/r/70795/#comment302972>
s/can not/cannot/
docs/ssl.md
Lines 194 (patched)
<https://reviews.apache.org/r/70795/#comment302973>
I wonder if we should already start a deprecation of the `libprocess` scheme - that would be:
- announcing that `openssl` will be standard soon on compatible boxes
- announcing it to be gone at some point
Or am I too eager for unification here?
- Till Toenshoff
On June 20, 2019, 5:27 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> -----------------------------------------------------------
>
> (Updated June 20, 2019, 5:27 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
>
>
> Diffs
> -----
>
> docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
>
>
> Diff: https://reviews.apache.org/r/70795/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------
(Updated July 4, 2019, 6:58 p.m.)
Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
Changes
-------
Renamed `libprocess` -> `legacy`.
Repository: mesos
Description
-------
Added a description of the new `--hostname_validation_scheme` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
environment variable.
Diffs (updated)
-----
docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
Diff: https://reviews.apache.org/r/70795/diff/5/
Changes: https://reviews.apache.org/r/70795/diff/4-5/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------
(Updated June 21, 2019, 3:05 p.m.)
Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
Repository: mesos
Description (updated)
-------
Added a description of the new `--hostname_validation_scheme` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_SCHEME`
environment variable.
Diffs (updated)
-----
docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
Diff: https://reviews.apache.org/r/70795/diff/4/
Changes: https://reviews.apache.org/r/70795/diff/3-4/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------
(Updated June 20, 2019, 5:27 p.m.)
Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
Repository: mesos
Description
-------
Added a description of the new `--hostname_validation_algorithm` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
environment variable.
Diffs (updated)
-----
docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
Diff: https://reviews.apache.org/r/70795/diff/3/
Changes: https://reviews.apache.org/r/70795/diff/2-3/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 70795: Updated SSL docs to include new libprocess
flag.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70795/
-----------------------------------------------------------
(Updated June 19, 2019, 2:46 p.m.)
Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
Summary (updated)
-----------------
Updated SSL docs to include new libprocess flag.
Repository: mesos
Description (updated)
-------
Added a description of the new `--hostname_validation_algorithm` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
environment variable.
Diffs (updated)
-----
docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c
Diff: https://reviews.apache.org/r/70795/diff/2/
Changes: https://reviews.apache.org/r/70795/diff/1-2/
Testing
-------
Thanks,
Benno Evers