diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index a7f3ff938c38de75c3bac7d7dfd387e524b701e7..4e2cfda22cd04726035886b3463af919f989e24b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -27,6 +27,11 @@ class Runner < Ci::ApplicationRecord project_type: 3 } + enum registration_type: { + registration_token: 0, + authenticated_user: 1 + } + enum executor_type: { unknown: 0, custom: 1, @@ -77,6 +82,8 @@ class Runner < Ci::ApplicationRecord has_one :last_build, -> { order('id DESC') }, class_name: 'Ci::Build' has_one :runner_version, primary_key: :version, foreign_key: :version, class_name: 'Ci::RunnerVersion' + belongs_to :creator, class_name: 'User', optional: true + before_save :ensure_token scope :active, -> (value = true) { where(active: value) } diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 5212ffbfd6bb933b3cb1795d5f76069babecd0a6..3651e7f6c29863a98430fde9a82f153c68807694 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -114,6 +114,10 @@ ci_runner_projects: - table: projects column: project_id on_delete: async_delete +ci_runners: + - table: users + column: creator_id + on_delete: async_nullify ci_running_builds: - table: projects column: project_id diff --git a/db/migrate/20221226105323_add_registration_columns_to_ci_runners.rb b/db/migrate/20221226105323_add_registration_columns_to_ci_runners.rb new file mode 100644 index 0000000000000000000000000000000000000000..6788f154a5cad474f37703fb43b587ef5ace5e08 --- /dev/null +++ b/db/migrate/20221226105323_add_registration_columns_to_ci_runners.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddRegistrationColumnsToCiRunners < Gitlab::Database::Migration[2.1] + enable_lock_retries! + + def change + add_column :ci_runners, :registration_type, :integer, limit: 1, default: 0, null: false + add_column :ci_runners, :creator_id, :bigint, null: true + end +end diff --git a/db/migrate/20221226132038_index_ci_runners_on_creator_id.rb b/db/migrate/20221226132038_index_ci_runners_on_creator_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..e163380533b4f71a87a416a471432f6e8215f621 --- /dev/null +++ b/db/migrate/20221226132038_index_ci_runners_on_creator_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class IndexCiRunnersOnCreatorId < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = 'index_ci_runners_on_creator_id_where_creator_id_not_null' + + def up + add_concurrent_index :ci_runners, :creator_id, where: 'creator_id IS NOT NULL', name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :ci_runners, INDEX_NAME + end +end diff --git a/db/schema_migrations/20221226105323 b/db/schema_migrations/20221226105323 new file mode 100644 index 0000000000000000000000000000000000000000..f8b82870f5f06e0e6f88d78892b3784f56f4a0f6 --- /dev/null +++ b/db/schema_migrations/20221226105323 @@ -0,0 +1 @@ +29ebddfcf7508f259eb4de595e194995b255a1a80d79aaa6d261323d8d273021 \ No newline at end of file diff --git a/db/schema_migrations/20221226132038 b/db/schema_migrations/20221226132038 new file mode 100644 index 0000000000000000000000000000000000000000..31a40e6d5f2c1a5e69ff8a07e4e5ad3464a46fe4 --- /dev/null +++ b/db/schema_migrations/20221226132038 @@ -0,0 +1 @@ +395dd3ad54b7854a12d9bf2faf575ee4d7842a75f0f16db40d26523e4e2ea21f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index a6d571e653c474ffa76688b9ff0632313e98b297..fe0be30cf53c5760da2a6365e468b92798272647 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13455,6 +13455,8 @@ CREATE TABLE ci_runners ( maintainer_note text, token_expires_at timestamp with time zone, allowed_plans text[] DEFAULT '{}'::text[] NOT NULL, + registration_type smallint DEFAULT 0 NOT NULL, + creator_id bigint, CONSTRAINT check_ce275cee06 CHECK ((char_length(maintainer_note) <= 1024)) ); @@ -28917,6 +28919,8 @@ CREATE INDEX index_ci_runners_on_created_at_and_id_where_inactive ON ci_runners CREATE INDEX index_ci_runners_on_created_at_desc_and_id_desc ON ci_runners USING btree (created_at DESC, id DESC); +CREATE INDEX index_ci_runners_on_creator_id_where_creator_id_not_null ON ci_runners USING btree (creator_id) WHERE (creator_id IS NOT NULL); + CREATE INDEX index_ci_runners_on_description_trigram ON ci_runners USING gin (description gin_trgm_ops); CREATE INDEX index_ci_runners_on_locked ON ci_runners USING btree (locked); diff --git a/doc/architecture/blueprints/runner_tokens/index.md b/doc/architecture/blueprints/runner_tokens/index.md index 67e9ae5f5edb538ce7c79e412d4a7e92201a5b42..9dcedb8796d9b1c2e0e3a8c6f6bc882c8806b8c1 100644 --- a/doc/architecture/blueprints/runner_tokens/index.md +++ b/doc/architecture/blueprints/runner_tokens/index.md @@ -140,7 +140,7 @@ instance (for example if the runner is offline). In addition, we should add the following columns to `ci_runners`: -- a `user_id` column to keep track of who created a runner; +- a `creator_id` column to keep track of who created a runner; - a `registration_type` enum column to `ci_runners` to signal whether a runner has been created using the legacy `register` method, or the new UI-based method. Possible values are `:registration_token` and `:authenticated_user`. @@ -150,7 +150,7 @@ In addition, we should add the following columns to `ci_runners`: ```sql CREATE TABLE ci_runner ( ... - user_id bigint + creator_id bigint registration_type int8 ) ``` diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 803b766c82240195dd28370eb889383857dbe116..d7db5f979fdae3824e90ef25fcc544680cf41956 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -11,6 +11,13 @@ let(:factory_name) { :ci_runner } end + context 'loose foreign key on ci_runners.creator_id' do + it_behaves_like 'cleanup by a loose foreign key' do + let!(:parent) { create(:user) } + let!(:model) { create(:ci_runner, creator: parent) } + end + end + describe 'groups association' do # Due to other associations such as projects this whole spec is allowed to # generate cross-database queries. So we have this temporary spec to diff --git a/spec/models/concerns/sensitive_serializable_hash_spec.rb b/spec/models/concerns/sensitive_serializable_hash_spec.rb index 591a4383a034bab993e186370f08e0c1648d637b..0bfd2d6a7deb28fa437c267e27723751398b4ef2 100644 --- a/spec/models/concerns/sensitive_serializable_hash_spec.rb +++ b/spec/models/concerns/sensitive_serializable_hash_spec.rb @@ -95,7 +95,7 @@ def attributes expect(model.attributes).to include(attribute) # double-check the attribute does exist expect(model.serializable_hash).not_to include(attribute) - expect(model.to_json).not_to include(attribute) + expect(model.to_json).not_to match(/\b#{attribute}\b/) expect(model.as_json).not_to include(attribute) end end