Commit ae4491b4 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'fix/error-when-job-variables-not-defined-but-specified' into 'master'

Fix error when CI job variables key used but not specified

## What does this MR do?

This MR fixes a an error when CI job variables specified, but not defined:

```yaml
image: ruby:2.2

test:
  variables:
  script:
     - rspec
```

## What are the relevant issue numbers?

Closes #18764  
Follow up discussion in: #18775 

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4745
parents 439e154c 6511b973
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.9.0 (unreleased) v 8.9.0 (unreleased)
- Fix error when CI job variables key specified but not defined
- Fix pipeline status when there are no builds in pipeline - Fix pipeline status when there are no builds in pipeline
- Fix Error 500 when using closes_issues API with an external issue tracker - Fix Error 500 when using closes_issues API with an external issue tracker
- Add more information into RSS feed for issues (Alexander Matyushentsev) - Add more information into RSS feed for issues (Alexander Matyushentsev)
......
...@@ -54,7 +54,7 @@ module Ci ...@@ -54,7 +54,7 @@ module Ci
job = @jobs[name.to_sym] job = @jobs[name.to_sym]
return [] unless job return [] unless job
job.fetch(:variables, []) job[:variables] || []
end end
private private
......
...@@ -344,13 +344,13 @@ module Ci ...@@ -344,13 +344,13 @@ module Ci
end end
end end
end end
describe "Scripts handling" do describe "Scripts handling" do
let(:config_data) { YAML.dump(config) } let(:config_data) { YAML.dump(config) }
let(:config_processor) { GitlabCiYamlProcessor.new(config_data, path) } let(:config_processor) { GitlabCiYamlProcessor.new(config_data, path) }
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 describe "before_script" do
context "in global context" do context "in global context" do
let(:config) do let(:config) do
...@@ -359,12 +359,12 @@ module Ci ...@@ -359,12 +359,12 @@ module Ci
test: { script: ["script"] } test: { script: ["script"] }
} }
end end
it "return commands with scripts concencaced" do it "return commands with scripts concencaced" do
expect(subject[:commands]).to eq("global script\nscript") expect(subject[:commands]).to eq("global script\nscript")
end end
end end
context "overwritten in local context" do context "overwritten in local context" do
let(:config) do let(:config) do
{ {
...@@ -522,19 +522,41 @@ module Ci ...@@ -522,19 +522,41 @@ module Ci
end end
context 'when syntax is incorrect' do context 'when syntax is incorrect' do
it 'raises error' do context 'when variables defined but invalid' do
variables = [:KEY1, 'value1', :KEY2, 'value2'] it 'raises error' do
variables = [:KEY1, 'value1', :KEY2, 'value2']
config = YAML.dump(
{ before_script: ['pwd'], config = YAML.dump(
rspec: { { before_script: ['pwd'],
variables: variables, rspec: {
script: 'rspec' } variables: variables,
}) script: 'rspec' }
})
expect { GitlabCiYamlProcessor.new(config, path) }
.to raise_error(GitlabCiYamlProcessor::ValidationError,
/job: variables should be a map/)
end
end
expect { GitlabCiYamlProcessor.new(config, path) } context 'when variables key defined but value not specified' do
.to raise_error(GitlabCiYamlProcessor::ValidationError, it 'returns empty array' do
/job: variables should be a map/) config = YAML.dump(
{ before_script: ['pwd'],
rspec: {
variables: nil,
script: 'rspec' }
})
config_processor = GitlabCiYamlProcessor.new(config, path)
##
# TODO, in next version of CI configuration processor this
# should be invalid configuration, see #18775 and #15060
#
expect(config_processor.job_variables(:rspec))
.to be_an_instance_of(Array).and be_empty
end
end end
end end
end end
......
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