mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Park" <mcyp...@gmail.com>
Subject Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'
Date Tue, 13 Oct 2015 08:15:24 GMT

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

Ship it!


I added the comments to the test you added, but some of them apply to `AuthenticatedSlaveText`
as well. Let's fix those as well :)


src/tests/credentials_tests.cpp (line 119)
<https://reviews.apache.org/r/39098/#comment160047>

    Let's rename this `s/flags/masterFlags/` and move it to just above `flags.load(values,
true)` below.



src/tests/credentials_tests.cpp (line 121)
<https://reviews.apache.org/r/39098/#comment160046>

    `path::join` returns a temporary string right?
    In which case: `s/const string &/string/`.



src/tests/credentials_tests.cpp (line 145)
<https://reviews.apache.org/r/39098/#comment160050>

    Add newline after



src/tests/credentials_tests.cpp (line 147)
<https://reviews.apache.org/r/39098/#comment160048>

    Let's just use `s/flags.credentials.get().value/path/` here?



src/tests/credentials_tests.cpp (line 158)
<https://reviews.apache.org/r/39098/#comment160049>

    Remove newline.


- Michael Park


On Oct. 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
>     https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When decoding the JSON credential file into the Credential protobuf object, the 'secret'
which is in 'bytes' is mapped into base64 string automatically by protobuf from JSON. This
creates an unintended behavior, forcing users to encode in base64 their secret when wanting
to pass a JSON file to the --credentials flag.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message