Repository: mesos Updated Branches: refs/heads/1.1.x 61f678ed7 -> d2b1888a5
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/7d2d16ff Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7d2d16ff Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7d2d16ff Branch: refs/heads/1.1.x Commit: 7d2d16ff85e758c333145dc7cdf852c5ef285ee6 Parents: 61f678e Author: Gilbert Song <[email protected]> Authored: Thu Apr 13 09:25:56 2017 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Apr 25 20:13:08 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/7d2d16ff/src/uri/fetchers/docker.cpp ---------------------------------------------------------------------- diff --git a/src/uri/fetchers/docker.cpp b/src/uri/fetchers/docker.cpp index 7d5acc1..6026a74 100644 --- a/src/uri/fetchers/docker.cpp +++ b/src/uri/fetchers/docker.cpp @@ -207,19 +207,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. @@ -228,7 +225,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( @@ -246,7 +243,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> { @@ -279,16 +276,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. //-------------------------------------------------------------------
