Will do; thanks Alex! On Thu, Mar 24, 2016 at 2:36 AM, Alex R <[email protected]> wrote:
> 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; >> >> >> >
