From b340b59743e8cd47fc1f4fa2020b400d82bfd86e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sat, 16 Apr 2016 18:46:18 +0200 Subject: [PATCH 1/6] Implement finally_script which allows to do cleanups as part of the build process --- lib/ci/gitlab_ci_yaml_processor.rb | 10 +++++-- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 29 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index b7209c1414..be2462949f 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -4,12 +4,12 @@ module Ci DEFAULT_STAGES = %w(build test deploy) DEFAULT_STAGE = 'test' - ALLOWED_YAML_KEYS = [:before_script, :image, :services, :types, :stages, :variables, :cache] + ALLOWED_YAML_KEYS = [:before_script, :finally_script, :image, :services, :types, :stages, :variables, :cache] ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage, :when, :artifacts, :cache, :dependencies] - attr_reader :before_script, :image, :services, :variables, :path, :cache + attr_reader :before_script, :finally_script, :image, :services, :variables, :path, :cache def initialize(config, path = nil) @config = YAML.safe_load(config, [Symbol], [], true) @@ -44,6 +44,7 @@ module Ci def initial_parsing @before_script = @config[:before_script] || [] + @finally_script = @config[:finally_script] @image = @config[:image] @services = @config[:services] @stages = @config[:stages] || @config[:types] @@ -85,6 +86,7 @@ module Ci artifacts: job[:artifacts], cache: job[:cache] || @cache, dependencies: job[:dependencies], + finally_script: @finally_script, }.compact } end @@ -102,6 +104,10 @@ module Ci raise ValidationError, "before_script should be an array of strings" end + unless @finally_script.nil? || validate_array_of_strings(@finally_script) + raise ValidationError, "finally_script should be an array of strings" + end + unless @image.nil? || @image.is_a?(String) raise ValidationError, "image should be a string" end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index dcb8a3451b..8e373ae55b 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -286,6 +286,28 @@ module Ci end end + + describe "Scripts handling" do + let(:config_data) { YAML.dump(config) } + let(:config_processor) { GitlabCiYamlProcessor.new(config_data, path) } + + subject { config_processor.builds_for_stage_and_ref("test", "master").first } + + describe "finally_script" do + context "in global context" do + let(:config) { + { + finally_script: ["finally_script"], + test: { script: ["script"] } + } + } + + it "return finally_script in options" do + expect(subject[:options][:finally_script]).to eq(["finally_script"]) + end + end + end + end describe "Image and service handling" do it "returns image and service when defined" do @@ -607,6 +629,13 @@ EOT end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings") end + it "returns errors if finally_script parameter is invalid" do + config = YAML.dump({ finally_script: "bundle update", rspec: { script: "test" } }) + expect do + GitlabCiYamlProcessor.new(config, path) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "finally_script should be an array of strings") + end + it "returns errors if image parameter is invalid" do config = YAML.dump({ image: ["test"], rspec: { script: "test" } }) expect do From cc57d61023c6dd5ef274bac5d4e6cde1cae97d2c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 17 Apr 2016 08:59:57 -0400 Subject: [PATCH 2/6] Rename finally_script to after_script --- lib/ci/gitlab_ci_yaml_processor.rb | 12 ++++++------ spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index be2462949f..a87329c296 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -4,12 +4,12 @@ module Ci DEFAULT_STAGES = %w(build test deploy) DEFAULT_STAGE = 'test' - ALLOWED_YAML_KEYS = [:before_script, :finally_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, :allow_failure, :type, :stage, :when, :artifacts, :cache, :dependencies] - attr_reader :before_script, :finally_script, :image, :services, :variables, :path, :cache + attr_reader :before_script, :after_script, :image, :services, :variables, :path, :cache def initialize(config, path = nil) @config = YAML.safe_load(config, [Symbol], [], true) @@ -44,7 +44,7 @@ module Ci def initial_parsing @before_script = @config[:before_script] || [] - @finally_script = @config[:finally_script] + @after_script = @config[:after_script] @image = @config[:image] @services = @config[:services] @stages = @config[:stages] || @config[:types] @@ -86,7 +86,7 @@ module Ci artifacts: job[:artifacts], cache: job[:cache] || @cache, dependencies: job[:dependencies], - finally_script: @finally_script, + after_script: @after_script, }.compact } end @@ -104,8 +104,8 @@ module Ci raise ValidationError, "before_script should be an array of strings" end - unless @finally_script.nil? || validate_array_of_strings(@finally_script) - raise ValidationError, "finally_script should be an array of strings" + unless @after_script.nil? || validate_array_of_strings(@after_script) + raise ValidationError, "after_script should be an array of strings" end unless @image.nil? || @image.is_a?(String) diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 8e373ae55b..2421d6eee8 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -293,17 +293,17 @@ module Ci subject { config_processor.builds_for_stage_and_ref("test", "master").first } - describe "finally_script" do + describe "after_script" do context "in global context" do let(:config) { { - finally_script: ["finally_script"], + after_script: ["after_script"], test: { script: ["script"] } } } - it "return finally_script in options" do - expect(subject[:options][:finally_script]).to eq(["finally_script"]) + it "return after_script in options" do + expect(subject[:options][:after_script]).to eq(["after_script"]) end end end @@ -629,11 +629,11 @@ EOT end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script should be an array of strings") end - it "returns errors if finally_script parameter is invalid" do - config = YAML.dump({ finally_script: "bundle update", rspec: { script: "test" } }) + it "returns errors if after_script parameter is invalid" do + config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "finally_script should be an array of strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings") end it "returns errors if image parameter is invalid" do From a0afeefd76ebfacae59343f48e127828b27ba77a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 17 Apr 2016 09:02:42 -0400 Subject: [PATCH 3/6] Add CHANGELOG and documentation --- CHANGELOG | 1 + doc/ci/yaml/README.md | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index c948e8e546..f52897892e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 8.7.0 (unreleased) - Make /profile/keys/new redirect to /profile/keys for back-compat. !3717 - Preserve time notes/comments have been updated at when moving issue - Make HTTP(s) label consistent on clone bar (Stan Hu) + - Add support for `after_script`, requires Runner 1.2 (Kamil Trzciński) - Expose label description in API (Mariusz Jachimowicz) - API: Ability to update a group (Robert Schilling) - API: Ability to move issues (Robert Schilling) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index abb6e97e5e..54b06f10b9 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -15,6 +15,7 @@ If you want a quick introduction to GitLab CI, follow our - [.gitlab-ci.yml](#gitlab-ci-yml) - [image and services](#image-and-services) - [before_script](#before_script) + - [after_script](#after_script) - [stages](#stages) - [types](#types) - [variables](#variables) @@ -80,6 +81,9 @@ services: before_script: - bundle install +after_script: + - rm secrets + stages: - build - test @@ -104,6 +108,7 @@ There are a few reserved `keywords` that **cannot** be used as job names: | stages | no | Define build stages | | types | no | Alias for `stages` | | before_script | no | Define commands that run before each job's script | +| after_script | no | Define commands that run after each job's script | | variables | no | Define build variables | | cache | no | Define list of files that should be cached between subsequent runs | @@ -118,6 +123,11 @@ used for time of the build. The configuration of this feature is covered in `before_script` is used to define the command that should be run before all builds, including deploy builds. This can be an array or a multi-line string. +### after_script + +`after_script` is used to define the command that will be run after for all +builds. This has to be an array or a multi-line string. + ### stages `stages` is used to define build stages that can be used by jobs. From c764b57f09ac55d181d17db018bdea9587fadea8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 17 Apr 2016 09:11:37 -0400 Subject: [PATCH 4/6] Add note about version --- doc/ci/yaml/README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 54b06f10b9..697445bcee 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -125,6 +125,9 @@ builds, including deploy builds. This can be an array or a multi-line string. ### after_script +>**Note:** +Introduced in GitLab 8.7 and GitLab Runner v1.2. + `after_script` is used to define the command that will be run after for all builds. This has to be an array or a multi-line string. From 63bd1f92d96c9d023723a78259be3c86846e30a5 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 18 Apr 2016 07:51:49 -0400 Subject: [PATCH 5/6] Fix rubocop complains --- lib/ci/gitlab_ci_yaml_processor.rb | 64 +++++++++++--------- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 4 +- 2 files changed, 37 insertions(+), 31 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index a87329c296..f1d6f2be1e 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -100,6 +100,29 @@ module Ci end def validate! + validate_global! + + @jobs.each do |name, job| + validate_job!(name, job) + end + + true + end + + def validate_job!(name, job) + validate_job_name!(name) + validate_job_keys!(name, job) + validate_job_types!(name, job) + + validate_job_stage!(name, job) if job[:stage] + validate_job_cache!(name, job) if job[:cache] + validate_job_artifacts!(name, job) if job[:artifacts] + validate_job_dependencies!(name, job) if job[:dependencies] + end + + private + + def validate_global! unless validate_array_of_strings(@before_script) raise ValidationError, "before_script should be an array of strings" end @@ -124,40 +147,23 @@ module Ci raise ValidationError, "variables should be a map of key-valued strings" end - if @cache - if @cache[:key] && !validate_string(@cache[:key]) - raise ValidationError, "cache:key parameter should be a string" - end - - if @cache[:untracked] && !validate_boolean(@cache[:untracked]) - raise ValidationError, "cache:untracked parameter should be an boolean" - end - - if @cache[:paths] && !validate_array_of_strings(@cache[:paths]) - raise ValidationError, "cache:paths parameter should be an array of strings" - end - end - - @jobs.each do |name, job| - validate_job!(name, job) - end - - true + validate_global_cache! if @cache end - def validate_job!(name, job) - validate_job_name!(name) - validate_job_keys!(name, job) - validate_job_types!(name, job) + def validate_global_cache! + if @cache[:key] && !validate_string(@cache[:key]) + raise ValidationError, "cache:key parameter should be a string" + end - validate_job_stage!(name, job) if job[:stage] - validate_job_cache!(name, job) if job[:cache] - validate_job_artifacts!(name, job) if job[:artifacts] - validate_job_dependencies!(name, job) if job[:dependencies] + if @cache[:untracked] && !validate_boolean(@cache[:untracked]) + raise ValidationError, "cache:untracked parameter should be an boolean" + end + + if @cache[:paths] && !validate_array_of_strings(@cache[:paths]) + raise ValidationError, "cache:paths parameter should be an array of strings" + end end - private - def validate_job_name!(name) if name.blank? || !validate_string(name) raise ValidationError, "job name should be non-empty string" diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 2421d6eee8..b94fec2ddf 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -295,12 +295,12 @@ module Ci describe "after_script" do context "in global context" do - let(:config) { + let(:config) do { after_script: ["after_script"], test: { script: ["script"] } } - } + end it "return after_script in options" do expect(subject[:options][:after_script]).to eq(["after_script"]) From 0ce5cc99621c84dfc61f6105d177ced4d9a4ed85 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 18 Apr 2016 10:18:46 -0400 Subject: [PATCH 6/6] Resolve merge --- lib/ci/gitlab_ci_yaml_processor.rb | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 2a228845ec..d8a7fcfe15 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -120,17 +120,6 @@ module Ci true end - def validate_job!(name, job) - validate_job_name!(name) - validate_job_keys!(name, job) - validate_job_types!(name, job) - - validate_job_stage!(name, job) if job[:stage] - validate_job_cache!(name, job) if job[:cache] - validate_job_artifacts!(name, job) if job[:artifacts] - validate_job_dependencies!(name, job) if job[:dependencies] - end - private def validate_global! @@ -170,12 +159,9 @@ module Ci raise ValidationError, "cache:untracked parameter should be an boolean" end -<<<<<<< HEAD if @cache[:paths] && !validate_array_of_strings(@cache[:paths]) raise ValidationError, "cache:paths parameter should be an array of strings" end -======= - true end def validate_job!(name, job) @@ -188,7 +174,6 @@ module Ci validate_job_cache!(name, job) if job[:cache] validate_job_artifacts!(name, job) if job[:artifacts] validate_job_dependencies!(name, job) if job[:dependencies] ->>>>>>> origin/master end def validate_job_name!(name)