Skip to content
Snippets Groups Projects
Verified Commit 0a3ea491 authored by Matija Čupić's avatar Matija Čupić :rocket:
Browse files

Refactor CI config lint resolver

Simplify stage extrapolation

Small GraphQL improvements

* Use connection_type for an array of objects
* Update GraphQL description for job need name

Remove include_merged_yaml parameter

Removes the include_merged_yaml parameter to make code more idiomatic.
The parameter isn't stored, it's computed dynamically when accessed so
this introduces a slight performance overhead, but it's far outweighed
by the pro of having cleaner code and less technical debt.

Stub YamlProcessor response in resolver specs

Refactor relation connection types

* Move request type tests to request specs
* Use proper status enum values

Simplify Ci::Config GraphQL resolver
parent 22883f79
No related branches found
No related tags found
1 merge request!48789Draft: Add lint tab to the pipeline editor
Showing
with 1053 additions and 138 deletions
...@@ -9,70 +9,72 @@ class ConfigResolver < BaseResolver ...@@ -9,70 +9,72 @@ class ConfigResolver < BaseResolver
required: true, required: true,
description: 'Contents of .gitlab-ci.yml' description: 'Contents of .gitlab-ci.yml'
argument :include_merged_yaml, GraphQL::BOOLEAN_TYPE, def resolve(content:)
required: false, result = ::Gitlab::Ci::YamlProcessor.new(content).execute
description: 'Whether or not to include merged CI yaml in the response'
def resolve(content:, include_merged_yaml: false)
result = Gitlab::Ci::YamlProcessor.new(content).execute
if result.errors.empty? if result.errors.empty?
stages = stages(result.stages) stages = process_stage_groups(result.stages, result.jobs)
jobs = jobs(result.jobs)
groups = groups(jobs)
stages = stage_groups(stages, groups)
response = { response = {
status: 'valid', status: :valid,
errors: [], errors: [],
stages: stages.select { |stage| !stage[:groups].empty? } stages: stages.select { |stage| !stage[:groups].empty? }
} }
else else
response = { response = {
status: 'invalid', status: :invalid,
errors: [result.errors.first] errors: [result.errors.first]
} }
end end
response.tap do |response| response.tap do |response|
response[:merged_yaml] = result.merged_yaml if include_merged_yaml response[:merged_yaml] = result.merged_yaml
end end
end end
private private
def stages(config_stages) def process_stages(config_stages)
config_stages.map { |stage| { name: stage, groups: [] } } config_stages.map { |stage| { name: stage, groups: [] } }
end end
def jobs(config_jobs) def process_jobs(config_jobs)
config_jobs.map do |job_name, job| config_jobs.map do |job_name, job|
{ {
name: job_name, name: job_name,
stage: job[:stage], stage: job[:stage],
group_name: CommitStatus.new(name: job_name).group_name, group_name: CommitStatus.new(name: job_name).group_name,
needs: needs(job) || [] needs: process_needs(job) || []
} }
end end
end end
def needs(job) def process_needs(job)
job.dig(:needs, :job)&.map do |job_need| job.dig(:needs, :job)&.map do |job_need|
{ name: job_need[:name], artifacts: job_need[:artifacts] } { name: job_need[:name], artifacts: job_need[:artifacts] }
end end
end end
def groups(jobs) def process_groups(job_data)
jobs = process_jobs(job_data)
group_names = jobs.map { |job| job[:group_name] }.uniq group_names = jobs.map { |job| job[:group_name] }.uniq
jobs_by_group = jobs.group_by { |job| job[:group_name] }
group_names.map do |group| group_names.map do |group|
group_jobs = jobs.select { |job| job[:group_name] == group } group_jobs = jobs_by_group[group]
{ jobs: group_jobs, name: group, stage: group_jobs.first[:stage], size: group_jobs.count } { jobs: group_jobs, name: group, stage: group_jobs.first[:stage], size: group_jobs.count }
end end
end end
def stage_groups(stage_data, groups) def process_stage_groups(stage_data, job_data)
stage_data.each do |stage| stages = process_stages(stage_data)
stage[:groups] = groups.select { |group| group[:stage] == stage[:name] } groups = process_groups(job_data)
groups_by_stage = groups.group_by { |group| group[:stage] }
stages.each do |stage|
next unless groups_by_stage[stage[:name]]
stage[:groups] = groups_by_stage[stage[:name]]
end end
end end
end end
......
...@@ -7,11 +7,11 @@ module Config ...@@ -7,11 +7,11 @@ module Config
class ConfigType < BaseObject class ConfigType < BaseObject
graphql_name 'CiConfig' graphql_name 'CiConfig'
field :errors, GraphQL::STRING_TYPE, null: true, field :errors, [GraphQL::STRING_TYPE], null: true,
description: 'Linting errors' description: 'Linting errors'
field :merged_yaml, GraphQL::STRING_TYPE, null: true, field :merged_yaml, GraphQL::STRING_TYPE, null: true,
description: 'Merged CI config YAML' description: 'Merged CI config YAML'
field :stages, [Types::Ci::Config::StageType], null: true, field :stages, Types::Ci::Config::StageType.connection_type, null: true,
description: 'Stages of the pipeline' description: 'Stages of the pipeline'
field :status, Types::Ci::Config::StatusEnum, null: true, field :status, Types::Ci::Config::StatusEnum, null: true,
description: 'Status of linting, can be either valid or invalid' description: 'Status of linting, can be either valid or invalid'
......
...@@ -9,7 +9,7 @@ class GroupType < BaseObject ...@@ -9,7 +9,7 @@ class GroupType < BaseObject
field :name, GraphQL::STRING_TYPE, null: true, field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job group' description: 'Name of the job group'
field :jobs, [Types::Ci::Config::JobType], null: true, field :jobs, Types::Ci::Config::JobType.connection_type, null: true,
description: 'Jobs in group' description: 'Jobs in group'
field :size, GraphQL::INT_TYPE, null: true, field :size, GraphQL::INT_TYPE, null: true,
description: 'Size of the job group' description: 'Size of the job group'
......
...@@ -9,7 +9,11 @@ class JobType < BaseObject ...@@ -9,7 +9,11 @@ class JobType < BaseObject
field :name, GraphQL::STRING_TYPE, null: true, field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job' description: 'Name of the job'
field :needs, [Types::Ci::Config::NeedType], null: true, field :group_name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job group'
field :stage, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job stage'
field :needs, Types::Ci::Config::NeedType.connection_type, null: true,
description: 'Builds that must complete before the jobs run' description: 'Builds that must complete before the jobs run'
end end
end end
......
...@@ -8,7 +8,7 @@ class NeedType < BaseObject ...@@ -8,7 +8,7 @@ class NeedType < BaseObject
graphql_name 'CiConfigNeed' graphql_name 'CiConfigNeed'
field :name, GraphQL::STRING_TYPE, null: true, field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job' description: 'Name of the need'
end end
end end
end end
......
...@@ -9,7 +9,7 @@ class StageType < BaseObject ...@@ -9,7 +9,7 @@ class StageType < BaseObject
field :name, GraphQL::STRING_TYPE, null: true, field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the stage' description: 'Name of the stage'
field :groups, [Types::Ci::Config::GroupType], null: true, field :groups, Types::Ci::Config::GroupType.connection_type, null: true,
description: 'Groups of jobs for the stage' description: 'Groups of jobs for the stage'
end end
end end
......
...@@ -7,8 +7,8 @@ class StatusEnum < BaseEnum ...@@ -7,8 +7,8 @@ class StatusEnum < BaseEnum
graphql_name 'CiConfigStatus' graphql_name 'CiConfigStatus'
description 'Values for YAML processor result' description 'Values for YAML processor result'
value 'VALID', 'Valid `gitlab-ci.yml`' value 'VALID', 'Valid `gitlab-ci.yml`', value: :valid
value 'INVALID', 'Invalid `gitlab-ci.yml`' value 'INVALID', 'Invalid `gitlab-ci.yml`', value: :invalid
end end
end end
end end
......
...@@ -93,8 +93,9 @@ class QueryType < ::Types::BaseObject ...@@ -93,8 +93,9 @@ class QueryType < ::Types::BaseObject
resolver: Resolvers::Ci::RunnerSetupResolver resolver: Resolvers::Ci::RunnerSetupResolver
field :ci_config, Types::Ci::Config::ConfigType, null: true, field :ci_config, Types::Ci::Config::ConfigType, null: true,
description: 'Contents of gitlab-ci.yml file', description: 'Get linted and processed contents of a CI config',
resolver: Resolvers::Ci::ConfigResolver resolver: Resolvers::Ci::ConfigResolver,
complexity: 220
def design_management def design_management
DesignManagementObject.new(nil) DesignManagementObject.new(nil)
......
--- ---
title: 'GraphQL: Adds ciConfig field and corresponding response fields' title: 'Expose GraphQL resolver for processing CI config'
merge_request: 46912 merge_request: 46912
author: author:
type: added type: added
...@@ -2241,7 +2241,7 @@ type CiConfig { ...@@ -2241,7 +2241,7 @@ type CiConfig {
""" """
Linting errors Linting errors
""" """
errors: String errors: [String!]
""" """
Merged CI config YAML Merged CI config YAML
...@@ -2251,7 +2251,27 @@ type CiConfig { ...@@ -2251,7 +2251,27 @@ type CiConfig {
""" """
Stages of the pipeline Stages of the pipeline
""" """
stages: [CiConfigStage!] stages(
"""
Returns the elements in the list that come after the specified cursor.
"""
after: String
"""
Returns the elements in the list that come before the specified cursor.
"""
before: String
"""
Returns the first _n_ elements from the list.
"""
first: Int
"""
Returns the last _n_ elements from the list.
"""
last: Int
): CiConfigStageConnection
""" """
Status of linting, can be either valid or invalid Status of linting, can be either valid or invalid
...@@ -2263,7 +2283,27 @@ type CiConfigGroup { ...@@ -2263,7 +2283,27 @@ type CiConfigGroup {
""" """
Jobs in group Jobs in group
""" """
jobs: [CiConfigJob!] jobs(
"""
Returns the elements in the list that come after the specified cursor.
"""
after: String
"""
Returns the elements in the list that come before the specified cursor.
"""
before: String
"""
Returns the first _n_ elements from the list.
"""
first: Int
"""
Returns the last _n_ elements from the list.
"""
last: Int
): CiConfigJobConnection
""" """
Name of the job group Name of the job group
...@@ -2276,7 +2316,47 @@ type CiConfigGroup { ...@@ -2276,7 +2316,47 @@ type CiConfigGroup {
size: Int size: Int
} }
"""
The connection type for CiConfigGroup.
"""
type CiConfigGroupConnection {
"""
A list of edges.
"""
edges: [CiConfigGroupEdge]
"""
A list of nodes.
"""
nodes: [CiConfigGroup]
"""
Information to aid in pagination.
"""
pageInfo: PageInfo!
}
"""
An edge in a connection.
"""
type CiConfigGroupEdge {
"""
A cursor for use in pagination.
"""
cursor: String!
"""
The item at the end of the edge.
"""
node: CiConfigGroup
}
type CiConfigJob { type CiConfigJob {
"""
Name of the job group
"""
groupName: String
""" """
Name of the job Name of the job
""" """
...@@ -2285,21 +2365,136 @@ type CiConfigJob { ...@@ -2285,21 +2365,136 @@ type CiConfigJob {
""" """
Builds that must complete before the jobs run Builds that must complete before the jobs run
""" """
needs: [CiConfigNeed!] needs(
"""
Returns the elements in the list that come after the specified cursor.
"""
after: String
"""
Returns the elements in the list that come before the specified cursor.
"""
before: String
"""
Returns the first _n_ elements from the list.
"""
first: Int
"""
Returns the last _n_ elements from the list.
"""
last: Int
): CiConfigNeedConnection
"""
Name of the job stage
"""
stage: String
}
"""
The connection type for CiConfigJob.
"""
type CiConfigJobConnection {
"""
A list of edges.
"""
edges: [CiConfigJobEdge]
"""
A list of nodes.
"""
nodes: [CiConfigJob]
"""
Information to aid in pagination.
"""
pageInfo: PageInfo!
}
"""
An edge in a connection.
"""
type CiConfigJobEdge {
"""
A cursor for use in pagination.
"""
cursor: String!
"""
The item at the end of the edge.
"""
node: CiConfigJob
} }
type CiConfigNeed { type CiConfigNeed {
""" """
Name of the job Name of the need
""" """
name: String name: String
} }
"""
The connection type for CiConfigNeed.
"""
type CiConfigNeedConnection {
"""
A list of edges.
"""
edges: [CiConfigNeedEdge]
"""
A list of nodes.
"""
nodes: [CiConfigNeed]
"""
Information to aid in pagination.
"""
pageInfo: PageInfo!
}
"""
An edge in a connection.
"""
type CiConfigNeedEdge {
"""
A cursor for use in pagination.
"""
cursor: String!
"""
The item at the end of the edge.
"""
node: CiConfigNeed
}
type CiConfigStage { type CiConfigStage {
""" """
Groups of jobs for the stage Groups of jobs for the stage
""" """
groups: [CiConfigGroup!] groups(
"""
Returns the elements in the list that come after the specified cursor.
"""
after: String
"""
Returns the elements in the list that come before the specified cursor.
"""
before: String
"""
Returns the first _n_ elements from the list.
"""
first: Int
"""
Returns the last _n_ elements from the list.
"""
last: Int
): CiConfigGroupConnection
""" """
Name of the stage Name of the stage
...@@ -2307,6 +2502,41 @@ type CiConfigStage { ...@@ -2307,6 +2502,41 @@ type CiConfigStage {
name: String name: String
} }
"""
The connection type for CiConfigStage.
"""
type CiConfigStageConnection {
"""
A list of edges.
"""
edges: [CiConfigStageEdge]
"""
A list of nodes.
"""
nodes: [CiConfigStage]
"""
Information to aid in pagination.
"""
pageInfo: PageInfo!
}
"""
An edge in a connection.
"""
type CiConfigStageEdge {
"""
A cursor for use in pagination.
"""
cursor: String!
"""
The item at the end of the edge.
"""
node: CiConfigStage
}
""" """
Values for YAML processor result Values for YAML processor result
""" """
...@@ -17746,18 +17976,13 @@ type PromoteToEpicPayload { ...@@ -17746,18 +17976,13 @@ type PromoteToEpicPayload {
type Query { type Query {
""" """
Contents of gitlab-ci.yml file Get linted and processed contents of a CI config
""" """
ciConfig( ciConfig(
""" """
Contents of .gitlab-ci.yml Contents of .gitlab-ci.yml
""" """
content: String! content: String!
"""
Whether or not to include merged CI yaml in the response
"""
includeMergedYaml: Boolean
): CiConfig ): CiConfig
""" """
......
This diff is collapsed.
...@@ -371,16 +371,16 @@ Represents the total number of issues and their weights for a particular day. ...@@ -371,16 +371,16 @@ Represents the total number of issues and their weights for a particular day.
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `errors` | String | Linting errors | | `errors` | String! => Array | Linting errors |
| `mergedYaml` | String | Merged CI config YAML | | `mergedYaml` | String | Merged CI config YAML |
| `stages` | CiConfigStage! => Array | Stages of the pipeline | | `stages` | CiConfigStageConnection | Stages of the pipeline |
| `status` | CiConfigStatus | Status of linting, can be either valid or invalid | | `status` | CiConfigStatus | Status of linting, can be either valid or invalid |
### CiConfigGroup ### CiConfigGroup
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `jobs` | CiConfigJob! => Array | Jobs in group | | `jobs` | CiConfigJobConnection | Jobs in group |
| `name` | String | Name of the job group | | `name` | String | Name of the job group |
| `size` | Int | Size of the job group | | `size` | Int | Size of the job group |
...@@ -388,20 +388,22 @@ Represents the total number of issues and their weights for a particular day. ...@@ -388,20 +388,22 @@ Represents the total number of issues and their weights for a particular day.
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `groupName` | String | Name of the job group |
| `name` | String | Name of the job | | `name` | String | Name of the job |
| `needs` | CiConfigNeed! => Array | Builds that must complete before the jobs run | | `needs` | CiConfigNeedConnection | Builds that must complete before the jobs run |
| `stage` | String | Name of the job stage |
### CiConfigNeed ### CiConfigNeed
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `name` | String | Name of the job | | `name` | String | Name of the need |
### CiConfigStage ### CiConfigStage
| Field | Type | Description | | Field | Type | Description |
| ----- | ---- | ----------- | | ----- | ---- | ----------- |
| `groups` | CiConfigGroup! => Array | Groups of jobs for the stage | | `groups` | CiConfigGroupConnection | Groups of jobs for the stage |
| `name` | String | Name of the stage | | `name` | String | Name of the stage |
### CiGroup ### CiGroup
......
...@@ -6,49 +6,49 @@ ...@@ -6,49 +6,49 @@
include GraphqlHelpers include GraphqlHelpers
describe '#resolve' do describe '#resolve' do
before do
yaml_processor_double = instance_double(::Gitlab::Ci::YamlProcessor)
allow(yaml_processor_double).to receive(:execute).and_return(fake_result)
allow(::Gitlab::Ci::YamlProcessor).to receive(:new).and_return(yaml_processor_double)
end
context 'with a valid .gitlab-ci.yml' do context 'with a valid .gitlab-ci.yml' do
let(:fake_result) do
::Gitlab::Ci::YamlProcessor::Result.new(
ci_config: ::Gitlab::Ci::Config.new(content),
errors: [],
warnings: []
)
end
let_it_be(:content) do let_it_be(:content) do
File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml')) File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml'))
end end
it 'lints the ci config file' do it 'lints the ci config file' do
response = resolve(described_class, args: { content: content, include_merged_yaml: false }, ctx: {}) response = resolve(described_class, args: { content: content }, ctx: {})
expect(response[:status]).to eq('valid') expect(response[:status]).to eq(:valid)
expect(response[:errors]).to be_empty expect(response[:errors]).to be_empty
end end
end
it 'returns the correct structure' do context 'with an invalid .gitlab-ci.yml' do
response = resolve(described_class, args: { content: content, include_merged_yaml: false }, ctx: {}) let(:content) { 'invalid' }
response_groups = response[:stages].map { |stage| stage[:groups] }.flatten
response_jobs = response.dig(:stages, 0, :groups, 0, :jobs)
response_needs = response.dig(:stages, -1, :groups, 0, :jobs, 0, :needs)
expect(response[:stages]).to include(
hash_including(name: 'build'), hash_including(name: 'test')
)
expect(response_groups).to include(
hash_including(name: 'rspec', size: 2),
hash_including(name: 'spinach', size: 1),
hash_including(name: 'docker', size: 1)
)
expect(response_jobs).to include( let(:fake_result) do
hash_including(group_name: 'rspec', name: :'rspec 0 1', needs: [], stage: 'build'), Gitlab::Ci::YamlProcessor::Result.new(
hash_including(group_name: 'rspec', name: :'rspec 0 2', needs: [], stage: 'build') ci_config: nil,
) errors: ['Invalid configuration format'],
expect(response_needs).to include( warnings: []
hash_including(name: 'rspec 0 1'), hash_including(name: 'spinach')
) )
end end
end
context 'with an invalid .gitlab-ci.yml' do
it 'responds with errors about invalid syntax' do it 'responds with errors about invalid syntax' do
response = resolve(described_class, args: { content: 'invalid', include_merged_yaml: false }, ctx: {}) response = resolve(described_class, args: { content: content }, ctx: {})
expect(response[:status]).to eq('invalid') expect(response[:status]).to eq(:invalid)
expect(response[:errors]).to eq(['Invalid configuration format']) expect(response[:errors]).to eq(['Invalid configuration format'])
end end
end end
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
it 'exposes the expected fields' do it 'exposes the expected fields' do
expected_fields = %i[ expected_fields = %i[
name name
group_name
stage
needs needs
] ]
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Query.ciConfig' do
include GraphqlHelpers
subject(:post_graphql_query) { post_graphql(query, current_user: user) }
let(:user) { create(:user) }
let_it_be(:content) do
File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml'))
end
let(:query) do
%(
query {
ciConfig(content: "#{content}") {
status
errors
stages {
nodes {
name
groups {
nodes {
name
size
jobs {
nodes {
name
groupName
stage
needs {
nodes {
name
}
}
}
}
}
}
}
}
}
}
)
end
before do
post_graphql_query
end
it_behaves_like 'a working graphql query'
it 'returns the correct structure' do
response_stages = graphql_data['ciConfig']['stages']['nodes']
response_groups = response_stages.map { |stage| stage['groups']['nodes'] }.flatten
response_jobs = response_groups.first['jobs']['nodes']
response_needs = response_groups.last['jobs']['nodes'].first['needs']['nodes']
expect(graphql_data['ciConfig']['status']).to eq('VALID')
expect(response_stages).to include(
hash_including('name' => 'build'), hash_including('name' => 'test')
)
expect(response_groups).to include(
hash_including('name' => 'rspec', 'size' => 2),
hash_including('name' => 'spinach', 'size' => 1),
hash_including('name' => 'docker', 'size' => 1)
)
expect(response_jobs).to include(
hash_including('groupName' => 'rspec', 'name' => 'rspec 0 1', 'needs' => { 'nodes' => [] }, 'stage' => 'build'),
hash_including('groupName' => 'rspec', 'name' => 'rspec 0 2', 'needs' => { 'nodes' => [] }, 'stage' => 'build')
)
expect(response_needs).to include(
hash_including('name' => 'rspec 0 1'), hash_including('name' => 'spinach')
)
end
end
rspec 0 1: rspec 0 1:
stage: build stage: build
script: "rake spec" script: 'rake spec'
needs: [] needs: []
rspec 0 2: rspec 0 2:
stage: build stage: build
script: "rake spec" script: 'rake spec'
needs: [] needs: []
spinach: spinach:
stage: build stage: build
script: "rake spinach" script: 'rake spinach'
needs: [] needs: []
docker: docker:
stage: test stage: test
script: "curl http://dockerhub/URL" script: 'curl http://dockerhub/URL'
needs: [spinach, rspec 0 1] needs: [spinach, rspec 0 1]
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment