Merge branch 'direct-upload-of-uploads' into 'master'

Allow to store uploads by default on Object Storage

See merge request gitlab-org/gitlab-ce!18156

app/uploaders/object_storage.rb

@@ -128,7 +128,7 @@ module ObjectStorage
       end
 
       def direct_upload_enabled?
-        object_store_options.direct_upload
+        object_store_options&.direct_upload
       end
 
       def background_upload_enabled?
@@ -184,6 +184,14 @@ module ObjectStorage
           StoreURL: connection.put_object_url(remote_store_path, upload_path, expire_at, options)
         }
       end
+
+      def default_object_store
+        if self.object_store_enabled? && self.direct_upload_enabled?
+          Store::REMOTE
+        else
+          Store::LOCAL
+        end
+      end
     end
 
     # allow to configure and overwrite the filename
@@ -204,12 +212,12 @@ module ObjectStorage
     end
 
     def object_store
-      @object_store ||= model.try(store_serialization_column) || Store::LOCAL
+      @object_store ||= model.try(store_serialization_column) || self.class.default_object_store
     end
 
     # rubocop:disable Gitlab/ModuleWithInstanceVariables
     def object_store=(value)
-      @object_store = value || Store::LOCAL
+      @object_store = value || self.class.default_object_store
       @storage = storage_for(object_store)
     end
     # rubocop:enable Gitlab/ModuleWithInstanceVariables

changelogs/unreleased/direct-upload-of-uploads.yml

+---
+title: Allow to store uploads by default on Object Storage
+merge_request:
+author:
+type: added

config/gitlab.yml.example

@@ -154,7 +154,7 @@ production: &base
     #     provider: AWS # Only AWS supported at the moment
     #     aws_access_key_id: AWS_ACCESS_KEY_ID
     #     aws_secret_access_key: AWS_SECRET_ACCESS_KEY
-    #     region: eu-central-1
+    #     region: us-east-1
 
   ## Git LFS
   lfs:
@@ -164,13 +164,14 @@ production: &base
     object_store:
       enabled: false
       remote_directory: lfs-objects # Bucket name
+      # direct_upload: false # Use Object Storage directly for uploads instead of background uploads if enabled (Default: false)
       # background_upload: false # Temporary option to limit automatic upload (Default: true)
       # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
       connection:
         provider: AWS
         aws_access_key_id: AWS_ACCESS_KEY_ID
         aws_secret_access_key: AWS_SECRET_ACCESS_KEY
-        region: eu-central-1
+        region: us-east-1
         # Use the following options to configure an AWS compatible host
         # host: 'localhost' # default: s3.amazonaws.com
         # endpoint: 'http://127.0.0.1:9000' # default: nil
@@ -184,13 +185,14 @@ production: &base
     object_store:
       enabled: false
       # remote_directory: uploads # Bucket name
+      # direct_upload: false # Use Object Storage directly for uploads instead of background uploads if enabled (Default: false)
       # background_upload: false # Temporary option to limit automatic upload (Default: true)
       # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
-    # connection:
-    #   provider: AWS
-    #   aws_access_key_id: AWS_ACCESS_KEY_ID
-    #   aws_secret_access_key: AWS_SECRET_ACCESS_KEY
-    #   region: eu-central-1
+    connection:
+      provider: AWS
+      aws_access_key_id: AWS_ACCESS_KEY_ID
+      aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+      region: us-east-1
       # host: 'localhost' # default: s3.amazonaws.com
       # endpoint: 'http://127.0.0.1:9000' # default: nil
       # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'

config/initializers/1_settings.rb

@@ -365,6 +365,7 @@ Settings.uploads['base_dir'] = Settings.uploads['base_dir'] || 'uploads/-/system
 Settings.uploads['object_store'] ||= Settingslogic.new({})
 Settings.uploads['object_store']['enabled'] = false if Settings.uploads['object_store']['enabled'].nil?
 Settings.uploads['object_store']['remote_directory'] ||= 'uploads'
+Settings.uploads['object_store']['direct_upload'] = false if Settings.uploads['object_store']['direct_upload'].nil?
 Settings.uploads['object_store']['background_upload'] = true if Settings.uploads['object_store']['background_upload'].nil?
 Settings.uploads['object_store']['proxy_download'] = false if Settings.uploads['object_store']['proxy_download'].nil?
 # Convert upload connection settings to use string keys, to make Fog happy

doc/administration/uploads.md

@@ -65,6 +65,7 @@ For source installations the following settings are nested under `uploads:` and
 |---------|-------------|---------|
 | `enabled` | Enable/disable object storage | `false` |
 | `remote_directory` | The bucket name where Uploads will be stored| |
+| `direct_upload` | Set to true to enable direct upload of Uploads without the need of local shared storage. Option may be removed once we decide to support only single storage for all files. This is beta option as it uses inefficient way of uploading data (via Unicorn). The accelerated uploads gonna be implemented in future releases | `false` |
 | `background_upload` | Set to false to disable automatic upload. Option may be removed once upload is direct to S3 | `true` |
 | `proxy_download` | Set to true to enable proxying all files served. Option allows to reduce egress traffic as this allows clients to download directly from remote storage instead of proxying all data | `false` |
 | `connection` | Various connection options described below | |

spec/uploaders/object_storage_spec.rb

@@ -62,10 +62,12 @@ describe ObjectStorage do
   end
 
   describe '#object_store' do
+    subject { uploader.object_store }
+
     it "delegates to <mount>_store on model" do
       expect(object).to receive(:file_store)
 
-      uploader.object_store
+      subject
     end
 
     context 'when store is null' do
@@ -73,8 +75,36 @@ describe ObjectStorage do
         expect(object).to receive(:file_store).and_return(nil)
       end
 
-      it "returns Store::LOCAL" do
-        expect(uploader.object_store).to eq(described_class::Store::LOCAL)
+      context 'when object storage is enabled' do
+        context 'when direct uploads are enabled' do
+          before do
+            stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: true)
+          end
+
+          it "uses Store::REMOTE" do
+            is_expected.to eq(described_class::Store::REMOTE)
+          end
+        end
+
+        context 'when direct uploads are disabled' do
+          before do
+            stub_uploads_object_storage(uploader_class, enabled: true, direct_upload: false)
+          end
+
+          it "uses Store::LOCAL" do
+            is_expected.to eq(described_class::Store::LOCAL)
+          end
+        end
+      end
+
+      context 'when object storage is disabled' do
+        before do
+          stub_uploads_object_storage(uploader_class, enabled: false)
+        end
+
+        it "uses Store::LOCAL" do
+          is_expected.to eq(described_class::Store::LOCAL)
+        end
       end
     end
 
@@ -84,7 +114,7 @@ describe ObjectStorage do
       end
 
       it "returns the given value" do
-        expect(uploader.object_store).to eq(described_class::Store::REMOTE)
+        is_expected.to eq(described_class::Store::REMOTE)
       end
     end
   end