You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2017/05/10 12:48:32 UTC

Review Request 59141: Added Secret::Value to the URI fetcher interface.

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
-------

Added Secret::Value to the URI fetcher interface.


Diffs
-----

  include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
  src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
  src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
  src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
  src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
  src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
  src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
  src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
  src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 


Diff: https://reviews.apache.org/r/59141/diff/1/


Testing
-------

make check


Thanks,

Gilbert Song


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/#review175749
-----------------------------------------------------------


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 22, 2017, 6:03 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 22, 2017, 6:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new parameter 'data' to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/tests/uri_fetcher_tests.cpp 4892909bbb47f60c95d8d9f0f175a5db32d4a164 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 8aab169f0628ad540654a7b6c15efbc282f5e940 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/#review175834
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/uri/fetcher.hpp
Line 72 (original), 74 (patched)
<https://reviews.apache.org/r/59141/#comment249168>

    add `@param data` doc comment?



include/mesos/uri/fetcher.hpp
Line 90 (original), 95 (patched)
<https://reviews.apache.org/r/59141/#comment249169>

    add `@param data` doc comment?



include/mesos/uri/fetcher.hpp
Line 103 (original), 109 (patched)
<https://reviews.apache.org/r/59141/#comment249170>

    add `@param data` doc comment?


- Vinod Kone


On May 22, 2017, 6:03 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 22, 2017, 6:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new parameter 'data' to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/tests/uri_fetcher_tests.cpp 4892909bbb47f60c95d8d9f0f175a5db32d4a164 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 8aab169f0628ad540654a7b6c15efbc282f5e940 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/
-----------------------------------------------------------

(Updated May 24, 2017, 8:43 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.


Changes
-------

Addressed Vinod's comment.


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


Repository: mesos


Description
-------

Added new parameter 'data' to the URI fetcher interface.


Diffs (updated)
-----

  include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/tests/uri_fetcher_tests.cpp 4892909bbb47f60c95d8d9f0f175a5db32d4a164 
  src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
  src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
  src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
  src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
  src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
  src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
  src/uri/fetchers/docker.cpp 8aab169f0628ad540654a7b6c15efbc282f5e940 
  src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
  src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 


Diff: https://reviews.apache.org/r/59141/diff/4/

Changes: https://reviews.apache.org/r/59141/diff/3-4/


Testing
-------

make check


Thanks,

Gilbert Song


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/
-----------------------------------------------------------

(Updated May 22, 2017, 11:03 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Added new parameter 'data' to the URI fetcher interface.


Diffs (updated)
-----

  include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/tests/uri_fetcher_tests.cpp 4892909bbb47f60c95d8d9f0f175a5db32d4a164 
  src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
  src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
  src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
  src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
  src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
  src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
  src/uri/fetchers/docker.cpp 8aab169f0628ad540654a7b6c15efbc282f5e940 
  src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
  src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 


Diff: https://reviews.apache.org/r/59141/diff/3/

Changes: https://reviews.apache.org/r/59141/diff/2-3/


Testing
-------

make check


Thanks,

Gilbert Song


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On May 18, 2017, 7:54 a.m., Qian Zhang wrote:
> > include/mesos/uri/fetcher.hpp
> > Line 72 (original), 74 (patched)
> > <https://reviews.apache.org/r/59141/diff/2/?file=1716658#file1716658line74>
> >
> >     Suggest to add a description for the newly introduced parameter `data` to explain what it is for.
> 
> Gilbert Song wrote:
>     Thanks, Qian. That is the reason why I added a TODO below. This interface for the URI fetcher is not finalized. We will update it asap.

We prorably also need some unit tests once we have finalized the interface and the functionalities supported for each fetcher.


- Chun-Hung


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


On May 22, 2017, 6:03 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 22, 2017, 6:03 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new parameter 'data' to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/tests/uri_fetcher_tests.cpp 4892909bbb47f60c95d8d9f0f175a5db32d4a164 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 8aab169f0628ad540654a7b6c15efbc282f5e940 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Gilbert Song <so...@gmail.com>.

> On May 18, 2017, 12:54 a.m., Qian Zhang wrote:
> > include/mesos/uri/fetcher.hpp
> > Line 72 (original), 74 (patched)
> > <https://reviews.apache.org/r/59141/diff/2/?file=1716658#file1716658line74>
> >
> >     Suggest to add a description for the newly introduced parameter `data` to explain what it is for.

Thanks, Qian. That is the reason why I added a TODO below. This interface for the URI fetcher is not finalized. We will update it asap.


- Gilbert


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


On May 12, 2017, 4:19 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 12, 2017, 4:19 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new parameter 'data' to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp dbfc1b2f2918ccaf90fa31496a0792f585489397 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/#review175360
-----------------------------------------------------------




include/mesos/uri/fetcher.hpp
Line 72 (original), 74 (patched)
<https://reviews.apache.org/r/59141/#comment248809>

    Suggest to add a description for the newly introduced parameter `data` to explain what it is for.



include/mesos/uri/fetcher.hpp
Line 91 (original), 97 (patched)
<https://reviews.apache.org/r/59141/#comment248811>

    Ditto



include/mesos/uri/fetcher.hpp
Line 104 (original), 111 (patched)
<https://reviews.apache.org/r/59141/#comment248812>

    Ditto


- Qian Zhang


On May 12, 2017, 7:19 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 12, 2017, 7:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added new parameter 'data' to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp dbfc1b2f2918ccaf90fa31496a0792f585489397 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added new parameter 'data' to the URI fetcher interface.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/
-----------------------------------------------------------

(Updated May 12, 2017, 4:19 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.


Summary (updated)
-----------------

Added new parameter 'data' to the URI fetcher interface.


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


Repository: mesos


Description (updated)
-------

Added new parameter 'data' to the URI fetcher interface.


Diffs (updated)
-----

  include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
  src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
  src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
  src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
  src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
  src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
  src/uri/fetchers/docker.cpp dbfc1b2f2918ccaf90fa31496a0792f585489397 
  src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
  src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 


Diff: https://reviews.apache.org/r/59141/diff/2/

Changes: https://reviews.apache.org/r/59141/diff/1-2/


Testing
-------

make check


Thanks,

Gilbert Song


Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

Posted by Gilbert Song <so...@gmail.com>.

> On May 10, 2017, 12:36 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/uri/fetcher.hpp
> > Lines 78 (patched)
> > <https://reviews.apache.org/r/59141/diff/1/?file=1713946#file1713946line78>
> >
> >     I'm thinking about what is a proper interface extension for this fetcher plugin. Instead of having a `Secret::Value`, how about a more general `config` or some session info parameter for the fetcher, which may or may not come from a secret?

This new parameter in URI fetcher should be used for authentication, no matter which plugin it is. And it means this parameter represents sensitive information. By introducing an optinal field secretValue, we are expecting this sensitive information is always from our new secret resolver infterface (Secret::Value).

So I think the assumption that the URI fetcher is expecting a resolved secret sounds reasonable.

```
  virtual process::Future<Secret::Value> resolve(
      const Secret& secret) const = 0;
```

Thoughts?


- Gilbert


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


On May 10, 2017, 5:48 a.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 5:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On May 10, 2017, 7:36 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/uri/fetcher.hpp
> > Lines 78 (patched)
> > <https://reviews.apache.org/r/59141/diff/1/?file=1713946#file1713946line78>
> >
> >     I'm thinking about what is a proper interface extension for this fetcher plugin. Instead of having a `Secret::Value`, how about a more general `config` or some session info parameter for the fetcher, which may or may not come from a secret?
> 
> Gilbert Song wrote:
>     This new parameter in URI fetcher should be used for authentication, no matter which plugin it is. And it means this parameter represents sensitive information. By introducing an optinal field secretValue, we are expecting this sensitive information is always from our new secret resolver infterface (Secret::Value).
>     
>     So I think the assumption that the URI fetcher is expecting a resolved secret sounds reasonable.
>     
>     ```
>       virtual process::Future<Secret::Value> resolve(
>           const Secret& secret) const = 0;
>     ```
>     
>     Thoughts?

I was wondering if there would be a scenario that we might what to have a plugin that could use some extra dynamic input for more than just authentication in the future. This is more of a design choice about how flexible this plugin interface could/should be.


- Chun-Hung


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


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59141/#review174535
-----------------------------------------------------------




include/mesos/uri/fetcher.hpp
Lines 78 (patched)
<https://reviews.apache.org/r/59141/#comment247701>

    I'm thinking about what is a proper interface extension for this fetcher plugin. Instead of having a `Secret::Value`, how about a more general `config` or some session info parameter for the fetcher, which may or may not come from a secret?


- Chun-Hung Hsiao


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> -----------------------------------------------------------
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
>     https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -----
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>