From 08416f0ec7bd76938e8925ebbd4fbc52a8185ad3 Mon Sep 17 00:00:00 2001 From: Tom Grace Date: Fri, 2 Feb 2018 09:39:41 +0000 Subject: Update Jira integration docs for new Jira UI Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/42663 The new Jira UI doesn't show transition IDs in the admin interface. This updates the docs to list a couple of alternative methods of getting the required ID. --- .../integrations/img/jira_workflow_screenshot.png | Bin 66685 -> 0 bytes doc/user/project/integrations/jira.md | 11 ++++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) delete mode 100644 doc/user/project/integrations/img/jira_workflow_screenshot.png diff --git a/doc/user/project/integrations/img/jira_workflow_screenshot.png b/doc/user/project/integrations/img/jira_workflow_screenshot.png deleted file mode 100644 index e62fb202613..00000000000 Binary files a/doc/user/project/integrations/img/jira_workflow_screenshot.png and /dev/null differ diff --git a/doc/user/project/integrations/jira.md b/doc/user/project/integrations/jira.md index f77569e4886..0be86915448 100644 --- a/doc/user/project/integrations/jira.md +++ b/doc/user/project/integrations/jira.md @@ -113,7 +113,16 @@ in the table below. | `JIRA API URL` | The base URL to the JIRA instance API. Web URL value will be used if not set. E.g., `https://jira-api.example.com`. | | `Username` | The user name created in [configuring JIRA step](#configuring-jira). | | `Password` |The password of the user created in [configuring JIRA step](#configuring-jira). | -| `Transition ID` | This is the ID of a transition that moves issues to a closed state. You can find this number under JIRA workflow administration ([see screenshot](img/jira_workflow_screenshot.png)). **Closing JIRA issues via commits or Merge Requests won't work if you don't set the ID correctly.** | +| `Transition ID` | This is the ID of a transition that moves issues to the desired state. **Closing JIRA issues via commits or Merge Requests won't work if you don't set the ID correctly.** | + +### Getting a Transition ID + +In the most recent Jira UI, you can no longer see transition IDs in the workflow administration UI. You can get the ID you need in either of the following ways: + +1. Use the API, with a request like https://yourcompany.atlassian.net/rest/api/2/issue/ISSUE-123/transitions using an issue that is in the appropriate "open" state +1. By mousing over the link for the transition you want and looking for the "action" parameter in the URL + +Note that the transition ID may vary between workflows (i.e. bug vs. story), even if the status you are changing to is the same. After saving the configuration, your GitLab project will be able to interact with all JIRA projects in your JIRA instance. -- cgit v1.2.1 From da6157a544a01faf5aa99537c8bb35d3ef2a3a6f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 23 Feb 2018 14:47:13 +0100 Subject: Move some predefined variables to reflect hierarchy --- app/models/ci/build.rb | 11 ----------- app/models/ci/pipeline.rb | 12 +++++++++++- app/models/project.rb | 5 ++++- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 490edf4ac57..acba0c7d72a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -282,19 +282,14 @@ module Ci # contain unexpanded variables. def variables(environment: persisted_environment) variables = predefined_variables - variables += project.predefined_variables variables += pipeline.predefined_variables variables += runner.predefined_variables if runner - variables += project.container_registry_variables variables += project.deployment_variables if has_environment? - variables += project.auto_devops_variables variables += yaml_variables variables += user_variables variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group variables += secret_variables(environment: environment) variables += trigger_request.user_variables if trigger_request - variables += pipeline.variables.map(&:to_runner_variable) - variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule variables += persisted_environment_variables if environment variables @@ -570,12 +565,6 @@ module Ci def predefined_variables variables = [ - { key: 'CI', value: 'true', public: true }, - { key: 'GITLAB_CI', value: 'true', public: true }, - { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true }, - { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, - { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, - { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, { key: 'CI_JOB_ID', value: id.to_s, public: true }, { key: 'CI_JOB_NAME', value: name, public: true }, { key: 'CI_JOB_STAGE', value: stage, public: true }, diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2abe90dd181..d5424dee798 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -473,11 +473,21 @@ module Ci end def predefined_variables - [ + variables = [ + { key: 'CI', value: 'true', public: true }, + { key: 'GITLAB_CI', value: 'true', public: true }, + { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true }, + { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, + { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, + { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, { key: 'CI_PIPELINE_ID', value: id.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: ci_yaml_file_path, public: true }, { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true } ] + + variables += project.predefined_variables + variables += variables.map(&:to_runner_variable) + variables += pipeline_schedule.job_variables if pipeline_schedule end def queued_duration diff --git a/app/models/project.rb b/app/models/project.rb index 2ba6a863500..995575718f8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1554,7 +1554,7 @@ class Project < ActiveRecord::Base end def predefined_variables - [ + variables = [ { key: 'CI_PROJECT_ID', value: id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: path, public: true }, { key: 'CI_PROJECT_PATH', value: full_path, public: true }, @@ -1563,6 +1563,9 @@ class Project < ActiveRecord::Base { key: 'CI_PROJECT_URL', value: web_url, public: true }, { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true } ] + + variables += container_registry_variables + variables += auto_devops_variables end def container_registry_variables -- cgit v1.2.1 From 7bb0c79b8e0c3b28d79a9471ed516651abfa1a6c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 23 Feb 2018 14:59:34 +0100 Subject: Fix pipeline variables when pipeline has no schedule --- app/models/ci/pipeline.rb | 10 ++++++---- app/models/project.rb | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d5424dee798..85024408cf2 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -473,7 +473,7 @@ module Ci end def predefined_variables - variables = [ + predefined = [ { key: 'CI', value: 'true', public: true }, { key: 'GITLAB_CI', value: 'true', public: true }, { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true }, @@ -485,9 +485,11 @@ module Ci { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true } ] - variables += project.predefined_variables - variables += variables.map(&:to_runner_variable) - variables += pipeline_schedule.job_variables if pipeline_schedule + predefined += project.predefined_variables + predefined += pipeline_schedule.job_variables if pipeline_schedule + predefined += self.variables.map(&:to_runner_variable) + + predefined end def queued_duration diff --git a/app/models/project.rb b/app/models/project.rb index 995575718f8..f7c41ab5fa4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1566,6 +1566,8 @@ class Project < ActiveRecord::Base variables += container_registry_variables variables += auto_devops_variables + + variables end def container_registry_variables -- cgit v1.2.1 From d25c4803f4429d5aeb2ec1eeb920da216d33f024 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 27 Feb 2018 11:21:40 +0100 Subject: Fix pipeline build specs related to variables order --- spec/models/ci/build_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 2b6b6a61182..43fba1349e3 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1854,7 +1854,8 @@ describe Ci::Build do end allow_any_instance_of(Ci::Pipeline) - .to receive(:predefined_variables) { [pipeline_pre_var] } + .to receive(:predefined_variables) + .and_return([project_pre_var] + [pipeline_pre_var]) end it do -- cgit v1.2.1 From 4f472d49b52452b0377fa0701dfe970f3e83bb85 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 27 Feb 2018 12:24:16 +0100 Subject: Make pipeline priority variables concept explicit --- app/models/ci/build.rb | 3 ++- app/models/ci/pipeline.rb | 16 +++++++++------ app/models/project.rb | 3 ++- spec/models/ci/build_spec.rb | 26 +++++++++++++++++++++++++ spec/models/ci/pipeline_spec.rb | 43 +++++++++++++++++++++++++++++++++++------ 5 files changed, 77 insertions(+), 14 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 8e2b9dd570b..137ccfbed89 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -262,8 +262,9 @@ module Ci variables += secret_variables(environment: environment) variables += trigger_request.user_variables if trigger_request variables += persisted_environment_variables if environment + variables += pipeline.priority_variables - variables + variables.reverse.uniq { |variable| variable.fetch(:key) }.reverse end def features diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 2eb0fa1897f..0435f6b2f9d 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -473,10 +473,9 @@ module Ci end def predefined_variables - predefined = [ + pipeline_predefined = [ { key: 'CI', value: 'true', public: true }, { key: 'GITLAB_CI', value: 'true', public: true }, - { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true }, { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, @@ -485,11 +484,16 @@ module Ci { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true } ] - predefined += project.predefined_variables - predefined += pipeline_schedule.job_variables if pipeline_schedule - predefined += self.variables.map(&:to_runner_variable) + Array(project.predefined_variables) + pipeline_predefined + end + + def priority_variables + Array(pipeline_schedule&.job_variables) + + self.variables.map(&:to_runner_variable) + end - predefined + def runtime_variables + predefined_variables + priority_variables end def queued_duration diff --git a/app/models/project.rb b/app/models/project.rb index fdaa41a79d6..62a7e10a5cf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1589,7 +1589,8 @@ class Project < ActiveRecord::Base { key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug, public: true }, { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true }, { key: 'CI_PROJECT_URL', value: web_url, public: true }, - { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true } + { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true }, + { key: 'GITLAB_FEATURES', value: namespace.features.join(','), public: true } ] variables += container_registry_variables diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 5a4080f9c6c..d1db61084c3 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1891,6 +1891,32 @@ describe Ci::Build do end end end + + context 'when there are duplicated variables present ' do + context 'when there are duplicated YAML variables' do + before do + build.yaml_variables = [{ key: 'MYVAR', value: 'first', public: true }, + { key: 'MYVAR', value: 'second', public: true}] + end + + it 'keeps the last occurence of a variable by given key' do + expect(subject).not_to include(key: 'MYVAR', value: 'first', public: true) + expect(subject).to include(key: 'MYVAR', value: 'second', public: true) + end + end + + context 'when pipeline trigger variable overrides YAML variables' do + before do + build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }] + pipeline.variables.build(key: 'MYVAR', value: 'pipeline value') + end + + it 'overrides YAML variable with a pipeline trigger variable' do + expect(subject).not_to include(key: 'MYVAR', value: 'myvar', public: true) + expect(subject).to include(key: 'MYVAR', value: 'pipeline value', public: false) + end + end + end end describe 'state transition: any => [:pending]' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 14d234f6aab..4d5fc1dfcd8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -167,15 +167,46 @@ describe Ci::Pipeline, :mailer do end end - describe '#predefined_variables' do - subject { pipeline.predefined_variables } + describe 'pipeline variables' do + describe '#predefined_variables' do + subject { pipeline.predefined_variables } - it { is_expected.to be_an(Array) } + it { is_expected.to be_an(Array) } + + it 'includes the defined keys' do + keys = subject.map { |v| v.fetch(:key) } + + expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + end + + it 'includes project-level predefined variables' do + keys = subject.map { |v| v.fetch(:key) } + + expect(keys).to include('CI_PROJECT_NAME') + end + end + + describe '#priority_variables' do + before do + pipeline.variables.build(key: 'MY_VAR', value: 'my var') + end - it 'includes the defined keys' do - keys = subject.map { |v| v[:key] } + it 'returns trigger variables' do + expect(pipeline.priority_variables) + .to include(key: 'MY_VAR', value: 'my var', public: false) + end + end - expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + describe '#runtime_variables' do + before do + pipeline.variables.build(key: 'MY_VAR', value: 'my var') + end + + it 'includes predefined and priority variables' do + variables = pipeline.runtime_variables.map { |v| v.fetch(:key) } + + expect(variables).to include('MY_VAR', 'CI_PIPELINE_ID', 'CI_PROJECT_ID') + end end end -- cgit v1.2.1 From c52255884ea1a54d66c0e5e52a67c25ad1a62644 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 10:50:02 +0100 Subject: Add support for only/except: variables CI/CD config --- lib/gitlab/ci/config/entry/policy.rb | 20 +++++++++-- lib/gitlab/ci/pipeline/expression/statement.rb | 14 +++++++- spec/lib/gitlab/ci/config/entry/policy_spec.rb | 25 ++++++++++++++ .../ci/pipeline/expression/statement_spec.rb | 40 +++++++++++++++++----- spec/models/ci/build_spec.rb | 2 +- 5 files changed, 88 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/ci/config/entry/policy.rb b/lib/gitlab/ci/config/entry/policy.rb index 0027e9ec8c5..b6d137a7e68 100644 --- a/lib/gitlab/ci/config/entry/policy.rb +++ b/lib/gitlab/ci/config/entry/policy.rb @@ -25,15 +25,31 @@ module Gitlab include Entry::Validatable include Entry::Attributable - attributes :refs, :kubernetes + attributes :refs, :kubernetes, :variables validations do validates :config, presence: true - validates :config, allowed_keys: %i[refs kubernetes] + validates :config, allowed_keys: %i[refs kubernetes variables] + validate :variables_expressions_syntax with_options allow_nil: true do validates :refs, array_of_strings_or_regexps: true validates :kubernetes, allowed_values: %w[active] + validates :variables, array_of_strings: true + end + + def variables_expressions_syntax + return unless variables.is_a?(Array) + + statements = variables.map do |statement| + ::Gitlab::Ci::Pipeline::Expression::Statement.new(statement) + end + + statements.each do |statement| + unless statement.valid? + errors.add(:variables, "Invalid expression #{statement.inspect}") + end + end end end end diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 08e662eccf9..616a9fe204c 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -14,9 +14,11 @@ module Gitlab %w[variable] ].freeze - def initialize(statement, pipeline) + def initialize(statement, pipeline = nil) @lexer = Expression::Lexer.new(statement) + return if pipeline.nil? + @variables = pipeline.variables.map do |variable| [variable.key.to_sym, variable.value] end @@ -35,6 +37,16 @@ module Gitlab def evaluate parse_tree.evaluate(@variables.to_h) end + + def inspect + "syntax: #{@lexer.lexemes.join(' ')}" + end + + def valid? + parse_tree.is_a?(Lexeme::Base) + rescue StatementError + false + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb index 5e83abf645b..f06d3a13ce0 100644 --- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -83,6 +83,31 @@ describe Gitlab::Ci::Config::Entry::Policy do end end + context 'when specifying valid variables expressions policy' do + let(:config) { { variables: ['$VAR == null'] } } + + it 'is a correct configuraton' do + expect(entry).to be_valid + expect(entry.value).to eq(config) + end + end + + context 'when specifying variables expressions in invalid format' do + let(:config) { { variables: '$MY_VAR' } } + + it 'reports an error about invalid format' do + expect(entry.errors).to include /should be an array of strings/ + end + end + + context 'when specifying invalid variables expressions statement' do + let(:config) { { variables: ['$MY_VAR =='] } } + + it 'reports an error about invalid statement' do + expect(entry.errors).to include /invalid expression syntax: variable equals/ + end + end + context 'when specifying unknown policy' do let(:config) { { refs: ['master'], invalid: :something } } diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 472a58599d8..3d97d71d629 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -11,6 +11,16 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do pipeline.variables.build([key: 'VARIABLE', value: 'my variable']) end + describe '.new' do + context 'when pipeline is not provided' do + it 'allows to properly initialize the statement' do + statement = described_class.new('$VARIABLE') + + expect(statement.evaluate).to be_nil + end + end + end + describe '#parse_tree' do context 'when expression is empty' do let(:text) { '' } @@ -23,18 +33,26 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do context 'when expression grammar is incorrect' do table = [ - '$VAR "text"', # missing operator - '== "123"', # invalid right side - "'single quotes'", # single quotes string - '$VAR ==', # invalid right side - '12345', # unknown syntax - '' # empty statement + '$VAR "text"', # missing operator + '== "123"', # invalid left side + '"some string"', # only string provided + '$VAR ==', # invalid right side + '12345', # unknown syntax + '' # empty statement ] table.each do |syntax| - it "raises an error when syntax is `#{syntax}`" do - expect { described_class.new(syntax, pipeline).parse_tree } - .to raise_error described_class::StatementError + context "when expression grammar is #{syntax.inspect}" do + let(:text) { syntax } + + it 'aises a statement error exception' do + expect { subject.parse_tree } + .to raise_error described_class::StatementError + end + + it 'is an invalid statement' do + expect(subject).not_to be_valid + end end end end @@ -47,6 +65,10 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do expect(subject.parse_tree) .to be_a Gitlab::Ci::Pipeline::Expression::Lexeme::Equals end + + it 'is a valid statement' do + expect(subject).to be_valid + end end context 'when using a single token' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index d1db61084c3..2328c3ee902 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1896,7 +1896,7 @@ describe Ci::Build do context 'when there are duplicated YAML variables' do before do build.yaml_variables = [{ key: 'MYVAR', value: 'first', public: true }, - { key: 'MYVAR', value: 'second', public: true}] + { key: 'MYVAR', value: 'second', public: true }] end it 'keeps the last occurence of a variable by given key' do -- cgit v1.2.1 From 8a7c89adfb6f6013a8db7550958637d4815ed0d5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 11:37:23 +0100 Subject: Use pipeline runtime variables in expressions --- lib/gitlab/ci/pipeline/expression/statement.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index 0b522d90e40..cd0d3089d0c 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -19,8 +19,8 @@ module Gitlab return if pipeline.nil? - @variables = pipeline.variables.map do |variable| - [variable.key, variable.value] + @variables = pipeline.runtime_variables.map do |variable| + [variable.fetch(:key), variable.fetch(:value)] end end -- cgit v1.2.1 From 64e86dab627681c52ffa78e99415b3b9d00b2f43 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 12:06:50 +0100 Subject: Allow using an empty string with pipeline expressions --- lib/gitlab/ci/pipeline/expression/lexeme/string.rb | 2 +- .../gitlab/ci/pipeline/expression/lexeme/string_spec.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/string.rb b/lib/gitlab/ci/pipeline/expression/lexeme/string.rb index 48bde213d44..346c92dc51e 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/string.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/string.rb @@ -4,7 +4,7 @@ module Gitlab module Expression module Lexeme class String < Lexeme::Value - PATTERN = /("(?.+?)")|('(?.+?)')/.freeze + PATTERN = /("(?.*?)")|('(?.*?)')/.freeze def initialize(value) @value = value diff --git a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb index 86234dfb9e5..1ccb792d1da 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/lexeme/string_spec.rb @@ -73,6 +73,22 @@ describe Gitlab::Ci::Pipeline::Expression::Lexeme::String do expect(token).not_to be_nil expect(token.build.evaluate).to eq 'some " string' end + + it 'allows to use an empty string inside single quotes' do + scanner = StringScanner.new(%('')) + + token = described_class.scan(scanner) + + expect(token.build.evaluate).to eq '' + end + + it 'allow to use an empty string inside double quotes' do + scanner = StringScanner.new(%("")) + + token = described_class.scan(scanner) + + expect(token.build.evaluate).to eq '' + end end end -- cgit v1.2.1 From 1926eca04d36793b9d9646d5a0e720e4b835fd3c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 12:10:32 +0100 Subject: Add method that checks if pipeline expression is truthy --- lib/gitlab/ci/pipeline/expression/statement.rb | 4 ++ .../ci/pipeline/expression/statement_spec.rb | 49 +++++++++++++++++----- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index cd0d3089d0c..017c7f11657 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -38,6 +38,10 @@ module Gitlab parse_tree.evaluate(@variables.to_h) end + def truthful? + evaluate.present? + end + def inspect "syntax: #{@lexer.lexemes.join(' ')}" end diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 3d97d71d629..475ac7afcb3 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -8,13 +8,16 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do end before do - pipeline.variables.build([key: 'VARIABLE', value: 'my variable']) + variables = [{ key: 'PRESENT_VARIABLE', value: 'my variable' }, + { key: 'EMPTY_VARIABLE', value: '' }] + + pipeline.variables.build(variables) end describe '.new' do context 'when pipeline is not provided' do it 'allows to properly initialize the statement' do - statement = described_class.new('$VARIABLE') + statement = described_class.new('$PRESENT_VARIABLE') expect(statement.evaluate).to be_nil end @@ -72,7 +75,7 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do end context 'when using a single token' do - let(:text) { '$VARIABLE' } + let(:text) { '$PRESENT_VARIABLE' } it 'returns a single token instance' do expect(subject.parse_tree) @@ -84,14 +87,17 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do describe '#evaluate' do statements = [ - ['$VARIABLE == "my variable"', true], - ["$VARIABLE == 'my variable'", true], - ['"my variable" == $VARIABLE', true], - ['$VARIABLE == null', false], - ['$VAR == null', true], - ['null == $VAR', true], - ['$VARIABLE', 'my variable'], - ['$VAR', nil] + ['$PRESENT_VARIABLE == "my variable"', true], + ["$PRESENT_VARIABLE == 'my variable'", true], + ['"my variable" == $PRESENT_VARIABLE', true], + ['$PRESENT_VARIABLE == null', false], + ['$EMPTY_VARIABLE == null', false], + ['"" == $EMPTY_VARIABLE', true], + ['$EMPTY_VARIABLE', ''], + ['$UNDEFINED_VARIABLE == null', true], + ['null == $UNDEFINED_VARIABLE', true], + ['$PRESENT_VARIABLE', 'my variable'], + ['$UNDEFINED_VARIABLE', nil] ] statements.each do |expression, value| @@ -104,4 +110,25 @@ describe Gitlab::Ci::Pipeline::Expression::Statement do end end end + + describe '#truthful?' do + statements = [ + ['$PRESENT_VARIABLE == "my variable"', true], + ["$PRESENT_VARIABLE == 'no match'", false], + ['$UNDEFINED_VARIABLE == null', true], + ['$PRESENT_VARIABLE', true], + ['$UNDEFINED_VARIABLE', false], + ['$EMPTY_VARIABLE', false] + ] + + statements.each do |expression, value| + context "when using expression `#{expression}`" do + let(:text) { expression } + + it "returns `#{value.inspect}`" do + expect(subject.truthful?).to eq value + end + end + end + end end -- cgit v1.2.1 From 52b34969a50312104d2d439c452a5bbb79085945 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 13:08:42 +0100 Subject: Add build policy for pipeline expressions --- lib/gitlab/ci/build/policy/variables.rb | 22 ++++++++++++ spec/lib/gitlab/ci/build/policy/variables_spec.rb | 41 +++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 lib/gitlab/ci/build/policy/variables.rb create mode 100644 spec/lib/gitlab/ci/build/policy/variables_spec.rb diff --git a/lib/gitlab/ci/build/policy/variables.rb b/lib/gitlab/ci/build/policy/variables.rb new file mode 100644 index 00000000000..eeb0f3f2cb7 --- /dev/null +++ b/lib/gitlab/ci/build/policy/variables.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + module Build + module Policy + class Variables < Policy::Specification + def initialize(expressions) + @expressions = Array(expressions) + end + + def satisfied_by?(pipeline) + statements = @expressions.map do |statement| + ::Gitlab::Ci::Pipeline::Expression::Statement + .new(statement, pipeline) + end + + statements.any? { |statement| statement.truthful? } + end + end + end + end + end +end diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb new file mode 100644 index 00000000000..6596237ff9c --- /dev/null +++ b/spec/lib/gitlab/ci/build/policy/variables_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Policy::Variables do + let(:pipeline) { build(:ci_pipeline, ref: 'master') } + + before do + pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '') + end + + describe '#satisfied_by?' do + it 'is satisfied by a defined and existing variable' do + policy = described_class.new(['$CI_PROJECT_ID', '$UNDEFINED']) + + expect(policy).to be_satisfied_by(pipeline) + end + + it 'is not satisfied by an overriden empty variable' do + policy = described_class.new(['$CI_PROJECT_NAME']) + + expect(policy).not_to be_satisfied_by(pipeline) + end + + it 'is satisfied by a truthy pipeline expression' do + policy = described_class.new([%($CI_PIPELINE_SOURCE == "#{pipeline.source}")]) + + expect(policy).to be_satisfied_by(pipeline) + end + + it 'is not satisfied by a falsy pipeline expression' do + policy = described_class.new([%($CI_PIPELINE_SOURCE == "invalid source")]) + + expect(policy).not_to be_satisfied_by(pipeline) + end + + it 'is satisfied by a truthy expression using undefined variable' do + policy = described_class.new(['$UNDEFINED', '$UNDEFINED == null']) + + expect(policy).to be_satisfied_by(pipeline) + end + end +end -- cgit v1.2.1 From 51727377a243238984425d7e3c8ca48658613733 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 28 Feb 2018 13:13:14 +0100 Subject: Use squiggly heredoc to improve YAML processor specs --- spec/lib/gitlab/ci/yaml_processor_spec.rb | 50 +++++++++++++++---------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index f83f932e61e..b9fccf47e79 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1344,13 +1344,13 @@ module Gitlab context 'when template is a job' do let(:config) do - <