Commit c7bd7b6d authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'make-before-after-overridable' into 'master'

Make before_script and after_script overridable

This is makes it possible to overwrite the before_script and after_script at job level.

This is continuation of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3771


See merge request !3772
parents 479f4ccc 3f66f447
...@@ -52,6 +52,7 @@ v 8.7.0 (unreleased) ...@@ -52,6 +52,7 @@ v 8.7.0 (unreleased)
- Use rugged to change HEAD in Project#change_head (P.S.V.R) - Use rugged to change HEAD in Project#change_head (P.S.V.R)
- API: Ability to filter milestones by state `active` and `closed` (Robert Schilling) - API: Ability to filter milestones by state `active` and `closed` (Robert Schilling)
- API: Fix milestone filtering by `iid` (Robert Schilling) - API: Fix milestone filtering by `iid` (Robert Schilling)
- Make before_script and after_script overridable on per-job (Kamil Trzciński)
- API: Delete notes of issues, snippets, and merge requests (Robert Schilling) - API: Delete notes of issues, snippets, and merge requests (Robert Schilling)
- Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.) - Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
- Better errors handling when creating milestones inside groups - Better errors handling when creating milestones inside groups
......
...@@ -31,6 +31,7 @@ If you want a quick introduction to GitLab CI, follow our ...@@ -31,6 +31,7 @@ If you want a quick introduction to GitLab CI, follow our
- [artifacts](#artifacts) - [artifacts](#artifacts)
- [artifacts:name](#artifacts-name) - [artifacts:name](#artifacts-name)
- [dependencies](#dependencies) - [dependencies](#dependencies)
- [before_script and after_script](#before_script-and-after_script)
- [Hidden jobs](#hidden-jobs) - [Hidden jobs](#hidden-jobs)
- [Special YAML features](#special-yaml-features) - [Special YAML features](#special-yaml-features)
- [Anchors](#anchors) - [Anchors](#anchors)
...@@ -349,6 +350,8 @@ job_name: ...@@ -349,6 +350,8 @@ job_name:
| dependencies | no | Define other builds that a build depends on so that you can pass artifacts between them| | dependencies | no | Define other builds that a build depends on so that you can pass artifacts between them|
| artifacts | no | Define list build artifacts | | artifacts | no | Define list build artifacts |
| cache | no | Define list of files that should be cached between subsequent runs | | cache | no | Define list of files that should be cached between subsequent runs |
| before_script | no | Override a set of commands that are executed before build |
| after_script | no | Override a set of commands that are executed after build |
### script ### script
...@@ -705,6 +708,23 @@ deploy: ...@@ -705,6 +708,23 @@ deploy:
script: make deploy script: make deploy
``` ```
### before_script and after_script
It's possible to overwrite globally defined `before_script` and `after_script`:
```yaml
before_script
- global before script
job:
before_script:
- execute this instead of global before script
script:
- my command
after_script:
- execute this after my script
```
## Hidden jobs ## Hidden jobs
>**Note:** >**Note:**
......
...@@ -7,7 +7,7 @@ module Ci ...@@ -7,7 +7,7 @@ module Ci
ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache] ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache]
ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services,
:allow_failure, :type, :stage, :when, :artifacts, :cache, :allow_failure, :type, :stage, :when, :artifacts, :cache,
:dependencies, :variables] :dependencies, :before_script, :after_script, :variables]
attr_reader :before_script, :after_script, :image, :services, :path, :cache attr_reader :before_script, :after_script, :image, :services, :path, :cache
...@@ -84,7 +84,7 @@ module Ci ...@@ -84,7 +84,7 @@ module Ci
{ {
stage_idx: stages.index(job[:stage]), stage_idx: stages.index(job[:stage]),
stage: job[:stage], stage: job[:stage],
commands: "#{@before_script.join("\n")}\n#{normalize_script(job[:script])}", commands: [job[:before_script] || @before_script, job[:script]].flatten.join("\n"),
tag_list: job[:tags] || [], tag_list: job[:tags] || [],
name: name, name: name,
only: job[:only], only: job[:only],
...@@ -97,19 +97,11 @@ module Ci ...@@ -97,19 +97,11 @@ module Ci
artifacts: job[:artifacts], artifacts: job[:artifacts],
cache: job[:cache] || @cache, cache: job[:cache] || @cache,
dependencies: job[:dependencies], dependencies: job[:dependencies],
after_script: @after_script, after_script: job[:after_script] || @after_script,
}.compact }.compact
} }
end end
def normalize_script(script)
if script.is_a? Array
script.join("\n")
else
script
end
end
def validate! def validate!
validate_global! validate_global!
...@@ -168,6 +160,7 @@ module Ci ...@@ -168,6 +160,7 @@ module Ci
validate_job_name!(name) validate_job_name!(name)
validate_job_keys!(name, job) validate_job_keys!(name, job)
validate_job_types!(name, job) validate_job_types!(name, job)
validate_job_script!(name, job)
validate_job_stage!(name, job) if job[:stage] validate_job_stage!(name, job) if job[:stage]
validate_job_variables!(name, job) if job[:variables] validate_job_variables!(name, job) if job[:variables]
...@@ -191,10 +184,6 @@ module Ci ...@@ -191,10 +184,6 @@ module Ci
end end
def validate_job_types!(name, job) def validate_job_types!(name, job)
if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
raise ValidationError, "#{name} job: script should be a string or an array of a strings"
end
if job[:image] && !validate_string(job[:image]) if job[:image] && !validate_string(job[:image])
raise ValidationError, "#{name} job: image should be a string" raise ValidationError, "#{name} job: image should be a string"
end end
...@@ -224,6 +213,20 @@ module Ci ...@@ -224,6 +213,20 @@ module Ci
end end
end end
def validate_job_script!(name, job)
if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
raise ValidationError, "#{name} job: script should be a string or an array of a strings"
end
if job[:before_script] && !validate_array_of_strings(job[:before_script])
raise ValidationError, "#{name} job: before_script should be an array of strings"
end
if job[:after_script] && !validate_array_of_strings(job[:after_script])
raise ValidationError, "#{name} job: after_script should be an array of strings"
end
end
def validate_job_stage!(name, job) def validate_job_stage!(name, job)
unless job[:stage].is_a?(String) && job[:stage].in?(stages) unless job[:stage].is_a?(String) && job[:stage].in?(stages)
raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}" raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
......
...@@ -293,6 +293,46 @@ module Ci ...@@ -293,6 +293,46 @@ module Ci
subject { config_processor.builds_for_stage_and_ref("test", "master").first } subject { config_processor.builds_for_stage_and_ref("test", "master").first }
describe "before_script" do
context "in global context" do
let(:config) do
{
before_script: ["global script"],
test: { script: ["script"] }
}
end
it "return commands with scripts concencaced" do
expect(subject[:commands]).to eq("global script\nscript")
end
end
context "overwritten in local context" do
let(:config) do
{
before_script: ["global script"],
test: { before_script: ["local script"], script: ["script"] }
}
end
it "return commands with scripts concencaced" do
expect(subject[:commands]).to eq("local script\nscript")
end
end
end
describe "script" do
let(:config) do
{
test: { script: ["script"] }
}
end
it "return commands with scripts concencaced" do
expect(subject[:commands]).to eq("script")
end
end
describe "after_script" do describe "after_script" do
context "in global context" do context "in global context" do
let(:config) do let(:config) do
...@@ -306,6 +346,19 @@ module Ci ...@@ -306,6 +346,19 @@ module Ci
expect(subject[:options][:after_script]).to eq(["after_script"]) expect(subject[:options][:after_script]).to eq(["after_script"])
end end
end end
context "overwritten in local context" do
let(:config) do
{
after_script: ["local after_script"],
test: { after_script: ["local after_script"], script: ["script"] }
}
end
it "return after_script in options" do
expect(subject[:options][:after_script]).to eq(["local after_script"])
end
end
end end
end end
...@@ -614,7 +667,7 @@ module Ci ...@@ -614,7 +667,7 @@ module Ci
stage_idx: 1, stage_idx: 1,
name: :normal_job, name: :normal_job,
only: nil, only: nil,
commands: "\ntest", commands: "test",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
...@@ -641,7 +694,7 @@ EOT ...@@ -641,7 +694,7 @@ EOT
stage_idx: 1, stage_idx: 1,
name: :job1, name: :job1,
only: nil, only: nil,
commands: "\nexecute-script-for-job", commands: "execute-script-for-job",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
...@@ -653,7 +706,7 @@ EOT ...@@ -653,7 +706,7 @@ EOT
stage_idx: 1, stage_idx: 1,
name: :job2, name: :job2,
only: nil, only: nil,
commands: "\nexecute-script-for-job", commands: "execute-script-for-job",
tag_list: [], tag_list: [],
options: {}, options: {},
when: "on_success", when: "on_success",
...@@ -685,6 +738,13 @@ EOT ...@@ -685,6 +738,13 @@ EOT
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings") end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings")
end end
it "returns errors if job before_script parameter is not an array of strings" do
config = YAML.dump({ rspec: { script: "test", before_script: [10, "test"] } })
expect do
GitlabCiYamlProcessor.new(config, path)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: before_script should be an array of strings")
end
it "returns errors if after_script parameter is invalid" do it "returns errors if after_script parameter is invalid" do
config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } }) config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
expect do expect do
...@@ -692,6 +752,13 @@ EOT ...@@ -692,6 +752,13 @@ EOT
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings") end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings")
end end
it "returns errors if job after_script parameter is not an array of strings" do
config = YAML.dump({ rspec: { script: "test", after_script: [10, "test"] } })
expect do
GitlabCiYamlProcessor.new(config, path)
end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: after_script should be an array of strings")
end
it "returns errors if image parameter is invalid" do it "returns errors if image parameter is invalid" do
config = YAML.dump({ image: ["test"], rspec: { script: "test" } }) config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
expect do expect do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment