From 0275bc79ca332c0952da8bbba8e59ab181fc25a5 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Tue, 8 Nov 2022 17:15:13 +0100 Subject: [PATCH] Add hooks:pre_get_sources_script syntax for CI config We have `pre_clone_script` in the Runner config. Here, we are providing the same config in the CI YAML. Implementation steps: 1. Add the basic job syntax. <-- This commit 2. Add default:hooks. 3. Add to JobRequest::Response for Runner. The changes are behind a feature flag ci_hooks_pre_get_sources_script. --- .rubocop_todo/rspec/file_path.yml | 1 + .../ci_hooks_pre_get_sources_script.yml | 8 +++ lib/gitlab/ci/config/entry/hooks.rb | 23 +++++++ lib/gitlab/ci/config/entry/job.rb | 11 +++- lib/gitlab/ci/yaml_processor/result.rb | 1 + spec/lib/gitlab/ci/config/entry/hooks_spec.rb | 37 +++++++++++ spec/lib/gitlab/ci/config/entry/job_spec.rb | 27 +++++++- spec/lib/gitlab/ci/yaml_processor_spec.rb | 32 ++++++++++ .../create_pipeline_service/scripts_spec.rb | 62 +++++++++++++++++++ 9 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 config/feature_flags/development/ci_hooks_pre_get_sources_script.yml create mode 100644 lib/gitlab/ci/config/entry/hooks.rb create mode 100644 spec/lib/gitlab/ci/config/entry/hooks_spec.rb create mode 100644 spec/services/ci/create_pipeline_service/scripts_spec.rb diff --git a/.rubocop_todo/rspec/file_path.yml b/.rubocop_todo/rspec/file_path.yml index 8930b709bfdb68..6811d228c95be8 100644 --- a/.rubocop_todo/rspec/file_path.yml +++ b/.rubocop_todo/rspec/file_path.yml @@ -65,3 +65,4 @@ RSpec/FilePath: - 'spec/services/ci/create_pipeline_service/rules_spec.rb' - 'spec/services/ci/create_pipeline_service/tags_spec.rb' - 'spec/services/ci/create_pipeline_service/variables_spec.rb' + - 'spec/services/ci/create_pipeline_service/scripts_spec.rb' diff --git a/config/feature_flags/development/ci_hooks_pre_get_sources_script.yml b/config/feature_flags/development/ci_hooks_pre_get_sources_script.yml new file mode 100644 index 00000000000000..42afd4235ccee8 --- /dev/null +++ b/config/feature_flags/development/ci_hooks_pre_get_sources_script.yml @@ -0,0 +1,8 @@ +--- +name: ci_hooks_pre_get_sources_script +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/102332 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/381840 +milestone: '15.6' +type: development +group: group::pipeline authoring +default_enabled: false diff --git a/lib/gitlab/ci/config/entry/hooks.rb b/lib/gitlab/ci/config/entry/hooks.rb new file mode 100644 index 00000000000000..d979dd497b262a --- /dev/null +++ b/lib/gitlab/ci/config/entry/hooks.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + class Config + module Entry + class Hooks < ::Gitlab::Config::Entry::Node + # `Configurable` alreadys adds `Validatable` + include ::Gitlab::Config::Entry::Configurable + + ALLOWED_HOOKS = %i[pre_get_sources_script].freeze + + validations do + validates :config, type: Hash, allowed_keys: ALLOWED_HOOKS + end + + entry :pre_get_sources_script, Entry::Commands, + description: 'Commands that will be executed on Runner before cloning/fetching the Git repository.' + end + end + end + end +end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index ab17e1e3870b3f..29335c4679c30b 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -12,7 +12,7 @@ class Job < ::Gitlab::Config::Entry::Node ALLOWED_WHEN = %w[on_success on_failure always manual delayed].freeze ALLOWED_KEYS = %i[tags script image services start_in artifacts - cache dependencies before_script after_script + cache dependencies before_script after_script hooks environment coverage retry parallel interruptible timeout release id_tokens].freeze @@ -59,6 +59,10 @@ class Job < ::Gitlab::Config::Entry::Node description: 'Commands that will be executed when finishing job.', inherit: true + entry :hooks, Entry::Hooks, + description: 'Commands that will be executed on Runner before/after some events; clone, build-script.', + inherit: false # This will be true in next iterations + entry :cache, Entry::Caches, description: 'Cache definition for this job.', inherit: true @@ -160,6 +164,7 @@ def value artifacts: artifacts_value, release: release_value, after_script: after_script_value, + hooks: hooks_pre_get_sources_script_enabled? ? hooks_value : nil, ignore: ignored?, allow_failure_criteria: allow_failure_criteria, needs: needs_defined? ? needs_value : nil, @@ -189,6 +194,10 @@ def static_allow_failure allow_failure_value end + + def hooks_pre_get_sources_script_enabled? + YamlProcessor::FeatureFlags.enabled?(:ci_hooks_pre_get_sources_script) + end end end end diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb index 31a4e10a53511d..f2c1ad0575dd44 100644 --- a/lib/gitlab/ci/yaml_processor/result.rb +++ b/lib/gitlab/ci/yaml_processor/result.rb @@ -119,6 +119,7 @@ def build_attributes(name) before_script: job[:before_script], script: job[:script], after_script: job[:after_script], + hooks: job[:hooks], environment: job[:environment], resource_group_key: job[:resource_group], retry: job[:retry], diff --git a/spec/lib/gitlab/ci/config/entry/hooks_spec.rb b/spec/lib/gitlab/ci/config/entry/hooks_spec.rb new file mode 100644 index 00000000000000..7a5ff244e189a7 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/hooks_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +RSpec.describe Gitlab::Ci::Config::Entry::Hooks do + subject(:entry) { described_class.new(config) } + + before do + entry.compose! + end + + describe 'validations' do + context 'when passing a valid hook' do + let(:config) { { pre_get_sources_script: ['ls'] } } + + it { is_expected.to be_valid } + end + + context 'when passing an invalid hook' do + let(:config) { { x_get_something: ['ls'] } } + + it { is_expected.not_to be_valid } + end + + context 'when entry config is not a hash' do + let(:config) { 'ls' } + + it { is_expected.not_to be_valid } + end + end + + describe '#value' do + let(:config) { { pre_get_sources_script: ['ls'] } } + + it 'returns a hash' do + expect(entry.value).to eq(config) + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb index 22cb1f480c56da..becb46ac2e7eaa 100644 --- a/spec/lib/gitlab/ci/config/entry/job_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb @@ -27,7 +27,7 @@ subject { described_class.nodes.keys } let(:result) do - %i[before_script script stage after_script cache + %i[before_script script after_script hooks stage cache image services only except rules needs variables artifacts environment coverage retry interruptible timeout release tags inherit parallel] @@ -717,7 +717,8 @@ { before_script: %w[ls pwd], script: 'rspec', after_script: %w[cleanup], - id_tokens: { TEST_ID_TOKEN: { aud: 'https://gitlab.com' } } } + id_tokens: { TEST_ID_TOKEN: { aud: 'https://gitlab.com' } }, + hooks: { pre_get_sources_script: 'echo hello' } } end it 'returns correct value' do @@ -728,12 +729,34 @@ stage: 'test', ignore: false, after_script: %w[cleanup], + hooks: { pre_get_sources_script: ['echo hello'] }, only: { refs: %w[branches tags] }, job_variables: {}, root_variables_inheritance: true, scheduling_type: :stage, id_tokens: { TEST_ID_TOKEN: { aud: 'https://gitlab.com' } }) end + + context 'when the FF ci_hooks_pre_get_sources_script is disabled' do + before do + stub_feature_flags(ci_hooks_pre_get_sources_script: false) + end + + it 'returns correct value' do + expect(entry.value) + .to eq(name: :rspec, + before_script: %w[ls pwd], + script: %w[rspec], + stage: 'test', + ignore: false, + after_script: %w[cleanup], + only: { refs: %w[branches tags] }, + job_variables: {}, + root_variables_inheritance: true, + scheduling_type: :stage, + id_tokens: { TEST_ID_TOKEN: { aud: 'https://gitlab.com' } }) + end + end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 5de813f7739732..fee3731c662f8b 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -870,6 +870,38 @@ module Ci end end end + + describe "hooks" do + context 'when it is a simple script' do + let(:config) do + { + test: { script: ["script"], + hooks: { pre_get_sources_script: ["echo 1", "echo 2", "pwd"] } } + } + end + + it "returns hooks in options" do + expect(subject[:options][:hooks]).to eq( + { pre_get_sources_script: ["echo 1", "echo 2", "pwd"] } + ) + end + end + + context 'when it is nested arrays of strings' do + let(:config) do + { + test: { script: ["script"], + hooks: { pre_get_sources_script: [[["global script"], "echo 1"], "echo 2", ["ls"], "pwd"] } } + } + end + + it "returns hooks in options" do + expect(subject[:options][:hooks]).to eq( + { pre_get_sources_script: ["global script", "echo 1", "echo 2", "ls", "pwd"] } + ) + end + end + end end describe "Image and service handling" do diff --git a/spec/services/ci/create_pipeline_service/scripts_spec.rb b/spec/services/ci/create_pipeline_service/scripts_spec.rb new file mode 100644 index 00000000000000..493e341395b28a --- /dev/null +++ b/spec/services/ci/create_pipeline_service/scripts_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::CreatePipelineService, :yaml_processor_feature_flag_corectness do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { project.first_owner } + + let(:service) { described_class.new(project, user, { ref: 'master' }) } + let(:pipeline) { service.execute(:push).payload } + + before do + stub_ci_pipeline_yaml_file(config) + end + + context 'when job has script and nested before_script and after_script' do + let(:config) do + <<-CI_CONFIG + default: + before_script: echo 'hello default before_script' + after_script: echo 'hello default after_script' + + job: + before_script: echo 'hello job before_script' + after_script: echo 'hello job after_script' + script: echo 'hello job script' + CI_CONFIG + end + + it 'creates a job with script data' do + expect(pipeline).to be_created_successfully + expect(pipeline.builds.first).to have_attributes( + name: 'job', + stage: 'test', + options: { script: ["echo 'hello job script'"], + before_script: ["echo 'hello job before_script'"], + after_script: ["echo 'hello job after_script'"] } + ) + end + end + + context 'when job has hooks' do + let(:config) do + <<-CI_CONFIG + job: + hooks: + pre_get_sources_script: echo 'hello job pre_get_sources_script' + script: echo 'hello job script' + CI_CONFIG + end + + it 'creates a job with script data' do + expect(pipeline).to be_created_successfully + expect(pipeline.builds.first).to have_attributes( + name: 'job', + stage: 'test', + options: { script: ["echo 'hello job script'"], + hooks: { pre_get_sources_script: ["echo 'hello job pre_get_sources_script'"] } } + ) + end + end +end -- GitLab