You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2018/01/11 10:52:39 UTC
Review Request 65090: Added specific flag loader for zookeeper urls
preventing password leaks.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/
-----------------------------------------------------------
Review request for mesos and Greg Mann.
Bugs: MESOS-8413
https://issues.apache.org/jira/browse/MESOS-8413
Repository: mesos
Description
-------
Adds the struct `zookeeper::SafeUrlLoader` which is used to load
zookeeper url's from flags.
This new struct will not shown the contents of a file in logs or the
`/flags` endpoint if the urls are given using a file (with the
`file://` prefix). If the url's are given in the command line, not
changes are noticed.
Diffs
-----
include/mesos/zookeeper/url.hpp 72287d6d811de571c7a14d09ffc2f57e1b3f4446
src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
Diff: https://reviews.apache.org/r/65090/diff/1/
Testing
-------
```sh
make -j12 check
# We don't seem to test flags in unit tests anywhere,
# so additionally I ran:
docker pull zookeeper
cat <<EOF > /tmp/$USER/zk-stack.yml
version: '3.1'
services:
zoo1:
image: zookeeper
restart: always
hostname: zoo1
ports:
- 2181:2181
environment:
ZOO_MY_ID: 1
ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
zoo2:
image: zookeeper
restart: always
hostname: zoo2
ports:
- 2182:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
zoo3:
image: zookeeper
restart: always
hostname: zoo3
ports:
- 2183:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
EOF
docker-compose -f /tmp/$USER/zk-stack.yml up
cd ${MESOS_BUILD_DIR}
# This command should fail to launch because there is no file zk.conf
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf
cat <<EOF > /tmp/$USER/zk/zk.conf
zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
EOF
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %1
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--master=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
kill %2
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %2
kill %1
```
Thanks,
Alexander Rojas
Re: Review Request 65090: Added specific flag loader for zookeeper
urls preventing password leaks.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/#review195220
-----------------------------------------------------------
PASS: Mesos patch 65090 was successfully built and tested.
Reviews applied: `['65090']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090
- Mesos Reviewbot Windows
On Jan. 11, 2018, 10:52 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2018, 10:52 a.m.)
>
>
> Review request for mesos and Greg Mann.
>
>
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds the struct `zookeeper::SafeUrlLoader` which is used to load
> zookeeper url's from flags.
>
> This new struct will not shown the contents of a file in logs or the
> `/flags` endpoint if the urls are given using a file (with the
> `file://` prefix). If the url's are given in the command line, not
> changes are noticed.
>
>
> Diffs
> -----
>
> include/mesos/zookeeper/url.hpp 72287d6d811de571c7a14d09ffc2f57e1b3f4446
> src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
> src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
> src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
> src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
>
>
> Diff: https://reviews.apache.org/r/65090/diff/1/
>
>
> Testing
> -------
>
> ```sh
> make -j12 check
>
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
>
> docker pull zookeeper
>
> cat <<EOF > /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
> zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
> - 2181:2181
> environment:
> ZOO_MY_ID: 1
> ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
> zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
> - 2182:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
> zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
> - 2183:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
> EOF
>
> docker-compose -f /tmp/$USER/zk-stack.yml up
>
> cd ${MESOS_BUILD_DIR}
>
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
>
> cat <<EOF > /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %1
>
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
>
> kill %2
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %2
> kill %1
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 65090: Added specific flag loader for zookeeper
urls preventing password leaks.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/#review195629
-----------------------------------------------------------
include/mesos/zookeeper/url.hpp
Lines 32-36 (patched)
<https://reviews.apache.org/r/65090/#comment274897>
Code looks good to me, but I wonder if we should add this as a more generic abstraction? It’s possible that other people may want to use this in the future; placing it in a common location and giving it a more generic name may help people recognize that this could be useful to them.
The functionality which the new struct adds is the ability to have a flag which can be set to _either_ a path _or_ a value, without displaying the file’s contents in logs or API requests when a path is chosen.
So, perhaps a more generic name like `SecurePathOrValue` would improve readability? If we went this route, I think we could place the object and its related function overloads in stout.
WDYT?
src/slave/flags.hpp
Line 195 (original), 197 (patched)
<https://reviews.apache.org/r/65090/#comment274893>
Indentation.
- Greg Mann
On Jan. 11, 2018, 10:52 a.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2018, 10:52 a.m.)
>
>
> Review request for mesos and Greg Mann.
>
>
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adds the struct `zookeeper::SafeUrlLoader` which is used to load
> zookeeper url's from flags.
>
> This new struct will not shown the contents of a file in logs or the
> `/flags` endpoint if the urls are given using a file (with the
> `file://` prefix). If the url's are given in the command line, not
> changes are noticed.
>
>
> Diffs
> -----
>
> include/mesos/zookeeper/url.hpp 72287d6d811de571c7a14d09ffc2f57e1b3f4446
> src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
> src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
> src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
> src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
>
>
> Diff: https://reviews.apache.org/r/65090/diff/1/
>
>
> Testing
> -------
>
> ```sh
> make -j12 check
>
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
>
> docker pull zookeeper
>
> cat <<EOF > /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
> zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
> - 2181:2181
> environment:
> ZOO_MY_ID: 1
> ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
> zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
> - 2182:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
> zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
> - 2183:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
> EOF
>
> docker-compose -f /tmp/$USER/zk-stack.yml up
>
> cd ${MESOS_BUILD_DIR}
>
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
>
> cat <<EOF > /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %1
>
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
>
> kill %2
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %2
> kill %1
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 65090: Mesos flags related to ZooKeeper use
SecurePathOrValue.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/#review195762
-----------------------------------------------------------
Fix it, then Ship it!
src/master/flags.hpp
Line 105 (original), 105 (patched)
<https://reviews.apache.org/r/65090/#comment275064>
Nit: in the description, I would recommend:
s/This prevents this leakage/This patch prevents this leakage/
I think it's just a bit more clear.
- Greg Mann
On Jan. 18, 2018, 7:01 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2018, 7:01 p.m.)
>
>
> Review request for mesos and Greg Mann.
>
>
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Up until now the Mesos master flag `--zk` as well as the Mesos agent
> flag `--master` would leak ZooKeeper authentication credentials in
> both logs and results for the `/flags` endpoint, if the credentials
> were part of the configuration url.
>
> This prevents this leakage if a user decides to store the ZooKeeper
> url in a file and pass the file as a value to the flags mentioned
> above (using the preffix `file://`).
>
>
> Diffs
> -----
>
> src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
> src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
> src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
> src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
>
>
> Diff: https://reviews.apache.org/r/65090/diff/2/
>
>
> Testing
> -------
>
> ```sh
> make -j12 check
>
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
>
> docker pull zookeeper
>
> cat <<EOF > /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
> zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
> - 2181:2181
> environment:
> ZOO_MY_ID: 1
> ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
> zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
> - 2182:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
> zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
> - 2183:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
> EOF
>
> docker-compose -f /tmp/$USER/zk-stack.yml up
>
> cd ${MESOS_BUILD_DIR}
>
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
>
> cat <<EOF > /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %1
>
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
>
> kill %2
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %2
> kill %1
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 65090: Mesos flags related to ZooKeeper use
SecurePathOrValue.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/#review195813
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 65226.
Failed command: `python.exe .\support\apply-reviews.py -n -r 65226`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090
Relevant logs:
- [apply-review-65226-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090/logs/apply-review-65226-stdout.log):
```
error: patch failed: 3rdparty/stout/include/stout/flags/fetch.hpp:64
error: 3rdparty/stout/include/stout/flags/fetch.hpp: patch does not apply
error: patch failed: 3rdparty/stout/include/stout/flags/flag.hpp:100
error: 3rdparty/stout/include/stout/flags/flag.hpp: patch does not apply
error: patch failed: 3rdparty/stout/include/stout/flags/parse.hpp:25
error: 3rdparty/stout/include/stout/flags/parse.hpp: patch does not apply
```
- Mesos Reviewbot Windows
On Jan. 19, 2018, 2:05 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2018, 2:05 p.m.)
>
>
> Review request for mesos and Greg Mann.
>
>
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Up until now the Mesos master flag `--zk` as well as the Mesos agent
> flag `--master` would leak ZooKeeper authentication credentials in
> both logs and results for the `/flags` endpoint, if the credentials
> were part of the configuration url.
>
> This patch prevents this leakage if a user decides to store the
> ZooKeeper url in a file and pass the file as a value to the flags
> mentioned above (using the preffix `file://`).
>
>
> Diffs
> -----
>
> src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
> src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
> src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
> src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
>
>
> Diff: https://reviews.apache.org/r/65090/diff/3/
>
>
> Testing
> -------
>
> ```sh
> make -j12 check
>
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
>
> docker pull zookeeper
>
> cat <<EOF > /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
> zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
> - 2181:2181
> environment:
> ZOO_MY_ID: 1
> ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
> zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
> - 2182:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
> zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
> - 2183:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
> EOF
>
> docker-compose -f /tmp/$USER/zk-stack.yml up
>
> cd ${MESOS_BUILD_DIR}
>
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
>
> cat <<EOF > /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %1
>
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
>
> kill %2
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %2
> kill %1
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 65090: Mesos flags related to ZooKeeper use
SecurePathOrValue.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/
-----------------------------------------------------------
(Updated Jan. 19, 2018, 2:05 p.m.)
Review request for mesos and Greg Mann.
Bugs: MESOS-8413
https://issues.apache.org/jira/browse/MESOS-8413
Repository: mesos
Description (updated)
-------
Up until now the Mesos master flag `--zk` as well as the Mesos agent
flag `--master` would leak ZooKeeper authentication credentials in
both logs and results for the `/flags` endpoint, if the credentials
were part of the configuration url.
This patch prevents this leakage if a user decides to store the
ZooKeeper url in a file and pass the file as a value to the flags
mentioned above (using the preffix `file://`).
Diffs (updated)
-----
src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
Diff: https://reviews.apache.org/r/65090/diff/3/
Changes: https://reviews.apache.org/r/65090/diff/2-3/
Testing
-------
```sh
make -j12 check
# We don't seem to test flags in unit tests anywhere,
# so additionally I ran:
docker pull zookeeper
cat <<EOF > /tmp/$USER/zk-stack.yml
version: '3.1'
services:
zoo1:
image: zookeeper
restart: always
hostname: zoo1
ports:
- 2181:2181
environment:
ZOO_MY_ID: 1
ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
zoo2:
image: zookeeper
restart: always
hostname: zoo2
ports:
- 2182:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
zoo3:
image: zookeeper
restart: always
hostname: zoo3
ports:
- 2183:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
EOF
docker-compose -f /tmp/$USER/zk-stack.yml up
cd ${MESOS_BUILD_DIR}
# This command should fail to launch because there is no file zk.conf
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf
cat <<EOF > /tmp/$USER/zk/zk.conf
zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
EOF
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %1
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--master=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
kill %2
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %2
kill %1
```
Thanks,
Alexander Rojas
Re: Review Request 65090: Mesos flags related to ZooKeeper use
SecurePathOrValue.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/#review195747
-----------------------------------------------------------
PASS: Mesos patch 65090 was successfully built and tested.
Reviews applied: `['65226', '65090']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65090
- Mesos Reviewbot Windows
On Jan. 18, 2018, 7:01 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65090/
> -----------------------------------------------------------
>
> (Updated Jan. 18, 2018, 7:01 p.m.)
>
>
> Review request for mesos and Greg Mann.
>
>
> Bugs: MESOS-8413
> https://issues.apache.org/jira/browse/MESOS-8413
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Up until now the Mesos master flag `--zk` as well as the Mesos agent
> flag `--master` would leak ZooKeeper authentication credentials in
> both logs and results for the `/flags` endpoint, if the credentials
> were part of the configuration url.
>
> This prevents this leakage if a user decides to store the ZooKeeper
> url in a file and pass the file as a value to the flags mentioned
> above (using the preffix `file://`).
>
>
> Diffs
> -----
>
> src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
> src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
> src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
> src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
>
>
> Diff: https://reviews.apache.org/r/65090/diff/2/
>
>
> Testing
> -------
>
> ```sh
> make -j12 check
>
> # We don't seem to test flags in unit tests anywhere,
> # so additionally I ran:
>
> docker pull zookeeper
>
> cat <<EOF > /tmp/$USER/zk-stack.yml
> version: '3.1'
> services:
> zoo1:
> image: zookeeper
> restart: always
> hostname: zoo1
> ports:
> - 2181:2181
> environment:
> ZOO_MY_ID: 1
> ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
> zoo2:
> image: zookeeper
> restart: always
> hostname: zoo2
> ports:
> - 2182:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
> zoo3:
> image: zookeeper
> restart: always
> hostname: zoo3
> ports:
> - 2183:2181
> environment:
> ZOO_MY_ID: 2
> ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
> EOF
>
> docker-compose -f /tmp/$USER/zk-stack.yml up
>
> cd ${MESOS_BUILD_DIR}
>
> # This command should fail to launch because there is no file zk.conf
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf
>
> cat <<EOF > /tmp/$USER/zk/zk.conf
> zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
> EOF
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %1
>
>
> ./bin/mesos-master.sh \
> --work_dir=/tmp/$USER/mesos/master \
> --log_dir=/tmp/$USER/mesos/master/log \
> --ip=$PUBLIC_IP \
> --quorum=1 \
> --zk=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --master=file:///tmp/$USER/zk/zk.conf &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
>
> kill %2
>
> ./bin/mesos-agent.sh \
> --work_dir=/tmp/$USER/mesos/agent \
> --log_dir=/tmp/$USER/mesos/agent/log \
> --zk=`cat /tmp/$USER/zk/zk.conf` &
>
> [[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
>
> kill %2
> kill %1
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 65090: Mesos flags related to ZooKeeper use
SecurePathOrValue.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65090/
-----------------------------------------------------------
(Updated Jan. 18, 2018, 8:01 p.m.)
Review request for mesos and Greg Mann.
Summary (updated)
-----------------
Mesos flags related to ZooKeeper use SecurePathOrValue.
Bugs: MESOS-8413
https://issues.apache.org/jira/browse/MESOS-8413
Repository: mesos
Description (updated)
-------
Up until now the Mesos master flag `--zk` as well as the Mesos agent
flag `--master` would leak ZooKeeper authentication credentials in
both logs and results for the `/flags` endpoint, if the credentials
were part of the configuration url.
This prevents this leakage if a user decides to store the ZooKeeper
url in a file and pass the file as a value to the flags mentioned
above (using the preffix `file://`).
Diffs (updated)
-----
src/master/flags.hpp dabb414560f506787b6e821a27af623c8da44b11
src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71
src/slave/flags.hpp 42c4861b5ecdc808d04bfe8f5c35572074fd2bdc
src/slave/main.cpp f38fec6028ade0e0a51fd2cce7470c5c36e66396
Diff: https://reviews.apache.org/r/65090/diff/2/
Changes: https://reviews.apache.org/r/65090/diff/1-2/
Testing
-------
```sh
make -j12 check
# We don't seem to test flags in unit tests anywhere,
# so additionally I ran:
docker pull zookeeper
cat <<EOF > /tmp/$USER/zk-stack.yml
version: '3.1'
services:
zoo1:
image: zookeeper
restart: always
hostname: zoo1
ports:
- 2181:2181
environment:
ZOO_MY_ID: 1
ZOO_SERVERS: server.1=0.0.0.0:2888:3888 server.2=zoo2:2888:3888 server.3=zoo3:2888:3888
zoo2:
image: zookeeper
restart: always
hostname: zoo2
ports:
- 2182:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=0.0.0.0:2888:3888 server.3=zoo3:2888:3888
zoo3:
image: zookeeper
restart: always
hostname: zoo3
ports:
- 2183:2181
environment:
ZOO_MY_ID: 2
ZOO_SERVERS: server.1=zoo1:2888:3888 server.2=zoo2:2888:3888 server.3=0.0.0.0:2888:3888
EOF
docker-compose -f /tmp/$USER/zk-stack.yml up
cd ${MESOS_BUILD_DIR}
# This command should fail to launch because there is no file zk.conf
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf
cat <<EOF > /tmp/$USER/zk/zk.conf
zk://$PUBLIC_IP:2181,$PUBLIC_IP:2182,$PUBLIC_IP:2183/mesos
EOF
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %1
./bin/mesos-master.sh \
--work_dir=/tmp/$USER/mesos/master \
--log_dir=/tmp/$USER/mesos/master/log \
--ip=$PUBLIC_IP \
--quorum=1 \
--zk=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5050/flags | jq -r '.flags.zk') == "/tmp/$USER/zk/zk.conf" ]]
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--master=file:///tmp/$USER/zk/zk.conf &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == "/tmp/$USER/zk/zk.conf" ]]
kill %2
./bin/mesos-agent.sh \
--work_dir=/tmp/$USER/mesos/agent \
--log_dir=/tmp/$USER/mesos/agent/log \
--zk=`cat /tmp/$USER/zk/zk.conf` &
[[ $(http -b $PUBLIC_IP:5051/flags | jq -r '.flags.master') == `cat /tmp/$USER/zk/zk.conf` ]]
kill %2
kill %1
```
Thanks,
Alexander Rojas