Commit 2f345de9 authored by Marcos Rocha's avatar Marcos Rocha Committed by Patrick Bair

Remove the existing duplicates of DastSiteTokens

The MR !67933
added an application-level uniqueness constraint on to facilitate token reuse by url.

However we may have existing duplicate tokens in the database and race conditions could result in duplicates being created.

This MR adds a data migration to remove the existing duplicate tokens

Changelog: fixed
MR: !68578
parent 070ae2bc
# frozen_string_literal: true
class RemoveDuplicateDastSiteTokens < ActiveRecord::Migration[6.1]
disable_ddl_transaction!
class DastSiteToken < ApplicationRecord
self.table_name = 'dast_site_tokens'
self.inheritance_column = :_type_disabled
scope :duplicates, -> do
all_duplicates = select(:project_id, :url)
.distinct
.group(:project_id, :url)
.having('count(*) > 1')
.pluck('array_agg(id) as ids')
duplicate_ids = extract_duplicate_ids(all_duplicates)
where(id: duplicate_ids)
end
def self.extract_duplicate_ids(duplicates)
duplicates.flat_map { |ids| ids.first(ids.size - 1) }
end
end
def up
DastSiteToken.duplicates.delete_all
end
def down
end
end
# frozen_string_literal: true
class AddUniqueIndexDastSiteTokenProjectIdAndUrl < ActiveRecord::Migration[6.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
INDEX_NAME = 'index_dast_site_token_on_project_id_and_url'
def up
add_concurrent_index :dast_site_tokens, [:project_id, :url], name: INDEX_NAME, unique: true
end
def down
remove_concurrent_index_by_name :dast_site_tokens, name: INDEX_NAME
end
end
7324c3803c910338261556c65cae5d0827e78b77890386e402e056d480c3486b
\ No newline at end of file
b7916e025131f11da97ab89a01b32d1dbacf94bb96dc84877ba18404c8b0b2ba
\ No newline at end of file
...@@ -23834,6 +23834,8 @@ CREATE UNIQUE INDEX index_dast_site_profiles_on_project_id_and_name ON dast_site ...@@ -23834,6 +23834,8 @@ CREATE UNIQUE INDEX index_dast_site_profiles_on_project_id_and_name ON dast_site
CREATE UNIQUE INDEX index_dast_site_profiles_pipelines_on_ci_pipeline_id ON dast_site_profiles_pipelines USING btree (ci_pipeline_id); CREATE UNIQUE INDEX index_dast_site_profiles_pipelines_on_ci_pipeline_id ON dast_site_profiles_pipelines USING btree (ci_pipeline_id);
CREATE UNIQUE INDEX index_dast_site_token_on_project_id_and_url ON dast_site_tokens USING btree (project_id, url);
CREATE INDEX index_dast_site_tokens_on_project_id ON dast_site_tokens USING btree (project_id); CREATE INDEX index_dast_site_tokens_on_project_id ON dast_site_tokens USING btree (project_id);
CREATE INDEX index_dast_site_validations_on_dast_site_token_id ON dast_site_validations USING btree (dast_site_token_id); CREATE INDEX index_dast_site_validations_on_dast_site_token_id ON dast_site_validations USING btree (dast_site_token_id);
# frozen_string_literal: true
require 'spec_helper'
require_migration!
RSpec.describe RemoveDuplicateDastSiteTokens do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:dast_site_tokens) { table(:dast_site_tokens) }
let!(:namespace) { namespaces.create!(id: 1, name: 'group', path: 'group') }
let!(:project1) { projects.create!(id: 1, namespace_id: namespace.id, path: 'project1') }
# create non duplicate dast site token
let!(:dast_site_token1) { dast_site_tokens.create!(project_id: project1.id, url: 'https://gitlab.com', token: SecureRandom.uuid) }
context 'when duplicate dast site tokens exists' do
# create duplicate dast site token
let_it_be(:duplicate_url) { 'https://about.gitlab.com' }
let!(:project2) { projects.create!(id: 2, namespace_id: namespace.id, path: 'project2') }
let!(:dast_site_token2) { dast_site_tokens.create!(project_id: project2.id, url: duplicate_url, token: SecureRandom.uuid) }
let!(:dast_site_token3) { dast_site_tokens.create!(project_id: project2.id, url: 'https://temp_url.com', token: SecureRandom.uuid) }
let!(:dast_site_token4) { dast_site_tokens.create!(project_id: project2.id, url: 'https://other_temp_url.com', token: SecureRandom.uuid) }
before 'update URL to bypass uniqueness validation' do
dast_site_tokens.where(project_id: 2).update_all(url: duplicate_url)
end
describe 'migration up' do
it 'does remove duplicated dast site tokens' do
expect(dast_site_tokens.count).to eq(4)
expect(dast_site_tokens.where(project_id: 2, url: duplicate_url).size).to eq(3)
migrate!
expect(dast_site_tokens.count).to eq(2)
expect(dast_site_tokens.where(project_id: 2, url: duplicate_url).size).to eq(1)
end
end
end
context 'when duplicate dast site tokens does not exists' do
before do
dast_site_tokens.create!(project_id: 1, url: 'https://about.gitlab.com/handbook', token: SecureRandom.uuid)
end
describe 'migration up' do
it 'does remove duplicated dast site tokens' do
expect { migrate! }.not_to change(dast_site_tokens, :count)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment