From 17f2052a71fdba57005cad8f0082ed7c5f752c2e Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Sat, 27 Jun 2020 16:49:28 -0700
Subject: [PATCH] Use S3 Workhorse client with consolidated object store
settings
In GitLab 13.1.0, we added an S3 client to Workhorse
(https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/466).
Previously this client would only be enabled if AWS instance profiles
(use_iam_profile) were used. We extend this functionality if the
consolidated object storage settings are enabled for AWS.
This will fix ETag Mismatch errors with non-AWS S3 providers and pave
the way for supporting encrypted S3 buckets with customer-provided keys.
Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/220288
---
app/uploaders/object_storage.rb | 6 +++++-
...sh-enable-workhorse-s3-client-consolidated.yml | 5 +++++
config/gitlab.yml.example | 1 -
config/object_store_settings.rb | 6 ++++--
lib/object_storage/direct_upload.rb | 15 +++++++++------
spec/lib/object_storage/direct_upload_spec.rb | 11 ++++++++++-
6 files changed, 33 insertions(+), 11 deletions(-)
create mode 100644 changelogs/unreleased/sh-enable-workhorse-s3-client-consolidated.yml
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index 5297112eef8d9..63b6197a04d45 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -169,6 +169,10 @@ def object_store_credentials
object_store_options.connection.to_hash.deep_symbolize_keys
end
+ def consolidated_settings?
+ object_store_options.fetch('consolidated_settings', false)
+ end
+
def remote_store_path
object_store_options.remote_directory
end
@@ -196,7 +200,7 @@ def workhorse_remote_upload_options(has_length:, maximum_size: nil)
id = [CarrierWave.generate_cache_id, SecureRandom.hex].join('-')
upload_path = File.join(TMP_UPLOAD_PATH, id)
direct_upload = ObjectStorage::DirectUpload.new(self.object_store_credentials, remote_store_path, upload_path,
- has_length: has_length, maximum_size: maximum_size)
+ has_length: has_length, maximum_size: maximum_size, consolidated_settings: consolidated_settings?)
direct_upload.to_hash.merge(ID: id)
end
diff --git a/changelogs/unreleased/sh-enable-workhorse-s3-client-consolidated.yml b/changelogs/unreleased/sh-enable-workhorse-s3-client-consolidated.yml
new file mode 100644
index 0000000000000..3289469af5f90
--- /dev/null
+++ b/changelogs/unreleased/sh-enable-workhorse-s3-client-consolidated.yml
@@ -0,0 +1,5 @@
+---
+title: Enable S3 Workhorse client if consolidated object settings used
+merge_request: 35480
+author:
+type: added
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index c6b4bcc36d803..dbbc946eaa737 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -210,7 +210,6 @@ production: &base
## within the types (e.g. artifacts, lfs, etc.).
# object_store:
# enabled: false
- # remote_directory: artifacts # The bucket name
# proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
# connection:
# provider: AWS # Only AWS supported at the moment
diff --git a/config/object_store_settings.rb b/config/object_store_settings.rb
index 0cd8fc98fb213..d8e1939a346c0 100644
--- a/config/object_store_settings.rb
+++ b/config/object_store_settings.rb
@@ -109,6 +109,7 @@ def parse!
# Map bucket (external name) -> remote_directory (internal representation)
target_config['remote_directory'] = target_config.delete('bucket')
+ target_config['consolidated_settings'] = true
section['object_store'] = target_config
end
end
@@ -120,7 +121,7 @@ def parse!
# 2. The legacy settings are not defined
def use_consolidated_settings?
return false unless settings.dig('object_store', 'enabled')
- return false unless settings.dig('object_store', 'connection')
+ return false unless settings.dig('object_store', 'connection').present?
SUPPORTED_TYPES.each do |store|
# to_h is needed because something strange happens to
@@ -135,7 +136,8 @@ def use_consolidated_settings?
next unless section
return false if section.dig('object_store', 'enabled')
- return false if section.dig('object_store', 'connection')
+ # Omnibus defaults to an empty hash
+ return false if section.dig('object_store', 'connection').present?
end
true
diff --git a/lib/object_storage/direct_upload.rb b/lib/object_storage/direct_upload.rb
index f973114e035d8..76f92f62e9c1d 100644
--- a/lib/object_storage/direct_upload.rb
+++ b/lib/object_storage/direct_upload.rb
@@ -23,9 +23,9 @@ class DirectUpload
MINIMUM_MULTIPART_SIZE = 5.megabytes
attr_reader :credentials, :bucket_name, :object_name
- attr_reader :has_length, :maximum_size
+ attr_reader :has_length, :maximum_size, :consolidated_settings
- def initialize(credentials, bucket_name, object_name, has_length:, maximum_size: nil)
+ def initialize(credentials, bucket_name, object_name, has_length:, maximum_size: nil, consolidated_settings: false)
unless has_length
raise ArgumentError, 'maximum_size has to be specified if length is unknown' unless maximum_size
end
@@ -35,6 +35,7 @@ def initialize(credentials, bucket_name, object_name, has_length:, maximum_size:
@object_name = object_name
@has_length = has_length
@maximum_size = maximum_size
+ @consolidated_settings = consolidated_settings
end
def to_hash
@@ -80,10 +81,12 @@ def workhorse_client_hash
end
def use_workhorse_s3_client?
- Feature.enabled?(:use_workhorse_s3_client, default_enabled: true) &&
- credentials.fetch(:use_iam_profile, false) &&
- # The Golang AWS SDK does not support V2 signatures
- credentials.fetch(:aws_signature_version, 4).to_i >= 4
+ return false unless Feature.enabled?(:use_workhorse_s3_client, default_enabled: true)
+ return false unless credentials.fetch(:use_iam_profile, false) || consolidated_settings
+ # The Golang AWS SDK does not support V2 signatures
+ return false unless credentials.fetch(:aws_signature_version, 4).to_i >= 4
+
+ true
end
def provider
diff --git a/spec/lib/object_storage/direct_upload_spec.rb b/spec/lib/object_storage/direct_upload_spec.rb
index e7d8b1de34ef0..1c1455e245604 100644
--- a/spec/lib/object_storage/direct_upload_spec.rb
+++ b/spec/lib/object_storage/direct_upload_spec.rb
@@ -6,6 +6,7 @@
let(:region) { 'us-east-1' }
let(:path_style) { false }
let(:use_iam_profile) { false }
+ let(:consolidated_settings) { false }
let(:credentials) do
{
provider: 'AWS',
@@ -23,7 +24,7 @@
let(:object_name) { 'tmp/uploads/my-file' }
let(:maximum_size) { 1.gigabyte }
- let(:direct_upload) { described_class.new(credentials, bucket_name, object_name, has_length: has_length, maximum_size: maximum_size) }
+ let(:direct_upload) { described_class.new(credentials, bucket_name, object_name, has_length: has_length, maximum_size: maximum_size, consolidated_settings: consolidated_settings) }
before do
Fog.unmock!
@@ -141,6 +142,14 @@
expect(subject[:UseWorkhorseClient]).to eq(use_iam_profile)
end
end
+
+ context 'when consolidated settings are used' do
+ let(:consolidated_settings) { true }
+
+ it 'enables the Workhorse client' do
+ expect(subject[:UseWorkhorseClient]).to be true
+ end
+ end
end
shared_examples 'a valid Google upload' do
--
GitLab