You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Isabel Jimenez <co...@isabeljimenez.com> on 2015/10/07 21:06:01 UTC
Review Request 39098: Changed secret field in Credential from 'bytes'
to 'string'
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/
-----------------------------------------------------------
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
Diff: https://reviews.apache.org/r/39098/diff/
Testing
-------
make check
Thanks,
Isabel Jimenez
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/#review101844
-----------------------------------------------------------
Let's include a test that breaks with `bytes` but works with `string` in this patch!
- Michael Park
On Oct. 7, 2015, 7:06 p.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 7:06 p.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
>
> Diff: https://reviews.apache.org/r/39098/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/#review102420
-----------------------------------------------------------
Ship it!
I've committed this with the following minor style fixes!
src/tests/credentials_tests.cpp (line 55)
<https://reviews.apache.org/r/39098/#comment160058>
`s/> >/>>/`
src/tests/credentials_tests.cpp (line 61)
<https://reviews.apache.org/r/39098/#comment160059>
`s/> >/>>/`
src/tests/credentials_tests.cpp (line 85)
<https://reviews.apache.org/r/39098/#comment160060>
`s/const std::string&/std::string/`
src/tests/credentials_tests.cpp (line 86)
<https://reviews.apache.org/r/39098/#comment160061>
newline after this line
src/tests/credentials_tests.cpp (line 88)
<https://reviews.apache.org/r/39098/#comment160062>
3 space indent => 4 space indent
src/tests/credentials_tests.cpp (line 142)
<https://reviews.apache.org/r/39098/#comment160063>
3 space indent => 4 space indent
src/tests/credentials_tests.cpp (lines 146 - 147)
<https://reviews.apache.org/r/39098/#comment160064>
Fits in one line.
- Michael Park
On Oct. 13, 2015, 8:45 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, 8:45 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
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/
-----------------------------------------------------------
(Updated Oct. 13, 2015, 8:45 a.m.)
Review request for mesos and Michael Park.
Changes
-------
review comments
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 (updated)
-----
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
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/#review102409
-----------------------------------------------------------
src/tests/credentials_tests.cpp (line 116)
<https://reviews.apache.org/r/39098/#comment160044>
Can you pls add some comments here for this unit test?
- Guangya Liu
On 十月 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 十月 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
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
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.
Changes
-------
+ test
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 (updated)
-----
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
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/#review102043
-----------------------------------------------------------
Ship it!
Ship It!
- Till Toenshoff
On Oct. 8, 2015, 1 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2015, 1 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
>
> Diff: https://reviews.apache.org/r/39098/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/#review101867
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On 十月 8, 2015, 1 a.m., Isabel Jimenez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> -----------------------------------------------------------
>
> (Updated 十月 8, 2015, 1 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
>
> Diff: https://reviews.apache.org/r/39098/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Isabel Jimenez
>
>
Re: Review Request 39098: Changed secret field in Credential from
'bytes' to 'string'
Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39098/
-----------------------------------------------------------
(Updated Oct. 8, 2015, 1 a.m.)
Review request for mesos and Michael Park.
Changes
-------
Added java framework example changes
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 (updated)
-----
include/mesos/mesos.proto 4a16be1
src/examples/java/TestExceptionFramework.java 78720b0
src/examples/java/TestFramework.java aad94c0
Diff: https://reviews.apache.org/r/39098/diff/
Testing
-------
make check
Thanks,
Isabel Jimenez