mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Adam B" <a...@mesosphere.io>
Subject Re: Review Request 27760: Revised authenticator interface to allow for two fold implementations.
Date Thu, 29 Jan 2015 02:11:11 GMT


> On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote:
> > Can you expand description on why you designed the authenticator inteface this way?
Particularly, expand on "The initial design and implementation of the authenticator module
interface caused issues and was not optimal for heavy lifting setup of external dependencies"
and "The new design also gets us back on track to the goal of makeing SASL a soft dependency
of mesos".
> > 
> > An issue with the new design is that now users have to implement an Authenticator
and an AuthenticatorSession to implement a new type of authenticator. Also, we lost the symmetry
between authenticatee and authenticator. An Authenticatee now communicates with AuthenticatorSession
instead of Authenticator, which is confusing. Is the plan to refactor the Authenticatee similarly?

We need to load/initialize the auxprop plugin at most once per linux process, so we need the
authenticator to live for the lifetime of the master, but we need the sasl connection to be
created/disposed with the lifetime of a per-authenticatee auth session. We could probably
get away with hiding the Session variable underneath the Authenticator interface, and just
call Authenticator::authenticate(pid) which will spin up a new session/connection and Authenticator::cancel/complete(pid)
which erases/disposes the session/connection. The sasl-based authenticators will then manage
the pid->session map internally, and other non-sasl authenticators don't have to know about
that implementation detail. (I think I mentioned this before, but Till will have to remind
me why we decided against it.)


> On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 4173-4190
> > <https://reviews.apache.org/r/27760/diff/12/?file=834181#file834181line4173>
> >
> >     Hmm. I think this is a serious enough error that the master should fail during
initilization rather than sending an error message here. Do you have use cases in mind that
warranted this?

The default authenticator is CRAM-MD5 rather than none, and that authenticator fails to initialize
if no credentials are provided (or if auxprops cannot be loaded). Since the default parameters
specify CRAM-MD5 authenticator, no required authentication, and no credentials, we must support
this starting successfully, although we do log a warning ("Only non-authenticating frameworks
and slaves are allowed to connect. Authentication is disabled: error_message"). In this case,
we must allow non-authenticating frameworks/slaves to register without authentication, but
we will return an AuthenticationError if they actually try to authenticate. This is the same
behavior as before.
For scenarios where the authenticator fails to load, but authentication is explicitly enabled
for frameworks/slaves, we do exit the master during initialization "Cannot start master with
authentication enabled: error_message")


> On Jan. 28, 2015, 12:33 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 4193-4203
> > <https://reviews.apache.org/r/27760/diff/12/?file=834181#file834181line4193>
> >
> >     IIRC, sending an AuthenticationErrorMessage here causes the driver to simply
retry the authentication and thus fall into a retry loop?

Truth. There is a TODO about at least adding a backoff, but maybe we should also attempt registering
without authentication?
Maybe authentication shouldn't even cause retries under certain scenarios where it will never
eventually succeed (bad credentials, no authenticator).


- Adam


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


On Jan. 26, 2015, 12:49 a.m., Till Toenshoff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27760/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 12:49 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2050
>     https://issues.apache.org/jira/browse/MESOS-2050
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The initial design and implementation of the authenticator module interface caused issues
and was not optimal for heavy lifting setup of external dependencies. By introducing a two
fold design, this has been decoupled from the authentication message processing. The new design
also gets us back on track to the goal of makeing SASL a soft dependency of mesos.
> 
> 
> Diffs
> -----
> 
>   src/authentication/authenticator.hpp 460494a 
>   src/authentication/cram_md5/authenticator.hpp d739a02 
>   src/authentication/cram_md5/auxprop.hpp b894386 
>   src/authentication/cram_md5/auxprop.cpp cf503a2 
>   src/master/master.hpp 1d342e5 
>   src/master/master.cpp bda8fda 
>   src/tests/cram_md5_authentication_tests.cpp a356aa1 
> 
> Diff: https://reviews.apache.org/r/27760/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>


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