From f69e33d77ef3fa980979b56684c566524de8ee92 Mon Sep 17 00:00:00 2001
From: Erick Bajao <fbajao@gitlab.com>
Date: Tue, 14 Mar 2023 11:15:30 +0000
Subject: [PATCH] Resolve object storage config via symbol
This adds support for referencing storage config
via symbols.
---
.../ci/pipeline_artifact_uploader.rb | 2 +-
app/uploaders/ci/secure_file_uploader.rb | 2 +-
app/uploaders/deleted_object_uploader.rb | 2 +-
.../dependency_proxy/file_uploader.rb | 2 +-
app/uploaders/external_diff_uploader.rb | 2 +-
app/uploaders/gitlab_uploader.rb | 17 +++-
app/uploaders/job_artifact_uploader.rb | 2 +-
app/uploaders/lfs_object_uploader.rb | 2 +-
.../packages/composer/cache_uploader.rb | 2 +-
.../debian/component_file_uploader.rb | 2 +-
.../distribution_release_file_uploader.rb | 2 +-
.../packages/package_file_uploader.rb | 2 +-
.../packages/rpm/repository_file_uploader.rb | 2 +-
app/uploaders/pages/deployment_uploader.rb | 2 +-
app/uploaders/terraform/state_uploader.rb | 2 +-
lib/object_storage/config.rb | 12 +++
.../concerns/send_file_upload_spec.rb | 2 +-
spec/lib/object_storage/config_spec.rb | 2 +-
spec/uploaders/gitlab_uploader_spec.rb | 17 +++-
spec/uploaders/object_storage/cdn_spec.rb | 88 +++++++++----------
spec/uploaders/object_storage_spec.rb | 2 +-
21 files changed, 101 insertions(+), 67 deletions(-)
diff --git a/app/uploaders/ci/pipeline_artifact_uploader.rb b/app/uploaders/ci/pipeline_artifact_uploader.rb
index d3a83c5d633a3..62e00fe1a663b 100644
--- a/app/uploaders/ci/pipeline_artifact_uploader.rb
+++ b/app/uploaders/ci/pipeline_artifact_uploader.rb
@@ -4,7 +4,7 @@ module Ci
class PipelineArtifactUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.artifacts
+ storage_location :artifacts
alias_method :upload, :model
diff --git a/app/uploaders/ci/secure_file_uploader.rb b/app/uploaders/ci/secure_file_uploader.rb
index 11cbfc6c1f210..09d9b3abafb8b 100644
--- a/app/uploaders/ci/secure_file_uploader.rb
+++ b/app/uploaders/ci/secure_file_uploader.rb
@@ -4,7 +4,7 @@ module Ci
class SecureFileUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.ci_secure_files
+ storage_location :ci_secure_files
# Use Lockbox to encrypt/decrypt the stored file (registers CarrierWave callbacks)
encrypt(key: :key)
diff --git a/app/uploaders/deleted_object_uploader.rb b/app/uploaders/deleted_object_uploader.rb
index fc0f62b920c74..eaf584c5dfa5b 100644
--- a/app/uploaders/deleted_object_uploader.rb
+++ b/app/uploaders/deleted_object_uploader.rb
@@ -3,7 +3,7 @@
class DeletedObjectUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.artifacts
+ storage_location :artifacts
def store_dir
model.store_dir
diff --git a/app/uploaders/dependency_proxy/file_uploader.rb b/app/uploaders/dependency_proxy/file_uploader.rb
index f0222d4cf0693..d4e486bfe8427 100644
--- a/app/uploaders/dependency_proxy/file_uploader.rb
+++ b/app/uploaders/dependency_proxy/file_uploader.rb
@@ -5,7 +5,7 @@ class DependencyProxy::FileUploader < GitlabUploader
include ObjectStorage::Concern
before :cache, :set_content_type
- storage_options Gitlab.config.dependency_proxy
+ storage_location :dependency_proxy
alias_method :upload, :model
diff --git a/app/uploaders/external_diff_uploader.rb b/app/uploaders/external_diff_uploader.rb
index d2707cd07774c..86c3d73417424 100644
--- a/app/uploaders/external_diff_uploader.rb
+++ b/app/uploaders/external_diff_uploader.rb
@@ -3,7 +3,7 @@
class ExternalDiffUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.external_diffs
+ storage_location :external_diffs
alias_method :upload, :model
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 62024bff4c080..2eb34288bd71b 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -3,7 +3,7 @@
class GitlabUploader < CarrierWave::Uploader::Base
include ContentTypeWhitelist::Concern
- class_attribute :options
+ class_attribute :storage_location_identifier
PROTECTED_METHODS = %i(filename cache_dir work_dir store_dir).freeze
@@ -11,8 +11,13 @@ class GitlabUploader < CarrierWave::Uploader::Base
class << self
# DSL setter
- def storage_options(options)
- self.options = options
+ def storage_location(location)
+ self.storage_location_identifier = location
+ _ = options # Ensures that we have a valid storage_location_identifier
+ end
+
+ def options
+ ObjectStorage::Config::LOCATIONS.fetch(storage_location_identifier)
end
def root
@@ -41,7 +46,7 @@ def version(*args, **kwargs, &block)
end
end
- storage_options Gitlab.config.uploads
+ storage_location :uploads
delegate :base_dir, :file_storage?, to: :class
@@ -51,6 +56,10 @@ def initialize(model, mounted_as = nil, **uploader_context)
super(model, mounted_as)
end
+ def options
+ self.class.options
+ end
+
def file_cache_storage?
cache_storage.is_a?(CarrierWave::Storage::File)
end
diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
index b38e7d93eac8a..5ee8c42f51024 100644
--- a/app/uploaders/job_artifact_uploader.rb
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -7,7 +7,7 @@ class JobArtifactUploader < GitlabUploader
UnknownFileLocationError = Class.new(StandardError)
- storage_options Gitlab.config.artifacts
+ storage_location :artifacts
alias_method :upload, :model
diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb
index 027857500f4da..4111bb923226d 100644
--- a/app/uploaders/lfs_object_uploader.rb
+++ b/app/uploaders/lfs_object_uploader.rb
@@ -4,7 +4,7 @@ class LfsObjectUploader < GitlabUploader
extend Workhorse::UploadPath
include ObjectStorage::Concern
- storage_options Gitlab.config.lfs
+ storage_location :lfs
alias_method :upload, :model
diff --git a/app/uploaders/packages/composer/cache_uploader.rb b/app/uploaders/packages/composer/cache_uploader.rb
index ad7c017c4ba21..ef581b5d6a1f6 100644
--- a/app/uploaders/packages/composer/cache_uploader.rb
+++ b/app/uploaders/packages/composer/cache_uploader.rb
@@ -2,7 +2,7 @@
class Packages::Composer::CacheUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.packages
+ storage_location :packages
alias_method :upload, :model
diff --git a/app/uploaders/packages/debian/component_file_uploader.rb b/app/uploaders/packages/debian/component_file_uploader.rb
index 2de4743d7f746..b1ed8d853f16d 100644
--- a/app/uploaders/packages/debian/component_file_uploader.rb
+++ b/app/uploaders/packages/debian/component_file_uploader.rb
@@ -3,7 +3,7 @@ class Packages::Debian::ComponentFileUploader < GitlabUploader
extend Workhorse::UploadPath
include ObjectStorage::Concern
- storage_options Gitlab.config.packages
+ storage_location :packages
alias_method :upload, :model
diff --git a/app/uploaders/packages/debian/distribution_release_file_uploader.rb b/app/uploaders/packages/debian/distribution_release_file_uploader.rb
index 268d42796e9a6..fe10861b77f02 100644
--- a/app/uploaders/packages/debian/distribution_release_file_uploader.rb
+++ b/app/uploaders/packages/debian/distribution_release_file_uploader.rb
@@ -3,7 +3,7 @@ class Packages::Debian::DistributionReleaseFileUploader < GitlabUploader
extend Workhorse::UploadPath
include ObjectStorage::Concern
- storage_options Gitlab.config.packages
+ storage_location :packages
alias_method :upload, :model
diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb
index c8a09c50dc6a7..57feee9f19d8f 100644
--- a/app/uploaders/packages/package_file_uploader.rb
+++ b/app/uploaders/packages/package_file_uploader.rb
@@ -3,7 +3,7 @@ class Packages::PackageFileUploader < GitlabUploader
extend Workhorse::UploadPath
include ObjectStorage::Concern
- storage_options Gitlab.config.packages
+ storage_location :packages
alias_method :upload, :model
diff --git a/app/uploaders/packages/rpm/repository_file_uploader.rb b/app/uploaders/packages/rpm/repository_file_uploader.rb
index f95f861585c72..399e9fa07d511 100644
--- a/app/uploaders/packages/rpm/repository_file_uploader.rb
+++ b/app/uploaders/packages/rpm/repository_file_uploader.rb
@@ -4,7 +4,7 @@ module Rpm
class RepositoryFileUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.packages
+ storage_location :packages
alias_method :upload, :model
diff --git a/app/uploaders/pages/deployment_uploader.rb b/app/uploaders/pages/deployment_uploader.rb
index c5ba65673ab8d..bb4f1a2235df1 100644
--- a/app/uploaders/pages/deployment_uploader.rb
+++ b/app/uploaders/pages/deployment_uploader.rb
@@ -4,7 +4,7 @@ module Pages
class DeploymentUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.pages
+ storage_location :pages
alias_method :upload, :model
diff --git a/app/uploaders/terraform/state_uploader.rb b/app/uploaders/terraform/state_uploader.rb
index 61e7ed7b0e656..5fe3048f7b0cf 100644
--- a/app/uploaders/terraform/state_uploader.rb
+++ b/app/uploaders/terraform/state_uploader.rb
@@ -4,7 +4,7 @@ module Terraform
class StateUploader < GitlabUploader
include ObjectStorage::Concern
- storage_options Gitlab.config.terraform_state
+ storage_location :terraform_state
# TODO: Remove this line
# See https://gitlab.com/gitlab-org/gitlab/-/issues/232917
diff --git a/lib/object_storage/config.rb b/lib/object_storage/config.rb
index fb0334959a3f6..8a2044a8cba31 100644
--- a/lib/object_storage/config.rb
+++ b/lib/object_storage/config.rb
@@ -6,6 +6,18 @@ class Config
AZURE_PROVIDER = 'AzureRM'
GOOGLE_PROVIDER = 'Google'
+ LOCATIONS = {
+ artifacts: Gitlab.config.artifacts,
+ ci_secure_files: Gitlab.config.ci_secure_files,
+ dependency_proxy: Gitlab.config.dependency_proxy,
+ external_diffs: Gitlab.config.external_diffs,
+ lfs: Gitlab.config.lfs,
+ packages: Gitlab.config.packages,
+ pages: Gitlab.config.pages,
+ terraform_state: Gitlab.config.terraform_state,
+ uploads: Gitlab.config.uploads
+ }.freeze
+
attr_reader :options
def initialize(options)
diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb
index 6acbff6e74549..3fe6d62329ed7 100644
--- a/spec/controllers/concerns/send_file_upload_spec.rb
+++ b/spec/controllers/concerns/send_file_upload_spec.rb
@@ -7,7 +7,7 @@
Class.new(GitlabUploader) do
include ObjectStorage::Concern
- storage_options Gitlab.config.uploads
+ storage_location :uploads
private
diff --git a/spec/lib/object_storage/config_spec.rb b/spec/lib/object_storage/config_spec.rb
index 3099468c07d5f..412fcb9b6b878 100644
--- a/spec/lib/object_storage/config_spec.rb
+++ b/spec/lib/object_storage/config_spec.rb
@@ -4,7 +4,7 @@
require 'rspec-parameterized'
require 'fog/core'
-RSpec.describe ObjectStorage::Config do
+RSpec.describe ObjectStorage::Config, feature_category: :shared do
using RSpec::Parameterized::TableSyntax
let(:region) { 'us-east-1' }
diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb
index f62ab7266319f..bd86f1fe08aca 100644
--- a/spec/uploaders/gitlab_uploader_spec.rb
+++ b/spec/uploaders/gitlab_uploader_spec.rb
@@ -3,7 +3,7 @@
require 'spec_helper'
require 'carrierwave/storage/fog'
-RSpec.describe GitlabUploader do
+RSpec.describe GitlabUploader, feature_category: :shared do
let(:uploader_class) { Class.new(described_class) }
subject(:uploader) { uploader_class.new(double) }
@@ -179,4 +179,19 @@
it { expect { subject }.to raise_error(RuntimeError, /not supported/) }
end
+
+ describe '.storage_location' do
+ it 'sets the identifier for the storage location options' do
+ uploader_class.storage_location(:artifacts)
+
+ expect(uploader_class.options).to eq(Gitlab.config.artifacts)
+ end
+
+ context 'when given identifier is not known' do
+ it 'raises an error' do
+ expect { uploader_class.storage_location(:foo) }
+ .to raise_error(KeyError)
+ end
+ end
+ end
end
diff --git a/spec/uploaders/object_storage/cdn_spec.rb b/spec/uploaders/object_storage/cdn_spec.rb
index d6c638297fac5..0c1966b4df24a 100644
--- a/spec/uploaders/object_storage/cdn_spec.rb
+++ b/spec/uploaders/object_storage/cdn_spec.rb
@@ -3,19 +3,6 @@
require 'spec_helper'
RSpec.describe ObjectStorage::CDN, feature_category: :build_artifacts do
- let(:cdn_options) do
- {
- 'object_store' => {
- 'cdn' => {
- 'provider' => 'google',
- 'url' => 'https://gitlab.example.com',
- 'key_name' => 'test-key',
- 'key' => Base64.urlsafe_encode64('12345')
- }
- }
- }.freeze
- end
-
let(:uploader_class) do
Class.new(GitlabUploader) do
include ObjectStorage::Concern
@@ -39,44 +26,66 @@ def dynamic_segment
subject { uploader_class.new(object, :file) }
context 'with CDN config' do
+ let(:cdn_options) do
+ {
+ 'object_store' => {
+ 'cdn' => {
+ 'provider' => cdn_provider,
+ 'url' => 'https://gitlab.example.com',
+ 'key_name' => 'test-key',
+ 'key' => Base64.urlsafe_encode64('12345')
+ }
+ }
+ }.freeze
+ end
+
before do
stub_artifacts_object_storage(enabled: true)
- uploader_class.options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options))
+ options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options))
+ allow(uploader_class).to receive(:options).and_return(options)
end
- describe '#cdn_enabled_url' do
- it 'calls #cdn_signed_url' do
- expect(subject).not_to receive(:url)
- expect(subject).to receive(:cdn_signed_url).with(query_params).and_call_original
+ context 'with a known CDN provider' do
+ let(:cdn_provider) { 'google' }
- result = subject.cdn_enabled_url(public_ip, query_params)
+ describe '#cdn_enabled_url' do
+ it 'calls #cdn_signed_url' do
+ expect(subject).not_to receive(:url)
+ expect(subject).to receive(:cdn_signed_url).with(query_params).and_call_original
+
+ result = subject.cdn_enabled_url(public_ip, query_params)
- expect(result.used_cdn).to be true
+ expect(result.used_cdn).to be true
+ end
end
- end
- describe '#use_cdn?' do
- it 'returns true' do
- expect(subject.use_cdn?(public_ip)).to be true
+ describe '#use_cdn?' do
+ it 'returns true' do
+ expect(subject.use_cdn?(public_ip)).to be true
+ end
end
- end
- describe '#cdn_signed_url' do
- it 'returns a URL' do
- expect_next_instance_of(ObjectStorage::CDN::GoogleCDN) do |cdn|
- expect(cdn).to receive(:signed_url).and_return("https://cdn.example.com/path")
+ describe '#cdn_signed_url' do
+ it 'returns a URL' do
+ expect_next_instance_of(ObjectStorage::CDN::GoogleCDN) do |cdn|
+ expect(cdn).to receive(:signed_url).and_return("https://cdn.example.com/path")
+ end
+
+ expect(subject.cdn_signed_url).to eq("https://cdn.example.com/path")
end
+ end
+ end
+
+ context 'with an unknown CDN provider' do
+ let(:cdn_provider) { 'amazon' }
- expect(subject.cdn_signed_url).to eq("https://cdn.example.com/path")
+ it 'raises an error' do
+ expect { subject.use_cdn?(public_ip) }.to raise_error("Unknown CDN provider: amazon")
end
end
end
context 'without CDN config' do
- before do
- uploader_class.options = Gitlab.config.uploads
- end
-
describe '#cdn_enabled_url' do
it 'calls #url' do
expect(subject).not_to receive(:cdn_signed_url)
@@ -94,15 +103,4 @@ def dynamic_segment
end
end
end
-
- context 'with an unknown CDN provider' do
- before do
- cdn_options['object_store']['cdn']['provider'] = 'amazon'
- uploader_class.options = Settingslogic.new(Gitlab.config.uploads.deep_merge(cdn_options))
- end
-
- it 'raises an error' do
- expect { subject.use_cdn?(public_ip) }.to raise_error("Unknown CDN provider: amazon")
- end
- end
end
diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb
index 5344dbeb51229..0e293ec973cd4 100644
--- a/spec/uploaders/object_storage_spec.rb
+++ b/spec/uploaders/object_storage_spec.rb
@@ -8,7 +8,7 @@ class Implementation < GitlabUploader
include ::RecordsUploads::Concern
prepend ::ObjectStorage::Extension::RecordsUploads
- storage_options Gitlab.config.uploads
+ storage_location :uploads
private
--
GitLab