From 0b4748cc2f44617eec88f24ff72ede2d6977d401 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Wed, 20 Aug 2025 18:28:55 -0300 Subject: [PATCH 01/10] Adds GCP bucket config for SQL traffic replay It adds the initial configuration to store sql data for traffic replay Changelog: changed --- config/gitlab.yml.example | 10 +++++ config/initializers/1_settings.rb | 10 +++++ lib/gitlab/database/capture/storage.rb | 39 +++++++++++++++++++ .../capture/storage_connectors/gcp.rb | 30 ++++++++++++++ .../capture/storage_connectors/mock.rb | 32 +++++++++++++++ 5 files changed, 121 insertions(+) create mode 100644 lib/gitlab/database/capture/storage.rb create mode 100644 lib/gitlab/database/capture/storage_connectors/gcp.rb create mode 100644 lib/gitlab/database/capture/storage_connectors/mock.rb diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index bda4f90e7e91ff..a4bfdee4efef51 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1315,6 +1315,16 @@ production: &base # Default is '.gitlab_workhorse_secret' relative to Rails.root (i.e. root of the GitLab app). # secret_file: /home/git/gitlab/.gitlab_workhorse_secret + database_traffic_capture: + enabled: false + config: + storage: + connector: + provider: Google + google_project: 'Project Name' + google_json_key_location: '/path/to/gcp-project.json' + google_storage_bucket_name: 'my-bucket' + cell: # enabled: false # id: null diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index a2ff67a533f5ee..e5f652e01fb5f3 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -1138,6 +1138,16 @@ Settings['workhorse'] ||= {} Settings.workhorse['secret_file'] ||= Rails.root.join('.gitlab_workhorse_secret') +# +# Database Traffic Capture Settings +# + +Settings['database_traffic_capture'] ||= {} +Settings['database_traffic_capture']['enabled'] = false +Settings.database_traffic_capture['config'] ||= {} +Settings.database_traffic_capture.config['storage'] ||= {} +Settings.database_traffic_capture.config.storage['connector'] ||= {} + # # Cells # diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb new file mode 100644 index 00000000000000..c333ae245c3e35 --- /dev/null +++ b/lib/gitlab/database/capture/storage.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Capture + class Storage + CONNECTORS = { + 'Google' => StorageConnectors::Gcp + }.freeze + + def upload(filename, data) + log("Upload request for database capture", filename) + start_monotonic_time = ::Gitlab::Metrics::System.monotonic_time + + CONNECTORS.fetch(connector_provider, StorageConnectors::Mock).new(connector_settings).upload(filename, data) + + duration_s = ::Gitlab::Metrics::System.monotonic_time - start_monotonic_time + log("Database capture upload completed", filename, duration_s) + end + + private + + def connector_provider + connector_settings.try(:provider) || 'LocalStorage' + end + + def connector_settings + Settings.database_traffic_capture.config.storage.connector + end + + def log(message, filename, duration = nil) + info = { message: message, connector: connector_provider, filename: filename, duration: duration } + + Gitlab::AppLogger.info(info.compact) + end + end + end + end +end diff --git a/lib/gitlab/database/capture/storage_connectors/gcp.rb b/lib/gitlab/database/capture/storage_connectors/gcp.rb new file mode 100644 index 00000000000000..f80aa4b472cdfa --- /dev/null +++ b/lib/gitlab/database/capture/storage_connectors/gcp.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Capture + module StorageConnectors + class Gcp + def initialize(settings) + @settings = settings + end + + def upload(filename, data) + client.put_object(settings.google_storage_bucket_name, filename, data) + end + + private + + attr_reader :settings + + def client + @client ||= Fog::Storage::Google.new( + google_project: settings.google_project, + google_json_key_location: settings.google_json_key_location + ) + end + end + end + end + end +end diff --git a/lib/gitlab/database/capture/storage_connectors/mock.rb b/lib/gitlab/database/capture/storage_connectors/mock.rb new file mode 100644 index 00000000000000..aa0e3745e17627 --- /dev/null +++ b/lib/gitlab/database/capture/storage_connectors/mock.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Capture + module StorageConnectors + # To simplify testing and development + class Mock + def initialize(settings) + @settings = settings + end + + def upload(filename, data) + path = File.join(tmp_dir, File.basename(filename)) + + FileUtils.mkdir_p(tmp_dir) + + File.open(path, 'w') do |file| + file.write(data) + end + end + + private + + def tmp_dir + @tmp_dir ||= Rails.root.join('tmp/database-traffic-capture/storage') + end + end + end + end + end +end -- GitLab From 1d620425f673a4a64d013429ed74e533426f4aa4 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Thu, 4 Sep 2025 19:19:43 -0300 Subject: [PATCH 02/10] Updates upload class to use GCS gem --- config/gitlab.yml.example | 7 +- lib/gitlab/database/capture/storage.rb | 4 +- .../capture/storage_connectors/gcp.rb | 30 ------ .../capture/storage_connectors/gcs.rb | 44 +++++++++ .../capture/storage_connectors/mock.rb | 20 ++-- .../capture/storage_connectors/gcs_spec.rb | 54 +++++++++++ .../gitlab/database/capture/storage_spec.rb | 94 +++++++++++++++++++ 7 files changed, 208 insertions(+), 45 deletions(-) delete mode 100644 lib/gitlab/database/capture/storage_connectors/gcp.rb create mode 100644 lib/gitlab/database/capture/storage_connectors/gcs.rb create mode 100644 spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb create mode 100644 spec/lib/gitlab/database/capture/storage_spec.rb diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index a4bfdee4efef51..700a17812fe772 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1316,14 +1316,13 @@ production: &base # secret_file: /home/git/gitlab/.gitlab_workhorse_secret database_traffic_capture: - enabled: false config: storage: connector: provider: Google - google_project: 'Project Name' - google_json_key_location: '/path/to/gcp-project.json' - google_storage_bucket_name: 'my-bucket' + project_id: 'my-project' + credentials: '/path/to/keyfile.json' + bucket: 'my-bucket' cell: # enabled: false diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb index c333ae245c3e35..73828e0004e4da 100644 --- a/lib/gitlab/database/capture/storage.rb +++ b/lib/gitlab/database/capture/storage.rb @@ -5,7 +5,7 @@ module Database module Capture class Storage CONNECTORS = { - 'Google' => StorageConnectors::Gcp + 'Google' => StorageConnectors::Gcs }.freeze def upload(filename, data) @@ -21,7 +21,7 @@ def upload(filename, data) private def connector_provider - connector_settings.try(:provider) || 'LocalStorage' + connector_settings.try(:provider) || 'StorageConnectors::Mock' end def connector_settings diff --git a/lib/gitlab/database/capture/storage_connectors/gcp.rb b/lib/gitlab/database/capture/storage_connectors/gcp.rb deleted file mode 100644 index f80aa4b472cdfa..00000000000000 --- a/lib/gitlab/database/capture/storage_connectors/gcp.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Database - module Capture - module StorageConnectors - class Gcp - def initialize(settings) - @settings = settings - end - - def upload(filename, data) - client.put_object(settings.google_storage_bucket_name, filename, data) - end - - private - - attr_reader :settings - - def client - @client ||= Fog::Storage::Google.new( - google_project: settings.google_project, - google_json_key_location: settings.google_json_key_location - ) - end - end - end - end - end -end diff --git a/lib/gitlab/database/capture/storage_connectors/gcs.rb b/lib/gitlab/database/capture/storage_connectors/gcs.rb new file mode 100644 index 00000000000000..ab5e86a5149cac --- /dev/null +++ b/lib/gitlab/database/capture/storage_connectors/gcs.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'google/cloud/storage' + +module Gitlab + module Database + module Capture + module StorageConnectors + # Google Cloud Storage Connector + class Gcs + def initialize(settings) + @settings = settings + end + + # We have to scape "/" from the filename to avoid gcp to interpret as a subfolder. This can be a problem if we + # use the primary write location compose the filename, which can include an address like +"1F/4BE69098"+. + def upload(filename, data) + bucket.create_file( + StringIO.new(data), + CGI.escape(filename), + metadata: { + original_filename: filename, + encoded: true + } + ) + end + + private + + attr_reader :settings + + def client + @client ||= Google::Cloud::Storage.new(project_id: settings.project_id, credentials: settings.credentials) + end + + # Permission 'storage.buckets.get' must be granted to access to the Google Cloud Storage bucket + def bucket + @bucket ||= client.bucket(settings.bucket) + end + end + end + end + end +end diff --git a/lib/gitlab/database/capture/storage_connectors/mock.rb b/lib/gitlab/database/capture/storage_connectors/mock.rb index aa0e3745e17627..fb9752a343e9c9 100644 --- a/lib/gitlab/database/capture/storage_connectors/mock.rb +++ b/lib/gitlab/database/capture/storage_connectors/mock.rb @@ -6,24 +6,26 @@ module Capture module StorageConnectors # To simplify testing and development class Mock + TEMP_PATH = Rails.root.join('tmp/database-traffic-capture/storage') + def initialize(settings) @settings = settings end def upload(filename, data) - path = File.join(tmp_dir, File.basename(filename)) + return unless Rails.env.development? || Rails.env.test? - FileUtils.mkdir_p(tmp_dir) + Gitlab::PathTraversal.check_path_traversal!(filename) - File.open(path, 'w') do |file| - file.write(data) - end - end + filepath = Rails.root.join(TEMP_PATH, File.basename(filename)) - private + Gitlab::PathTraversal.check_allowed_absolute_path!(File.dirname(filepath), [TEMP_PATH.to_s]) - def tmp_dir - @tmp_dir ||= Rails.root.join('tmp/database-traffic-capture/storage') + FileUtils.mkdir_p(File.dirname(filepath)) + + File.open(filepath, 'w') do |file| + file.write(data) + end end end end diff --git a/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb b/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb new file mode 100644 index 00000000000000..1764a76d4af47b --- /dev/null +++ b/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Capture::StorageConnectors::Gcs, feature_category: :database do + let(:connector) { described_class.new(settings) } + let(:client) { instance_double(Google::Cloud::Storage::Project) } + let(:bucket) { instance_double(Google::Cloud::Storage::Bucket) } + let(:settings) do + GitlabSettings::Options.build( + provider: 'Google', + project_id: 'my-project', + credentials: '/path/to/keyfile.json', + bucket: 'my-bucket' + ) + end + + let(:data) do + <<~NDJSON + {"id": 1, "sql": "SELECT 1 FROM \"public\".\"users\" LIMIT 1;"} + {"id": 2, "sql": "SELECT * FROM \"public\".\"projects\" WHERE \"projects\".\"id\" = 1;"} + {"id": 3, "sql": "DELETE FROM \"public\".\"users\" WHERE \"users\".\"id\" = 1;"} + NDJSON + end + + before do + allow(Google::Cloud::Storage).to receive(:new).and_return(client) + allow(client).to receive(:bucket).and_return(bucket) + end + + describe '#upload' do + let(:filename) { "v1-main-cadf8f5a--1F/4BEAABE0" } + let(:encoded_filename) { "v1-main-cadf8f5a--1F%2F4BEAABE0" } + let(:metadata) { { original_filename: filename, encoded: true } } + + it 'uploads data to the specified bucket' do + expect(bucket).to receive(:create_file).with(instance_of(StringIO), encoded_filename, metadata: metadata) + + connector.upload(filename, data) + end + + context 'when upload fails' do + let(:error) { ArgumentError.new('Upload failed') } + + before do + allow(bucket).to receive(:create_file).and_raise(error) + end + + it 'propagates the error' do + expect { connector.upload(filename, data) }.to raise_error(error) + end + end + end +end diff --git a/spec/lib/gitlab/database/capture/storage_spec.rb b/spec/lib/gitlab/database/capture/storage_spec.rb new file mode 100644 index 00000000000000..6916281c0db84d --- /dev/null +++ b/spec/lib/gitlab/database/capture/storage_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Capture::Storage, feature_category: :database do + let(:filename) { 'wal_capture' } + let(:storage) { described_class.new } + let(:mock_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Mock) } + let(:gcs_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Gcs) } + let(:configured_settings) { {} } + let(:data) do + <<~NDJSON + {"id": 1, "sql": "SELECT 1 FROM \"public\".\"users\" LIMIT 1;"} + {"id": 2, "sql": "SELECT * FROM \"public\".\"projects\" WHERE \"projects\".\"id\" = 1;"} + {"id": 3, "sql": "DELETE FROM \"public\".\"users\" WHERE \"users\".\"id\" = 1;"} + NDJSON + end + + before do + allow(Gitlab::AppLogger).to receive(:info) + allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(0.0, 1.5) + + allow(Settings).to( + receive_message_chain(:database_traffic_capture, :config, :storage, :connector).and_return( + GitlabSettings::Options.build(configured_settings) + ) + ) + end + + describe '#upload' do + context 'when using Google connector' do + let(:configured_settings) do + { + provider: 'Google', + project_id: 'my-project', + credentials: '/path/to/keyfile.json', + bucket: 'my-bucket' + } + end + + before do + allow(Gitlab::Database::Capture::StorageConnectors::Gcs).to receive(:new).and_return(gcs_connector) + allow(gcs_connector).to receive(:upload) + end + + it 'uses the GCS connector' do + expect(Gitlab::Database::Capture::StorageConnectors::Gcs).to( + receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(gcs_connector) + ) + expect(gcs_connector).to receive(:upload).with(filename, data) + + storage.upload(filename, data) + end + + it 'logs the upload request and completion' do + expect(Gitlab::AppLogger).to receive(:info).with( + { + message: 'Upload request for database capture', + connector: 'Google', + filename: filename, + duration: nil + }.compact + ) + + expect(Gitlab::AppLogger).to receive(:info).with( + { + message: 'Database capture upload completed', + connector: 'Google', + filename: filename, + duration: 1.5 + }.compact + ) + + storage.upload(filename, data) + end + end + + context 'when Settings are not configured' do + before do + allow(Gitlab::Database::Capture::StorageConnectors::Mock).to receive(:new).and_return(mock_connector) + allow(mock_connector).to receive(:upload) + end + + it 'falls back to Mock connector' do + expect(Gitlab::Database::Capture::StorageConnectors::Mock).to( + receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(mock_connector) + ) + expect(mock_connector).to receive(:upload).with(filename, data) + + storage.upload(filename, data) + end + end + end +end -- GitLab From fdf8aebb292c4ee1ee761d9ba617d4da41c9cc58 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Thu, 4 Sep 2025 19:22:13 -0300 Subject: [PATCH 03/10] Update settings --- config/initializers/1_settings.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index e5f652e01fb5f3..8a7eaa7d971c7a 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -1143,7 +1143,6 @@ # Settings['database_traffic_capture'] ||= {} -Settings['database_traffic_capture']['enabled'] = false Settings.database_traffic_capture['config'] ||= {} Settings.database_traffic_capture.config['storage'] ||= {} Settings.database_traffic_capture.config.storage['connector'] ||= {} -- GitLab From 6a119f748440e1cc6644b720f45e004a1951c7af Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Fri, 5 Sep 2025 10:19:09 -0300 Subject: [PATCH 04/10] Add a class-level access to the instance upload method --- lib/gitlab/database/capture/storage.rb | 12 +++++++++++- .../database/capture/storage_connectors/gcs.rb | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb index 73828e0004e4da..d7fe38be9c5f8a 100644 --- a/lib/gitlab/database/capture/storage.rb +++ b/lib/gitlab/database/capture/storage.rb @@ -8,18 +8,28 @@ class Storage 'Google' => StorageConnectors::Gcs }.freeze + def self.upload(...) + new.upload(...) + end + def upload(filename, data) log("Upload request for database capture", filename) start_monotonic_time = ::Gitlab::Metrics::System.monotonic_time - CONNECTORS.fetch(connector_provider, StorageConnectors::Mock).new(connector_settings).upload(filename, data) + result = connector.upload(filename, data) duration_s = ::Gitlab::Metrics::System.monotonic_time - start_monotonic_time log("Database capture upload completed", filename, duration_s) + + result end private + def connector + CONNECTORS.fetch(connector_provider, StorageConnectors::Mock).new(connector_settings) + end + def connector_provider connector_settings.try(:provider) || 'StorageConnectors::Mock' end diff --git a/lib/gitlab/database/capture/storage_connectors/gcs.rb b/lib/gitlab/database/capture/storage_connectors/gcs.rb index ab5e86a5149cac..1fc1d45883be31 100644 --- a/lib/gitlab/database/capture/storage_connectors/gcs.rb +++ b/lib/gitlab/database/capture/storage_connectors/gcs.rb @@ -7,6 +7,7 @@ module Database module Capture module StorageConnectors # Google Cloud Storage Connector + # https://cloud.google.com/ruby/docs/reference/google-cloud-storage/latest class Gcs def initialize(settings) @settings = settings -- GitLab From d25d9d3fd99136ad149013f0637e1136158e7e94 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Fri, 5 Sep 2025 10:29:47 -0300 Subject: [PATCH 05/10] return nil if gcs connection is not configure --- lib/gitlab/database/capture/storage.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb index d7fe38be9c5f8a..613dc2f6eea515 100644 --- a/lib/gitlab/database/capture/storage.rb +++ b/lib/gitlab/database/capture/storage.rb @@ -31,7 +31,7 @@ def connector end def connector_provider - connector_settings.try(:provider) || 'StorageConnectors::Mock' + connector_settings.try(:provider) end def connector_settings -- GitLab From d2dedba44188a28cdcbdca437cbf32402688cc81 Mon Sep 17 00:00:00 2001 From: Leonardo da Rosa Date: Fri, 5 Sep 2025 10:33:00 -0300 Subject: [PATCH 06/10] fix a typo --- lib/gitlab/database/capture/storage_connectors/gcs.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/database/capture/storage_connectors/gcs.rb b/lib/gitlab/database/capture/storage_connectors/gcs.rb index 1fc1d45883be31..2b79a1ce13509f 100644 --- a/lib/gitlab/database/capture/storage_connectors/gcs.rb +++ b/lib/gitlab/database/capture/storage_connectors/gcs.rb @@ -13,7 +13,7 @@ def initialize(settings) @settings = settings end - # We have to scape "/" from the filename to avoid gcp to interpret as a subfolder. This can be a problem if we + # We have to escape "/" from the filename to avoid gcp to interpret as a subfolder. This can be a problem if we # use the primary write location compose the filename, which can include an address like +"1F/4BE69098"+. def upload(filename, data) bucket.create_file( -- GitLab From 57408dbacd9053d4397a49c573c7354ef28d042c Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Fri, 5 Sep 2025 10:45:39 -0300 Subject: [PATCH 07/10] Fix Rubocop offenses --- lib/gitlab/database/capture/storage_connectors/gcs.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/database/capture/storage_connectors/gcs.rb b/lib/gitlab/database/capture/storage_connectors/gcs.rb index 2b79a1ce13509f..8a1d20a9293881 100644 --- a/lib/gitlab/database/capture/storage_connectors/gcs.rb +++ b/lib/gitlab/database/capture/storage_connectors/gcs.rb @@ -13,8 +13,8 @@ def initialize(settings) @settings = settings end - # We have to escape "/" from the filename to avoid gcp to interpret as a subfolder. This can be a problem if we - # use the primary write location compose the filename, which can include an address like +"1F/4BE69098"+. + # We have to escape "/" from the filename to avoid gcp to interpret as a subfolder. This can be a problem + # if we use the primary write location compose the filename, which can include an address like +"1F/4BE69098"+ def upload(filename, data) bucket.create_file( StringIO.new(data), -- GitLab From 49d8fb3521c9e207ccc8f3de8ee5fa094bd46411 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Mon, 8 Sep 2025 18:04:43 -0300 Subject: [PATCH 08/10] Handle upload failures --- lib/gitlab/database/capture/storage.rb | 5 +++ .../gitlab/database/capture/storage_spec.rb | 44 ++++++++++++++----- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb index 613dc2f6eea515..4d324609fc1fcc 100644 --- a/lib/gitlab/database/capture/storage.rb +++ b/lib/gitlab/database/capture/storage.rb @@ -22,10 +22,15 @@ def upload(filename, data) log("Database capture upload completed", filename, duration_s) result + rescue StandardError => error + log("Database capture upload failed: #{error}", filename) + + raise end private + # Fetches the configured provider or uses +StorageConnectors::Mock+ as default. def connector CONNECTORS.fetch(connector_provider, StorageConnectors::Mock).new(connector_settings) end diff --git a/spec/lib/gitlab/database/capture/storage_spec.rb b/spec/lib/gitlab/database/capture/storage_spec.rb index 6916281c0db84d..c6bbfe9ee9979a 100644 --- a/spec/lib/gitlab/database/capture/storage_spec.rb +++ b/spec/lib/gitlab/database/capture/storage_spec.rb @@ -5,8 +5,8 @@ RSpec.describe Gitlab::Database::Capture::Storage, feature_category: :database do let(:filename) { 'wal_capture' } let(:storage) { described_class.new } - let(:mock_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Mock) } - let(:gcs_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Gcs) } + let(:mock_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Mock, upload: true) } + let(:gcs_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Gcs, upload: true) } let(:configured_settings) { {} } let(:data) do <<~NDJSON @@ -19,6 +19,8 @@ before do allow(Gitlab::AppLogger).to receive(:info) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(0.0, 1.5) + allow(Gitlab::Database::Capture::StorageConnectors::Mock).to receive(:new).and_return(mock_connector) + allow(Gitlab::Database::Capture::StorageConnectors::Gcs).to receive(:new).and_return(gcs_connector) allow(Settings).to( receive_message_chain(:database_traffic_capture, :config, :storage, :connector).and_return( @@ -38,11 +40,6 @@ } end - before do - allow(Gitlab::Database::Capture::StorageConnectors::Gcs).to receive(:new).and_return(gcs_connector) - allow(gcs_connector).to receive(:upload) - end - it 'uses the GCS connector' do expect(Gitlab::Database::Capture::StorageConnectors::Gcs).to( receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(gcs_connector) @@ -76,11 +73,6 @@ end context 'when Settings are not configured' do - before do - allow(Gitlab::Database::Capture::StorageConnectors::Mock).to receive(:new).and_return(mock_connector) - allow(mock_connector).to receive(:upload) - end - it 'falls back to Mock connector' do expect(Gitlab::Database::Capture::StorageConnectors::Mock).to( receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(mock_connector) @@ -90,5 +82,33 @@ storage.upload(filename, data) end end + + context 'when the connection fails to upload the file' do + let(:configured_settings) do + { + provider: 'Google', + project_id: 'my-project', + credentials: '/path/to/keyfile.json', + bucket: 'my-bucket' + } + end + + before do + allow(gcs_connector).to receive(:upload).and_raise(StandardError, 'GCS unavailable.') + end + + it 'logs the message and re-raise the error' do + expect(Gitlab::AppLogger).to receive(:info).with( + { + message: 'Database capture upload failed: GCS unavailable.', + connector: 'Google', + filename: filename, + duration: nil + }.compact + ) + + expect { storage.upload(filename, data) }.to raise_error('GCS unavailable.') + end + end end end -- GitLab From e57a054cbec8e1173cc039e3b83b5da10ac639a1 Mon Sep 17 00:00:00 2001 From: Leonardo Rosa Date: Wed, 10 Sep 2025 10:11:45 -0300 Subject: [PATCH 09/10] Apply reviewer suggestions --- config/gitlab.yml.example | 2 +- lib/gitlab/database/capture/storage.rb | 6 ++--- .../storage_connectors/{mock.rb => local.rb} | 8 ++++--- .../gitlab/database/capture/storage_spec.rb | 22 +++++++++---------- 4 files changed, 20 insertions(+), 18 deletions(-) rename lib/gitlab/database/capture/storage_connectors/{mock.rb => local.rb} (79%) diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 700a17812fe772..a3b3493e2e5847 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -1319,7 +1319,7 @@ production: &base config: storage: connector: - provider: Google + provider: Gcs project_id: 'my-project' credentials: '/path/to/keyfile.json' bucket: 'my-bucket' diff --git a/lib/gitlab/database/capture/storage.rb b/lib/gitlab/database/capture/storage.rb index 4d324609fc1fcc..f897f9751aee30 100644 --- a/lib/gitlab/database/capture/storage.rb +++ b/lib/gitlab/database/capture/storage.rb @@ -5,7 +5,7 @@ module Database module Capture class Storage CONNECTORS = { - 'Google' => StorageConnectors::Gcs + 'Gcs' => StorageConnectors::Gcs }.freeze def self.upload(...) @@ -30,9 +30,9 @@ def upload(filename, data) private - # Fetches the configured provider or uses +StorageConnectors::Mock+ as default. + # Fetches the configured provider or uses +StorageConnectors::Local+ as fallback connector. def connector - CONNECTORS.fetch(connector_provider, StorageConnectors::Mock).new(connector_settings) + CONNECTORS.fetch(connector_provider, StorageConnectors::Local).new(connector_settings) end def connector_provider diff --git a/lib/gitlab/database/capture/storage_connectors/mock.rb b/lib/gitlab/database/capture/storage_connectors/local.rb similarity index 79% rename from lib/gitlab/database/capture/storage_connectors/mock.rb rename to lib/gitlab/database/capture/storage_connectors/local.rb index fb9752a343e9c9..1b3077e850334d 100644 --- a/lib/gitlab/database/capture/storage_connectors/mock.rb +++ b/lib/gitlab/database/capture/storage_connectors/local.rb @@ -5,16 +5,18 @@ module Database module Capture module StorageConnectors # To simplify testing and development - class Mock + class Local TEMP_PATH = Rails.root.join('tmp/database-traffic-capture/storage') def initialize(settings) + unless Rails.env.development? || Rails.env.test? + raise ConfigurationError, "Local connector provider it's not intended to be used in production" + end + @settings = settings end def upload(filename, data) - return unless Rails.env.development? || Rails.env.test? - Gitlab::PathTraversal.check_path_traversal!(filename) filepath = Rails.root.join(TEMP_PATH, File.basename(filename)) diff --git a/spec/lib/gitlab/database/capture/storage_spec.rb b/spec/lib/gitlab/database/capture/storage_spec.rb index c6bbfe9ee9979a..1804b523e13d97 100644 --- a/spec/lib/gitlab/database/capture/storage_spec.rb +++ b/spec/lib/gitlab/database/capture/storage_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Gitlab::Database::Capture::Storage, feature_category: :database do let(:filename) { 'wal_capture' } let(:storage) { described_class.new } - let(:mock_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Mock, upload: true) } + let(:local_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Local, upload: true) } let(:gcs_connector) { instance_double(Gitlab::Database::Capture::StorageConnectors::Gcs, upload: true) } let(:configured_settings) { {} } let(:data) do @@ -19,7 +19,7 @@ before do allow(Gitlab::AppLogger).to receive(:info) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(0.0, 1.5) - allow(Gitlab::Database::Capture::StorageConnectors::Mock).to receive(:new).and_return(mock_connector) + allow(Gitlab::Database::Capture::StorageConnectors::Local).to receive(:new).and_return(local_connector) allow(Gitlab::Database::Capture::StorageConnectors::Gcs).to receive(:new).and_return(gcs_connector) allow(Settings).to( @@ -33,7 +33,7 @@ context 'when using Google connector' do let(:configured_settings) do { - provider: 'Google', + provider: 'Gcs', project_id: 'my-project', credentials: '/path/to/keyfile.json', bucket: 'my-bucket' @@ -53,7 +53,7 @@ expect(Gitlab::AppLogger).to receive(:info).with( { message: 'Upload request for database capture', - connector: 'Google', + connector: 'Gcs', filename: filename, duration: nil }.compact @@ -62,7 +62,7 @@ expect(Gitlab::AppLogger).to receive(:info).with( { message: 'Database capture upload completed', - connector: 'Google', + connector: 'Gcs', filename: filename, duration: 1.5 }.compact @@ -73,11 +73,11 @@ end context 'when Settings are not configured' do - it 'falls back to Mock connector' do - expect(Gitlab::Database::Capture::StorageConnectors::Mock).to( - receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(mock_connector) + it 'falls back to Local connector' do + expect(Gitlab::Database::Capture::StorageConnectors::Local).to( + receive(:new).with(Settings.database_traffic_capture.config.storage.connector).and_return(local_connector) ) - expect(mock_connector).to receive(:upload).with(filename, data) + expect(local_connector).to receive(:upload).with(filename, data) storage.upload(filename, data) end @@ -86,7 +86,7 @@ context 'when the connection fails to upload the file' do let(:configured_settings) do { - provider: 'Google', + provider: 'Gcs', project_id: 'my-project', credentials: '/path/to/keyfile.json', bucket: 'my-bucket' @@ -101,7 +101,7 @@ expect(Gitlab::AppLogger).to receive(:info).with( { message: 'Database capture upload failed: GCS unavailable.', - connector: 'Google', + connector: 'Gcs', filename: filename, duration: nil }.compact -- GitLab From 149e418809ad69c3e3c2f1882e3482dc8c8e8717 Mon Sep 17 00:00:00 2001 From: Leonardo da Rosa Date: Wed, 10 Sep 2025 15:42:52 -0300 Subject: [PATCH 10/10] Fix a typo --- spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb b/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb index 1764a76d4af47b..7e1a6b978142de 100644 --- a/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb +++ b/spec/lib/gitlab/database/capture/storage_connectors/gcs_spec.rb @@ -8,7 +8,7 @@ let(:bucket) { instance_double(Google::Cloud::Storage::Bucket) } let(:settings) do GitlabSettings::Options.build( - provider: 'Google', + provider: 'Gcs', project_id: 'my-project', credentials: '/path/to/keyfile.json', bucket: 'my-bucket' -- GitLab