Merge branch 'revert-fb45e290' into 'master'

Fix S3 object storage failing when endpoint is not specified

See merge request gitlab-org/gitlab!54868

changelogs/unreleased/revert-fb45e290.yml

+---
+title: Fix S3 object storage failing when endpoint is not specified
+merge_request: 54868
+author:
+type: fixed

lib/object_storage/config.rb

@@ -2,8 +2,6 @@
 
 module ObjectStorage
   class Config
-    include Gitlab::Utils::StrongMemoize
-
     AWS_PROVIDER = 'AWS'
     AZURE_PROVIDER = 'AzureRM'
     GOOGLE_PROVIDER = 'Google'
@@ -68,36 +66,6 @@ module ObjectStorage
     def provider
       credentials[:provider].to_s
     end
-
-    # This method converts fog-aws parameters to an endpoint for the
-    # Workhorse S3 client.
-    def s3_endpoint
-      strong_memoize(:s3_endpoint) do
-        # We could omit this line and let the following code handle this, but
-        # this will ensure that working configurations that use `endpoint`
-        # will continue to work.
-        next credentials[:endpoint] if credentials[:endpoint].present?
-
-        generate_s3_endpoint_from_credentials
-      end
-    end
-
-    def generate_s3_endpoint_from_credentials
-      # fog-aws has special handling of the host, region, scheme, etc:
-      # https://github.com/fog/fog-aws/blob/c7a11ba377a76d147861d0e921eb1e245bc11b6c/lib/fog/aws/storage.rb#L440-L449
-      # Rather than reimplement this, we derive it from a sample GET URL.
-      url = fog_connection.get_object_url(bucket, "tmp", nil)
-      uri = ::Addressable::URI.parse(url)
-
-      return unless uri&.scheme && uri&.host
-
-      endpoint = "#{uri.scheme}://#{uri.host}"
-      endpoint += ":#{uri.port}" if uri.port
-      endpoint
-    rescue ::URI::InvalidComponentError, ::Addressable::URI::InvalidURIError => e
-      Gitlab::ErrorTracking.track_exception(e)
-      nil
-    end
     # End AWS-specific options
 
     # Begin Azure-specific options
@@ -123,10 +91,6 @@ module ObjectStorage
       end
     end
 
-    def fog_connection
-      @connection ||= ::Fog::Storage.new(credentials)
-    end
-
     private
 
     # This returns a Hash of HTTP encryption headers to send along to S3.

lib/object_storage/direct_upload.rb

@@ -80,7 +80,7 @@ module ObjectStorage
           S3Config: {
             Bucket: bucket_name,
             Region: credentials[:region],
-            Endpoint: config.s3_endpoint,
+            Endpoint: credentials[:endpoint],
             PathStyle: config.use_path_style?,
             UseIamProfile: config.use_iam_profile?,
             ServerSideEncryption: config.server_side_encryption,
@@ -229,7 +229,7 @@ module ObjectStorage
     end
 
     def connection
-      config.fog_connection
+      @connection ||= ::Fog::Storage.new(credentials)
     end
   end
 end

spec/lib/object_storage/config_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 require 'rspec-parameterized'
+require 'fog/core'
 
 RSpec.describe ObjectStorage::Config do
   using RSpec::Parameterized::TableSyntax
@@ -33,9 +34,7 @@ RSpec.describe ObjectStorage::Config do
     }
   end
 
-  subject do
-    described_class.new(raw_config.as_json)
-  end
+  subject { described_class.new(raw_config.as_json) }
 
   describe '#load_provider' do
     before do
@@ -46,10 +45,6 @@ RSpec.describe ObjectStorage::Config do
       it 'registers AWS as a provider' do
         expect(Fog.providers.keys).to include(:aws)
       end
-
-      describe '#fog_connection' do
-        it { expect(subject.fog_connection).to be_a_kind_of(Fog::AWS::Storage::Real) }
-      end
     end
 
     context 'with Google' do
@@ -64,10 +59,6 @@ RSpec.describe ObjectStorage::Config do
       it 'registers Google as a provider' do
         expect(Fog.providers.keys).to include(:google)
       end
-
-      describe '#fog_connection' do
-        it { expect(subject.fog_connection).to be_a_kind_of(Fog::Storage::GoogleXML::Real) }
-      end
     end
 
     context 'with Azure' do
@@ -82,10 +73,6 @@ RSpec.describe ObjectStorage::Config do
       it 'registers AzureRM as a provider' do
         expect(Fog.providers.keys).to include(:azurerm)
       end
-
-      describe '#fog_connection' do
-        it { expect(subject.fog_connection).to be_a_kind_of(Fog::Storage::AzureRM::Real) }
-      end
     end
   end
 
@@ -183,50 +170,6 @@ RSpec.describe ObjectStorage::Config do
     it { expect(subject.provider).to eq('AWS') }
     it { expect(subject.aws?).to be true }
     it { expect(subject.google?).to be false }
-
-    it 'returns the default S3 endpoint' do
-      subject.load_provider
-
-      expect(subject.s3_endpoint).to eq("https://test-bucket.s3.amazonaws.com")
-    end
-
-    describe 'with a custom endpoint' do
-      let(:endpoint) { 'https://my.example.com' }
-
-      before do
-        credentials[:endpoint] = endpoint
-      end
-
-      it 'returns the custom endpoint' do
-        subject.load_provider
-
-        expect(subject.s3_endpoint).to eq(endpoint)
-      end
-    end
-
-    context 'with custom S3 host and port' do
-      where(:host, :port, :scheme, :expected) do
-        's3.example.com' | 8080    | nil     | 'https://test-bucket.s3.example.com:8080'
-        's3.example.com' | 443     | nil     | 'https://test-bucket.s3.example.com'
-        's3.example.com' | 443     | "https" | 'https://test-bucket.s3.example.com'
-        's3.example.com' | nil     | nil     | 'https://test-bucket.s3.example.com'
-        's3.example.com' | 80      | "http"  | 'http://test-bucket.s3.example.com'
-        's3.example.com' | "bogus" | nil     | nil
-      end
-
-      with_them do
-        before do
-          credentials[:host] = host
-          credentials[:port] = port
-          credentials[:scheme] = scheme
-          subject.load_provider
-        end
-
-        it 'returns expected host' do
-          expect(subject.s3_endpoint).to eq(expected)
-        end
-      end
-    end
   end
 
   context 'with Google credentials' do