Commit d7c17a58 authored by lauraMon's avatar lauraMon

Implement warnings for deprecated keywords

* Add `warning` to CiConfigResolver and type
* Add deprecation property to deprecated entry
* Update a lot of specs that were still using `types`
parent c0acad51
......@@ -47,11 +47,13 @@ module Resolvers
{
status: :valid,
errors: [],
warnings: result.warnings,
stages: make_stages(result.jobs)
}
else
{
status: :invalid,
warnings: result.warnings,
errors: result.errors
}
end
......
......@@ -15,6 +15,8 @@ module Types
description: 'Stages of the pipeline.'
field :status, Types::Ci::Config::StatusEnum, null: true,
description: 'Status of linting, can be either valid or invalid.'
field :warnings, [GraphQL::Types::String], null: true,
description: 'Linting warnings.'
end
end
end
......
......@@ -40,7 +40,8 @@ module Users
profile_personal_access_token_expiry: 37, # EE-only
terraform_notification_dismissed: 38,
security_newsletter_callout: 39,
verification_reminder: 40 # EE-only
verification_reminder: 40, # EE-only
ci_deprecation_warning_for_types_keyword: 41
}
validates :feature_name,
......
......@@ -8730,6 +8730,7 @@ Represents the total number of issues and their weights for a particular day.
| <a id="ciconfigmergedyaml"></a>`mergedYaml` | [`String`](#string) | Merged CI configuration YAML. |
| <a id="ciconfigstages"></a>`stages` | [`CiConfigStageConnection`](#ciconfigstageconnection) | Stages of the pipeline. (see [Connections](#connections)) |
| <a id="ciconfigstatus"></a>`status` | [`CiConfigStatus`](#ciconfigstatus) | Status of linting, can be either valid or invalid. |
| <a id="ciconfigwarnings"></a>`warnings` | [`[String!]`](#string) | Linting warnings. |
### `CiConfigGroup`
......@@ -17392,6 +17393,7 @@ Name of the feature that the callout is for.
| <a id="usercalloutfeaturenameenumactive_user_count_threshold"></a>`ACTIVE_USER_COUNT_THRESHOLD` | Callout feature name for active_user_count_threshold. |
| <a id="usercalloutfeaturenameenumbuy_pipeline_minutes_notification_dot"></a>`BUY_PIPELINE_MINUTES_NOTIFICATION_DOT` | Callout feature name for buy_pipeline_minutes_notification_dot. |
| <a id="usercalloutfeaturenameenumcanary_deployment"></a>`CANARY_DEPLOYMENT` | Callout feature name for canary_deployment. |
| <a id="usercalloutfeaturenameenumci_deprecation_warning_for_types_keyword"></a>`CI_DEPRECATION_WARNING_FOR_TYPES_KEYWORD` | Callout feature name for ci_deprecation_warning_for_types_keyword. |
| <a id="usercalloutfeaturenameenumcloud_licensing_subscription_activation_banner"></a>`CLOUD_LICENSING_SUBSCRIPTION_ACTIVATION_BANNER` | Callout feature name for cloud_licensing_subscription_activation_banner. |
| <a id="usercalloutfeaturenameenumcluster_security_warning"></a>`CLUSTER_SECURITY_WARNING` | Callout feature name for cluster_security_warning. |
| <a id="usercalloutfeaturenameenumeoa_bronze_plan_banner"></a>`EOA_BRONZE_PLAN_BANNER` | Callout feature name for eoa_bronze_plan_banner. |
......@@ -36,7 +36,7 @@ module Gitlab
end
@root = self.logger.instrument(:config_compose) do
Entry::Root.new(@config).tap(&:compose!)
Entry::Root.new(@config, project: project, user: user).tap(&:compose!)
end
rescue *rescue_errors => e
raise Config::ConfigError, e.message
......
......@@ -59,7 +59,8 @@ module Gitlab
entry :types, Entry::Stages,
description: 'Deprecated: stages for this pipeline.',
reserved: true
reserved: true,
deprecation: { deprecated: '9.0', warning: '14.8', removed: '15.0', documentation: 'https://docs.gitlab.com/ee/ci/yaml/#deprecated-keywords' }
entry :cache, Entry::Caches,
description: 'Configure caching between build jobs.',
......@@ -122,8 +123,9 @@ module Gitlab
# Deprecated `:types` key workaround - if types are defined and
# stages are not defined we use types definition as stages.
#
if types_defined? && !stages_defined?
@entries[:stages] = @entries[:types]
if types_defined?
@entries[:stages] = @entries[:types] unless stages_defined?
log_and_warn_deprecated_entry(@entries[:types])
end
@entries.delete(:types)
......
......@@ -76,7 +76,7 @@ module Gitlab
private
# rubocop: disable CodeReuse/ActiveRecord
def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil, metadata: {})
def entry(key, entry, description: nil, default: nil, inherit: nil, reserved: nil, deprecation: nil, metadata: {})
entry_name = key.to_sym
raise ArgumentError, "Entry '#{key}' already defined in '#{name}'" if @nodes.to_h[entry_name]
......@@ -85,6 +85,7 @@ module Gitlab
.with(default: default)
.with(inherit: inherit)
.with(reserved: reserved)
.with(deprecation: deprecation)
.metadata(metadata)
@nodes ||= {}
......
......@@ -32,6 +32,10 @@ module Gitlab
self
end
def deprecation
@attributes[:deprecation]
end
def description
@attributes[:description]
end
......@@ -84,6 +88,7 @@ module Gitlab
node.parent = @attributes[:parent]
node.default = @attributes[:default]
node.description = @attributes[:description]
node.deprecation = @attributes[:deprecation]
end
end
end
......
......@@ -10,7 +10,7 @@ module Gitlab
InvalidError = Class.new(StandardError)
attr_reader :config, :metadata
attr_accessor :key, :parent, :default, :description
attr_accessor :key, :parent, :default, :description, :deprecation
def initialize(config, **metadata)
@config = config
......@@ -128,6 +128,25 @@ module Gitlab
private
attr_reader :entries
def log_and_warn_deprecated_entry(entry)
user = metadata[:user]
project = metadata[:project]
if project && user
Gitlab::AppJsonLogger.info(event: 'ci_used_deprecated_keyword',
entry: entry.key.to_s,
user_id: user.id,
project_id: project.id)
end
deprecation = entry.deprecation
add_warning(
"`#{entry.key}` is deprecated in " \
"#{deprecation[:deprecated]} and will be removed in #{deprecation[:removed]} " \
"- read more: #{deprecation[:documentation]}"
)
end
end
end
end
......
......@@ -11,6 +11,7 @@ RSpec.describe Types::Ci::Config::ConfigType do
mergedYaml
stages
status
warnings
]
expect(described_class).to have_graphql_fields(*expected_fields)
......
......@@ -3,7 +3,9 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Config::Entry::Root do
let(:root) { described_class.new(hash) }
let(:user) {}
let(:project) {}
let(:root) { described_class.new(hash, user: user, project: project) }
describe '.nodes' do
it 'returns a hash' do
......@@ -53,6 +55,37 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do
}
end
context 'when deprecated types keyword is defined' do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:hash) do
{ types: %w(test deploy),
rspec: { script: 'rspec' } }
end
before do
root.compose!
end
it 'returns array of types as stages with a warning' do
expect(root.stages_value).to eq %w[test deploy]
expect(root.warnings).to eq(["root `types` is deprecated in 9.0 and will be removed in 15.0 - read more: https://docs.gitlab.com/ee/ci/yaml/#deprecated-keywords"])
end
it 'logs usage of types keyword' do
expect(Gitlab::AppJsonLogger).to(
receive(:info)
.with(event: 'ci_used_deprecated_keyword',
entry: root[:stages].key.to_s,
user_id: user.id,
project_id: project.id)
)
root.compose!
end
end
describe '#compose!' do
before do
root.compose!
......@@ -108,17 +141,6 @@ RSpec.describe Gitlab::Ci::Config::Entry::Root do
expect(root.stages_value).to eq %w[build pages release]
end
end
context 'when deprecated types key defined' do
let(:hash) do
{ types: %w(test deploy),
rspec: { script: 'rspec' } }
end
it 'returns array of types as stages' do
expect(root.stages_value).to eq %w[test deploy]
end
end
end
describe '#jobs_value' do
......
......@@ -2608,7 +2608,7 @@ module Gitlab
end
context 'returns errors if job stage is not a defined stage' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", type: "acceptance" } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", type: "acceptance" } }) }
it_behaves_like 'returns errors', 'rspec job: chosen stage does not exist; available stages are .pre, build, test, .post'
end
......@@ -2644,37 +2644,37 @@ module Gitlab
end
context 'returns errors if job artifacts:name is not an a string' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { name: 1 } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { name: 1 } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:artifacts name should be a string'
end
context 'returns errors if job artifacts:when is not an a predefined value' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { when: 1 } } }) }
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'
end
context 'returns errors if job artifacts:expire_in is not an a string' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { expire_in: 1 } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { expire_in: 1 } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:artifacts expire in should be a duration'
end
context 'returns errors if job artifacts:expire_in is not an a valid duration' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { expire_in: "7 elephants" } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { expire_in: "7 elephants" } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:artifacts expire in should be a duration'
end
context 'returns errors if job artifacts:untracked is not an array of strings' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { untracked: "string" } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { untracked: "string" } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:artifacts untracked should be a boolean value'
end
context 'returns errors if job artifacts:paths is not an array of strings' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", artifacts: { paths: "string" } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", artifacts: { paths: "string" } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:artifacts paths should be an array of strings'
end
......@@ -2698,49 +2698,49 @@ module Gitlab
end
context 'returns errors if job cache:key is not an a string' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { key: 1 } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { key: 1 } } }) }
it_behaves_like 'returns errors', "jobs:rspec:cache:key should be a hash, a string or a symbol"
end
context 'returns errors if job cache:key:files is not an array of strings' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { key: { files: [1] } } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { key: { files: [1] } } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:cache:key:files config should be an array of strings'
end
context 'returns errors if job cache:key:files is an empty array' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { key: { files: [] } } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { key: { files: [] } } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:cache:key:files config requires at least 1 item'
end
context 'returns errors if job defines only cache:key:prefix' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { key: { prefix: 'prefix-key' } } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { key: { prefix: 'prefix-key' } } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:cache:key config missing required keys: files'
end
context 'returns errors if job cache:key:prefix is not an a string' do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { key: { prefix: 1, files: ['file'] } } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { key: { prefix: 1, files: ['file'] } } } }) }
it_behaves_like 'returns errors', 'jobs:rspec:cache:key:prefix config should be a string or symbol'
end
context "returns errors if job cache:untracked is not an array of strings" do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { untracked: "string" } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { untracked: "string" } } }) }
it_behaves_like 'returns errors', "jobs:rspec:cache:untracked config should be a boolean value"
end
context "returns errors if job cache:paths is not an array of strings" do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", cache: { paths: "string" } } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", cache: { paths: "string" } } }) }
it_behaves_like 'returns errors', "jobs:rspec:cache:paths config should be an array of strings"
end
context "returns errors if job dependencies is not an array of strings" do
let(:config) { YAML.dump({ types: %w(build test), rspec: { script: "test", dependencies: "string" } }) }
let(:config) { YAML.dump({ stages: %w(build test), rspec: { script: "test", dependencies: "string" } }) }
it_behaves_like 'returns errors', "jobs:rspec dependencies should be an array of strings"
end
......
......@@ -39,7 +39,8 @@ RSpec.describe Gitlab::Config::Entry::Configurable do
entry :object, entry_class,
description: 'test object',
inherit: true,
reserved: true
reserved: true,
deprecation: { deprecated: '10.0', warning: '10.1', removed: '11.0', documentation: 'docs.gitlab.com' }
end
end
......@@ -52,6 +53,12 @@ RSpec.describe Gitlab::Config::Entry::Configurable do
factory = entry.nodes[:object]
expect(factory).to be_an_instance_of(Gitlab::Config::Entry::Factory)
expect(factory.deprecation).to eq(
deprecated: '10.0',
warning: '10.1',
removed: '11.0',
documentation: 'docs.gitlab.com'
)
expect(factory.description).to eq('test object')
expect(factory.inheritable?).to eq(true)
expect(factory.reserved?).to eq(true)
......
......@@ -115,5 +115,16 @@ RSpec.describe Gitlab::Config::Entry::Factory do
.with('some value', { some: 'hash' })
end
end
context 'when setting deprecation information' do
it 'passes deprecation as a parameter' do
entry = factory
.value('some value')
.with(deprecation: { deprecated: '10.0', warning: '10.1', removed: '11.0', documentation: 'docs' })
.create!
expect(entry.deprecation).to eq({ deprecated: '10.0', warning: '10.1', removed: '11.0', documentation: 'docs' })
end
end
end
end
......@@ -20,6 +20,7 @@ RSpec.describe 'Query.ciConfig' do
ciConfig(projectPath: "#{project.full_path}", content: "#{content}", dryRun: false) {
status
errors
warnings
stages {
nodes {
name
......@@ -73,6 +74,7 @@ RSpec.describe 'Query.ciConfig' do
expect(graphql_data['ciConfig']).to eq(
"status" => "VALID",
"errors" => [],
"warnings" => [],
"stages" =>
{
"nodes" =>
......@@ -220,6 +222,21 @@ RSpec.describe 'Query.ciConfig' do
)
end
context 'when using deprecated keywords' do
let_it_be(:content) do
YAML.dump(
rspec: { script: 'ls' },
types: ['test']
)
end
it 'returns a warning' do
post_graphql_query
expect(graphql_data['ciConfig']['warnings']).to include('root `types` is deprecated in 9.0 and will be removed in 15.0 - read more: https://docs.gitlab.com/ee/ci/yaml/#deprecated-keywords')
end
end
context 'when the config file includes other files' do
let_it_be(:content) do
YAML.dump(
......@@ -250,6 +267,7 @@ RSpec.describe 'Query.ciConfig' do
expect(graphql_data['ciConfig']).to eq(
"status" => "VALID",
"errors" => [],
"warnings" => [],
"stages" =>
{
"nodes" =>
......
......@@ -121,8 +121,8 @@ RSpec.describe API::Lint do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to be_an Hash
expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).to eq([])
expect(json_response['errors']).to eq([])
expect(json_response['warnings']).to match_array([])
expect(json_response['errors']).to match_array([])
end
it 'outputs expanded yaml content' do
......@@ -149,7 +149,20 @@ RSpec.describe API::Lint do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).not_to be_empty
expect(json_response['errors']).to eq([])
expect(json_response['errors']).to match_array([])
end
end
context 'with valid .gitlab-ci.yaml using deprecated keywords' do
let(:yaml_content) { { job: { script: 'ls' }, types: ['test'] }.to_yaml }
it 'passes validation but returns warnings' do
post api('/ci/lint', api_user), params: { content: yaml_content }
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['status']).to eq('valid')
expect(json_response['warnings']).not_to be_empty
expect(json_response['errors']).to match_array([])
end
end
......
......@@ -9,7 +9,7 @@ before_script:
variables:
DB_NAME: postgres
types:
stages:
- test
- deploy
- notify
......@@ -36,7 +36,7 @@ staging:
KEY1: value1
KEY2: value2
script: "cap deploy stating"
type: deploy
stage: deploy
tags:
- ruby
- mysql
......@@ -46,7 +46,7 @@ staging:
production:
variables:
DB_NAME: mysql
type: deploy
stage: deploy
script:
- cap deploy production
- cap notify
......@@ -58,7 +58,7 @@ production:
- /^deploy-.*$/
dockerhub:
type: notify
stage: notify
script: "curl http://dockerhub/URL"
tags:
- ruby
......
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