diff --git a/lib/gitlab/ci/config/entry/default.rb b/lib/gitlab/ci/config/entry/default.rb index 12d68b755b3fdf62fd6f707e751dac66c54faba2..e996b6b1312d73995a6d89334c614031b9714e11 100644 --- a/lib/gitlab/ci/config/entry/default.rb +++ b/lib/gitlab/ci/config/entry/default.rb @@ -13,9 +13,8 @@ class Default < ::Gitlab::Config::Entry::Node include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Inheritable - ALLOWED_KEYS = %i[before_script image services - after_script cache interruptible - timeout retry tags artifacts].freeze + ALLOWED_KEYS = %i[before_script after_script hooks cache image services + interruptible timeout retry tags artifacts].freeze validations do validates :config, allowed_keys: ALLOWED_KEYS @@ -25,22 +24,27 @@ class Default < ::Gitlab::Config::Entry::Node description: 'Script that will be executed before each job.', inherit: true - entry :image, Entry::Image, - description: 'Docker image that will be used to execute jobs.', - inherit: true - - entry :services, Entry::Services, - description: 'Docker images that will be linked to the container.', - inherit: true - entry :after_script, Entry::Commands, description: 'Script that will be executed after each job.', inherit: true + entry :hooks, Entry::Hooks, + description: 'Commands that will be executed on Runner before/after some events ' \ + 'such as `clone` and `build-script`.', + inherit: false + entry :cache, Entry::Caches, description: 'Configure caching between build jobs.', inherit: true + entry :image, Entry::Image, + description: 'Docker image that will be used to execute jobs.', + inherit: true + + entry :services, Entry::Services, + description: 'Docker images that will be linked to the container.', + inherit: true + entry :interruptible, ::Gitlab::Config::Entry::Boolean, description: 'Set jobs interruptible default value.', inherit: false diff --git a/lib/gitlab/ci/config/entry/hooks.rb b/lib/gitlab/ci/config/entry/hooks.rb index d979dd497b262a2387464203033d890ce713cfac..28bc2e4e7ce8a4705f67a8eb4d245fc0fc3c8ecc 100644 --- a/lib/gitlab/ci/config/entry/hooks.rb +++ b/lib/gitlab/ci/config/entry/hooks.rb @@ -8,6 +8,8 @@ class Hooks < ::Gitlab::Config::Entry::Node # `Configurable` alreadys adds `Validatable` include ::Gitlab::Config::Entry::Configurable + # NOTE: If a new hook is added, inheriting should be changed because a `job:hooks` overrides all + # `default:hooks` now. We should implement merging; each hook must be overridden individually. ALLOWED_HOOKS = %i[pre_get_sources_script].freeze validations do diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 29335c4679c30b84e75a20c4c4c398a92b8c992c..7c49b59a7f07ec5974ce687e323164174660caa2 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -61,7 +61,7 @@ class Job < ::Gitlab::Config::Entry::Node 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 + inherit: true entry :cache, Entry::Caches, description: 'Cache definition for this job.', diff --git a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb index 8da46561b738d9c560343b8df84fac888a260768..736c184a289ebdbaa0b4d0fe11e2552cdaaa4d88 100644 --- a/spec/lib/gitlab/ci/config/entry/bridge_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/bridge_spec.rb @@ -13,7 +13,7 @@ # that we know that we don't want to inherit # as they do not have sense in context of Bridge let(:ignored_inheritable_columns) do - %i[before_script after_script image services cache interruptible timeout + %i[before_script after_script hooks image services cache interruptible timeout retry tags artifacts] end end diff --git a/spec/lib/gitlab/ci/config/entry/default_spec.rb b/spec/lib/gitlab/ci/config/entry/default_spec.rb index 5613b0f09d118939e5ced39158014376defaf72f..46e96843ee38e7b2515907851fb267de3eb8cb52 100644 --- a/spec/lib/gitlab/ci/config/entry/default_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/default_spec.rb @@ -26,9 +26,8 @@ context 'when filtering all the entry/node names' do it 'contains the expected node names' do expect(described_class.nodes.keys) - .to match_array(%i[before_script image services - after_script cache interruptible - timeout retry tags artifacts]) + .to match_array(%i[before_script after_script hooks cache image services + interruptible timeout retry tags artifacts]) end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index fee3731c662f8bd10dacd936b5d416ee759e4c7f..0d1deb863b1e2e268176fd7997bc988e945a954d 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -901,6 +901,37 @@ module Ci ) end end + + context 'when receiving from the default' do + let(:config) do + { + default: { hooks: { pre_get_sources_script: ["echo 1", "echo 2", "pwd"] } }, + test: { script: ["script"] } + } + end + + it "inherits hooks" do + expect(subject[:options][:hooks]).to eq( + { pre_get_sources_script: ["echo 1", "echo 2", "pwd"] } + ) + end + end + + context 'when overriding the default' do + let(:config) do + { + default: { hooks: { pre_get_sources_script: ["echo 1", "echo 2", "pwd"] } }, + test: { script: ["script"], + hooks: { pre_get_sources_script: ["echo 3", "echo 4", "pwd"] } } + } + end + + it "overrides hooks" do + expect(subject[:options][:hooks]).to eq( + { pre_get_sources_script: ["echo 3", "echo 4", "pwd"] } + ) + end + end end end diff --git a/spec/services/ci/create_pipeline_service/scripts_spec.rb b/spec/services/ci/create_pipeline_service/scripts_spec.rb index 493e341395b28afc6cb6deacae92bb0b18fc589b..50b558e505a5638b99e9a9dd08afe8e1cf539cf1 100644 --- a/spec/services/ci/create_pipeline_service/scripts_spec.rb +++ b/spec/services/ci/create_pipeline_service/scripts_spec.rb @@ -39,24 +39,74 @@ end end - context 'when job has hooks' do + context 'when job has hooks and default hooks' do let(:config) do <<-CI_CONFIG - job: + default: hooks: - pre_get_sources_script: echo 'hello job pre_get_sources_script' - script: echo 'hello job script' + pre_get_sources_script: + - echo 'hello default pre_get_sources_script' + + job1: + hooks: + pre_get_sources_script: + - echo 'hello job1 pre_get_sources_script' + script: echo 'hello job1 script' + + job2: + script: echo 'hello job2 script' + + job3: + inherit: + default: false + script: echo 'hello job3 script' CI_CONFIG end - it 'creates a job with script data' do + it 'creates jobs with hook data' do expect(pipeline).to be_created_successfully - expect(pipeline.builds.first).to have_attributes( - name: 'job', + expect(pipeline.builds.find_by(name: 'job1')).to have_attributes( + name: 'job1', stage: 'test', - options: { script: ["echo 'hello job script'"], - hooks: { pre_get_sources_script: ["echo 'hello job pre_get_sources_script'"] } } + options: { script: ["echo 'hello job1 script'"], + hooks: { pre_get_sources_script: ["echo 'hello job1 pre_get_sources_script'"] } } ) + expect(pipeline.builds.find_by(name: 'job2')).to have_attributes( + name: 'job2', + stage: 'test', + options: { script: ["echo 'hello job2 script'"], + hooks: { pre_get_sources_script: ["echo 'hello default pre_get_sources_script'"] } } + ) + expect(pipeline.builds.find_by(name: 'job3')).to have_attributes( + name: 'job3', + stage: 'test', + options: { script: ["echo 'hello job3 script'"] } + ) + 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 'creates jobs without hook data' do + expect(pipeline).to be_created_successfully + expect(pipeline.builds.find_by(name: 'job1')).to have_attributes( + name: 'job1', + stage: 'test', + options: { script: ["echo 'hello job1 script'"] } + ) + expect(pipeline.builds.find_by(name: 'job2')).to have_attributes( + name: 'job2', + stage: 'test', + options: { script: ["echo 'hello job2 script'"] } + ) + expect(pipeline.builds.find_by(name: 'job3')).to have_attributes( + name: 'job3', + stage: 'test', + options: { script: ["echo 'hello job3 script'"] } + ) + end end end end