You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/10/17 06:00:31 UTC
Review Request 26859: Integrated CRAM-MD5 Authenticator module.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
see summary.
Diffs
-----
src/local/local.cpp 66de798
src/master/flags.hpp 9d06856
src/master/master.hpp 14f1d0f
src/master/master.cpp 0a5c9a3
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
> On Oct. 22, 2014, 7:01 a.m., Adam B wrote:
> > src/master/master.cpp, line 3803
> > <https://reviews.apache.org/r/26859/diff/2/?file=727115#file727115line3803>
> >
> > Would there be any problem creating thousands of instances of the same module, one for each authentication? This doesn't actually load the library each time, right?
Correct, it does not. A module create is simply invoking the factory method for the specific module class -- e.g. src/examples/test_authenticator_module.cpp:
static Authenticator* createCRAMMD5Authenticator(const Parameters& parameters)
{
return new mesos::internal::sasl::CRAMMD5Authenticator();
}
The actual dlopen is triggered within src/master/main.cpp, via ModuleManager::load.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review57763
-----------------------------------------------------------
On Oct. 21, 2014, 1:01 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 1:01 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 9d06856
> src/master/master.hpp 14f1d0f
> src/master/master.cpp 0a5c9a3
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review57763
-----------------------------------------------------------
Great job!
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment98637>
Should this really be a CHECK? What happens if an authenticatee (e.g. slave) tries to authenticate with a master not configured for authentication?
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment98638>
Would there be any problem creating thousands of instances of the same module, one for each authentication? This doesn't actually load the library each time, right?
- Adam B
On Oct. 21, 2014, 6:01 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 6:01 a.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 9d06856
> src/master/master.hpp 14f1d0f
> src/master/master.cpp 0a5c9a3
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
> On Oct. 29, 2014, 12:18 a.m., Cody Maloney wrote:
> > src/master/master.cpp, line 3912
> > <https://reviews.apache.org/r/26859/diff/6/?file=735286#file735286line3912>
> >
> > All the right angle bracket fixes are nice, but the ones in chunks of code which aren't otherwise touched make the diff harder to read.
Yes, I feared this would come up. I think that we won't touch all areas of this implementation for a loooong time, hence I simply took the freedom to get it done now.
> On Oct. 29, 2014, 12:18 a.m., Cody Maloney wrote:
> > src/module/authenticator.hpp, line 1
> > <https://reviews.apache.org/r/26859/diff/6/?file=735287#file735287line1>
> >
> > This needs to be added to a Makefile.am so that distcheck will pass.
Ow, great point - thanks a bunch! As this file should have been added in 26857, I will fix it there.
> On Oct. 29, 2014, 12:18 a.m., Cody Maloney wrote:
> > src/master/master.cpp, line 3873
> > <https://reviews.apache.org/r/26859/diff/6/?file=735286#file735286line3873>
> >
> > Since all this code only can load one authenticator module it would be good to check the authetnicatorModules argument only produces a list of length 1 max and warn / exit early. Currently extra arguments are silently ignored.
Yes indeed.
> On Oct. 29, 2014, 12:18 a.m., Cody Maloney wrote:
> > src/master/master.cpp, line 3876
> > <https://reviews.apache.org/r/26859/diff/6/?file=735286#file735286line3876>
> >
> > This assuems crammd5 can only appear first in the list of authenticator modules.
MESOS-1939 will fix this, as noted.
> On Oct. 29, 2014, 12:18 a.m., Cody Maloney wrote:
> > src/master/master.cpp, line 3875
> > <https://reviews.apache.org/r/26859/diff/6/?file=735286#file735286line3875>
> >
> > The strings::split() API makes this a lot more complicated than it should be. split of an empty string returning an empty vector I think would make more sense... But alas...
> >
> > I think it would make sense though to default the argument to a list of one element ['crammd5'] by default. Then you should just error if not --authenticator= is passed without listing any authenticators, rather than try to fall back. Ideally the error would list available authentication modules.
Using a default makes much more sense, even though it complicates the flags description a little. Changed that accordingly - thank you!.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review58921
-----------------------------------------------------------
On Oct. 29, 2014, 5:19 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 5:19 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/examples/test_authenticator_module.cpp PRE-CREATION
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review58921
-----------------------------------------------------------
src/master/master.hpp
<https://reviews.apache.org/r/26859/#comment100101>
There shouldn't be a newline added here
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100105>
Since all this code only can load one authenticator module it would be good to check the authetnicatorModules argument only produces a list of length 1 max and warn / exit early. Currently extra arguments are silently ignored.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100103>
The strings::split() API makes this a lot more complicated than it should be. split of an empty string returning an empty vector I think would make more sense... But alas...
I think it would make sense though to default the argument to a list of one element ['crammd5'] by default. Then you should just error if not --authenticator= is passed without listing any authenticators, rather than try to fall back. Ideally the error would list available authentication modules.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100104>
This assuems crammd5 can only appear first in the list of authenticator modules.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100113>
If authenticator should really be Owned here it would be a lot better to make it owned much earlier so that it is never unowned / a resource which could potentially be lost if / when the unexpected happens.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100114>
All the right angle bracket fixes are nice, but the ones in chunks of code which aren't otherwise touched make the diff harder to read.
src/module/authenticator.hpp
<https://reviews.apache.org/r/26859/#comment100115>
This needs to be added to a Makefile.am so that distcheck will pass.
- Cody Maloney
On Oct. 28, 2014, 12:44 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 12:44 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/module/authenticator.hpp PRE-CREATION
> src/tests/crammd5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
> On Oct. 31, 2014, 12:43 a.m., Vinod Kone wrote:
> > src/master/master.cpp, line 124
> > <https://reviews.apache.org/r/26859/diff/7/?file=741143#file741143line124>
> >
> > i wish you did this cleanup as a dependent patch. oh well.
That indeed would have been better - sry for the inconvenience caused.
> On Oct. 31, 2014, 12:43 a.m., Vinod Kone wrote:
> > src/master/master.cpp, line 386
> > <https://reviews.apache.org/r/26859/diff/7/?file=741143#file741143line386>
> >
> > Do you also want to specify how to specify the modules, e.g., "via --modules flag"?
Seems you were looking at an older RR - the latest one used the following:
EXIT(1) << "Authenticator '" << authenticatorNames[0] << "' not found."
<< "Check the spelling (compare to "
<< BUILTIN_AUTHENTICATOR_NAME << ") or verify that the "
<< "authenticator was loaded from a module (see --modules)";
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59108
-----------------------------------------------------------
On Oct. 30, 2014, 11:58 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 11:58 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/constants.hpp a8298bc
> src/master/constants.cpp 3ebd246
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59108
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100425>
i wish you did this cleanup as a dependent patch. oh well.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100426>
Why not just EXIT if we don't support multiple modules?
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100427>
Do you also want to specify how to specify the modules, e.g., "via --modules flag"?
- Vinod Kone
On Oct. 30, 2014, 11:58 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 11:58 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/constants.hpp a8298bc
> src/master/constants.cpp 3ebd246
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59308
-----------------------------------------------------------
Ship it!
A few minor re-wordings, then we can commit it!
src/master/flags.hpp
<https://reviews.apache.org/r/26859/#comment100577>
Update this comment to match the actually accepted inputs. No longer can there be multiple authenticators or even a comma.
Suggested new text:
"Authenticator implementation to use when authenticating frameworks and/or slaves. Use the default '" + DEFAULT_AUTHENTICATOR + "', or load an alternate authenticator module using --modules."
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100579>
These TODOs are probably obvious enough you could remove them. If/when we support multiple authenticators, this is the first place we'll have to change the code, and the EXIT error makes that obvious. :)
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100580>
I'd add single quotes (') around the default authenticator name, so it reads "(compare to 'crammd5')"
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100581>
"... verify that the authenticator module was loaded successfully (see --modules)"
- Adam B
On Oct. 30, 2014, 6:37 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:37 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/constants.hpp a8298bc
> src/master/constants.cpp 3ebd246
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59353
-----------------------------------------------------------
Ship it!
Ship It!
- Adam B
On Oct. 31, 2014, 3:18 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 31, 2014, 3:18 a.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/constants.hpp a8298bc
> src/master/constants.cpp 3ebd246
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 31, 2014, 10:18 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Addressed comments.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/constants.hpp a8298bc
src/master/constants.cpp 3ebd246
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59270
-----------------------------------------------------------
src/master/flags.hpp
<https://reviews.apache.org/r/26859/#comment100563>
s/module //?
s/loaded and //?
src/master/constants.hpp
<https://reviews.apache.org/r/26859/#comment100536>
s/BUILTIN/DEFAULT/?
- Adam B
On Oct. 30, 2014, 6:37 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2014, 6:37 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/constants.hpp a8298bc
> src/master/constants.cpp 3ebd246
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 31, 2014, 1:37 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Addressed comments.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/constants.hpp a8298bc
src/master/constants.cpp 3ebd246
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 30, 2014, 11:58 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
tiny text correction.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/constants.hpp a8298bc
src/master/constants.cpp 3ebd246
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 30, 2014, 10:18 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Addressed comments.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/constants.hpp a8298bc
src/master/constants.cpp 3ebd246
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review59088
-----------------------------------------------------------
Looks really good, but I think we need one more round to clear out the references to 'modules' when using generic 'authenticators'.
src/examples/test_authenticator_module.cpp
<https://reviews.apache.org/r/26859/#comment100396>
Strange, I didn't see these merge annotations in your previous patch that introduced this file. Not sure why it shows up in this diff, but as long as the lhs never gets checked in, we're fine.
src/master/flags.hpp
<https://reviews.apache.org/r/26859/#comment100400>
Why does this even talk about modules? It should talk about authenticators, and the --modules flag can talk about modules. And if there's a module with a new authenticator, you can tell them in your module's howto doc to use --modules to load it and --authenticators to enable it.
Suggested new text:
"Comma separated list of authenticator(s) to use.
Only uses the first authenticator in the list for now."
src/master/master.hpp
<https://reviews.apache.org/r/26859/#comment100401>
These are no longer necessarily 'modules', so maybe rename the variable to 'authenticatorNames'
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100403>
s/module //
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100404>
Shouldn't you also check that !authenticatorNames.empty() in case somebody overrides the default to set --authenticators=""?
Error and exit
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100402>
It would be nice to see the name of the authenticator being used:
"Multiple authenticators are not supported. Only using: " + authenticatorNames[0]
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100406>
Let's get rid of the magic string and make "crammd5" a string constant.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100405>
Can we reword this to not discuss 'modules'?
Maybe just "Authenticator '" << authenticatorNames[0] << "' not found."
If you want to suggest how to fix it, you could suggest they check the spelling (compare to "crammd5") or verify that they have the authenticator loaded in a module from --modules"
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100407>
Is this an error to 'load' or an error to 'create'? s/load/create/?
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100409>
'auth' is to short/ambiguous.
Use authenticator and authenticator_.
You can pick which one is the Owned ptr vs. the normal pointer.
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100412>
Blank like between these; pretty much always want a blank line before comments.
src/tests/cram_md5_authentication_tests.cpp
<https://reviews.apache.org/r/26859/#comment100450>
Two blank lines between tests
- Adam B
On Oct. 29, 2014, 11:51 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 11:51 a.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/examples/test_authenticator_module.cpp PRE-CREATION
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 29, 2014, 6:51 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Fixed dependency list.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs
-----
src/examples/test_authenticator_module.cpp PRE-CREATION
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 29, 2014, 5:19 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Addressed comments.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/examples/test_authenticator_module.cpp PRE-CREATION
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing (updated)
-------
make check
NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
> On Oct. 28, 2014, 11:58 p.m., Kapil Arya wrote:
> > src/master/master.cpp, lines 3879-3888
> > <https://reviews.apache.org/r/26859/diff/6/?file=735286#file735286line3879>
> >
> > Here if the user specifies an invalid modulename but doesn't specify the module with --modules, shouldn't we give a simple error that the authenticator 'name' was not found?
> >
> > Sth along the lines of :
> >
> > } else if (modules::ModuleManager::contains<Authenticator>(authenticatorModules[0])) {
> > ... ModuleManager::create ...
> > } else {
> > ERROR(invalid module)
> > }
Yes, always better to have deterministic error messages where possible. Thank you! I did however move that check further up, into initialize as we can check at that point already.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review58924
-----------------------------------------------------------
On Oct. 29, 2014, 5:19 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2014, 5:19 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/examples/test_authenticator_module.cpp PRE-CREATION
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/tests/cram_md5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all four CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review58924
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment100107>
Here if the user specifies an invalid modulename but doesn't specify the module with --modules, shouldn't we give a simple error that the authenticator 'name' was not found?
Sth along the lines of :
} else if (modules::ModuleManager::contains<Authenticator>(authenticatorModules[0])) {
... ModuleManager::create ...
} else {
ERROR(invalid module)
}
- Kapil Arya
On Oct. 28, 2014, 8:44 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 28, 2014, 8:44 a.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
>
> Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
>
>
> Diffs
> -----
>
> src/master/flags.hpp c931fd9
> src/master/master.hpp b1a2cd0
> src/master/master.cpp 762d2ff
> src/module/authenticator.hpp PRE-CREATION
> src/tests/crammd5_authentication_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 28, 2014, 12:44 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
Fixed style issue.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 762d2ff
src/module/authenticator.hpp PRE-CREATION
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 25, 2014, 8:28 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
namespace glitch fixed.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/flags.hpp 9d06856
src/master/master.hpp b1a2cd0
src/master/master.cpp 95589b8
src/module/authenticator.hpp PRE-CREATION
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 25, 2014, 8:22 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Changes
-------
rebased and fixed all "> >" towards ">>" in master.cpp/.hpp
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description (updated)
-------
Enables selecting a module based authenticator via the new --authenticators flag for mesos master.
Additionally, all "> >" have been fixed towards ">>" in master.hpp and master.cpp.
Diffs (updated)
-----
src/master/flags.hpp 9d06856
src/master/master.hpp b1a2cd0
src/master/master.cpp 95589b8
src/module/authenticator.hpp PRE-CREATION
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 24, 2014, 2:28 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
see summary.
Diffs
-----
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 95589b8
src/module/authenticator.hpp PRE-CREATION
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 24, 2014, 2:26 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
see summary.
Diffs (updated)
-----
src/master/flags.hpp c931fd9
src/master/master.hpp b1a2cd0
src/master/master.cpp 95589b8
src/module/authenticator.hpp PRE-CREATION
src/tests/crammd5_authentication_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
> On Oct. 22, 2014, 6:24 p.m., Vinod Kone wrote:
> > src/master/flags.hpp, line 339
> > <https://reviews.apache.org/r/26859/diff/2/?file=727113#file727113line339>
> >
> > I thought the default is going to be cram-md5 to make it backwards compatible?
Indeed, the logic has entirely changed in the now updated review-request proposal, following MESOS-1966.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review57845
-----------------------------------------------------------
On Oct. 21, 2014, 1:01 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 1:01 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 9d06856
> src/master/master.hpp 14f1d0f
> src/master/master.cpp 0a5c9a3
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/#review57845
-----------------------------------------------------------
src/master/flags.hpp
<https://reviews.apache.org/r/26859/#comment98718>
I thought the default is going to be cram-md5 to make it backwards compatible?
src/master/master.cpp
<https://reviews.apache.org/r/26859/#comment98720>
+1
This should not be a CHECK.
Also, the module name should be extracted out in initialize() and not here.
- Vinod Kone
On Oct. 21, 2014, 1:01 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26859/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 1:01 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
>
>
> Bugs: MESOS-1889
> https://issues.apache.org/jira/browse/MESOS-1889
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> see summary.
>
>
> Diffs
> -----
>
> src/master/flags.hpp 9d06856
> src/master/master.hpp 14f1d0f
> src/master/master.cpp 0a5c9a3
>
> Diff: https://reviews.apache.org/r/26859/diff/
>
>
> Testing
> -------
>
> make check
>
> NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 1:01 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
see summary.
Diffs (updated)
-----
src/master/flags.hpp 9d06856
src/master/master.hpp 14f1d0f
src/master/master.cpp 0a5c9a3
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff
Re: Review Request 26859: Integrated CRAM-MD5 Authenticator module.
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26859/
-----------------------------------------------------------
(Updated Oct. 17, 2014, 4:08 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Vinod Kone.
Bugs: MESOS-1889
https://issues.apache.org/jira/browse/MESOS-1889
Repository: mesos-git
Description
-------
see summary.
Diffs
-----
src/local/local.cpp 66de798
src/master/flags.hpp 9d06856
src/master/master.hpp 14f1d0f
src/master/master.cpp 0a5c9a3
Diff: https://reviews.apache.org/r/26859/diff/
Testing
-------
make check
NOTE all three CRAM-MD5 authenticator module related RRs need to get applied before running make check.
Thanks,
Till Toenshoff