You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/05/16 03:33:38 UTC
[1/2] mesos git commit: Supported GCE container registry.
Repository: mesos
Updated Branches:
refs/heads/1.3.x 76f662c21 -> b03229969
Supported GCE container registry.
Certain registries, such as GCE registry, reply 403 instead of 401 for
unauthorized requests. When fetching image manifests and blobs, instead
of sending out unauthorized requests first and waiting for a possible
401, we should always look up the docker config and send requests with
basic authorization when possible.
Review: https://reviews.apache.org/r/58778/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ca042724
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ca042724
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ca042724
Branch: refs/heads/1.3.x
Commit: ca04272467f3b81126257a3cced8f133c25eb3e1
Parents: 76f662c
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Mon May 15 20:10:50 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon May 15 20:29:29 2017 -0700
----------------------------------------------------------------------
src/uri/fetchers/docker.cpp | 220 ++++++++++++++++++---------------------
1 file changed, 101 insertions(+), 119 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/ca042724/src/uri/fetchers/docker.cpp
----------------------------------------------------------------------
diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp
index dbfc1b2..8aab169 100644
--- a/src/uri/fetchers/docker.cpp
+++ b/src/uri/fetchers/docker.cpp
@@ -310,6 +310,62 @@ static Future<int> download(
return download(strings::trim(stringify(uri)), blobPath, headers);
}
+
+// Returns the 'Basic' credential as a header for pulling an image
+// from a registry, if the host of the image's repository exists in
+// the docker config file, or empty if there is none.
+static http::Headers getAuthHeaderBasic(
+ const URI& uri,
+ const hashmap<string, spec::Config::Auth>& auths)
+{
+ http::Headers headers;
+
+ // NOTE: The host field of uri can be either domain or IP
+ // address, which is merged in docker registry puller.
+ const string registry = uri.has_port()
+ ? uri.host() + ":" + stringify(uri.port())
+ : uri.host();
+
+ foreachpair (const string& key, const spec::Config::Auth& value, auths) {
+ // Handle domains including 'docker.io' as a special case,
+ // because the url is set differently for different version
+ // of docker default registry, but all of them should depend
+ // on the same default namespace 'docker.io'. Please see:
+ // https://github.com/docker/docker/blob/master/registry/config.go#L34
+ const bool isDocker =
+ strings::contains(uri.host(), "docker.io") &&
+ strings::contains(key, "docker.io");
+
+ // Should not use 'http::URL::parse()' here, since many
+ // registry domain recorded in docker config file does
+ // not start with 'https://' or 'http://'. They are pure
+ // domain only (e.g., 'quay.io', 'localhost:5000').
+ // Please see 'ResolveAuthConfig()' in:
+ // https://github.com/docker/docker/blob/master/registry/auth.go
+ if (isDocker || (registry == spec::parseAuthUrl(key))) {
+ if (value.has_auth()) {
+ headers["Authorization"] = "Basic " + value.auth();
+ break;
+ }
+ }
+ }
+
+ return headers;
+}
+
+
+static http::Headers getAuthHeaderBearer(
+ const Option<string>& authToken)
+{
+ http::Headers headers;
+
+ if (authToken.isSome()) {
+ headers["Authorization"] = "Bearer " + authToken.get();
+ }
+
+ return headers;
+}
+
//-------------------------------------------------------------------
// DockerFetcherPlugin implementation.
//-------------------------------------------------------------------
@@ -330,6 +386,7 @@ private:
const string& directory,
const URI& manifestUri,
const http::Headers& manifestHeaders,
+ const http::Headers& basicAuthHeaders,
const http::Response& response);
Future<Nothing> __fetch(
@@ -346,10 +403,16 @@ private:
Future<Nothing> _fetchBlob(
const URI& uri,
const string& directory,
- const URI& blobUri);
+ const URI& blobUri,
+ const http::Headers& basicAuthHeaders);
+ Future<Nothing> __fetchBlob(int code);
+
+ // Returns a token-based authorization header. Basic authorization
+ // header may be required to get a proper authorization token.
Future<http::Headers> getAuthHeader(
const URI& uri,
+ const http::Headers& basicAuthHeaders,
const http::Response& response);
URI getManifestUri(const URI& uri);
@@ -464,8 +527,11 @@ Future<Nothing> DockerFetcherPluginProcess::fetch(
directory + "': " + mkdir.error());
}
+ // Use the 'Basic' credential to pull the manifest/blob by default.
+ http::Headers basicAuthHeaders = getAuthHeaderBasic(uri, auths);
+
if (uri.scheme() == "docker-blob") {
- return fetchBlob(uri, directory, http::Headers());
+ return fetchBlob(uri, directory, basicAuthHeaders);
}
URI manifestUri = getManifestUri(uri);
@@ -479,13 +545,14 @@ Future<Nothing> DockerFetcherPluginProcess::fetch(
{"Accept", "application/vnd.docker.distribution.manifest.v1+json"}
};
- return curl(manifestUri, manifestHeaders)
+ return curl(manifestUri, manifestHeaders + basicAuthHeaders)
.then(defer(self(),
&Self::_fetch,
uri,
directory,
manifestUri,
manifestHeaders,
+ basicAuthHeaders,
lambda::_1));
}
@@ -495,10 +562,12 @@ Future<Nothing> DockerFetcherPluginProcess::_fetch(
const string& directory,
const URI& manifestUri,
const http::Headers& manifestHeaders,
+ const http::Headers& basicAuthHeaders,
const http::Response& response)
{
if (response.code == http::Status::UNAUTHORIZED) {
- return getAuthHeader(manifestUri, response)
+ // Use the 'Basic' credential to request an auth token by default.
+ return getAuthHeader(manifestUri, basicAuthHeaders, response)
.then(defer(self(), [=](
const http::Headers& authHeaders) -> Future<Nothing> {
return curl(manifestUri, manifestHeaders + authHeaders)
@@ -511,7 +580,7 @@ Future<Nothing> DockerFetcherPluginProcess::_fetch(
}));
}
- return __fetch(uri, directory, http::Headers(), response);
+ return __fetch(uri, directory, basicAuthHeaders, response);
}
@@ -590,6 +659,7 @@ Future<Nothing> DockerFetcherPluginProcess::__fetch(
? Option<int>(uri.port())
: None()));
+ // Use the same 'authHeaders' as for the manifest to pull the blobs.
futures.push_back(fetchBlob(
blob,
directory,
@@ -610,20 +680,15 @@ Future<Nothing> DockerFetcherPluginProcess::fetchBlob(
return download(blobUri, directory, authHeaders)
.then(defer(self(), [=](int code) -> Future<Nothing> {
- if (code == http::Status::OK) {
- return Nothing();
- }
-
- // Note that if 'authHeaders' is not empty, but we still get a
- // '401 Unauthorized' response, we return a Failure. This can
- // prevent us from entering an infinite loop.
- if (code == http::Status::UNAUTHORIZED && authHeaders.empty()) {
- return _fetchBlob(uri, directory, blobUri);
+ if (code == http::Status::UNAUTHORIZED) {
+ // If we get a '401 Unauthorized', we assume that 'authHeaders'
+ // is either empty or contains the 'Basic' credential, and we
+ // can use it to request an auth token.
+ // TODO(chhsiao): What if 'authHeaders' has an expired token?
+ return _fetchBlob(uri, directory, blobUri, authHeaders);
}
- return Failure(
- "Unexpected HTTP response '" + http::Status::string(code) + "' "
- "when trying to download the blob");
+ return __fetchBlob(code);
}));
}
@@ -631,13 +696,14 @@ Future<Nothing> DockerFetcherPluginProcess::fetchBlob(
Future<Nothing> DockerFetcherPluginProcess::_fetchBlob(
const URI& uri,
const string& directory,
- const URI& blobUri)
+ const URI& blobUri,
+ const http::Headers& basicAuthHeaders)
{
// TODO(jieyu): This extra 'curl' call can be avoided if we can get
// HTTP headers from 'download'. Currently, 'download' only returns
// the HTTP response code because we don't support parsing HTTP
// headers alone. Revisit this once that's supported.
- return curl(blobUri)
+ return curl(blobUri, basicAuthHeaders)
.then(defer(self(), [=](const http::Response& response) -> Future<Nothing> {
// We expect a '401 Unauthorized' response here since the
// 'download' with the same URI returns a '401 Unauthorized'.
@@ -647,47 +713,33 @@ Future<Nothing> DockerFetcherPluginProcess::_fetchBlob(
"but get '" + response.status + "' instead");
}
- return getAuthHeader(blobUri, response)
- .then(defer(self(),
- &Self::fetchBlob,
- uri,
- directory,
- lambda::_1));
+ return getAuthHeader(blobUri, basicAuthHeaders, response)
+ .then(defer(self(), [=](
+ const http::Headers& authHeaders) -> Future<Nothing> {
+ return download(blobUri, directory, authHeaders)
+ .then(defer(self(),
+ &Self::__fetchBlob,
+ lambda::_1));
+ }));
}));
}
-static http::Headers getAuthHeaderBasic(
- const Option<string>& credential)
+Future<Nothing> DockerFetcherPluginProcess::__fetchBlob(int code)
{
- http::Headers headers;
-
- if (credential.isSome()) {
- // NOTE: The 'Basic' credential would be attached as a header
- // when pulling a public image from a registry, if the host
- // of the image's repository exists in the docker config file.
- headers["Authorization"] = "Basic " + credential.get();
- }
-
- return headers;
-}
-
-
-static http::Headers getAuthHeaderBearer(
- const Option<string>& authToken)
-{
- http::Headers headers;
-
- if (authToken.isSome()) {
- headers["Authorization"] = "Bearer " + authToken.get();
+ if (code == http::Status::OK) {
+ return Nothing();
}
- return headers;
+ return Failure(
+ "Unexpected HTTP response '" + http::Status::string(code) + "' "
+ "when trying to download the blob");
}
Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader(
const URI& uri,
+ const http::Headers& basicAuthHeaders,
const http::Response& response)
{
Result<http::header::WWWAuthenticate> header =
@@ -704,24 +756,6 @@ Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader(
const string authScheme = strings::upper(header->authScheme());
// If a '401 Unauthorized' response is received and the auth-scheme
- // is 'Basic', we do basic authentication with the server directly.
- if (authScheme == "BASIC") {
- const string registry = uri.has_port()
- ? uri.host() + ":" + stringify(uri.port())
- : uri.host();
-
- Option<string> auth;
- foreachpair (const string& key, const spec::Config::Auth& value, auths) {
- if (registry == spec::parseAuthUrl(key) && value.has_auth()) {
- auth = value.auth();
- break;
- }
- }
-
- return getAuthHeaderBasic(auth);
- }
-
- // If a '401 Unauthorized' response is received and the auth-scheme
// is 'Bearer', we expect a header 'Www-Authenticate' containing the
// auth server information. We extract the auth server information
// from the auth-param, and then contacts the auth server to get the
@@ -747,58 +781,6 @@ Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader(
return Failure("Missing 'scope' in WWW-Authenticate header");
}
- // NOTE: The host field of uri can be either domain or IP
- // address, which is merged in docker registry puller.
- const string registry = uri.has_port()
- ? uri.host() + ":" + stringify(uri.port())
- : uri.host();
-
- // TODO(gilbert): Ideally, this should be done after getting
- // the '401 Unauthorized' response. Then, the workflow should
- // be:
- // 1. Send a requst to registry for pulling.
- // 2. The registry returns '401 Unauthorized' HTTP response.
- // 3. The registry client makes a request (without a Basic header)
- // to the authorization server for a Bearer token.
- // 4. The authorization servicer returns an unacceptable
- // Bearer token.
- // 5. Re-send a request to registry with the Bearer token attached.
- // 6. The registry returns '401 Unauthorized' HTTP response.
- // 7. The registry client makes a request (with a correct Basic
- // header attached) to the authorization server for a Bearer
- // token.
- // 8. The authorization servicer returns a corrent Bearer token.
- // 9. Re-send a request to registry with the right Bearer token
- // attached.
- // 10. The registry authorizes the client, and the docker fetcher
- // starts pulling.
- // The step 3 ~ 6 are exactly what this TODO describes.
- Option<string> auth;
-
- foreachpair (const string& key, const spec::Config::Auth& value, auths) {
- // Handle domains including 'docker.io' as a special case,
- // because the url is set differently for different version
- // of docker default registry, but all of them should depend
- // on the same default namespace 'docker.io'. Please see:
- // https://github.com/docker/docker/blob/master/registry/config.go#L34
- const bool isDocker =
- strings::contains(uri.host(), "docker.io") &&
- strings::contains(key, "docker.io");
-
- // Should not use 'http::URL::parse()' here, since many
- // registry domain recorded in docker config file does
- // not start with 'https://' or 'http://'. They are pure
- // domain only (e.g., 'quay.io', 'localhost:5000').
- // Please see 'ResolveAuthConfig()' in:
- // https://github.com/docker/docker/blob/master/registry/auth.go
- if (isDocker || (registry == spec::parseAuthUrl(key))) {
- if (value.has_auth()) {
- auth = value.auth();
- break;
- }
- }
- }
-
// TODO(jieyu): Currently, we don't expect the auth server to return
// a service or a scope that needs encoding.
string authServerUri =
@@ -806,7 +788,7 @@ Future<http::Headers> DockerFetcherPluginProcess::getAuthHeader(
"service=" + authParam.at("service") + "&" +
"scope=" + authParam.at("scope");
- return curl(authServerUri, getAuthHeaderBasic(auth))
+ return curl(authServerUri, basicAuthHeaders)
.then([authServerUri](
const http::Response& response) -> Future<http::Headers> {
if (response.code != http::Status::OK) {
[2/2] mesos git commit: Added MESOS-7431 to 1.3.0 CHANGELOG.
Posted by ji...@apache.org.
Added MESOS-7431 to 1.3.0 CHANGELOG.
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b0322996
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b0322996
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b0322996
Branch: refs/heads/1.3.x
Commit: b032299694c4dfb9597bd3cb0d111ebabe060cdd
Parents: ca04272
Author: Jie Yu <yu...@gmail.com>
Authored: Mon May 15 20:32:31 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon May 15 20:33:32 2017 -0700
----------------------------------------------------------------------
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b0322996/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index 94a981a..fd8ef3a 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -203,6 +203,7 @@ All Resolved Issues:
* [MESOS-7400] - The mesos master crashes due to an incorrect invariant check in the decoder.
* [MESOS-7427] - Registry puller cannot fetch manifests from Amazon ECR: 405 Unsupported.
* [MESOS-7430] - Per-role Suppress call implementation is broken.
+ * [MESOS-7431] - Registry puller cannot fetch manifests from Google GCR: 403 Forbidden.
* [MESOS-7453] - glyphicons-halflings-regular.woff2 is missing in WebUI.
* [MESOS-7456] - Compilation error on recent glibc in cgroups device subsystem.
* [MESOS-7464] - Recent Docker versions cannot be parsed by stout.