diff --git a/app/models/concerns/ci/metadatable.rb b/app/models/concerns/ci/metadatable.rb index 8d5df06c10792ea4bf18e36b7b8d018bb2ca3b25..832d17cf0a555877ff4248e6bb31fd64680d737d 100644 --- a/app/models/concerns/ci/metadatable.rb +++ b/app/models/concerns/ci/metadatable.rb @@ -21,7 +21,6 @@ module Metadatable accepts_nested_attributes_for :metadata delegate :interruptible, to: :metadata, prefix: false, allow_nil: true - delegate :id_tokens, to: :metadata, allow_nil: true delegate :exit_code, to: :metadata, allow_nil: true before_validation :ensure_metadata, on: :create @@ -109,12 +108,16 @@ def interruptible=(value) ensure_metadata.interruptible = value end + def id_tokens + read_metadata_attribute(nil, :id_tokens, :id_tokens, {}).deep_stringify_keys + end + def id_tokens? - metadata&.id_tokens.present? + id_tokens.present? end def id_tokens=(value) - ensure_metadata.id_tokens = value + ensure_metadata.id_tokens = value if can_write_metadata? end def debug_trace_enabled? diff --git a/ee/spec/presenters/ci/build_runner_presenter_spec.rb b/ee/spec/presenters/ci/build_runner_presenter_spec.rb index 36022508bccc74cb6ec178ae0cb9b75472146769..4a1531d0202500dd3705cada6f70e800d8dab131 100644 --- a/ee/spec/presenters/ci/build_runner_presenter_spec.rb +++ b/ee/spec/presenters/ci/build_runner_presenter_spec.rb @@ -6,8 +6,9 @@ subject(:presenter) { described_class.new(ci_build) } describe '#secrets_configuration' do - let!(:ci_build) { create(:ci_build, secrets: secrets) } + let!(:ci_build) { create(:ci_build, secrets: secrets, id_tokens: id_tokens) } let(:jwt_token) { "TESTING" } + let(:id_tokens) { nil } context 'build has no secrets' do let(:secrets) { {} } @@ -111,13 +112,16 @@ end context 'when there are ID tokens available' do + let(:id_tokens) do + { + 'VAULT_ID_TOKEN_1' => { aud: 'https://gitlab.test' }, + 'VAULT_ID_TOKEN_2' => { aud: 'https://gitlab.link' } + } + end + before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - ci_build.id_tokens = { - 'VAULT_ID_TOKEN_1' => { id_token: { aud: 'https://gitlab.test' } }, - 'VAULT_ID_TOKEN_2' => { id_token: { aud: 'https://gitlab.link' } } - } ci_build.runner = build_stubbed(:ci_runner) end @@ -216,13 +220,16 @@ end context 'when there are ID tokens available' do + let(:id_tokens) do + { + 'VAULT_ID_TOKEN_1' => { aud: 'https://gitlab.test' }, + 'VAULT_ID_TOKEN_2' => { aud: 'https://gitlab.link' } + } + end + before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - ci_build.id_tokens = { - 'VAULT_ID_TOKEN_1' => { id_token: { aud: 'https://gitlab.test' } }, - 'VAULT_ID_TOKEN_2' => { id_token: { aud: 'https://gitlab.link' } } - } ci_build.runner = build_stubbed(:ci_runner) end @@ -326,13 +333,16 @@ end context 'when there are ID tokens available' do + let(:id_tokens) do + { + 'VAULT_ID_TOKEN_2' => { aud: 'https://gitlab.link' }, + 'AWS_ID_TOKEN' => { aud: 'https://gitlab.test' } + } + end + before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - ci_build.id_tokens = { - 'VAULT_ID_TOKEN_2' => { id_token: { aud: 'https://gitlab.link' } }, - 'AWS_ID_TOKEN' => { id_token: { aud: 'https://gitlab.test' } } - } ci_build.runner = build_stubbed(:ci_runner) end @@ -372,18 +382,16 @@ end context 'with all AWS configuration variables present' do + let(:id_tokens) { { 'AWS_ID_TOKEN' => { aud: 'https://aws.example.com' } } } + before do create(:ci_variable, project: ci_build.project, key: 'AWS_REGION', value: 'us-west-2') create(:ci_variable, project: ci_build.project, key: 'AWS_ROLE_ARN', value: 'arn:aws:iam::123456789012:role/test-role') create(:ci_variable, project: ci_build.project, key: 'AWS_ROLE_SESSION_NAME', value: 'gitlab-ci-session') - # Add ID token rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - ci_build.id_tokens = { - 'AWS_ID_TOKEN' => { id_token: { aud: 'https://aws.example.com' } } - } ci_build.runner = build_stubbed(:ci_runner) end diff --git a/ee/spec/services/ci/register_job_service_spec.rb b/ee/spec/services/ci/register_job_service_spec.rb index c96af99035b0101f5e1b5a656583e3c7c1cfaf6b..17f793c219843d657aeaabf58b1e7c0572a3a0df 100644 --- a/ee/spec/services/ci/register_job_service_spec.rb +++ b/ee/spec/services/ci/register_job_service_spec.rb @@ -14,7 +14,8 @@ end let!(:pipeline) { create(:ci_empty_pipeline, project: project) } - let!(:pending_build) { create(:ci_build, :pending, :queued, pipeline: pipeline) } + let!(:pending_build) { create(:ci_build, :pending, :queued, pipeline: pipeline, id_tokens: id_tokens) } + let(:id_tokens) { nil } shared_examples 'namespace minutes quota' do context 'shared runners minutes limit' do @@ -231,13 +232,12 @@ end context 'when build has id_tokens defined and there is secrets provider defined' do + let(:id_tokens) { { 'TEST_ID_TOKEN' => { aud: 'https://client.test' } } } + before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - pending_build.metadata.update!( - id_tokens: { 'TEST_ID_TOKEN' => { aud: 'https://client.test' } } - ) create(:ci_variable, project: project, key: 'VAULT_SERVER_URL', value: 'https://vault.example.com') end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 5e71c60ea981fdceefd0cf33c37a310122011f1f..97fbd9e4e387e5bfe781288fe113aae6c714679a 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -34,6 +34,8 @@ { key: 'DB_NAME', value: 'postgres', public: true } ] end + + id_tokens { nil } end after(:build) do |build, evaluator| @@ -41,6 +43,16 @@ build.runner = evaluator.runner_manager.runner create(:ci_runner_machine_build, build: build, runner_manager: evaluator.runner_manager) end + + if evaluator.id_tokens + # TODO: Remove this when FF `stop_writing_builds_metadata` is removed. + # https://gitlab.com/gitlab-org/gitlab/-/issues/552065 + build.metadata.write_attribute(:id_tokens, evaluator.id_tokens) + next unless build.job_definition + + updated_config = build.job_definition.config.merge(id_tokens: evaluator.id_tokens) + build.job_definition.write_attribute(:config, updated_config) + end end trait :with_token do diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index 3007b03edc71911948d48893675fc4d19d40d76d..9f49873ce50f0d5e17d886741bdfc656a626abd7 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -25,8 +25,6 @@ expect(bridge).to have_one(:sourced_pipeline) end - it_behaves_like 'has ID tokens', :ci_bridge - it_behaves_like 'a retryable job' it_behaves_like 'a deployable job' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index dbb59bb73720ed20e1bf986285de7939174fcda1..85b4e3315b7e731606bc7dbc98cbc6941ac25c84 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -221,8 +221,6 @@ end end - it_behaves_like 'has ID tokens', :ci_build - it_behaves_like 'a retryable job' it_behaves_like 'a deployable job' do @@ -2747,9 +2745,7 @@ context 'when the build has ID tokens' do before do - build.update!( - id_tokens: { 'TEST_ID_TOKEN' => { 'aud' => 'https://client.test' } } - ) + allow(build).to receive(:id_tokens).and_return({ 'TEST_ID_TOKEN' => { 'aud' => 'https://client.test' } }) end it 'includes the tokens and excludes the predefined JWT variables' do @@ -3295,10 +3291,12 @@ def insert_expected_predefined_variables(variables, after:) ci_stage: pipeline.stages.first, ref: 'feature', project: project, - pipeline: pipeline + pipeline: pipeline, + id_tokens: id_tokens ) end + let(:id_tokens) { nil } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'feature') } context 'and id_tokens are not present in the build' do @@ -3309,8 +3307,8 @@ def insert_expected_predefined_variables(variables, after:) end context 'and id_tokens are present in the build' do - before do - build.id_tokens = { + let(:id_tokens) do + { 'ID_TOKEN_1' => { aud: 'developers' }, 'ID_TOKEN_2' => { aud: 'maintainers' } } @@ -3620,9 +3618,9 @@ def insert_expected_predefined_variables(variables, after:) before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - build.metadata.update!(id_tokens: { - 'ID_TOKEN_1' => { aud: 'developers' }, - 'ID_TOKEN_2' => { aud: 'maintainers' } + allow(build).to receive(:id_tokens).and_return({ + 'ID_TOKEN_1' => { 'aud' => 'developers' }, + 'ID_TOKEN_2' => { 'aud' => 'maintainers' } }) build.runner = build_stubbed(:ci_runner) end @@ -3668,10 +3666,10 @@ def insert_expected_predefined_variables(variables, after:) before do rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - build.metadata.update!(id_tokens: { - 'ID_TOKEN_1' => { aud: '$CI_SERVER_URL' }, - 'ID_TOKEN_2' => { aud: 'https://$CI_SERVER_HOST' }, - 'ID_TOKEN_3' => { aud: ['developers', '$CI_SERVER_URL', 'https://$CI_SERVER_HOST'] } + allow(build).to receive(:id_tokens).and_return({ + 'ID_TOKEN_1' => { 'aud' => '$CI_SERVER_URL' }, + 'ID_TOKEN_2' => { 'aud' => 'https://$CI_SERVER_HOST' }, + 'ID_TOKEN_3' => { 'aud' => ['developers', '$CI_SERVER_URL', 'https://$CI_SERVER_HOST'] } }) build.runner = build_stubbed(:ci_runner) end @@ -3710,9 +3708,9 @@ def insert_expected_predefined_variables(variables, after:) build.update!(environment: 'production') rsa_key = OpenSSL::PKey::RSA.generate(3072).to_s stub_application_setting(ci_jwt_signing_key: rsa_key) - build.metadata.update!(id_tokens: { - 'ID_TOKEN_1' => { aud: '$ENVIRONMENT_SCOPED_VAR' }, - 'ID_TOKEN_2' => { aud: ['$CI_ENVIRONMENT_NAME', '$ENVIRONMENT_SCOPED_VAR'] } + allow(build).to receive(:id_tokens).and_return({ + 'ID_TOKEN_1' => { 'aud' => '$ENVIRONMENT_SCOPED_VAR' }, + 'ID_TOKEN_2' => { 'aud' => ['$CI_ENVIRONMENT_NAME', '$ENVIRONMENT_SCOPED_VAR'] } }) build.runner = build_stubbed(:ci_runner) end @@ -4956,6 +4954,11 @@ def run_job_without_exception subject { build.send(:read_metadata_attribute, :options, :config_options, :options, default_options) } + before do + # Remove when FF `read_from_new_ci_destinations` is removed + build.clear_memoization(:read_from_new_destination?) + end + context 'when all destination options are set' do before do build.write_attribute(:options, build_options) diff --git a/spec/models/concerns/ci/metadatable_spec.rb b/spec/models/concerns/ci/metadatable_spec.rb index acbfd021ca19259304dfaa3912a166caf41e2067..535b3857b9c9edc656df910adaf136a06b5cb53f 100644 --- a/spec/models/concerns/ci/metadatable_spec.rb +++ b/spec/models/concerns/ci/metadatable_spec.rb @@ -5,6 +5,13 @@ RSpec.describe Ci::Metadatable, feature_category: :continuous_integration do let_it_be_with_refind(:processable) { create(:ci_processable, options: { script: 'echo' }) } + before do + # Remove when FF `read_from_new_ci_destinations` is removed + processable.clear_memoization(:read_from_new_destination?) + # Remove when FF `stop_writing_builds_metadata` is removed + processable.clear_memoization(:can_write_metadata?) + end + describe '#timeout_value' do using RSpec::Parameterized::TableSyntax @@ -169,4 +176,73 @@ end end end + + describe '#id_tokens' do + let(:metadata_id_tokens) { { 'TEST_ID_TOKEN' => { 'aud' => 'https://metadata' } } } + let(:job_definition_id_tokens) { { 'TEST_ID_TOKEN' => { 'aud' => 'https://job.definition' } } } + + subject(:id_tokens) { processable.id_tokens } + + it 'defaults to an empty hash' do + expect(id_tokens).to eq({}) + expect(processable.id_tokens?).to be(false) + end + + context 'when metadata id_tokens are present' do + before do + processable.ensure_metadata.write_attribute(:id_tokens, metadata_id_tokens) + end + + it 'returns metadata id_tokens' do + expect(id_tokens).to eq(metadata_id_tokens) + expect(processable.id_tokens?).to be(true) + end + + context 'when job definition id_tokens are present' do + before do + updated_config = processable.job_definition.config.merge(id_tokens: job_definition_id_tokens) + processable.job_definition.write_attribute(:config, updated_config) + end + + it 'returns job definition id_tokens' do + expect(id_tokens).to eq(job_definition_id_tokens) + expect(processable.id_tokens?).to be(true) + end + + context 'when FF `read_from_new_ci_destinations` is disabled' do + before do + stub_feature_flags(read_from_new_ci_destinations: false) + end + + it 'returns metadata id_tokens' do + expect(id_tokens).to eq(metadata_id_tokens) + expect(processable.id_tokens?).to be(true) + end + end + end + end + end + + describe '#id_tokens=' do + let(:id_tokens) { { 'TEST_ID_TOKEN' => { 'aud' => 'https://client.test' } } } + + subject(:set_id_tokens) { processable.id_tokens = id_tokens } + + it 'does not change metadata.id_tokens' do + expect { set_id_tokens } + .to not_change { processable.metadata.id_tokens } + end + + context 'when FF `stop_writing_builds_metadata` is disabled' do + before do + stub_feature_flags(stop_writing_builds_metadata: false) + end + + it 'sets the value into metadata.id_tokens' do + set_id_tokens + + expect(processable.metadata.id_tokens).to eq(id_tokens) + end + end + end end diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 0438754bb94038a94f8b3b31055554cb9d0d5495..f0c367504ad727367f5019ac5bd83035409568f5 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -722,6 +722,12 @@ def previous_commit_sha_from_ref(ref) end context 'when the configuration includes ID tokens' do + before do + # TODO: We can remove this stub after we start writing `id_tokens` to ci_job_definitions + # on pipeline creation. https://gitlab.com/gitlab-org/gitlab/-/issues/551860 + stub_feature_flags(stop_writing_builds_metadata: false) + end + it 'creates variables for the ID tokens' do config = YAML.dump({ job_with_id_tokens: { diff --git a/spec/support/shared_examples/models/ci/metadata_id_tokens_shared_examples.rb b/spec/support/shared_examples/models/ci/metadata_id_tokens_shared_examples.rb deleted file mode 100644 index 0c71ebe7a4d999744b115592023d2d22f4fe80ca..0000000000000000000000000000000000000000 --- a/spec/support/shared_examples/models/ci/metadata_id_tokens_shared_examples.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -RSpec.shared_examples_for 'has ID tokens' do |ci_type| - subject(:ci) { FactoryBot.build(ci_type) } - - describe 'delegations' do - it { is_expected.to delegate_method(:id_tokens).to(:metadata).allow_nil } - end - - describe '#id_tokens?' do - subject { ci.id_tokens? } - - context 'without metadata' do - let(:ci) { FactoryBot.build(ci_type) } - - it { is_expected.to be_falsy } - end - - context 'with metadata' do - let(:ci) { FactoryBot.build(ci_type, metadata: FactoryBot.build(:ci_build_metadata, id_tokens: id_tokens)) } - - context 'when ID tokens exist' do - let(:id_tokens) { { TEST_JOB_JWT: { id_token: { aud: 'developers ' } } } } - - it { is_expected.to be_truthy } - end - - context 'when ID tokens do not exist' do - let(:id_tokens) { {} } - - it { is_expected.to be_falsy } - end - end - end - - describe '#id_tokens=' do - it 'assigns the ID tokens to the CI job' do - id_tokens = [{ 'JOB_ID_TOKEN' => { 'id_token' => { 'aud' => 'https://gitlab.test ' } } }] - ci.id_tokens = id_tokens - - expect(ci.id_tokens).to match_array(id_tokens) - end - end -end