From 4e2af1a4d13d179d06254bba32bca21a49b6b7c3 Mon Sep 17 00:00:00 2001
From: Shubham Kumar <shukumar@gitlab.com>
Date: Fri, 4 Oct 2024 22:27:11 +0000
Subject: [PATCH] Add and backfill project_id for ci_secure_file_states
## What does this MR do and why?
Add and backfill project_id for ci_secure_file_states.
This table has a
[desired sharding key](https://docs.gitlab.com/ee/development/database/multiple_databases.html#define-a-desired_sharding_key-to-automatically-backfill-a-sharding_key)
configured ([view configuration](https://gitlab.com/gitlab-org/gitlab/-/blob/master/db/docs/ci_secure_file_states.yml)).
This merge request is the first step towards transforming the desired sharding key into a
[sharding key](https://docs.gitlab.com/ee/development/database/multiple_databases.html#defining-a-sharding-key-for-all-cell-local-tables).
This involves three changes:
- Adding a new column that will serve as the sharding key (along with the relevant index and foreign key).
- Populating the sharding key when new records are created by adding a database function and trigger.
- Scheduling a [batched background migration](https://docs.gitlab.com/ee/development/database/batched_background_migrations.html)
to set the sharding key for existing records.
Once the background migration has completed, a second merge request will be created to finalize the background
migration and validate the not null constraint.
## How to verify
We have assigned a random backend engineer from ~"group::pipeline security" to review these changes. Please review this merge
request from a ~backend perspective. The main thing we are looking to verify is that the added column and association
match the values specified by the [desired sharding key](https://gitlab.com/gitlab-org/gitlab/-/blob/master/db/docs/ci_secure_file_states.yml)
configuration and that backfilling the column from this other table makes sense in the context of this feature.
When you are finished, please:
1. Trigger the [database testing pipeline](https://docs.gitlab.com/ee/development/database/database_migration_pipeline.html)
as instructed by Danger.
1. Request a review from the ~backend maintainer and ~database reviewer suggested by Danger.
If you have any questions or concerns, reach out to `@tigerwnz` or @shubhamkrai.
This merge request was generated by a once off keep implemented in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143774
This change was generated by
[gitlab-housekeeper](https://gitlab.com/gitlab-org/gitlab/-/tree/master/gems/gitlab-housekeeper)
using the Keeps::BackfillDesiredShardingKeySmallTable keep.
To provide feedback on your experience with `gitlab-housekeeper` please create an issue with the
label ~"GitLab Housekeeper" and consider pinging the author of this keep.
Changelog: other
---
...kfill_ci_secure_file_states_project_id.yml | 9 +++++
db/docs/ci_secure_file_states.yml | 1 +
...add_project_id_to_ci_secure_file_states.rb | 9 +++++
...dex_ci_secure_file_states_on_project_id.rb | 16 ++++++++
...i_secure_file_states_project_id_trigger.rb | 25 ++++++++++++
...ckfill_ci_secure_file_states_project_id.rb | 40 +++++++++++++++++++
db/schema_migrations/20240930125308 | 1 +
db/schema_migrations/20240930125309 | 1 +
db/schema_migrations/20240930125310 | 1 +
db/schema_migrations/20240930125311 | 1 +
db/structure.sql | 21 ++++++++++
...ckfill_ci_secure_file_states_project_id.rb | 10 +++++
spec/db/schema_spec.rb | 1 +
...l_ci_secure_file_states_project_id_spec.rb | 17 ++++++++
...l_ci_secure_file_states_project_id_spec.rb | 33 +++++++++++++++
15 files changed, 186 insertions(+)
create mode 100644 db/docs/batched_background_migrations/backfill_ci_secure_file_states_project_id.yml
create mode 100644 db/migrate/20240930125308_add_project_id_to_ci_secure_file_states.rb
create mode 100644 db/post_migrate/20240930125309_index_ci_secure_file_states_on_project_id.rb
create mode 100644 db/post_migrate/20240930125310_add_ci_secure_file_states_project_id_trigger.rb
create mode 100644 db/post_migrate/20240930125311_queue_backfill_ci_secure_file_states_project_id.rb
create mode 100644 db/schema_migrations/20240930125308
create mode 100644 db/schema_migrations/20240930125309
create mode 100644 db/schema_migrations/20240930125310
create mode 100644 db/schema_migrations/20240930125311
create mode 100644 lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id.rb
create mode 100644 spec/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id_spec.rb
create mode 100644 spec/migrations/20240930125312_queue_backfill_ci_secure_file_states_project_id_spec.rb
diff --git a/db/docs/batched_background_migrations/backfill_ci_secure_file_states_project_id.yml b/db/docs/batched_background_migrations/backfill_ci_secure_file_states_project_id.yml
new file mode 100644
index 000000000000..71c8c76c7bd0
--- /dev/null
+++ b/db/docs/batched_background_migrations/backfill_ci_secure_file_states_project_id.yml
@@ -0,0 +1,9 @@
+---
+migration_job_name: BackfillCiSecureFileStatesProjectId
+description: Backfills sharding key `ci_secure_file_states.project_id` from `ci_secure_files`.
+feature_category: secrets_management
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/167617
+milestone: '17.5'
+queued_migration_version: 20240930125311
+finalize_after: '2024-10-22'
+finalized_by: # version of the migration that finalized this BBM
diff --git a/db/docs/ci_secure_file_states.yml b/db/docs/ci_secure_file_states.yml
index 39f559491ece..796b24c4df45 100644
--- a/db/docs/ci_secure_file_states.yml
+++ b/db/docs/ci_secure_file_states.yml
@@ -17,3 +17,4 @@ desired_sharding_key:
table: ci_secure_files
sharding_key: project_id
belongs_to: ci_secure_file
+desired_sharding_key_migration_job_name: BackfillCiSecureFileStatesProjectId
diff --git a/db/migrate/20240930125308_add_project_id_to_ci_secure_file_states.rb b/db/migrate/20240930125308_add_project_id_to_ci_secure_file_states.rb
new file mode 100644
index 000000000000..1b096de187bb
--- /dev/null
+++ b/db/migrate/20240930125308_add_project_id_to_ci_secure_file_states.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddProjectIdToCiSecureFileStates < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+
+ def change
+ add_column :ci_secure_file_states, :project_id, :bigint
+ end
+end
diff --git a/db/post_migrate/20240930125309_index_ci_secure_file_states_on_project_id.rb b/db/post_migrate/20240930125309_index_ci_secure_file_states_on_project_id.rb
new file mode 100644
index 000000000000..09a1fe51fa27
--- /dev/null
+++ b/db/post_migrate/20240930125309_index_ci_secure_file_states_on_project_id.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+class IndexCiSecureFileStatesOnProjectId < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+ disable_ddl_transaction!
+
+ INDEX_NAME = 'index_ci_secure_file_states_on_project_id'
+
+ def up
+ add_concurrent_index :ci_secure_file_states, :project_id, name: INDEX_NAME
+ end
+
+ def down
+ remove_concurrent_index_by_name :ci_secure_file_states, INDEX_NAME
+ end
+end
diff --git a/db/post_migrate/20240930125310_add_ci_secure_file_states_project_id_trigger.rb b/db/post_migrate/20240930125310_add_ci_secure_file_states_project_id_trigger.rb
new file mode 100644
index 000000000000..faa0ff03e329
--- /dev/null
+++ b/db/post_migrate/20240930125310_add_ci_secure_file_states_project_id_trigger.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class AddCiSecureFileStatesProjectIdTrigger < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+
+ def up
+ install_sharding_key_assignment_trigger(
+ table: :ci_secure_file_states,
+ sharding_key: :project_id,
+ parent_table: :ci_secure_files,
+ parent_sharding_key: :project_id,
+ foreign_key: :ci_secure_file_id
+ )
+ end
+
+ def down
+ remove_sharding_key_assignment_trigger(
+ table: :ci_secure_file_states,
+ sharding_key: :project_id,
+ parent_table: :ci_secure_files,
+ parent_sharding_key: :project_id,
+ foreign_key: :ci_secure_file_id
+ )
+ end
+end
diff --git a/db/post_migrate/20240930125311_queue_backfill_ci_secure_file_states_project_id.rb b/db/post_migrate/20240930125311_queue_backfill_ci_secure_file_states_project_id.rb
new file mode 100644
index 000000000000..44645c60221e
--- /dev/null
+++ b/db/post_migrate/20240930125311_queue_backfill_ci_secure_file_states_project_id.rb
@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+class QueueBackfillCiSecureFileStatesProjectId < Gitlab::Database::Migration[2.2]
+ milestone '17.5'
+ restrict_gitlab_migration gitlab_schema: :gitlab_ci
+
+ MIGRATION = "BackfillCiSecureFileStatesProjectId"
+ DELAY_INTERVAL = 2.minutes
+ BATCH_SIZE = 1000
+ SUB_BATCH_SIZE = 100
+
+ def up
+ queue_batched_background_migration(
+ MIGRATION,
+ :ci_secure_file_states,
+ :ci_secure_file_id,
+ :project_id,
+ :ci_secure_files,
+ :project_id,
+ :ci_secure_file_id,
+ job_interval: DELAY_INTERVAL,
+ batch_size: BATCH_SIZE,
+ sub_batch_size: SUB_BATCH_SIZE
+ )
+ end
+
+ def down
+ delete_batched_background_migration(
+ MIGRATION,
+ :ci_secure_file_states,
+ :ci_secure_file_id,
+ [
+ :project_id,
+ :ci_secure_files,
+ :project_id,
+ :ci_secure_file_id
+ ]
+ )
+ end
+end
diff --git a/db/schema_migrations/20240930125308 b/db/schema_migrations/20240930125308
new file mode 100644
index 000000000000..3ad785c1bb05
--- /dev/null
+++ b/db/schema_migrations/20240930125308
@@ -0,0 +1 @@
+b226424a525abb340c104a6c8d5debc5ad0297220b37bff32aaeaa767702d21a
\ No newline at end of file
diff --git a/db/schema_migrations/20240930125309 b/db/schema_migrations/20240930125309
new file mode 100644
index 000000000000..248462abefa7
--- /dev/null
+++ b/db/schema_migrations/20240930125309
@@ -0,0 +1 @@
+fc748d24f09f804e50dc62a9e9f5f5941ecd7235a55760f84bfd1c5dd2a95edf
\ No newline at end of file
diff --git a/db/schema_migrations/20240930125310 b/db/schema_migrations/20240930125310
new file mode 100644
index 000000000000..abca0c65f009
--- /dev/null
+++ b/db/schema_migrations/20240930125310
@@ -0,0 +1 @@
+6634a974d38adbe6d05a40ccfbd6e5d7469bc2462e7784c70e52e8ecbdb8a447
\ No newline at end of file
diff --git a/db/schema_migrations/20240930125311 b/db/schema_migrations/20240930125311
new file mode 100644
index 000000000000..09a5867fa648
--- /dev/null
+++ b/db/schema_migrations/20240930125311
@@ -0,0 +1 @@
+64a55d736a19fb1a04f3f046808e3f9d2f88485a85a9c44b119e4756ad38187e
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 5d74c537b867..779638c76174 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -1785,6 +1785,22 @@ RETURN NEW;
END
$$;
+CREATE FUNCTION trigger_8b39d532224c() RETURNS trigger
+ LANGUAGE plpgsql
+ AS $$
+BEGIN
+IF NEW."project_id" IS NULL THEN
+ SELECT "project_id"
+ INTO NEW."project_id"
+ FROM "ci_secure_files"
+ WHERE "ci_secure_files"."id" = NEW."ci_secure_file_id";
+END IF;
+
+RETURN NEW;
+
+END
+$$;
+
CREATE FUNCTION trigger_8ba31bddd655() RETURNS trigger
LANGUAGE plpgsql
AS $$
@@ -9062,6 +9078,7 @@ CREATE TABLE ci_secure_file_states (
verification_retry_count smallint,
verification_checksum bytea,
verification_failure text,
+ project_id bigint,
CONSTRAINT check_a79e5a9261 CHECK ((char_length(verification_failure) <= 255))
);
@@ -28264,6 +28281,8 @@ CREATE INDEX index_ci_secure_file_states_needs_verification ON ci_secure_file_st
CREATE INDEX index_ci_secure_file_states_on_ci_secure_file_id ON ci_secure_file_states USING btree (ci_secure_file_id);
+CREATE INDEX index_ci_secure_file_states_on_project_id ON ci_secure_file_states USING btree (project_id);
+
CREATE INDEX index_ci_secure_file_states_on_verification_state ON ci_secure_file_states USING btree (verification_state);
CREATE INDEX index_ci_secure_file_states_pending_verification ON ci_secure_file_states USING btree (verified_at NULLS FIRST) WHERE (verification_state = 0);
@@ -33564,6 +33583,8 @@ CREATE TRIGGER trigger_8a38ce2327de BEFORE INSERT OR UPDATE ON boards_epic_user_
CREATE TRIGGER trigger_8ac78f164b2d BEFORE INSERT OR UPDATE ON design_management_repositories FOR EACH ROW EXECUTE FUNCTION trigger_8ac78f164b2d();
+CREATE TRIGGER trigger_8b39d532224c BEFORE INSERT OR UPDATE ON ci_secure_file_states FOR EACH ROW EXECUTE FUNCTION trigger_8b39d532224c();
+
CREATE TRIGGER trigger_8ba31bddd655 BEFORE INSERT OR UPDATE ON vulnerability_occurrence_pipelines FOR EACH ROW EXECUTE FUNCTION trigger_8ba31bddd655();
CREATE TRIGGER trigger_8d002f38bdef BEFORE INSERT OR UPDATE ON packages_debian_group_components FOR EACH ROW EXECUTE FUNCTION trigger_8d002f38bdef();
diff --git a/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id.rb b/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id.rb
new file mode 100644
index 000000000000..2d19fdaf963d
--- /dev/null
+++ b/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id.rb
@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module BackgroundMigration
+ class BackfillCiSecureFileStatesProjectId < BackfillDesiredShardingKeyJob
+ operation_name :backfill_ci_secure_file_states_project_id
+ feature_category :secrets_management
+ end
+ end
+end
diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb
index 12cefdf63e0f..632f81c9eeef 100644
--- a/spec/db/schema_spec.rb
+++ b/spec/db/schema_spec.rb
@@ -89,6 +89,7 @@
ci_pipeline_metadata: %w[partition_id],
ci_pipeline_variables: %w[partition_id pipeline_id project_id],
ci_pipelines: %w[partition_id auto_canceled_by_partition_id],
+ ci_secure_file_states: %w[project_id],
ci_unit_test_failures: %w[project_id],
ci_resources: %w[project_id],
p_ci_pipelines: %w[partition_id auto_canceled_by_partition_id auto_canceled_by_id],
diff --git a/spec/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id_spec.rb b/spec/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id_spec.rb
new file mode 100644
index 000000000000..cc28a1e2e101
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/backfill_ci_secure_file_states_project_id_spec.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillCiSecureFileStatesProjectId,
+ feature_category: :secrets_management,
+ schema: 20240930125308,
+ migration: :gitlab_ci do
+ include_examples 'desired sharding key backfill job' do
+ let(:batch_table) { :ci_secure_file_states }
+ let(:batch_column) { :ci_secure_file_id }
+ let(:backfill_column) { :project_id }
+ let(:backfill_via_table) { :ci_secure_files }
+ let(:backfill_via_column) { :project_id }
+ let(:backfill_via_foreign_key) { :ci_secure_file_id }
+ end
+end
diff --git a/spec/migrations/20240930125312_queue_backfill_ci_secure_file_states_project_id_spec.rb b/spec/migrations/20240930125312_queue_backfill_ci_secure_file_states_project_id_spec.rb
new file mode 100644
index 000000000000..d0ca7b6d9955
--- /dev/null
+++ b/spec/migrations/20240930125312_queue_backfill_ci_secure_file_states_project_id_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require_migration!
+
+RSpec.describe QueueBackfillCiSecureFileStatesProjectId, migration: :gitlab_ci, feature_category: :secrets_management do
+ let!(:batched_migration) { described_class::MIGRATION }
+
+ it 'schedules a new batched migration' do
+ reversible_migration do |migration|
+ migration.before -> {
+ expect(batched_migration).not_to have_scheduled_batched_migration
+ }
+
+ migration.after -> {
+ expect(batched_migration).to have_scheduled_batched_migration(
+ table_name: :ci_secure_file_states,
+ column_name: :ci_secure_file_id,
+ interval: described_class::DELAY_INTERVAL,
+ batch_size: described_class::BATCH_SIZE,
+ sub_batch_size: described_class::SUB_BATCH_SIZE,
+ gitlab_schema: :gitlab_ci,
+ job_arguments: [
+ :project_id,
+ :ci_secure_files,
+ :project_id,
+ :ci_secure_file_id
+ ]
+ )
+ }
+ end
+ end
+end
--
GitLab