Repository: mesos Updated Branches: refs/heads/1.2.x c2314e9bc -> 9e90ae535
Fixed docker fetcher 3xx redirect errors by header attached. The root cause for this issue is that, in private registry like quay.io, layer download request will be redirected to storage server in S3. However, the curl command with '-L' handles HTTP redirection automatically, in which case HTTP headers will be attached to all requests. AmazonS3 server will return 400 Bad Request if HTTP Authorization header is attached, with 'InvalidArgument' error code. So we need to touch the given URL first to add extra logic for HTTP redirections. Please note that the download() method is changed to be recursive since no header should be attached once the request get authenticated. Review: https://reviews.apache.org/r/48917/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/906c877c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/906c877c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/906c877c Branch: refs/heads/1.2.x Commit: 906c877c57386bfc92030351393302aa4946de92 Parents: c2314e9 Author: Gilbert Song <[email protected]> Authored: Thu Apr 13 09:25:56 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 25 20:12:43 2017 -0700 ---------------------------------------------------------------------- src/uri/fetchers/docker.cpp | 44 ++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/906c877c/src/uri/fetchers/docker.cpp ---------------------------------------------------------------------- diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp index 68f380d..ec79b6b 100644 --- a/src/uri/fetchers/docker.cpp +++ b/src/uri/fetchers/docker.cpp @@ -205,19 +205,16 @@ static Future<http::Response> curl( // TODO(jieyu): Add a comment here. static Future<int> download( - const URI& uri, - const string& directory, + const string& uri, + const string& blobPath, const http::Headers& headers = http::Headers()) { - const string output = path::join(directory, Path(uri.path()).basename()); - vector<string> argv = { "curl", "-s", // Don't show progress meter or error messages. "-S", // Make curl show an error message if it fails. - "-L", // Follow HTTP 3xx redirects. - "-w", "%{http_code}", // Display HTTP response code on stdout. - "-o", output // Write output to the file. + "-w", "%{http_code}\n%{redirect_url}", // Display HTTP response code and the redirected URL. // NOLINT(whitespace/line_length) + "-o", blobPath // Write output to the file. }; // Add additional headers. @@ -226,7 +223,7 @@ static Future<int> download( argv.push_back(key + ": " + value); } - argv.push_back(strings::trim(stringify(uri))); + argv.push_back(uri); // TODO(jieyu): Kill the process if discard is called. Try<Subprocess> s = subprocess( @@ -244,7 +241,7 @@ static Future<int> download( s.get().status(), io::read(s.get().out().get()), io::read(s.get().err().get())) - .then([](const tuple< + .then([=](const tuple< Future<Option<int>>, Future<string>, Future<string>>& t) -> Future<int> { @@ -277,16 +274,41 @@ static Future<int> download( (output.isFailed() ? output.failure() : "discarded")); } + vector<string> tokens = strings::tokenize(output.get(), "\n", 2); + if (tokens.empty()) { + return Failure("Unexpected 'curl' output: " + output.get()); + } + // Parse the output and get the HTTP response code. - Try<int> code = numify<int>(output.get()); + Try<int> code = numify<int>(tokens[0]); if (code.isError()) { - return Failure("Unexpected output from 'curl': " + output.get()); + return Failure( + "Unexpected HTTP response code from 'curl': " + tokens[0]); + } + + // If there are two tokens, it means that the redirect url + // exists in the stdout and the request to download the blob + // is already authenticated. + if (tokens.size() == 2) { + // Headers are not attached because the request is already + // authenticated. + return download(tokens[1], blobPath); } return code.get(); }); } + +static Future<int> download( + const URI& uri, + const string& directory, + const http::Headers& headers = http::Headers()) +{ + const string blobPath = path::join(directory, Path(uri.path()).basename()); + return download(strings::trim(stringify(uri)), blobPath, headers); +} + //------------------------------------------------------------------- // DockerFetcherPlugin implementation. //-------------------------------------------------------------------
