Merge branch 'docker-registry-work-with-redirect' into 'master'

Make docker registry work with location redirects when external storage is used

## What does this MR do?

Honor `Location:` header when working with local registry.
Location makes it possible to download manifests from external storage.

## What are the relevant issue numbers?

Fixed https://gitlab.com/gitlab-org/gitlab-ce/issues/18477

## Remark

Adding `FollowRedirects` makes that we leak `Authorization:` in followed requests.

That is why it is implemented manually and we are explicitly removing `Authorization` header.

cc @marin @twk3 

See merge request !4677

2 files changed, 50 insertions, 21 deletions

diff --git a/lib/container_registry/client.rb b/lib/container_registry/client.rb
index 42232b7129d..2edddb84fc3 100644
--- a/lib/container_registry/client.rb
+++ b/lib/container_registry/client.rb
@@ -7,62 +7,91 @@ module ContainerRegistry
 
     MANIFEST_VERSION = 'application/vnd.docker.distribution.manifest.v2+json'
 
+    # Taken from: FaradayMiddleware::FollowRedirects
+    REDIRECT_CODES = Set.new [301, 302, 303, 307]
+
     def initialize(base_uri, options = {})
       @base_uri = base_uri
-      @faraday = Faraday.new(@base_uri) do |conn|
-        initialize_connection(conn, options)
-      end
+      @options = options
     end
 
     def repository_tags(name)
-      response_body @faraday.get("/v2/#{name}/tags/list")
+      response_body faraday.get("/v2/#{name}/tags/list")
     end
 
     def repository_manifest(name, reference)
-      response_body @faraday.get("/v2/#{name}/manifests/#{reference}")
+      response_body faraday.get("/v2/#{name}/manifests/#{reference}")
     end
 
     def repository_tag_digest(name, reference)
-      response = @faraday.head("/v2/#{name}/manifests/#{reference}")
+      response = faraday.head("/v2/#{name}/manifests/#{reference}")
       response.headers['docker-content-digest'] if response.success?
     end
 
     def delete_repository_tag(name, reference)
-      @faraday.delete("/v2/#{name}/manifests/#{reference}").success?
+      faraday.delete("/v2/#{name}/manifests/#{reference}").success?
     end
 
     def blob(name, digest, type = nil)
-      headers = {}
-      headers['Accept'] = type if type
-      response_body @faraday.get("/v2/#{name}/blobs/#{digest}", nil, headers)
+      type ||= 'application/octet-stream'
+      response_body faraday_blob.get("/v2/#{name}/blobs/#{digest}", nil, 'Accept' => type), allow_redirect: true
     end
 
     def delete_blob(name, digest)
-      @faraday.delete("/v2/#{name}/blobs/#{digest}").success?
+      faraday.delete("/v2/#{name}/blobs/#{digest}").success?
     end
-    
+
     private
-    
+
     def initialize_connection(conn, options)
       conn.request :json
+
+      if options[:user] && options[:password]
+        conn.request(:basic_auth, options[:user].to_s, options[:password].to_s)
+      elsif options[:token]
+        conn.request(:authorization, :bearer, options[:token].to_s)
+      end
+
+      conn.adapter :net_http
+    end
+
+    def accept_manifest(conn)
       conn.headers['Accept'] = MANIFEST_VERSION
 
       conn.response :json, content_type: 'application/json'
       conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v1+prettyjws'
       conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v1+json'
       conn.response :json, content_type: 'application/vnd.docker.distribution.manifest.v2+json'
+    end
 
-      if options[:user] && options[:password]
-        conn.request(:basic_auth, options[:user].to_s, options[:password].to_s)
-      elsif options[:token]
-        conn.request(:authorization, :bearer, options[:token].to_s)
+    def response_body(response, allow_redirect: false)
+      if allow_redirect && REDIRECT_CODES.include?(response.status)
+        response = redirect_response(response.headers['location'])
       end
 
-      conn.adapter :net_http
+      response.body if response && response.success?
+    end
+
+    def redirect_response(location)
+      return unless location
+
+      # We explicitly remove authorization token
+      faraday_blob.get(location) do |req|
+        req['Authorization'] = ''
+      end
     end
 
-    def response_body(response)
-      response.body if response.success?
+    def faraday
+      @faraday ||= Faraday.new(@base_uri) do |conn|
+        initialize_connection(conn, @options)
+        accept_manifest(conn)
+      end
+    end
+
+    def faraday_blob
+      @faraday_blob ||= Faraday.new(@base_uri) do |conn|
+        initialize_connection(conn, @options)
+      end
     end
   end
 end
diff --git a/lib/container_registry/tag.rb b/lib/container_registry/tag.rb
index 708d01b95a1..59040199920 100644
--- a/lib/container_registry/tag.rb
+++ b/lib/container_registry/tag.rb
@@ -53,7 +53,7 @@ module ContainerRegistry
     def config
       return unless config_blob
 
-      @config ||= ContainerRegistry::Config.new(self, config_blob)
+      @config ||= ContainerRegistry::Config.new(self, config_blob) if config_blob.data
     end
 
     def created_at