Mailing-List: contact commits-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@mesos.apache.org
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: jieyu@apache.org
To: commits@mesos.apache.org
Date: Wed, 26 Apr 2017 03:16:43 -0000
Message-Id: <2d6e5386bf7542de8f6db1196de34a66@git.apache.org>
Subject: [1/3] mesos git commit: Fixed docker fetcher 3xx redirect errors by
 header attached.
archived-at: Wed, 26 Apr 2017 03:16:44 -0000

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 <songzihao1990@gmail.com>
Authored: Thu Apr 13 09:25:56 2017 -0700
Committer: Jie Yu <yujie.jay@gmail.com>
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.
 //-------------------------------------------------------------------