Commit 334e3c37 authored by Stan Hu's avatar Stan Hu

Fix Error 500 in parsing invalid CI needs and dependencies

Consider the following .gitlab-ci.yml:

```
stages:
    - test
    - build
    - deploy

test:
    stage: test
    script:
        - echo 1
build:
    stage: build
    script:
        - echo 1

deploy:
    stage: deploy
    needs: ["build"]
    dependencies: "test" # notice: no brackets
    script:
        - echo 1
```

If either `needs` or `dependencies` were not arrays of strings, the
validation for missing needs would fail because it was trying to
calculate `dependencies - needs`, which is invalid.

Even though `dependencies` and `needs` have separate validations that
check their types, the validation chain isn't halted if an error is
encountered. Thus, the validation for missing needs would still try to
run.

We fix this by checking the normalized (aka composed) job values in the
needs section.

Closes https://gitlab.com/gitlab-org/gitlab/issues/195653
parent 3029be8b
---
title: Fix Error 500 in parsing invalid CI needs and dependencies
merge_request: 22567
author:
type: fixed
......@@ -55,11 +55,12 @@ module Gitlab
validates :start_in, duration: { limit: '1 week' }, if: :delayed?
validates :start_in, absence: true, if: -> { has_rules? || !delayed? }
validate do
validate on: :composed do
next unless dependencies.present?
next unless needs.present?
next unless needs_value.present?
missing_needs = dependencies - needs_value[:job].pluck(:name) # rubocop:disable CodeReuse/ActiveRecord (Array#pluck)
missing_needs = dependencies - needs
if missing_needs.any?
errors.add(:dependencies, "the #{missing_needs.join(", ")} should be part of needs")
end
......
......@@ -1735,6 +1735,39 @@ module Gitlab
it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies the build2 should be part of needs') }
end
context 'needs with a Hash type and dependencies with a string type that are mismatching' do
let(:needs) do
[
"build1",
{ job: "build2" }
]
end
let(:dependencies) { %w(build3) }
it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies the build3 should be part of needs') }
end
context 'needs with an array type and dependency with a string type' do
let(:needs) { %w(build1) }
let(:dependencies) { 'deploy' }
it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies should be an array of strings') }
end
context 'needs with a string type and dependency with an array type' do
let(:needs) { 'build1' }
let(:dependencies) { %w(deploy) }
it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1:needs config can only be a hash or an array') }
end
context 'needs with a Hash type and dependency with a string type' do
let(:needs) { { job: 'build1' } }
let(:dependencies) { 'deploy' }
it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, 'jobs:test1 dependencies should be an array of strings') }
end
end
context 'with when/rules conflict' 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