diff --git a/changelogs/unreleased/sh-fix-container-registry-s3-redirects.yml b/changelogs/unreleased/sh-fix-container-registry-s3-redirects.yml new file mode 100644 index 00000000000..1e783811b66 --- /dev/null +++ b/changelogs/unreleased/sh-fix-container-registry-s3-redirects.yml @@ -0,0 +1,4 @@ +--- +title: Properly handle container registry redirects to fix metadata stored on a S3 backend +merge_request: +author: diff --git a/lib/container_registry/client.rb b/lib/container_registry/client.rb index 7f5f6d9ddb6..c7263f302ab 100644 --- a/lib/container_registry/client.rb +++ b/lib/container_registry/client.rb @@ -75,10 +75,7 @@ module ContainerRegistry def redirect_response(location) return unless location - # We explicitly remove authorization token - faraday_blob.get(location) do |req| - req['Authorization'] = '' - end + faraday_redirect.get(location) end def faraday @@ -93,5 +90,14 @@ module ContainerRegistry initialize_connection(conn, @options) end end + + # Create a new request to make sure the Authorization header is not inserted + # via the Faraday middleware + def faraday_redirect + @faraday_redirect ||= Faraday.new(@base_uri) do |conn| + conn.request :json + conn.adapter :net_http + end + end end end diff --git a/spec/lib/container_registry/blob_spec.rb b/spec/lib/container_registry/blob_spec.rb index f06e5fd54a2..ab010c6dfeb 100644 --- a/spec/lib/container_registry/blob_spec.rb +++ b/spec/lib/container_registry/blob_spec.rb @@ -98,7 +98,7 @@ describe ContainerRegistry::Blob do context 'for a valid address' do before do stub_request(:get, location). - with(headers: { 'Authorization' => nil }). + with { |request| !request.headers.include?('Authorization') }. to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, diff --git a/spec/lib/container_registry/client_spec.rb b/spec/lib/container_registry/client_spec.rb new file mode 100644 index 00000000000..ec03b533383 --- /dev/null +++ b/spec/lib/container_registry/client_spec.rb @@ -0,0 +1,39 @@ +# coding: utf-8 +require 'spec_helper' + +describe ContainerRegistry::Client do + let(:token) { '12345' } + let(:options) { { token: token } } + let(:client) { described_class.new("http://container-registry", options) } + + describe '#blob' do + it 'GET /v2/:name/blobs/:digest' do + stub_request(:get, "http://container-registry/v2/group/test/blobs/sha256:0123456789012345"). + with(headers: { + 'Accept' => 'application/octet-stream', + 'Authorization' => "bearer #{token}" + }). + to_return(status: 200, body: "Blob") + + expect(client.blob('group/test', 'sha256:0123456789012345')).to eq('Blob') + end + + it 'follows 307 redirect for GET /v2/:name/blobs/:digest' do + stub_request(:get, "http://container-registry/v2/group/test/blobs/sha256:0123456789012345"). + with(headers: { + 'Accept' => 'application/octet-stream', + 'Authorization' => "bearer #{token}" + }). + to_return(status: 307, body: "", headers: { Location: 'http://redirected' }) + # We should probably use hash_excluding here, but that requires an update to WebMock: + # https://github.com/bblimke/webmock/blob/master/lib/webmock/matchers/hash_excluding_matcher.rb + stub_request(:get, "http://redirected/"). + with { |request| !request.headers.include?('Authorization') }. + to_return(status: 200, body: "Successfully redirected") + + response = client.blob('group/test', 'sha256:0123456789012345') + + expect(response).to eq('Successfully redirected') + end + end +end