Greg, let's use `EXIT(EXIT_FAILURE)` instead of `EXIT(1)` in the future. I'll be fixing all occurrences (including these) in our codebase for consistency soon.
Alex. On 24 March 2016 at 05:07, <[email protected]> wrote: > Added agent flags for HTTP authentication. > > Three command-line flags have been added to the agent to enable > HTTP authentication: `--authenticate_http`, `--http_credentials`, > and `--http_authenticators`. > > Review: https://reviews.apache.org/r/44515/ > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/40138729 > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/40138729 > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/40138729 > > Branch: refs/heads/master > Commit: 401387299a04b71f62021adc6ee5e7545ac63376 > Parents: a1482ac > Author: Greg Mann <[email protected]> > Authored: Wed Mar 23 02:46:26 2016 -0700 > Committer: Adam B <[email protected]> > Committed: Wed Mar 23 21:07:23 2016 -0700 > > ---------------------------------------------------------------------- > src/slave/constants.hpp | 6 +++ > src/slave/flags.cpp | 36 ++++++++++++++- > src/slave/flags.hpp | 3 ++ > src/slave/slave.cpp | 103 +++++++++++++++++++++++++++++++++++++++++++ > src/tests/mesos.cpp | 61 +++++++++++++++++++------ > 5 files changed, 193 insertions(+), 16 deletions(-) > ---------------------------------------------------------------------- > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/40138729/src/slave/constants.hpp > ---------------------------------------------------------------------- > diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp > index 4189c07..d096cfe 100644 > --- a/src/slave/constants.hpp > +++ b/src/slave/constants.hpp > @@ -111,6 +111,12 @@ constexpr Duration DOCKER_VERSION_WAIT_TIMEOUT = > Seconds(5); > // Name of the default, CRAM-MD5 authenticatee. > constexpr char DEFAULT_AUTHENTICATEE[] = "crammd5"; > > +// Name of the default HTTP authenticator. > +constexpr char DEFAULT_HTTP_AUTHENTICATOR[] = "basic"; > + > +// Name of the default agent HTTP authentication realm. > +constexpr char DEFAULT_HTTP_AUTHENTICATION_REALM[] = "mesos-agent"; > + > // Default maximum storage space to be used by the fetcher cache. > constexpr Bytes DEFAULT_FETCHER_CACHE_SIZE = Gigabytes(2); > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/40138729/src/slave/flags.cpp > ---------------------------------------------------------------------- > diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp > index ce02882..71685ce 100644 > --- a/src/slave/flags.cpp > +++ b/src/slave/flags.cpp > @@ -428,10 +428,10 @@ mesos::internal::slave::Flags::Flags() > add(&Flags::credential, > "credential", > "Either a path to a text with a single line\n" > - "containing `principal` and `secret` separated by " > - "whitespace.\n" > + "containing `principal` and `secret` separated by whitespace.\n" > "Or a path containing the JSON-formatted " > "information used for one credential.\n" > + "This credential is used to identify the slave to the master.\n" > "Path could be of the form `file:///path/to/file` or > `/path/to/file`." > "\n" > "Example:\n" > @@ -676,6 +676,38 @@ mesos::internal::slave::Flags::Flags() > "load an alternate authenticatee module using `--modules`.", > DEFAULT_AUTHENTICATEE); > > + add(&Flags::http_authenticators, > + "http_authenticators", > + "HTTP authenticator implementation to use when handling requests > to\n" > + "authenticated endpoints. Use the default\n" > + "`" + string(DEFAULT_HTTP_AUTHENTICATOR) + "`, or load an > alternate\n" > + "HTTP authenticator module using `--modules`.\n" > + "\n" > + "Currently there is no support for multiple HTTP authenticators.", > + DEFAULT_HTTP_AUTHENTICATOR); > + > + add(&Flags::authenticate_http, > + "authenticate_http", > + "If `true`, only authenticated requests for HTTP endpoints > supporting\n" > + "authentication are allowed. If `false`, unauthenticated requests > to\n" > + "HTTP endpoints are also allowed.", > + false); > + > + add(&Flags::http_credentials, > + "http_credentials", > + "Path to a JSON-formatted file containing credentials used to\n" > + "authenticate HTTP endpoints on the slave.\n" > + "Path can be of the form `file:///path/to/file` or > `/path/to/file`.\n" > + "Example:\n" > + "{\n" > + " \"credentials\": [\n" > + " {\n" > + " \"principal\": \"yoda\",\n" > + " \"secret\": \"usetheforce\"\n" > + " }\n" > + " ]\n" > + "}"); > + > add(&Flags::hooks, > "hooks", > "A comma-separated list of hook modules to be\n" > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/40138729/src/slave/flags.hpp > ---------------------------------------------------------------------- > diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp > index 3067c12..9ee7f34 100644 > --- a/src/slave/flags.hpp > +++ b/src/slave/flags.hpp > @@ -127,6 +127,9 @@ public: > bool enforce_container_disk_quota; > Option<Modules> modules; > std::string authenticatee; > + std::string http_authenticators; > + bool authenticate_http; > + Option<Path> http_credentials; > Option<std::string> hooks; > Option<std::string> resource_estimator; > Option<std::string> qos_controller; > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/40138729/src/slave/slave.cpp > ---------------------------------------------------------------------- > diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp > index 840534f..80a4ef5 100644 > --- a/src/slave/slave.cpp > +++ b/src/slave/slave.cpp > @@ -30,7 +30,10 @@ > > #include <mesos/type_utils.hpp> > > +#include <mesos/authentication/http/basic_authenticator_factory.hpp> > + > #include <mesos/module/authenticatee.hpp> > +#include <mesos/module/http_authenticator.hpp> > > #include <process/async.hpp> > #include <process/check.hpp> > @@ -115,6 +118,9 @@ namespace slave { > > using namespace state; > > +namespace authentication = process::http::authentication; > + > +using mesos::http::authentication::BasicAuthenticatorFactory; > > Slave::Slave(const std::string& id, > const slave::Flags& _flags, > @@ -311,6 +317,7 @@ void Slave::initialize() > > authenticateeName = flags.authenticatee; > > + // Load credential for agent authentication with the master. > if (flags.credential.isSome()) { > Result<Credential> _credential = > credentials::readCredential(flags.credential.get()); > @@ -326,6 +333,102 @@ void Slave::initialize() > } > } > > + vector<string> httpAuthenticatorNames = > + strings::split(flags.http_authenticators, ","); > + > + // If the `http_authenticators` flag is not specified, the default > value will > + // be filled in. Passing an empty string into the `http_authenticators` > flag > + // is considered an error. > + if (httpAuthenticatorNames.empty()) { > + EXIT(1) << "No HTTP authenticator specified"; > + } > + if (httpAuthenticatorNames.size() > 1) { > + EXIT(1) << "Multiple HTTP authenticators not supported"; > + } > + if (httpAuthenticatorNames[0] != DEFAULT_HTTP_AUTHENTICATOR && > + !modules::ModuleManager::contains<authentication::Authenticator>( > + httpAuthenticatorNames[0])) { > + EXIT(1) << "HTTP authenticator '" << httpAuthenticatorNames[0] > + << "' not found. Check the spelling (compare to '" > + << DEFAULT_HTTP_AUTHENTICATOR << "') or verify that the " > + << "authenticator was loaded successfully (see --modules)"; > + } > + > + if (flags.authenticate_http) { > + authentication::Authenticator* httpAuthenticator = NULL; > + > + if (httpAuthenticatorNames[0] == DEFAULT_HTTP_AUTHENTICATOR) { > + // Load credentials for HTTP authentication. > + Credentials httpCredentials; > + if (flags.http_credentials.isSome()) { > + Result<Credentials> credentials = > + credentials::read(flags.http_credentials.get()); > + if (credentials.isError()) { > + EXIT(1) << credentials.error() << " (see --http_credentials > flag)"; > + } else if (credentials.isNone()) { > + EXIT(1) << "Credentials file must contain at least one > credential" > + << " (see --http_credentials flag)"; > + } > + > + httpCredentials = credentials.get(); > + } else { > + EXIT(1) << "No credentials provided for the default '" > + << DEFAULT_HTTP_AUTHENTICATOR > + << "' HTTP authenticator"; > + } > + > + LOG(INFO) << "Using default '" << DEFAULT_HTTP_AUTHENTICATOR > + << "' HTTP authenticator"; > + > + Try<authentication::Authenticator*> authenticator = > + BasicAuthenticatorFactory::create( > + DEFAULT_HTTP_AUTHENTICATION_REALM, > + httpCredentials); > + if (authenticator.isError()) { > + EXIT(1) << "Could not create HTTP authenticator module '" > + << httpAuthenticatorNames[0] << "': " << > authenticator.error(); > + } > + > + httpAuthenticator = authenticator.get(); > + } else { > + if (flags.http_credentials.isSome()) { > + EXIT(1) << "The '--http_credentials' flag is only used by the > default " > + << "basic HTTP authenticator, but a custom authenticator " > + << "module was specified via '--http_authenticators'"; > + } > + > + Try<authentication::Authenticator*> module = > + modules::ModuleManager::create<authentication::Authenticator>( > + httpAuthenticatorNames[0]); > + if (module.isError()) { > + EXIT(1) << "Could not create HTTP authenticator module '" > + << httpAuthenticatorNames[0] << "': " << module.error(); > + } > + > + LOG(INFO) << "Using '" << httpAuthenticatorNames[0] > + << "' HTTP authenticator"; > + > + httpAuthenticator = module.get(); > + } > + > + if (httpAuthenticator == NULL) { > + EXIT(1) << "An error occurred while initializing the '" > + << httpAuthenticatorNames[0] << "' HTTP authenticator"; > + } > + > + // Ownership of the `httpAuthenticator` is passed to libprocess. > + process::http::authentication::setAuthenticator( > + DEFAULT_HTTP_AUTHENTICATION_REALM, > + Owned<authentication::Authenticator>(httpAuthenticator)); > + } else if (flags.http_credentials.isSome()) { > + EXIT(1) << "The '--http_credentials' flag was provided, but HTTP " > + << "authentication was not enabled via '--authenticate_http'"; > + } else if (httpAuthenticatorNames[0] != DEFAULT_HTTP_AUTHENTICATOR) { > + EXIT(1) << "A custom HTTP authenticator was specified with the " > + << "'--http_authenticators' flag, but HTTP authentication was > not " > + << "enabled via '--authenticate_http'"; > + } > + > if ((flags.gc_disk_headroom < 0) || (flags.gc_disk_headroom > 1)) { > EXIT(1) << "Invalid value '" << flags.gc_disk_headroom > << "' for --gc_disk_headroom. Must be between 0.0 and 1.0."; > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/40138729/src/tests/mesos.cpp > ---------------------------------------------------------------------- > diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp > index 90aef6b..543b2a7 100644 > --- a/src/tests/mesos.cpp > +++ b/src/tests/mesos.cpp > @@ -147,26 +147,59 @@ slave::Flags MesosTest::CreateSlaveFlags() > > flags.launcher_dir = getLauncherDir(); > > - // Create a default credential file. > - const string& path = path::join(directory.get(), "credential"); > + { > + // Create a default credential file for master/agent authentication. > + const string& path = path::join(directory.get(), "credential"); > > - Try<int> fd = os::open( > - path, > - O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, > - S_IRUSR | S_IWUSR | S_IRGRP); > + Try<int> fd = os::open( > + path, > + O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, > + S_IRUSR | S_IWUSR | S_IRGRP); > > - CHECK_SOME(fd); > + CHECK_SOME(fd); > > - Credential credential; > - credential.set_principal(DEFAULT_CREDENTIAL.principal()); > - credential.set_secret(DEFAULT_CREDENTIAL.secret()); > + Credential credential; > + credential.set_principal(DEFAULT_CREDENTIAL.principal()); > + credential.set_secret(DEFAULT_CREDENTIAL.secret()); > > - CHECK_SOME(os::write(fd.get(), stringify(JSON::protobuf(credential)))) > - << "Failed to write slave credential to '" << path << "'"; > + CHECK_SOME(os::write(fd.get(), stringify(JSON::protobuf(credential)))) > + << "Failed to write slave credential to '" << path << "'"; > > - CHECK_SOME(os::close(fd.get())); > + CHECK_SOME(os::close(fd.get())); > + > + flags.credential = path; > + } > + > + flags.authenticate_http = true; > > - flags.credential = path; > + { > + // Create a default HTTP credentials file. > + const string& path = path::join(directory.get(), "http_credentials"); > + > + Try<int> fd = os::open( > + path, > + O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, > + S_IRUSR | S_IWUSR | S_IRGRP); > + > + CHECK_SOME(fd); > + > + Credentials httpCredentials; > + > + Credential* httpCredential = httpCredentials.add_credentials(); > + httpCredential->set_principal(DEFAULT_CREDENTIAL.principal()); > + httpCredential->set_secret(DEFAULT_CREDENTIAL.secret()); > + > + httpCredential = httpCredentials.add_credentials(); > + httpCredential->set_principal(DEFAULT_CREDENTIAL_2.principal()); > + httpCredential->set_secret(DEFAULT_CREDENTIAL_2.secret()); > + > + CHECK_SOME(os::write(fd.get(), > stringify(JSON::protobuf(httpCredentials)))) > + << "Failed to write HTTP credentials to '" << path << "'"; > + > + CHECK_SOME(os::close(fd.get())); > + > + flags.http_credentials = path; > + } > > flags.resources = defaultAgentResourcesString; > > >
