diff --git a/app/assets/javascripts/editor/schema/ci.json b/app/assets/javascripts/editor/schema/ci.json
index 4c4ad81fed73508c453e1ce8c8c202eacde50488..5facd858f62d4c0e3501185bcdb133f705b8238b 100644
--- a/app/assets/javascripts/editor/schema/ci.json
+++ b/app/assets/javascripts/editor/schema/ci.json
@@ -202,25 +202,11 @@
"when": {
"markdownDescription": "Configure when artifacts are uploaded depended on job status. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#artifactswhen).",
"default": "on_success",
- "oneOf": [
- {
- "enum": [
- "on_success"
- ],
- "description": "Upload artifacts only when the job succeeds (this is the default)."
- },
- {
- "enum": [
- "on_failure"
- ],
- "description": "Upload artifacts only when the job fails."
- },
- {
- "enum": [
- "always"
- ],
- "description": "Upload artifacts regardless of job status."
- }
+ "type": "string",
+ "enum": [
+ "on_success",
+ "on_failure",
+ "always"
]
},
"expire_in": {
@@ -989,6 +975,7 @@
"default": false
},
"when": {
+ "type": "string",
"markdownDescription": "Defines when to save the cache, based on the status of the job. [Learn More](https://docs.gitlab.com/ee/ci/yaml/#cachewhen).",
"default": "on_success",
"enum": [
diff --git a/lib/gitlab/ci/config/entry/artifacts.rb b/lib/gitlab/ci/config/entry/artifacts.rb
index 3b0cbc6b69efcdce6b4743a7506e285288dd9856..27206d7e3a837d15c4ff111775616f1fa3abc227 100644
--- a/lib/gitlab/ci/config/entry/artifacts.rb
+++ b/lib/gitlab/ci/config/entry/artifacts.rb
@@ -12,6 +12,7 @@ class Artifacts < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
include ::Gitlab::Config::Entry::Attributable
+ ALLOWED_WHEN = %w[on_success on_failure always].freeze
ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as exclude public].freeze
EXPOSE_AS_REGEX = /\A\w[-\w ]*\z/.freeze
EXPOSE_AS_ERROR_MESSAGE = "can contain only letters, digits, '-', '_' and spaces"
@@ -38,10 +39,10 @@ class Artifacts < ::Gitlab::Config::Entry::Node
validates :expose_as, format: { with: EXPOSE_AS_REGEX, message: EXPOSE_AS_ERROR_MESSAGE }, if: :expose_as_present?
validates :exclude, array_of_strings: true
validates :reports, type: Hash
- validates :when,
- inclusion: { in: %w[on_success on_failure always],
- message: 'should be on_success, on_failure ' \
- 'or always' }
+ validates :when, type: String, inclusion: {
+ in: ALLOWED_WHEN,
+ message: "should be one of: #{ALLOWED_WHEN.join(', ')}"
+ }
validates :expire_in, duration: { parser: ::Gitlab::Ci::Build::DurationParser }
end
end
diff --git a/lib/gitlab/ci/config/entry/cache.rb b/lib/gitlab/ci/config/entry/cache.rb
index ab79add688bb879d1c422a8771e7c6364b73113f..a5481071fc593702dc42c7d5a53c594cdc8080e0 100644
--- a/lib/gitlab/ci/config/entry/cache.rb
+++ b/lib/gitlab/ci/config/entry/cache.rb
@@ -17,16 +17,16 @@ class Cache < ::Gitlab::Config::Entry::Node
validations do
validates :config, type: Hash, allowed_keys: ALLOWED_KEYS
- validates :policy,
- inclusion: { in: ALLOWED_POLICY, message: 'should be pull-push, push, or pull' },
- allow_blank: true
+ validates :policy, type: String, allow_blank: true, inclusion: {
+ in: ALLOWED_POLICY,
+ message: "should be one of: #{ALLOWED_POLICY.join(', ')}"
+ }
with_options allow_nil: true do
- validates :when,
- inclusion: {
- in: ALLOWED_WHEN,
- message: 'should be on_success, on_failure or always'
- }
+ validates :when, type: String, inclusion: {
+ in: ALLOWED_WHEN,
+ message: "should be one of: #{ALLOWED_WHEN.join(', ')}"
+ }
end
end
diff --git a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml
index f5670376efc57b66ba1737b8540cda295018a6cd..29f4a0cd76df0932595c81bfc703be7b8953a572 100644
--- a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml
+++ b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/artifacts.yml
@@ -16,3 +16,16 @@ cyclonedx not an array or string:
paths:
- foo
- bar
+
+# invalid artifacts:when
+artifacts-when-unknown:
+ artifacts:
+ when: unknown
+
+artifacts-when-array:
+ artifacts:
+ when: [always]
+
+artifacts-when-boolean:
+ artifacts:
+ when: true
diff --git a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/cache.yml b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/cache.yml
index 3979c9ae2ac786ebeb69fd60dbcd6cfd0f99e83a..9baed2a7922dd5fa34d8662b1b762e2aac192131 100644
--- a/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/cache.yml
+++ b/spec/frontend/editor/schema/ci/yaml_tests/negative_tests/cache.yml
@@ -51,12 +51,39 @@ cache-untracked-string:
cache:
untracked: 'true'
-when_integer:
+# invalid cache:when
+cache-when-integer:
script: echo "This job uses a cache."
cache:
when: 0
-when_not_reserved_keyword:
+cache-when-array:
+ script: echo "This job uses a cache."
+ cache:
+ when: [always]
+
+cache-when-boolean:
+ script: echo "This job uses a cache."
+ cache:
+ when: true
+
+cache-when-never:
script: echo "This job uses a cache."
cache:
when: 'never'
+
+# invalid cache:policy
+cache-policy-array:
+ script: echo "This job uses a cache."
+ cache:
+ policy: [push]
+
+cache-policy-boolean:
+ script: echo "This job uses a cache."
+ cache:
+ policy: true
+
+cache-when-unknown:
+ script: echo "This job uses a cache."
+ cache:
+ policy: unknown
diff --git a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml
index 20c1fc2c50f5352d6d418c90d1c24d4ae7dde8bd..a5c9153ee13a130d3ac7833a54636e57d423c442 100644
--- a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml
+++ b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/artifacts.yml
@@ -23,3 +23,13 @@ cylonedx mixed list of string paths and globs:
cyclonedx:
- ./foo
- "bar/*.baz"
+
+# valid artifacts:when
+artifacts-when-on-failure:
+ artifacts:
+ when: on_failure
+
+artifacts-no-when:
+ artifacts:
+ paths:
+ - binaries/
diff --git a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/cache.yml b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/cache.yml
index 75918cd2a1ba7128f0d7d7680f43c27e2a798705..d50b74e14480ff35e4c6ad6aa7f08b4c8bdb6a7e 100644
--- a/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/cache.yml
+++ b/spec/frontend/editor/schema/ci/yaml_tests/positive_tests/cache.yml
@@ -122,3 +122,20 @@ cache-untracked-false:
script: test
cache:
untracked: false
+
+# valid cache:policy
+cache-policy-push:
+ script: echo "This job uses a cache."
+ cache:
+ policy: push
+
+cache-policy-pull:
+ script: echo "This job uses a cache."
+ cache:
+ policy: pull
+
+cache-no-policy:
+ script: echo "This job uses a cache."
+ cache:
+ paths:
+ - binaries/
diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb
index 7476fc6c25fb0f3861b71adf2f7071c3c1e2f80f..6264e0c8e33f211dd10953a1ba9da6ccceccd6b8 100644
--- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb
@@ -142,6 +142,26 @@
end
end
+ context 'when the `when` keyword is not a string' do
+ context 'when it is an array' do
+ let(:config) { { paths: %w[results.txt], when: ['always'] } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ expect(entry.errors).to include 'artifacts when should be a string'
+ end
+ end
+
+ context 'when it is a boolean' do
+ let(:config) { { paths: %w[results.txt], when: true } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ expect(entry.errors).to include 'artifacts when should be a string'
+ end
+ end
+ end
+
describe 'excluded artifacts' do
context 'when configuration is valid' do
let(:config) { { untracked: true, exclude: ['some/directory/'] } }
diff --git a/spec/lib/gitlab/ci/config/entry/cache_spec.rb b/spec/lib/gitlab/ci/config/entry/cache_spec.rb
index 247f4b639102283b964df0779759431bef39086a..414cbb169b92ec96f8e056c7069e6e9537a5e380 100644
--- a/spec/lib/gitlab/ci/config/entry/cache_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/cache_spec.rb
@@ -163,22 +163,6 @@
end
end
- context 'when policy is unknown' do
- let(:config) { { policy: 'unknown' } }
-
- it 'reports error' do
- is_expected.to include('cache policy should be pull-push, push, or pull')
- end
- end
-
- context 'when `when` is unknown' do
- let(:config) { { when: 'unknown' } }
-
- it 'reports error' do
- is_expected.to include('cache when should be on_success, on_failure or always')
- end
- end
-
context 'when descendants are invalid' do
context 'with invalid keys' do
let(:config) { { key: 1 } }
@@ -228,6 +212,62 @@
is_expected.to include 'cache config contains unknown keys: invalid'
end
end
+
+ context 'when the `when` keyword is not a valid string' do
+ context 'when `when` is unknown' do
+ let(:config) { { when: 'unknown' } }
+
+ it 'returns error' do
+ is_expected.to include('cache when should be one of: on_success, on_failure, always')
+ end
+ end
+
+ context 'when it is an array' do
+ let(:config) { { when: ['always'] } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ is_expected.to include('cache when should be a string')
+ end
+ end
+
+ context 'when it is a boolean' do
+ let(:config) { { when: true } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ is_expected.to include('cache when should be a string')
+ end
+ end
+ end
+
+ context 'when the `policy` keyword is not a valid string' do
+ context 'when `policy` is unknown' do
+ let(:config) { { policy: 'unknown' } }
+
+ it 'returns error' do
+ is_expected.to include('cache policy should be one of: pull-push, push, pull')
+ end
+ end
+
+ context 'when it is an array' do
+ let(:config) { { policy: ['pull-push'] } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ is_expected.to include('cache policy should be a string')
+ end
+ end
+
+ context 'when it is a boolean' do
+ let(:config) { { policy: true } }
+
+ it 'returns error' do
+ expect(entry).not_to be_valid
+ is_expected.to include('cache policy should be a string')
+ end
+ end
+ end
end
end
end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 0d1deb863b1e2e268176fd7997bc988e945a954d..ae98d2e0cad68e402f8e5e6bfd0e56c3fa5e47c9 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -2946,7 +2946,7 @@ module Ci
context 'returns errors if job artifacts:when is not an a predefined value' do
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { when: 1 } } }) }
- it_behaves_like 'returns errors', 'jobs:rspec:artifacts when should be on_success, on_failure or always'
+ it_behaves_like 'returns errors', 'jobs:rspec:artifacts when should be one of: on_success, on_failure, always'
end
context 'returns errors if job artifacts:expire_in is not an a string' do