Merge branch 'pedropombeiro/386491/add-registration-columns-to-ci_runners-table' into 'master'

Add registration columns to ci_runners table

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107793



Merged-by: Mayra Cabrera <mcabrera@gitlab.com>
Approved-by: Carla Drago <cdrago@gitlab.com>
Approved-by: Tarun Vellishetty <tvellishetty@gitlab.com>
Approved-by: Dylan Griffith <dyl.griffith@gmail.com>
Approved-by: Mayra Cabrera <mcabrera@gitlab.com>
Reviewed-by: Kamil Trzciński <ayufan@ayufan.eu>
Reviewed-by: Pedro Pombeiro <noreply@pedro.pombei.ro>
Co-authored-by: Pedro Pombeiro <noreply@pedro.pombei.ro>

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) }

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

db/migrate/20221226105323_add_registration_columns_to_ci_runners.rb

+# 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

db/migrate/20221226132038_index_ci_runners_on_creator_id.rb

+# 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

db/schema_migrations/20221226105323

+29ebddfcf7508f259eb4de595e194995b255a1a80d79aaa6d261323d8d273021
\ No newline at end of file

db/schema_migrations/20221226132038

+395dd3ad54b7854a12d9bf2faf575ee4d7842a75f0f16db40d26523e4e2ea21f
\ No newline at end of file

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);

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
 )
 ```

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

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