From d0f0f47e2c84285bc4c62bf82bcef44c25ba3d40 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 11 Jul 2025 13:36:51 -0700 Subject: [PATCH] Detect incorrect sequence owners This commit builds upon https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196044 and makes it possible to identify wrong owners for sequences. Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/396878 Changelog: added --- .../lib/gitlab/schema/validation.rb | 2 + .../validation/schema_objects/sequence.rb | 4 +- .../schema/validation/validators/base.rb | 3 +- .../validators/different_sequence_owners.rb | 24 +++++++ .../spec/fixtures/structure.sql | 4 ++ .../schema/validation/validators/base_spec.rb | 3 +- .../different_sequence_owners_spec.rb | 11 ++++ .../validators/missing_sequences_spec.rb | 1 - .../sequence_validators_shared_examples.rb | 66 +++++++++++++++---- 9 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/different_sequence_owners.rb create mode 100644 gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/different_sequence_owners_spec.rb diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb index d1cfeddee123b7..ff18f1e2e02e81 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation.rb @@ -15,6 +15,8 @@ require_relative 'validation/validators/different_definition_indexes' require_relative 'validation/validators/extra_indexes' require_relative 'validation/validators/missing_indexes' + +require_relative 'validation/validators/different_sequence_owners' require_relative 'validation/validators/missing_sequences' require_relative 'validation/validators/extra_table_columns' diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb index 4c00d5f3aa5fcf..8245a7d4026c8e 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/schema_objects/sequence.rb @@ -26,7 +26,9 @@ def table_name end def statement - "CREATE SEQUENCE #{name}" + statements = "CREATE SEQUENCE #{name};" + statements += "\nALTER SEQUENCE #{name} OWNED BY #{owner}" if owner + statements end private diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb index e3d02f869438a8..b340d9cec79c75 100644 --- a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/base.rb @@ -23,7 +23,8 @@ def self.all_validators DifferentDefinitionTables, DifferentDefinitionIndexes, DifferentDefinitionTriggers, - DifferentDefinitionForeignKeys + DifferentDefinitionForeignKeys, + DifferentSequenceOwners ] end diff --git a/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/different_sequence_owners.rb b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/different_sequence_owners.rb new file mode 100644 index 00000000000000..069d0d28afd7f5 --- /dev/null +++ b/gems/gitlab-schema-validation/lib/gitlab/schema/validation/validators/different_sequence_owners.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Schema + module Validation + module Validators + class DifferentSequenceOwners < Base + ERROR_MESSAGE = "The sequence %s has a different owner between structure.sql and database" + + def execute + structure_sql.sequences.filter_map do |sequence| + database_sequence = database.fetch_sequence_by_name(sequence.name) + + next if database_sequence.nil? + next if database_sequence.owner == sequence.owner + + build_inconsistency(self.class, sequence, database_sequence) + end + end + end + end + end + end +end diff --git a/gems/gitlab-schema-validation/spec/fixtures/structure.sql b/gems/gitlab-schema-validation/spec/fixtures/structure.sql index 7444324ac7b3f8..d13dfa295f2827 100644 --- a/gems/gitlab-schema-validation/spec/fixtures/structure.sql +++ b/gems/gitlab-schema-validation/spec/fixtures/structure.sql @@ -1,6 +1,10 @@ CREATE SEQUENCE missing_sequence; CREATE SEQUENCE shared_audit_event_id_seq; CREATE SEQUENCE abuse_events_id_seq; +CREATE SEQUENCE zoekt_repositories_id_seq; + +ALTER SEQUENCE abuse_events_id_seq OWNED by abuse_events.id; +ALTER SEQUENCE zoekt_repositories_id_seq OWNED by zoekt_repositories.id; CREATE INDEX missing_index ON events USING btree (created_at, author_id); diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb index d4e507f02e7915..2475e0fc022e7c 100644 --- a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/base_spec.rb @@ -28,7 +28,8 @@ Gitlab::Schema::Validation::Validators::DifferentDefinitionTables, Gitlab::Schema::Validation::Validators::DifferentDefinitionIndexes, Gitlab::Schema::Validation::Validators::DifferentDefinitionTriggers, - Gitlab::Schema::Validation::Validators::DifferentDefinitionForeignKeys + Gitlab::Schema::Validation::Validators::DifferentDefinitionForeignKeys, + Gitlab::Schema::Validation::Validators::DifferentSequenceOwners ]) end end diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/different_sequence_owners_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/different_sequence_owners_spec.rb new file mode 100644 index 00000000000000..cb8e91e9f06bb5 --- /dev/null +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/different_sequence_owners_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Schema::Validation::Validators::DifferentSequenceOwners, feature_category: :database do + wrong_sequence_owners = %w[ + public.zoekt_repositories_id_seq + ] + + include_examples 'sequence validators', described_class, wrong_sequence_owners +end diff --git a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb index 842de750216918..df42249a01ca48 100644 --- a/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb +++ b/gems/gitlab-schema-validation/spec/lib/gitlab/schema/validation/validators/missing_sequences_spec.rb @@ -5,7 +5,6 @@ RSpec.describe Gitlab::Schema::Validation::Validators::MissingSequences, feature_category: :database do missing_sequences = %w[ public.missing_sequence - public.shared_audit_event_id_seq public.abuse_events_id_seq ] diff --git a/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb b/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb index aae7013879dd58..ecac0bdf220f93 100644 --- a/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb +++ b/gems/gitlab-schema-validation/spec/support/shared_examples/sequence_validators_shared_examples.rb @@ -4,30 +4,68 @@ RSpec.shared_examples 'sequence validators' do |validator, expected_result| let(:structure_file_path) { 'spec/fixtures/structure.sql' } - let(:database_sequences) do - %w[ - wrong_sequence - extra_sequence - shared_audit_event_id_seq - ] - end - let(:inconsistency_type) { validator.name } let(:connection_class) { class_double(Class, name: 'ActiveRecord::ConnectionAdapters::PostgreSQLAdapter') } # rubocop:disable RSpec/VerifiedDoubleReference let(:connection) do - instance_double('connection', class: connection_class, current_schema: 'public') + instance_double('connection', class: connection_class, exec_query: database_sequences, current_schema: 'public') end # rubocop:enable RSpec/VerifiedDoubleReference let(:schema) { 'public' } let(:database) { Gitlab::Schema::Validation::Sources::Database.new(connection) } let(:structure_file) { Gitlab::Schema::Validation::Sources::StructureSql.new(structure_file_path, schema) } - - before do - allow(database).to receive(:sequence_exists?) do |sequence_name| - database_sequences.include?(sequence_name) - end + let(:database_sequences) do + [ + { + 'sequence_name' => 'wrong_sequence', + 'schema' => schema, + 'user_owner' => 'gitlab', + 'start_value' => '1', + 'increment_by' => '1', + 'min_value' => '1', + 'max_value' => '9223372036854775807', + 'cycle' => 'f', + 'cache_size' => '1', + 'owned_by_column' => 'some_table.id' + }, + { + 'sequence_name' => 'extra_sequence', + 'schema' => schema, + 'user_owner' => 'gitlab', + 'start_value' => '1', + 'increment_by' => '1', + 'min_value' => '1', + 'max_value' => '9223372036854775807', + 'cycle' => 'f', + 'cache_size' => '1', + 'owned_by_column' => 'extra_table.id' + }, + { + 'sequence_name' => 'zoekt_repositories_id_seq', + 'schema' => schema, + 'user_owner' => 'gitlab', + 'start_value' => '1', + 'increment_by' => '1', + 'min_value' => '1', + 'max_value' => '9223372036854775807', + 'cycle' => 'f', + 'cache_size' => '1', + 'owned_by_column' => "wrong_table.id" + }, + { + 'sequence_name' => 'shared_audit_event_id_seq', + 'schema' => schema, + 'user_owner' => 'gitlab', + 'start_value' => '1', + 'increment_by' => '1', + 'min_value' => '1', + 'max_value' => '9223372036854775807', + 'cycle' => 'f', + 'cache_size' => '1', + 'owned_by_column' => nil + } + ] end subject(:result) { validator.new(structure_file, database).execute } -- GitLab