From c5ec6a829ea78c97e71b72190955bdbc5808be12 Mon Sep 17 00:00:00 2001 From: Alper Akgun Date: Mon, 18 Sep 2023 18:07:38 +0300 Subject: [PATCH 1/2] Enforce not null constraints for workspace personal_access_token_id Changelog: changed --- ...ackfill_workspace_personal_access_token.rb | 22 +++++++++++++++++++ ...int_personal_access_token_in_workspaces.rb | 13 +++++++++++ db/schema_migrations/20230918143333 | 1 + db/schema_migrations/20230918145641 | 1 + db/structure.sql | 3 +++ ee/app/models/remote_development/workspace.rb | 1 + 6 files changed, 41 insertions(+) create mode 100644 db/post_migrate/20230918143333_finalize_backfill_workspace_personal_access_token.rb create mode 100644 db/post_migrate/20230918145641_add_not_null_constraint_personal_access_token_in_workspaces.rb create mode 100644 db/schema_migrations/20230918143333 create mode 100644 db/schema_migrations/20230918145641 diff --git a/db/post_migrate/20230918143333_finalize_backfill_workspace_personal_access_token.rb b/db/post_migrate/20230918143333_finalize_backfill_workspace_personal_access_token.rb new file mode 100644 index 00000000000000..c31c7b1c405e03 --- /dev/null +++ b/db/post_migrate/20230918143333_finalize_backfill_workspace_personal_access_token.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class FinalizeBackfillWorkspacePersonalAccessToken < Gitlab::Database::Migration[2.1] + MIGRATION = 'BackfillWorkspacePersonalAccessToken' + disable_ddl_transaction! + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + def up + ensure_batched_background_migration_is_finished( + job_class_name: MIGRATION, + table_name: :workspaces, + column_name: :id, + job_arguments: [], + finalize: true + ) + end + + def down + # no-op + end +end diff --git a/db/post_migrate/20230918145641_add_not_null_constraint_personal_access_token_in_workspaces.rb b/db/post_migrate/20230918145641_add_not_null_constraint_personal_access_token_in_workspaces.rb new file mode 100644 index 00000000000000..84f0ad11e32975 --- /dev/null +++ b/db/post_migrate/20230918145641_add_not_null_constraint_personal_access_token_in_workspaces.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddNotNullConstraintPersonalAccessTokenInWorkspaces < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + def up + add_not_null_constraint :workspaces, :personal_access_token_id, validate: false + end + + def down + remove_not_null_constraint :workspaces, :personal_access_token_id + end +end diff --git a/db/schema_migrations/20230918143333 b/db/schema_migrations/20230918143333 new file mode 100644 index 00000000000000..63dae73cc5dcf6 --- /dev/null +++ b/db/schema_migrations/20230918143333 @@ -0,0 +1 @@ +28ad98bd0151a3dac1b1495fd773ee8641ffd3c50df5f3e41ca5a0df58daa826 \ No newline at end of file diff --git a/db/schema_migrations/20230918145641 b/db/schema_migrations/20230918145641 new file mode 100644 index 00000000000000..61753f364691a2 --- /dev/null +++ b/db/schema_migrations/20230918145641 @@ -0,0 +1 @@ +f20056555ebccab049b57176ada21e61dc63b30061321786c6ec946e8ac951ec \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e86fe42ecb49c8..2d1d82594fa396 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -27534,6 +27534,9 @@ ALTER TABLE ONLY chat_names ALTER TABLE ONLY chat_teams ADD CONSTRAINT chat_teams_pkey PRIMARY KEY (id); +ALTER TABLE workspaces + ADD CONSTRAINT check_2a89035b04 CHECK ((personal_access_token_id IS NOT NULL)) NOT VALID; + ALTER TABLE vulnerability_scanners ADD CONSTRAINT check_37608c9db5 CHECK ((char_length(vendor) <= 255)) NOT VALID; diff --git a/ee/app/models/remote_development/workspace.rb b/ee/app/models/remote_development/workspace.rb index 4b55acd135ed83..20b170ffdf3960 100644 --- a/ee/app/models/remote_development/workspace.rb +++ b/ee/app/models/remote_development/workspace.rb @@ -22,6 +22,7 @@ class Workspace < ApplicationRecord validates :user, presence: true validates :agent, presence: true validates :editor, presence: true + validates :personal_access_token, presence: true # Ensure that the associated agent has an existing RemoteDevelopmentAgentConfig before we allow it # to be used to create a new workspace -- GitLab From 207b6b6cd9e88195e03c8313a0cf208003ce880a Mon Sep 17 00:00:00 2001 From: Alper Akgun Date: Tue, 19 Sep 2023 17:44:03 +0300 Subject: [PATCH 2/2] Remove unneeded test case --- ...ckfill_workspace_personal_access_token_spec.rb | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/ee/spec/lib/gitlab/background_migration/backfill_workspace_personal_access_token_spec.rb b/ee/spec/lib/gitlab/background_migration/backfill_workspace_personal_access_token_spec.rb index 7b0c1c2eb1b676..bb63526ece2c8e 100644 --- a/ee/spec/lib/gitlab/background_migration/backfill_workspace_personal_access_token_spec.rb +++ b/ee/spec/lib/gitlab/background_migration/backfill_workspace_personal_access_token_spec.rb @@ -37,13 +37,6 @@ expires_at: 4.days.from_now) end - let!(:workspace_without_personal_access_token) do - workspaces_table.create!({ - name: 'workspace1', - personal_access_token_id: nil - }.merge!(workspace_attrs)) - end - let!(:workspace_with_personal_access_token) do workspaces_table.create!({ name: 'workspace2', @@ -53,7 +46,7 @@ let(:migration) do described_class.new( - start_id: workspace_without_personal_access_token.id, + start_id: workspace_with_personal_access_token.id, end_id: workspace_with_personal_access_token.id, batch_table: :workspaces, batch_column: :id, @@ -63,12 +56,6 @@ ) end - it "creates a personal access token and updates the workspace" do - expect { migration.perform }.to change { - workspace_without_personal_access_token.reload.personal_access_token_id - }.from(nil) - end - it "does not modify workspace's existing token" do expect { migration.perform }.not_to change { workspace_with_personal_access_token.reload.personal_access_token_id -- GitLab