From 3708a233e8dc06df08fcc2658b0acccca37f2d0b Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Mon, 28 Nov 2022 17:22:34 +0100 Subject: [PATCH 1/2] Add hooks:pre_get_sources_script to jobs/request Implementation steps: 1. Add the basic job syntax. 2. Add default:hooks. 3. Add to JobRequest::Response for Runner. <-- This commit The changes are behind a feature flag ci_hooks_pre_get_sources_script. --- app/models/ci/build.rb | 4 ++++ lib/api/entities/ci/job_request/hook.rb | 13 ++++++++++ lib/api/entities/ci/job_request/response.rb | 2 ++ lib/gitlab/ci/build/hook.rb | 24 +++++++++++++++++++ spec/factories/ci/builds.rb | 5 +++- spec/lib/gitlab/ci/build/hook_spec.rb | 20 ++++++++++++++++ spec/models/ci/build_spec.rb | 15 ++++++++++++ .../api/ci/runner/jobs_request_post_spec.rb | 18 ++++++++++++++ 8 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 lib/api/entities/ci/job_request/hook.rb create mode 100644 lib/gitlab/ci/build/hook.rb create mode 100644 spec/lib/gitlab/ci/build/hook_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 12c9f45e4c462b..1e358fa2b5cb92 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -865,6 +865,10 @@ def steps Gitlab::Ci::Build::Step.from_after_script(self)].compact end + def hooks + Gitlab::Ci::Build::Hook.from_hooks(self) + end + def image Gitlab::Ci::Build::Image.from_image(self) end diff --git a/lib/api/entities/ci/job_request/hook.rb b/lib/api/entities/ci/job_request/hook.rb new file mode 100644 index 00000000000000..2d155bb1c4585b --- /dev/null +++ b/lib/api/entities/ci/job_request/hook.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module API + module Entities + module Ci + module JobRequest + class Hook < Grape::Entity + expose :name, :script + end + end + end + end +end diff --git a/lib/api/entities/ci/job_request/response.rb b/lib/api/entities/ci/job_request/response.rb index 9de415ebacbb6f..cd6a6d3899ade1 100644 --- a/lib/api/entities/ci/job_request/response.rb +++ b/lib/api/entities/ci/job_request/response.rb @@ -23,6 +23,8 @@ class Response < Grape::Entity expose :runner_variables, as: :variables expose :steps, using: Entities::Ci::JobRequest::Step + expose :hooks, using: Entities::Ci::JobRequest::Hook, + if: ->(job) { ::Feature.enabled?(:ci_hooks_pre_get_sources_script, job.project) } expose :image, using: Entities::Ci::JobRequest::Image expose :services, using: Entities::Ci::JobRequest::Service expose :artifacts, using: Entities::Ci::JobRequest::Artifacts diff --git a/lib/gitlab/ci/build/hook.rb b/lib/gitlab/ci/build/hook.rb new file mode 100644 index 00000000000000..b731228678c569 --- /dev/null +++ b/lib/gitlab/ci/build/hook.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Build + class Hook + attr_reader :name, :script + + class << self + def from_hooks(job) + job.options[:hooks].to_a.map do |name, script| + new(name.to_s, script) + end + end + end + + def initialize(name, script) + @name = name + @script = script + end + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index b88d6b5fda46ef..eccaa76a52a6dc 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -545,9 +545,12 @@ options do { image: { name: 'image:1.0', entrypoint: '/bin/sh' }, - services: ['postgres', { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }, { name: 'mysql:latest', variables: { MYSQL_ROOT_PASSWORD: 'root123.' } }], + services: ['postgres', + { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }, + { name: 'mysql:latest', variables: { MYSQL_ROOT_PASSWORD: 'root123.' } }], script: %w(echo), after_script: %w(ls date), + hooks: { pre_get_sources_script: ["echo 'hello pre_get_sources_script'"] }, artifacts: { name: 'artifacts_file', untracked: false, diff --git a/spec/lib/gitlab/ci/build/hook_spec.rb b/spec/lib/gitlab/ci/build/hook_spec.rb new file mode 100644 index 00000000000000..6ed40a44c97d22 --- /dev/null +++ b/spec/lib/gitlab/ci/build/hook_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Build::Hook, feature_category: :pipeline_authoring do + let_it_be(:build1) do + FactoryBot.build(:ci_build, + options: { hooks: { pre_get_sources_script: ["echo 'hello pre_get_sources_script'"] } }) + end + + describe '.from_hooks' do + subject(:from_hooks) { described_class.from_hooks(build1) } + + it 'initializes and returns hooks' do + expect(from_hooks.size).to eq(1) + expect(from_hooks[0].name).to eq('pre_get_sources_script') + expect(from_hooks[0].script).to eq(["echo 'hello pre_get_sources_script'"]) + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 22fa020ad02553..396f85aacce618 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5616,4 +5616,19 @@ def run_job_without_exception end end end + + describe '#hooks' do + let(:build1) do + FactoryBot.build(:ci_build, + options: { hooks: { pre_get_sources_script: ["echo 'hello pre_get_sources_script'"] } }) + end + + subject(:hooks) { build1.hooks } + + it 'returns an array of hook objects' do + expect(hooks.size).to eq(1) + expect(hooks[0].name).to eq('pre_get_sources_script') + expect(hooks[0].script).to eq(["echo 'hello pre_get_sources_script'"]) + end + end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 1cb4cc93ea569f..232c339de57b58 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -175,6 +175,10 @@ 'allow_failure' => true }] end + let(:expected_hooks) do + [{ 'name' => 'pre_get_sources_script', 'script' => ["echo 'hello pre_get_sources_script'"] }] + end + let(:expected_variables) do [{ 'key' => 'CI_JOB_NAME', 'value' => 'spinach', 'public' => true, 'masked' => false }, { 'key' => 'CI_JOB_STAGE', 'value' => 'test', 'public' => true, 'masked' => false }, @@ -230,6 +234,7 @@ 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }], 'pull_policy' => nil } ]) expect(json_response['steps']).to eq(expected_steps) + expect(json_response['hooks']).to eq(expected_hooks) expect(json_response['artifacts']).to eq(expected_artifacts) expect(json_response['cache']).to match(expected_cache) expect(json_response['variables']).to include(*expected_variables) @@ -769,6 +774,19 @@ end end 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 'does not return the pre_get_sources_script' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).not_to have_key('hooks') + end + end end describe 'port support' do -- GitLab From d85aeed960e53b777440356488915edd62b3a0b4 Mon Sep 17 00:00:00 2001 From: Furkan Ayhan Date: Tue, 29 Nov 2022 10:27:44 +0100 Subject: [PATCH 2/2] Change Build#hooks to runtime_hooks --- app/models/ci/build.rb | 2 +- lib/api/entities/ci/job_request/response.rb | 5 +++-- spec/models/ci/build_spec.rb | 10 +++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1e358fa2b5cb92..a60813a36a7e8d 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -865,7 +865,7 @@ def steps Gitlab::Ci::Build::Step.from_after_script(self)].compact end - def hooks + def runtime_hooks Gitlab::Ci::Build::Hook.from_hooks(self) end diff --git a/lib/api/entities/ci/job_request/response.rb b/lib/api/entities/ci/job_request/response.rb index cd6a6d3899ade1..cfdbeed79b6daa 100644 --- a/lib/api/entities/ci/job_request/response.rb +++ b/lib/api/entities/ci/job_request/response.rb @@ -23,8 +23,9 @@ class Response < Grape::Entity expose :runner_variables, as: :variables expose :steps, using: Entities::Ci::JobRequest::Step - expose :hooks, using: Entities::Ci::JobRequest::Hook, - if: ->(job) { ::Feature.enabled?(:ci_hooks_pre_get_sources_script, job.project) } + expose :runtime_hooks, as: :hooks, + using: Entities::Ci::JobRequest::Hook, + if: ->(job) { ::Feature.enabled?(:ci_hooks_pre_get_sources_script, job.project) } expose :image, using: Entities::Ci::JobRequest::Image expose :services, using: Entities::Ci::JobRequest::Service expose :artifacts, using: Entities::Ci::JobRequest::Artifacts diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 396f85aacce618..c03bb61f1f8b29 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -5617,18 +5617,18 @@ def run_job_without_exception end end - describe '#hooks' do + describe '#runtime_hooks' do let(:build1) do FactoryBot.build(:ci_build, options: { hooks: { pre_get_sources_script: ["echo 'hello pre_get_sources_script'"] } }) end - subject(:hooks) { build1.hooks } + subject(:runtime_hooks) { build1.runtime_hooks } it 'returns an array of hook objects' do - expect(hooks.size).to eq(1) - expect(hooks[0].name).to eq('pre_get_sources_script') - expect(hooks[0].script).to eq(["echo 'hello pre_get_sources_script'"]) + expect(runtime_hooks.size).to eq(1) + expect(runtime_hooks[0].name).to eq('pre_get_sources_script') + expect(runtime_hooks[0].script).to eq(["echo 'hello pre_get_sources_script'"]) end end end -- GitLab