diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index a940a5f03db1dd54c5e814fd93fc9b50b2d0dfc0..04e0979c54a57f462e0aa7fcf4a137aef1dc3c5d 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -35,7 +35,8 @@ class Runner < Ci::ApplicationRecord o: ->(token_owner_record) { token_owner_record.owner.organization_id }, g: ->(token_owner_record) { token_owner_record.group_type? ? token_owner_record.sharding_key_id : nil }, p: ->(token_owner_record) { token_owner_record.project_type? ? token_owner_record.sharding_key_id : nil }, - u: ->(token_owner_record) { token_owner_record.creator_id } + u: ->(token_owner_record) { token_owner_record.creator_id }, + t: ->(token_owner_record) { token_owner_record.partition_id } } } @@ -62,6 +63,7 @@ class Runner < Ci::ApplicationRecord # Prefix assigned to runners created from the UI, instead of registered via the command line CREATED_RUNNER_TOKEN_PREFIX = 'glrt-' + REGISTRATION_RUNNER_TOKEN_PREFIX = 'glrtr-' RUNNER_SHORT_SHA_LENGTH = 8 @@ -446,11 +448,19 @@ def only_for?(project) def short_sha return unless token - # We want to show the first characters of the hash, so we need to bypass any fixed components of the token, - # such as CREATED_RUNNER_TOKEN_PREFIX or partition_id_prefix_in_16_bit_encode - partition_prefix = partition_id_prefix_in_16_bit_encode - start_index = authenticated_user_registration_type? ? CREATED_RUNNER_TOKEN_PREFIX.length : 0 - start_index += partition_prefix.length if token[start_index..].start_with?(partition_prefix) + # We want to show the first characters of the hash, so we need to bypass any fixed components of the token, such + # as CREATED_RUNNER_TOKEN_PREFIX, REGISTRATION_RUNNER_TOKEN_PREFIX or legacy_partition_id_prefix_in_16_bit_encode + legacy_partition_prefix = legacy_partition_id_prefix_in_16_bit_encode + start_index = if authenticated_user_registration_type? + CREATED_RUNNER_TOKEN_PREFIX.length + elsif token.start_with?(REGISTRATION_RUNNER_TOKEN_PREFIX) + REGISTRATION_RUNNER_TOKEN_PREFIX.length + else + 0 + end + + start_index += legacy_partition_prefix.length if token[start_index..].start_with?(legacy_partition_prefix) + token[start_index..start_index + RUNNER_SHORT_SHA_LENGTH - 1] end @@ -568,6 +578,10 @@ def dedicated_gitlab_hosted? false end + def partition_id + self.class.runner_types[runner_type] + end + private scope :with_upgrade_status, ->(upgrade_status) do @@ -667,17 +681,17 @@ def no_allowed_plan_ids errors.add(:runner, 'cannot have allowed plans assigned') unless allowed_plan_ids.empty? end - def partition_id_prefix_in_16_bit_encode + def legacy_partition_id_prefix_in_16_bit_encode # Prefix with t1 / t2 / t3 (`t` as in runner type, to allow us to easily detect how a token got prefixed). # This is needed in order to ensure that tokens have unique values across partitions # in the new ci_runners partitioned table. - "t#{self.class.runner_types[runner_type].to_s(16)}_" + "t#{partition_id.to_s(16)}_" end def prefix_for_new_and_legacy_runner - return partition_id_prefix_in_16_bit_encode if registration_token_registration_type? + return REGISTRATION_RUNNER_TOKEN_PREFIX if registration_token_registration_type? - "#{CREATED_RUNNER_TOKEN_PREFIX}#{partition_id_prefix_in_16_bit_encode}" + CREATED_RUNNER_TOKEN_PREFIX end end end diff --git a/lib/authn/token_field/generator/routable_token.rb b/lib/authn/token_field/generator/routable_token.rb index 15d68a9d7f7c09ef25f760da51ec1b9fee7ce625..9c0e83ab7f40a02b58a481f2d0d2d4dff688ac84 100644 --- a/lib/authn/token_field/generator/routable_token.rb +++ b/lib/authn/token_field/generator/routable_token.rb @@ -7,7 +7,7 @@ class RoutableToken RANDOM_BYTES_LENGTH = 16 BASE64_PAYLOAD_LENGTH_HOLDER_BYTES = 2 CRC_BYTES = 7 - VALID_ROUTING_KEYS = %i[c g o p u].freeze + VALID_ROUTING_KEYS = %i[c g o p u t].freeze REQUIRED_ROUTING_KEYS = %i[o].freeze MAXIMUM_SIZE_OF_ROUTING_PAYLOAD = 159 DEFAULT_ROUTING_PAYLOAD_HASH = diff --git a/spec/lib/authn/token_field/generator/routable_token_spec.rb b/spec/lib/authn/token_field/generator/routable_token_spec.rb index 872dcb3200cc7b614e949ccff0839ea8e43c657d..57b04777154e78536f8419c6a238771504f7c8b6 100644 --- a/spec/lib/authn/token_field/generator/routable_token_spec.rb +++ b/spec/lib/authn/token_field/generator/routable_token_spec.rb @@ -41,7 +41,7 @@ it 'raises an exception' do expect { generator }.to raise_error(described_class::InvalidRoutingKeys, - "Invalid routing keys: :q, :k. Valid routing keys are: :c, :g, :o, :p, :u.") + "Invalid routing keys: :q, :k. Valid routing keys are: :c, :g, :o, :p, :u, :t.") end end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 7200e49bc460118db8aac2bca797c10bb602a645..92f152fd0b368a081985ec88e1ba9b76bef7f709 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -1810,18 +1810,40 @@ def does_db_update context 'when registered via command-line' do let_it_be(:runner) { create(:ci_runner) } + specify { expect(runner.token).to start_with(described_class::REGISTRATION_RUNNER_TOKEN_PREFIX) } specify { expect(runner.token).not_to start_with(described_class::CREATED_RUNNER_TOKEN_PREFIX) } it { is_expected.to match(/[0-9a-zA-Z_-]{8}/) } - it { is_expected.not_to start_with('t1_') } + it { is_expected.not_to start_with(described_class::REGISTRATION_RUNNER_TOKEN_PREFIX) } it { is_expected.not_to start_with(described_class::CREATED_RUNNER_TOKEN_PREFIX) } end + context 'when legacy token' do + context 'with legacy partition prefix' do + let_it_be(:runner) { create(:ci_runner, token: 't1_deadbeaf') } + + it { is_expected.to eq('deadbeaf') } + end + + context 'with runner token prefix and legacy partition prefix' do + let_it_be(:runner) { create(:ci_runner, registration_type: :authenticated_user, token: 'glrt-t1_deadbeaf') } + + it { is_expected.to eq('deadbeaf') } + end + + context 'without prefix' do + let_it_be(:runner) { create(:ci_runner, token: 'deadbeaf') } + + it { is_expected.to eq('deadbeaf') } + end + end + context 'when creating new runner via UI' do let_it_be(:runner) { create(:ci_runner, registration_type: :authenticated_user) } specify { expect(runner.token).to start_with(described_class::CREATED_RUNNER_TOKEN_PREFIX) } + specify { expect(runner.token).not_to start_with(described_class::REGISTRATION_RUNNER_TOKEN_PREFIX) } it { is_expected.to match(/[0-9a-zA-Z_-]{8}/) } - it { is_expected.not_to start_with('t1_') } + it { is_expected.not_to start_with(described_class::REGISTRATION_RUNNER_TOKEN_PREFIX) } it { is_expected.not_to start_with(described_class::CREATED_RUNNER_TOKEN_PREFIX) } end end @@ -1858,12 +1880,18 @@ def does_db_update end shared_examples 'an encrypted routable token for resource' do |prefix| + let(:routing_payload) do + { + c: 1, + **(resource.is_a?(Group) ? { g: resource.id.to_s(36) } : { p: resource.id.to_s(36) }), + o: resource.organization_id.to_s(36), + u: creator.id.to_s(36), + t: described_class.runner_types[runner_type].to_s(36) + }.sort.to_h + end + let(:expected_routing_payload) do - if resource.is_a?(Group) - "c:1\ng:#{resource.id.to_s(36)}\no:#{resource.organization_id.to_s(36)}\nu:#{creator.id.to_s(36)}" - else - "c:1\no:#{resource.organization_id.to_s(36)}\np:#{resource.id.to_s(36)}\nu:#{creator.id.to_s(36)}" - end + routing_payload.map { |pairs| pairs.join(':') }.join("\n") end context 'when :routable_runner_token feature flag is enabled for the resource' do @@ -1924,15 +1952,15 @@ def does_db_update let(:registration_type) { :registration_token } context 'when runner is instance type' do - it_behaves_like 'an instance runner encrypted token', 't1_' + it_behaves_like 'an instance runner encrypted token', described_class::REGISTRATION_RUNNER_TOKEN_PREFIX end context 'when runner is group type' do - it_behaves_like 'a group runner encrypted token', 't2_' + it_behaves_like 'a group runner encrypted token', described_class::REGISTRATION_RUNNER_TOKEN_PREFIX end context 'when runner is project type' do - it_behaves_like 'a project runner encrypted token', 't3_' + it_behaves_like 'a project runner encrypted token', described_class::REGISTRATION_RUNNER_TOKEN_PREFIX end end @@ -1940,15 +1968,15 @@ def does_db_update let(:registration_type) { :authenticated_user } context 'when runner is instance type' do - it_behaves_like 'an instance runner encrypted token', 'glrt-t1_' + it_behaves_like 'an instance runner encrypted token', described_class::CREATED_RUNNER_TOKEN_PREFIX end context 'when runner is group type' do - it_behaves_like 'a group runner encrypted token', 'glrt-t2_' + it_behaves_like 'a group runner encrypted token', described_class::CREATED_RUNNER_TOKEN_PREFIX end context 'when runner is project type' do - it_behaves_like 'a project runner encrypted token', 'glrt-t3_' + it_behaves_like 'a project runner encrypted token', described_class::CREATED_RUNNER_TOKEN_PREFIX end end end @@ -2347,6 +2375,28 @@ def does_db_update specify { expect(is_hosted).to be false } end + describe '#partition_id' do + subject(:partition) { runner.partition_id } + + context 'with an instance runner' do + let(:runner) { build(:ci_runner, :instance) } + + it { is_expected.to be(1) } + end + + context 'with a group runner' do + let(:runner) { build(:ci_runner, :group, groups: [group]) } + + it { is_expected.to be(2) } + end + + context 'with a project runner' do + let(:runner) { build(:ci_runner, :project, projects: [project]) } + + it { is_expected.to be(3) } + end + end + describe 'status scopes', :freeze_time do before_all do freeze_time # Freeze time before `let_it_be` runs, so that runner statuses are frozen during execution