From 176ab4ae575a0bb2c724cccb94dfff846166f657 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 17 Jan 2024 12:45:16 +0100 Subject: [PATCH 1/3] Add identity YAML config entry --- app/assets/javascripts/editor/schema/ci.json | 14 +++ doc/ci/yaml/index.md | 33 ++++++- ee/app/models/ee/ci/build.rb | 12 +++ .../ci_yaml_support_for_identity_provider.yml | 9 ++ ee/lib/ee/gitlab/ci/config/entry/job.rb | 16 +++- ee/lib/ee/gitlab/ci/yaml_processor/result.rb | 3 +- .../ci/config/entry/identity_provider.rb | 25 ++++++ ...ate_build_environment_variables_service.rb | 38 +++++++++ .../gitlab/ci/config/entry/job_spec.rb | 56 +++++++++++- .../gitlab/ci/yaml_processor/result_spec.rb | 32 +++++++ ...uild_environment_variables_service_spec.rb | 70 +++++++++++++++ ee/spec/lib/gitlab/ci/yaml_processor_spec.rb | 85 ++++++++++++------- ee/spec/models/ci/build_spec.rb | 52 ++++++++++++ lib/gitlab/ci/config/entry/job.rb | 2 +- lib/gitlab/ci/yaml_processor/feature_flags.rb | 4 +- .../negative_tests/identity_provider.yml | 5 ++ .../positive_tests/identity_provider.yml | 2 + .../ci/yaml_processor/feature_flags_spec.rb | 22 +++-- 18 files changed, 432 insertions(+), 48 deletions(-) create mode 100644 ee/config/feature_flags/beta/ci_yaml_support_for_identity_provider.yml create mode 100644 ee/lib/gitlab/ci/config/entry/identity_provider.rb create mode 100644 ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb rename ee/spec/lib/{ => ee}/gitlab/ci/config/entry/job_spec.rb (64%) create mode 100644 ee/spec/lib/ee/gitlab/ci/yaml_processor/result_spec.rb create mode 100644 ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb create mode 100644 spec/frontend/editor/schema/ci/yaml_tests/negative_tests/identity_provider.yml create mode 100644 spec/frontend/editor/schema/ci/yaml_tests/positive_tests/identity_provider.yml diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index 1da224c2174ffe..02cd2f99f3bbde 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -61,6 +61,9 @@ "id_tokens": { "$ref": "#/definitions/id_tokens" }, + "identity_provider": { + "$ref": "#/definitions/identity_provider" + }, "retry": { "$ref": "#/definitions/retry" }, @@ -703,6 +706,14 @@ } } }, + "identity_provider": { + "type": "string", + "markdownDescription": "Sets an identity provider (experimental), allowing automatic authentication with the external provider. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#identity_provider).", + "enum": [ + "google_cloud" + ], + "additionalProperties": false + }, "secrets": { "type": "object", "markdownDescription": "Defines secrets to be injected as environment variables. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#secrets).", @@ -1632,6 +1643,9 @@ "id_tokens": { "$ref": "#/definitions/id_tokens" }, + "identity_provider": { + "$ref": "#/definitions/identity_provider" + }, "secrets": { "$ref": "#/definitions/secrets" }, diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index b95397bbc735c4..bf050a761e10d5 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -1,7 +1,10 @@ --- stage: Verify group: Pipeline Authoring -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments +info: >- + To determine the technical writer assigned to the Stage/Group associated with + this page, see + https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments --- # CI/CD YAML syntax reference @@ -59,6 +62,7 @@ A GitLab CI/CD pipeline configuration includes: | [`dependencies`](#dependencies) | Restrict which artifacts are passed to a specific job by providing a list of jobs to fetch artifacts from. | | [`environment`](#environment) | Name of an environment to which the job deploys. | | [`extends`](#extends) | Configuration entries that this job inherits from. | + | [`identity_provider`](#identity_provider) | Authenticate with third party services. | | [`image`](#image) | Use Docker images. | | [`inherit`](#inherit) | Select which global defaults all jobs inherit. | | [`interruptible`](#interruptible) | Defines if a job can be canceled when made redundant by a newer run. | @@ -2437,6 +2441,33 @@ job1: - [GitLab Runner configuration](https://docs.gitlab.com/runner/configuration/advanced-configuration.html#the-runners-section) +### `identity_provider` **(EXPERIMENT)** + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142054) in GitLab 16.9. This feature is an [Experiment](../../policy/experiment-beta-support.md). + +FLAG: +On GitLab.com, this feature is not available. +The feature is not ready for production use. + +Use `identity_provider` to authenticate with third party services. + +**Keyword type**: Job keyword. You can use it only as part of a job or in the [`default:` section](#default). + +**Possible inputs**: A provider identifier. Supported providers: `google_cloud` (Google Cloud). The Google Cloud + +**Example of `identity_provider`**: + +```yaml +job_with_identity_provider: + identity_provider: google_cloud + script: + - gcloud compute instances list +``` + +**Related topics**: + +- [Workload Identity Federation](https://cloud.google.com/iam/docs/workload-identity-federation). + ### `id_tokens` > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356986) in GitLab 15.7. diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb index 9a6aec29e2b24e..62b1003108804a 100644 --- a/ee/app/models/ee/ci/build.rb +++ b/ee/app/models/ee/ci/build.rb @@ -83,6 +83,11 @@ def variables end end + override :job_jwt_variables + def job_jwt_variables + super.concat(identity_provider_variables) + end + def cost_factor_enabled? runner&.cost_factor_enabled?(project) end @@ -313,6 +318,13 @@ def azure_key_vault_provider? def hashicorp_vault_provider? variable_value('VAULT_SERVER_URL').present? end + + def identity_provider_variables + # NOTE: For now, only google_cloud identity provider is supported + return [] unless options[:identity_provider] == 'google_cloud' + + ::Gitlab::Ci::GoogleCloud::GenerateBuildEnvironmentVariablesService.new(self).execute + end end end end diff --git a/ee/config/feature_flags/beta/ci_yaml_support_for_identity_provider.yml b/ee/config/feature_flags/beta/ci_yaml_support_for_identity_provider.yml new file mode 100644 index 00000000000000..4034e91ac5b48f --- /dev/null +++ b/ee/config/feature_flags/beta/ci_yaml_support_for_identity_provider.yml @@ -0,0 +1,9 @@ +--- +name: ci_yaml_support_for_identity_provider +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/438546 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142054 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/438420 +milestone: '16.9' +group: group::pipeline security +type: beta +default_enabled: false diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb index 372cb00f3f73c4..b090c8d4210f84 100644 --- a/ee/lib/ee/gitlab/ci/config/entry/job.rb +++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb @@ -9,15 +9,19 @@ module Job extend ActiveSupport::Concern extend ::Gitlab::Utils::Override - EE_ALLOWED_KEYS = %i[dast_configuration secrets].freeze + EE_ALLOWED_KEYS = %i[dast_configuration identity_provider secrets].freeze prepended do - attributes :dast_configuration, :secrets + attributes :dast_configuration, :identity_provider, :secrets entry :dast_configuration, ::Gitlab::Ci::Config::Entry::DastConfiguration, description: 'DAST configuration for this job', inherit: false + entry :identity_provider, ::Gitlab::Ci::Config::Entry::IdentityProvider, + description: 'Configured identity provider for this job.', + inherit: false + entry :secrets, ::Gitlab::Config::Entry::ComposableHash, description: 'Configured secrets for this job', inherit: false, @@ -37,9 +41,17 @@ def allowed_keys def value super.merge({ dast_configuration: dast_configuration_value, + identity_provider: identity_provider_available? ? identity_provider : nil, secrets: secrets_value }.compact) end + + private + + def identity_provider_available? + ::Gitlab::Ci::YamlProcessor::FeatureFlags.enabled?(:ci_yaml_support_for_identity_provider, type: :beta) && + ::Gitlab::Saas.feature_available?(:google_artifact_registry) + end end end end diff --git a/ee/lib/ee/gitlab/ci/yaml_processor/result.rb b/ee/lib/ee/gitlab/ci/yaml_processor/result.rb index 2924ef2bfaf075..fd354c612c9fd1 100644 --- a/ee/lib/ee/gitlab/ci/yaml_processor/result.rb +++ b/ee/lib/ee/gitlab/ci/yaml_processor/result.rb @@ -16,7 +16,8 @@ def build_attributes(name) super.deep_merge( { options: { - dast_configuration: job[:dast_configuration] + dast_configuration: job[:dast_configuration], + identity_provider: job[:identity_provider] }.compact, secrets: job[:secrets] }.compact diff --git a/ee/lib/gitlab/ci/config/entry/identity_provider.rb b/ee/lib/gitlab/ci/config/entry/identity_provider.rb new file mode 100644 index 00000000000000..c1211dba7d3c9f --- /dev/null +++ b/ee/lib/gitlab/ci/config/entry/identity_provider.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + ## + # Entry that represents identity provider settings. + # + class IdentityProvider < ::Gitlab::Config::Entry::Node + include ::Gitlab::Config::Entry::Validatable + + ALLOWED_IDENTITY_PROVIDERS = %w[google_cloud].freeze + + validations do + validates :config, presence: false, type: String, inclusion: { + in: ALLOWED_IDENTITY_PROVIDERS, + message: "should be one of: #{ALLOWED_IDENTITY_PROVIDERS.join(', ')}" + } + end + end + end + end + end +end diff --git a/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb b/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb new file mode 100644 index 00000000000000..a5e14ed92cf807 --- /dev/null +++ b/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module GoogleCloud + class GenerateBuildEnvironmentVariablesService + def initialize(build) + @build = build + + # TODO: This will be refactored to use a new IAM integration + @integration = build.project.google_cloud_platform_artifact_registry_integration + end + + def execute + return [] unless @integration&.active + + json = ::GoogleCloudPlatform::BaseClient.credentials( + audience: @integration.wlif, + encoded_jwt: encoded_jwt + ).to_json + + var_attributes = { value: json, public: false, masked: true, file: true } + + [ + { key: 'CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE', **var_attributes }, + { key: 'GOOGLE_APPLICATION_CREDENTIALS', **var_attributes } + ] + end + + private + + def encoded_jwt + JwtV2.for_build(@build, aud: ::GoogleCloudPlatform::BaseClient::GLGO_BASE_URL, wlif: @integration.wlif) + end + end + end + end +end diff --git a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb b/ee/spec/lib/ee/gitlab/ci/config/entry/job_spec.rb similarity index 64% rename from ee/spec/lib/gitlab/ci/config/entry/job_spec.rb rename to ee/spec/lib/ee/gitlab/ci/config/entry/job_spec.rb index d0e1d4939ba52d..f791dcabd6c592 100644 --- a/ee/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/ee/spec/lib/ee/gitlab/ci/config/entry/job_spec.rb @@ -2,14 +2,14 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Config::Entry::Job do +RSpec.describe Gitlab::Ci::Config::Entry::Job, feature_category: :pipeline_composition do let(:entry) { described_class.new(config, name: :rspec) } describe '.nodes' do context 'when filtering all the entry/node names' do subject { described_class.nodes.keys } - let(:result) { %i[dast_configuration secrets] } + let(:result) { %i[dast_configuration identity_provider secrets] } it { is_expected.to include(*result) } end @@ -47,7 +47,9 @@ end context 'when has dast_configuration' do - let(:config) { { script: 'echo', dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' } } } + let(:config) do + { script: 'echo', dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' } } + end it_behaves_like 'a valid entry' end @@ -73,11 +75,19 @@ it_behaves_like 'an invalid entry', 'secrets config should be a hash' end + + context 'when entry has unknown identity provider' do + let(:config) { { script: 'rspec', identity_provider: 'unknown' } } + + it_behaves_like 'an invalid entry', 'identity_provider config should be one of: google_cloud' + end end end describe 'dast_configuration' do - let(:config) { { script: 'echo', dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' } } } + let(:config) do + { script: 'echo', dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' } } + end before do entry.compose! @@ -136,4 +146,42 @@ }) end end + + describe 'identity_provider', :aggregate_failures, feature_category: :secrets_management do + let(:feature_flag_enabled) { true } + let(:saas_feature_enabled) { true } + let(:config) do + { + script: 'rspec', + identity_provider: 'google_cloud' + } + end + + before do + stub_feature_flags(ci_yaml_support_for_identity_provider: feature_flag_enabled) + stub_saas_features(google_artifact_registry: saas_feature_enabled) + + entry.compose! + end + + it 'includes identity provider-related values' do + expect(entry.value).to include(identity_provider: 'google_cloud') + end + + context 'when ci_yaml_support_for_identity_provider FF is disabled' do + let(:feature_flag_enabled) { false } + + it 'does not include identity provider-related values' do + expect(entry.value).not_to match(a_hash_including(identity_provider: anything)) + end + end + + context 'when feature is disabled' do + let(:saas_feature_enabled) { false } + + it 'does not include identity provider-related values' do + expect(entry.value).not_to match(a_hash_including(identity_provider: anything)) + end + end + end end diff --git a/ee/spec/lib/ee/gitlab/ci/yaml_processor/result_spec.rb b/ee/spec/lib/ee/gitlab/ci/yaml_processor/result_spec.rb new file mode 100644 index 00000000000000..cd93559e5416d7 --- /dev/null +++ b/ee/spec/lib/ee/gitlab/ci/yaml_processor/result_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::YamlProcessor::Result, feature_category: :pipeline_composition do + include StubRequests + + let_it_be(:user) { create(:user) } + + let(:ci_config) { Gitlab::Ci::Config.new(config_content, user: user) } + let(:result) { described_class.new(ci_config: ci_config, warnings: ci_config&.warnings) } + + subject(:build) { result.builds.first } + + describe '#builds' do + context 'when a job has identity_provider', feature_category: :secrets_management do + let(:config_content) do + YAML.dump( + test: { stage: 'test', script: 'echo', identity_provider: 'google_cloud' } + ) + end + + before do + stub_saas_features(google_artifact_registry: true) + end + + it 'includes :identity_provider in :options' do + expect(build.dig(:options, :identity_provider)).to eq('google_cloud') + end + end + end +end diff --git a/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb b/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb new file mode 100644 index 00000000000000..86965ed6028116 --- /dev/null +++ b/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe ::Gitlab::Ci::GoogleCloud::GenerateBuildEnvironmentVariablesService, '#execute', feature_category: :secrets_management do + let_it_be_with_refind(:project) { create(:project) } + let_it_be_with_refind(:integration) { create(:google_cloud_platform_artifact_registry_integration, project: project) } + let_it_be(:user) { create(:user) } + let_it_be(:rsa_key) { OpenSSL::PKey::RSA.generate(3072) } + let_it_be(:rsa_key_data) { rsa_key.to_s } + + let(:build) { build_stubbed(:ci_build, project: project, user: user) } + let(:service) { described_class.new(build) } + + subject(:execute) { service.execute } + + before do + stub_application_setting(ci_jwt_signing_key: rsa_key_data) + end + + it 'returns variables containing valid config.json', :aggregate_failures do + expect(execute).to contain_exactly( + a_hash_including(key: 'GOOGLE_APPLICATION_CREDENTIALS', file: true, masked: true), + a_hash_including(key: 'CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE', file: true, masked: true) + ) + + expect(Gitlab::Json.parse(execute.first[:value])).to match( + 'type' => 'external_account', + 'audience' => integration.wlif, + 'subject_token_type' => 'urn:ietf:params:oauth:token-type:jwt', + 'token_url' => 'https://sts.googleapis.com/v1/token', + 'credential_source' => { + 'url' => 'https://auth.gcp.gitlab.com/token', + 'headers' => { 'Authorization' => a_string_matching(/Bearer [0-9a-zA-Z_\-.]+/) }, + 'format' => { 'type' => 'json', 'subject_token_field_name' => 'token' } + }) + expect(execute.pluck(:value)).to all(eq(execute.first[:value])) + end + + it 'creates a config with expected JWT token' do + config = Gitlab::Json.parse(execute.first[:value]) + authorization = config.dig(*%w[credential_source headers Authorization]) + encoded_token = authorization.split(' ').last + payload, _ = ::JWT.decode(encoded_token, rsa_key, true, { algorithm: 'RS256' }) + + expect(payload).to match(a_hash_including( + 'namespace_id' => project.namespace_id.to_s, + 'project_id' => project.id.to_s, + 'user_id' => user.id.to_s, + 'aud' => 'https://auth.gcp.gitlab.com', + 'wlif' => integration.wlif + )) + end + + context 'when integration is not present' do + before do + integration.destroy! + end + + it { is_expected.to be_empty } + end + + context 'when integration is inactive' do + before do + integration.update_column(:active, false) + end + + it { is_expected.to be_empty } + end +end diff --git a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb index 5d09aaf45c7257..35e8d8843c8845 100644 --- a/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/ee/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -2,7 +2,9 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::YamlProcessor do +RSpec.describe Gitlab::Ci::YamlProcessor, feature_category: :pipeline_composition do + subject(:result) { described_class.new(YAML.dump(config)).execute } + describe 'Bridge Needs' do let(:config) do { @@ -11,14 +13,12 @@ } end - subject { described_class.new(YAML.dump(config)).execute } - - context 'needs upstream pipeline' do + context 'when needs upstream pipeline' do let(:needs) { { pipeline: 'some/project' } } it 'creates jobs with valid specification' do - expect(subject.builds.size).to eq(2) - expect(subject.builds[0]).to eq( + expect(result.builds.size).to eq(2) + expect(result.builds[0]).to eq( stage: "build", stage_idx: 1, name: "build", @@ -32,7 +32,7 @@ root_variables_inheritance: true, scheduling_type: :stage ) - expect(subject.builds[1]).to eq( + expect(result.builds[1]).to eq( stage: "test", stage_idx: 2, name: "bridge", @@ -49,12 +49,12 @@ end end - context 'needs both job and pipeline' do + context 'when needs both job and pipeline' do let(:needs) { ['build', { pipeline: 'some/project' }] } it 'creates jobs with valid specification' do - expect(subject.builds.size).to eq(2) - expect(subject.builds[0]).to eq( + expect(result.builds.size).to eq(2) + expect(result.builds[0]).to eq( stage: "build", stage_idx: 1, name: "build", @@ -68,7 +68,7 @@ root_variables_inheritance: true, scheduling_type: :stage ) - expect(subject.builds[1]).to eq( + expect(result.builds[1]).to eq( stage: "test", stage_idx: 2, name: "bridge", @@ -88,7 +88,7 @@ end end - context 'needs cross projects artifacts' do + context 'when needs cross projects artifacts' do let(:config) do { build: { stage: 'build', script: 'test' }, @@ -121,9 +121,9 @@ end it 'creates jobs with valid specification' do - expect(subject.builds.size).to eq(3) + expect(result.builds.size).to eq(3) - expect(subject.builds[1]).to eq( + expect(result.builds[1]).to eq( stage: 'test', stage_idx: 2, name: 'test1', @@ -163,7 +163,7 @@ end end - context 'needs cross projects artifacts and pipelines' do + context 'when needs cross projects artifacts and pipelines' do let(:needs) do [ { @@ -179,7 +179,7 @@ end it 'returns errors' do - expect(subject.errors).to include( + expect(result.errors).to include( 'jobs:bridge config should contain either a trigger or a needs:pipeline') end end @@ -202,12 +202,12 @@ end it 'returns errors' do - expect(subject.errors).to contain_exactly( + expect(result.errors).to contain_exactly( 'jobs:test:needs:need ref should be a string') end end - describe 'cross pipeline needs' do + describe 'with cross pipeline needs' do context 'when job is not present' do let(:config) do { @@ -222,10 +222,10 @@ end it 'returns an error' do - expect(subject).not_to be_valid + expect(result).not_to be_valid # This currently shows a confusing error message because a conflict of syntax # with upstream pipeline status mirroring: https://gitlab.com/gitlab-org/gitlab/-/issues/280853 - expect(subject.errors).to include(/:needs config uses invalid types: bridge/) + expect(result.errors).to include(/:needs config uses invalid types: bridge/) end end end @@ -245,9 +245,9 @@ end it 'returns a valid specification' do - expect(subject).to be_valid + expect(result).to be_valid - rspec = subject.builds.last + rspec = result.builds.last expect(rspec.dig(:options, :cross_dependencies)).to eq( [ { pipeline: '$UPSTREAM_PIPELINE_ID', job: 'test', artifacts: true }, @@ -258,17 +258,25 @@ describe 'dast configuration' do let(:config) do - { build: { stage: 'build', dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' }, script: 'test' } } + { + build: { + stage: 'build', + dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' }, + script: 'test' + } + } end it 'creates a job with a valid specification' do - expect(subject.builds[0][:options]).to include(dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' }) + expect(result.builds[0][:options]).to include( + dast_configuration: { site_profile: 'Site profile', scanner_profile: 'Scanner profile' } + ) end end end describe 'secrets' do - context 'hashicorp vault' do + context 'on hashicorp vault' do let(:secrets) do { DATABASE_PASSWORD: { @@ -279,8 +287,6 @@ let(:config) { { deploy_to_production: { stage: 'deploy', script: ['echo'], secrets: secrets } } } - subject(:result) { described_class.new(YAML.dump(config)).execute } - it "returns secrets info" do secrets = result.builds.first.fetch(:secrets) @@ -296,7 +302,7 @@ end end - context 'azure key vault' do + context 'on azure key vault' do let(:secrets) do { DATABASE_PASSWORD: { @@ -310,8 +316,6 @@ let(:config) { { deploy_to_production: { stage: 'deploy', script: ['echo'], secrets: secrets } } } - subject(:result) { described_class.new(YAML.dump(config)).execute } - it "returns secrets info" do secrets = result.builds.first.fetch(:secrets) @@ -326,4 +330,25 @@ end end end + + describe 'identity_provider', feature_category: :secrets_management do + let(:config) do + { + build: { + stage: 'build', script: 'test', + identity_provider: 'google_cloud' + } + } + end + + before do + stub_saas_features(google_artifact_registry: true) + end + + it 'includes identity provider-related values' do + identity_provider = result.builds.first.dig(:options, :identity_provider) + + expect(identity_provider).to eq('google_cloud') + end + end end diff --git a/ee/spec/models/ci/build_spec.rb b/ee/spec/models/ci/build_spec.rb index 05d8eb2ba7fe8d..784fbcb80be102 100644 --- a/ee/spec/models/ci/build_spec.rb +++ b/ee/spec/models/ci/build_spec.rb @@ -1005,6 +1005,58 @@ end end + describe 'build identity_provider' do + let_it_be(:user) { create(:user) } + + let(:build) do + create(:ci_build, pipeline: pipeline, user: user, options: { identity_provider: 'google_cloud' }) + end + + subject(:variables) { build.variables } + + before do + rsa_key = OpenSSL::PKey::RSA.generate(3072) + stub_application_setting(ci_jwt_signing_key: rsa_key.to_s) + end + + it 'does not include the gcloud file variables' do + runner_vars = variables.to_runner_variables.index_by { |v| v[:key] } + runner_var_names = runner_vars.keys + + expect(runner_var_names).not_to include('CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE') + expect(runner_var_names).not_to include('GOOGLE_APPLICATION_CREDENTIALS') + end + + context 'with integration active' do + let!(:project) { create(:project) } + let!(:integration) { create(:google_cloud_platform_artifact_registry_integration, project: project) } + let!(:pipeline) { create(:ci_pipeline, project: project, status: 'success') } + + it 'includes the gcloud file variables' do + runner_vars = variables.to_runner_variables.index_by { |v| v[:key] } + runner_var_names = runner_vars.keys + + expect(runner_var_names).to include('CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE') + expect(runner_var_names).to include('GOOGLE_APPLICATION_CREDENTIALS') + expect(runner_vars['GOOGLE_APPLICATION_CREDENTIALS']).to include(file: true) + expect(runner_vars['GOOGLE_APPLICATION_CREDENTIALS'].except(:key)).to eq( + runner_vars['CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE'].except(:key)) + + json = Gitlab::Json.parse(runner_vars['CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE'][:value]) + expect(json).to match( + 'type' => 'external_account', + 'audience' => an_instance_of(String), + 'subject_token_type' => 'urn:ietf:params:oauth:token-type:jwt', + 'token_url' => 'https://sts.googleapis.com/v1/token', + 'credential_source' => { + 'url' => 'https://auth.gcp.gitlab.com/token', + 'headers' => { 'Authorization' => an_instance_of(String) }, + 'format' => { 'type' => 'json', 'subject_token_field_name' => 'token' } + }) + end + end + end + context 'with loose foreign keys for partitioned tables' do before do create(:security_scan, build: job) diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 7ea4b460640e18..74edf814803537 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -121,7 +121,7 @@ class Job < ::Gitlab::Config::Entry::Node inherit: false entry :id_tokens, ::Gitlab::Config::Entry::ComposableHash, - description: 'Configured JWTs for this job', + description: 'Configured JWTs for this job.', inherit: true, metadata: { composable_class: ::Gitlab::Ci::Config::Entry::IdToken } diff --git a/lib/gitlab/ci/yaml_processor/feature_flags.rb b/lib/gitlab/ci/yaml_processor/feature_flags.rb index 50d37f6e4a009a..5a2fa7c8bc4edd 100644 --- a/lib/gitlab/ci/yaml_processor/feature_flags.rb +++ b/lib/gitlab/ci/yaml_processor/feature_flags.rb @@ -27,8 +27,8 @@ def with_actor(actor) end # Use this to check if a feature flag is enabled - def enabled?(feature_flag) - ::Feature.enabled?(feature_flag, current_actor) + def enabled?(feature_flag, type: :development) + ::Feature.enabled?(feature_flag, current_actor, type: type) end def ensure_correct_usage diff --git a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/identity_provider.yml b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/identity_provider.yml new file mode 100644 index 00000000000000..76836cc1cea8a4 --- /dev/null +++ b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/identity_provider.yml @@ -0,0 +1,5 @@ +id_token_with_missing_identity_provider_value: + identity_provider: "" + +id_token_with_unknown_identity_provider_value: + identity_provider: unknown diff --git a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/identity_provider.yml b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/identity_provider.yml new file mode 100644 index 00000000000000..6a54422bc31480 --- /dev/null +++ b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/identity_provider.yml @@ -0,0 +1,2 @@ +valid_identity_provider: + identity_provider: google_cloud diff --git a/spec/lib/gitlab/ci/yaml_processor/feature_flags_spec.rb b/spec/lib/gitlab/ci/yaml_processor/feature_flags_spec.rb index 77346f328ca852..72b87ec07e2ac8 100644 --- a/spec/lib/gitlab/ci/yaml_processor/feature_flags_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor/feature_flags_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::YamlProcessor::FeatureFlags do +RSpec.describe Gitlab::Ci::YamlProcessor::FeatureFlags, feature_category: :pipeline_composition do let(:feature_flag) { :my_feature_flag } context 'when the actor is set' do @@ -11,12 +11,20 @@ it 'checks the feature flag using the given actor' do described_class.with_actor(actor) do - expect(Feature).to receive(:enabled?).with(feature_flag, actor) + expect(Feature).to receive(:enabled?).with(feature_flag, actor, type: :development) described_class.enabled?(feature_flag) end end + it 'checks the feature flag using the given type' do + described_class.with_actor(actor) do + expect(Feature).to receive(:enabled?).with(feature_flag, actor, type: :beta) + + described_class.enabled?(feature_flag, type: :beta) + end + end + it 'returns the value of the block' do result = described_class.with_actor(actor) do :test @@ -28,12 +36,12 @@ it 'restores the existing actor if any' do described_class.with_actor(actor) do described_class.with_actor(another_actor) do - expect(Feature).to receive(:enabled?).with(feature_flag, another_actor) + expect(Feature).to receive(:enabled?).with(feature_flag, another_actor, type: anything) described_class.enabled?(feature_flag) end - expect(Feature).to receive(:enabled?).with(feature_flag, actor) + expect(Feature).to receive(:enabled?).with(feature_flag, actor, type: anything) described_class.enabled?(feature_flag) end end @@ -59,7 +67,7 @@ end it 'checks the feature flag without actor' do - expect(Feature).to receive(:enabled?).with(feature_flag, nil) + expect(Feature).to receive(:enabled?).with(feature_flag, nil, type: anything) expect(Gitlab::ErrorTracking) .to receive(:track_and_raise_for_dev_exception) .and_call_original @@ -71,7 +79,7 @@ context 'when yaml_processor_feature_flag_corectness is not used' do it 'checks the feature flag without actor' do - expect(Feature).to receive(:enabled?).with(feature_flag, nil) + expect(Feature).to receive(:enabled?).with(feature_flag, nil, type: anything) expect(Gitlab::ErrorTracking) .to receive(:track_exception) @@ -83,7 +91,7 @@ context 'when actor is explicitly nil' do it 'checks the feature flag without actor' do described_class.with_actor(nil) do - expect(Feature).to receive(:enabled?).with(feature_flag, nil) + expect(Feature).to receive(:enabled?).with(feature_flag, nil, type: anything) described_class.enabled?(feature_flag) end -- GitLab From 03d99968ed62d08d9abe947c157c0ef1a714f165 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 31 Jan 2024 13:21:38 +0100 Subject: [PATCH 2/3] Address MR review comments --- app/assets/javascripts/editor/schema/ci.json | 3 +-- ee/app/models/ee/ci/build.rb | 10 +++++++--- ee/lib/ee/gitlab/ci/config/entry/job.rb | 4 ++-- ee/lib/gitlab/ci/config/entry/identity_provider.rb | 2 +- .../generate_build_environment_variables_service.rb | 6 +++--- ...enerate_build_environment_variables_service_spec.rb | 4 ++-- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json index 02cd2f99f3bbde..a2a02d2fc90ca1 100644 --- a/app/assets/javascripts/editor/schema/ci.json +++ b/app/assets/javascripts/editor/schema/ci.json @@ -711,8 +711,7 @@ "markdownDescription": "Sets an identity provider (experimental), allowing automatic authentication with the external provider. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#identity_provider).", "enum": [ "google_cloud" - ], - "additionalProperties": false + ] }, "secrets": { "type": "object", diff --git a/ee/app/models/ee/ci/build.rb b/ee/app/models/ee/ci/build.rb index 62b1003108804a..efca5da43de182 100644 --- a/ee/app/models/ee/ci/build.rb +++ b/ee/app/models/ee/ci/build.rb @@ -320,10 +320,14 @@ def hashicorp_vault_provider? end def identity_provider_variables - # NOTE: For now, only google_cloud identity provider is supported - return [] unless options[:identity_provider] == 'google_cloud' + return [] if options[:identity_provider].blank? - ::Gitlab::Ci::GoogleCloud::GenerateBuildEnvironmentVariablesService.new(self).execute + case options[:identity_provider] + when 'google_cloud' + ::Gitlab::Ci::GoogleCloud::GenerateBuildEnvironmentVariablesService.new(self).execute + else + raise ArgumentError, "Unknown identity_provider value: #{options[:identity_provider]}" + end end end end diff --git a/ee/lib/ee/gitlab/ci/config/entry/job.rb b/ee/lib/ee/gitlab/ci/config/entry/job.rb index b090c8d4210f84..b7bc743ecab20a 100644 --- a/ee/lib/ee/gitlab/ci/config/entry/job.rb +++ b/ee/lib/ee/gitlab/ci/config/entry/job.rb @@ -12,7 +12,7 @@ module Job EE_ALLOWED_KEYS = %i[dast_configuration identity_provider secrets].freeze prepended do - attributes :dast_configuration, :identity_provider, :secrets + attributes :dast_configuration, :secrets entry :dast_configuration, ::Gitlab::Ci::Config::Entry::DastConfiguration, description: 'DAST configuration for this job', @@ -41,7 +41,7 @@ def allowed_keys def value super.merge({ dast_configuration: dast_configuration_value, - identity_provider: identity_provider_available? ? identity_provider : nil, + identity_provider: identity_provider_available? ? identity_provider_value : nil, secrets: secrets_value }.compact) end diff --git a/ee/lib/gitlab/ci/config/entry/identity_provider.rb b/ee/lib/gitlab/ci/config/entry/identity_provider.rb index c1211dba7d3c9f..a6eb4bfc57a2a2 100644 --- a/ee/lib/gitlab/ci/config/entry/identity_provider.rb +++ b/ee/lib/gitlab/ci/config/entry/identity_provider.rb @@ -13,7 +13,7 @@ class IdentityProvider < ::Gitlab::Config::Entry::Node ALLOWED_IDENTITY_PROVIDERS = %w[google_cloud].freeze validations do - validates :config, presence: false, type: String, inclusion: { + validates :config, type: String, inclusion: { in: ALLOWED_IDENTITY_PROVIDERS, message: "should be one of: #{ALLOWED_IDENTITY_PROVIDERS.join(', ')}" } diff --git a/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb b/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb index a5e14ed92cf807..67ed067a84e39c 100644 --- a/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb +++ b/ee/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service.rb @@ -7,19 +7,19 @@ class GenerateBuildEnvironmentVariablesService def initialize(build) @build = build - # TODO: This will be refactored to use a new IAM integration + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/439200 @integration = build.project.google_cloud_platform_artifact_registry_integration end def execute return [] unless @integration&.active - json = ::GoogleCloudPlatform::BaseClient.credentials( + config_json = ::GoogleCloudPlatform::BaseClient.credentials( audience: @integration.wlif, encoded_jwt: encoded_jwt ).to_json - var_attributes = { value: json, public: false, masked: true, file: true } + var_attributes = { value: config_json, public: false, masked: true, file: true } [ { key: 'CLOUDSDK_AUTH_CREDENTIAL_FILE_OVERRIDE', **var_attributes }, diff --git a/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb b/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb index 86965ed6028116..b620ceddaf8b2c 100644 --- a/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb +++ b/ee/spec/lib/gitlab/ci/google_cloud/generate_build_environment_variables_service_spec.rb @@ -57,7 +57,7 @@ integration.destroy! end - it { is_expected.to be_empty } + it { is_expected.to eq([]) } end context 'when integration is inactive' do @@ -65,6 +65,6 @@ integration.update_column(:active, false) end - it { is_expected.to be_empty } + it { is_expected.to eq([]) } end end -- GitLab From e810dd196786105be6c8cd79a39049b7407da85f Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Wed, 31 Jan 2024 16:17:06 +0100 Subject: [PATCH 3/3] Add test coverage --- ee/spec/models/ci/build_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ee/spec/models/ci/build_spec.rb b/ee/spec/models/ci/build_spec.rb index 784fbcb80be102..1a3be73c3a7f72 100644 --- a/ee/spec/models/ci/build_spec.rb +++ b/ee/spec/models/ci/build_spec.rb @@ -1008,8 +1008,9 @@ describe 'build identity_provider' do let_it_be(:user) { create(:user) } + let(:identity_provider) { 'google_cloud' } let(:build) do - create(:ci_build, pipeline: pipeline, user: user, options: { identity_provider: 'google_cloud' }) + create(:ci_build, pipeline: pipeline, user: user, options: { identity_provider: identity_provider }) end subject(:variables) { build.variables } @@ -1054,6 +1055,14 @@ 'format' => { 'type' => 'json', 'subject_token_field_name' => 'token' } }) end + + context 'when identity_provider is unknown' do + let(:identity_provider) { 'unknown' } + + it 'raises an error' do + expect { subject }.to raise_error ArgumentError, "Unknown identity_provider value: #{identity_provider}" + end + end end end -- GitLab