Commit 4a285e4d authored by Matija Čupić's avatar Matija Čupić

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 ca5a53f5
......@@ -9,71 +9,51 @@ module Resolvers
required: true,
description: 'Contents of .gitlab-ci.yml'
argument :include_merged_yaml, GraphQL::BOOLEAN_TYPE,
required: false,
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?
stages = stages(result.stages)
jobs = jobs(result.jobs)
groups = groups(jobs)
stages = stage_groups(stages, groups)
response = {
status: 'valid',
errors: [],
stages: stages.select { |stage| !stage[:groups].empty? }
def resolve(content:)
result = ::Gitlab::Ci::YamlProcessor.new(content).execute
response = if result.errors.empty?
{
status: :valid,
errors: [],
stages: make_stages(result.jobs)
}
else
response = {
status: 'invalid',
errors: [result.errors.first]
else
{
status: :invalid,
errors: result.errors
}
end
end
response.tap do |response|
response[:merged_yaml] = result.merged_yaml if include_merged_yaml
end
response.merge(merged_yaml: result.merged_yaml)
end
private
def stages(config_stages)
config_stages.map { |stage| { name: stage, groups: [] } }
end
def jobs(config_jobs)
def make_jobs(config_jobs)
config_jobs.map do |job_name, job|
{
name: job_name,
stage: job[:stage],
group_name: CommitStatus.new(name: job_name).group_name,
needs: needs(job) || []
needs: job.dig(:needs, :job) || []
}
end
end
def needs(job)
job.dig(:needs, :job)&.map do |job_need|
{ name: job_need[:name], artifacts: job_need[:artifacts] }
end
end
def make_groups(job_data)
jobs = make_jobs(job_data)
def groups(jobs)
group_names = jobs.map { |job| job[:group_name] }.uniq
group_names.map do |group|
group_jobs = jobs.select { |job| job[:group_name] == group }
{ jobs: group_jobs, name: group, stage: group_jobs.first[:stage], size: group_jobs.count }
jobs_by_group = jobs.group_by { |job| job[:group_name] }
jobs_by_group.map do |name, jobs|
{ jobs: jobs, name: name, stage: jobs.first[:stage], size: jobs.size }
end
end
def stage_groups(stage_data, groups)
stage_data.each do |stage|
stage[:groups] = groups.select { |group| group[:stage] == stage[:name] }
end
def make_stages(jobs)
make_groups(jobs)
.group_by { |group| group[:stage] }
.map { |name, groups| { name: name, groups: groups } }
end
end
end
......
......@@ -7,7 +7,7 @@ module Types
class ConfigType < BaseObject
graphql_name 'CiConfig'
field :errors, GraphQL::STRING_TYPE, null: true,
field :errors, [GraphQL::STRING_TYPE], null: true,
description: 'Linting errors'
field :merged_yaml, GraphQL::STRING_TYPE, null: true,
description: 'Merged CI config YAML'
......
......@@ -9,6 +9,10 @@ module Types
field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job'
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], null: true,
description: 'Builds that must complete before the jobs run'
end
......
......@@ -8,7 +8,7 @@ module Types
graphql_name 'CiConfigNeed'
field :name, GraphQL::STRING_TYPE, null: true,
description: 'Name of the job'
description: 'Name of the need'
end
end
end
......
......@@ -7,8 +7,8 @@ module Types
graphql_name 'CiConfigStatus'
description 'Values for YAML processor result'
value 'VALID', 'Valid `gitlab-ci.yml`'
value 'INVALID', 'Invalid `gitlab-ci.yml`'
value 'VALID', 'The configuration file is valid', value: :valid
value 'INVALID', 'The configuration file is not valid', value: :invalid
end
end
end
......
......@@ -92,8 +92,9 @@ module Types
resolver: Resolvers::Ci::RunnerSetupResolver
field :ci_config, Types::Ci::Config::ConfigType, null: true,
description: 'Contents of gitlab-ci.yml file',
resolver: Resolvers::Ci::ConfigResolver
description: 'Get linted and processed contents of a CI config. Should not be requested more than once per request.',
resolver: Resolvers::Ci::ConfigResolver,
complexity: 126 # AUTHENTICATED_COMPLEXITY / 2 + 1
def design_management
DesignManagementObject.new(nil)
......
---
title: 'GraphQL: Adds ciConfig field and corresponding response fields'
title: 'Expose GraphQL resolver for processing CI config'
merge_request: 46912
author:
type: added
......@@ -2241,7 +2241,7 @@ type CiConfig {
"""
Linting errors
"""
errors: String
errors: [String!]
"""
Merged CI config YAML
......@@ -2277,6 +2277,11 @@ type CiConfigGroup {
}
type CiConfigJob {
"""
Name of the job group
"""
groupName: String
"""
Name of the job
"""
......@@ -2286,11 +2291,16 @@ type CiConfigJob {
Builds that must complete before the jobs run
"""
needs: [CiConfigNeed!]
"""
Name of the job stage
"""
stage: String
}
type CiConfigNeed {
"""
Name of the job
Name of the need
"""
name: String
}
......@@ -2312,12 +2322,12 @@ Values for YAML processor result
"""
enum CiConfigStatus {
"""
Invalid `gitlab-ci.yml`
The configuration file is not valid
"""
INVALID
"""
Valid `gitlab-ci.yml`
The configuration file is valid
"""
VALID
}
......@@ -17935,18 +17945,13 @@ type PromoteToEpicPayload {
type Query {
"""
Contents of gitlab-ci.yml file
Get linted and processed contents of a CI config. Should not be requested more than once per request.
"""
ciConfig(
"""
Contents of .gitlab-ci.yml
"""
content: String!
"""
Whether or not to include merged CI yaml in the response
"""
includeMergedYaml: Boolean
): CiConfig
"""
......
......@@ -6012,9 +6012,17 @@
],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}
},
"isDeprecated": false,
"deprecationReason": null
......@@ -6145,6 +6153,20 @@
"name": "CiConfigJob",
"description": null,
"fields": [
{
"name": "groupName",
"description": "Name of the job group",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "name",
"description": "Name of the job",
......@@ -6180,6 +6202,20 @@
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "stage",
"description": "Name of the job stage",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
......@@ -6196,7 +6232,7 @@
"fields": [
{
"name": "name",
"description": "Name of the job",
"description": "Name of the need",
"args": [
],
......@@ -6275,13 +6311,13 @@
"enumValues": [
{
"name": "VALID",
"description": "Valid `gitlab-ci.yml`",
"description": "The configuration file is valid",
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "INVALID",
"description": "Invalid `gitlab-ci.yml`",
"description": "The configuration file is not valid",
"isDeprecated": false,
"deprecationReason": null
}
......@@ -52431,7 +52467,7 @@
"fields": [
{
"name": "ciConfig",
"description": "Contents of gitlab-ci.yml file",
"description": "Get linted and processed contents of a CI config. Should not be requested more than once per request.",
"args": [
{
"name": "content",
......@@ -52446,16 +52482,6 @@
}
},
"defaultValue": null
},
{
"name": "includeMergedYaml",
"description": "Whether or not to include merged CI yaml in the response",
"type": {
"kind": "SCALAR",
"name": "Boolean",
"ofType": null
},
"defaultValue": null
}
],
"type": {
......@@ -371,7 +371,7 @@ Represents the total number of issues and their weights for a particular day.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `errors` | String | Linting errors |
| `errors` | String! => Array | Linting errors |
| `mergedYaml` | String | Merged CI config YAML |
| `stages` | CiConfigStage! => Array | Stages of the pipeline |
| `status` | CiConfigStatus | Status of linting, can be either valid or invalid |
......@@ -388,14 +388,16 @@ Represents the total number of issues and their weights for a particular day.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `groupName` | String | Name of the job group |
| `name` | String | Name of the job |
| `needs` | CiConfigNeed! => Array | Builds that must complete before the jobs run |
| `stage` | String | Name of the job stage |
### CiConfigNeed
| Field | Type | Description |
| ----- | ---- | ----------- |
| `name` | String | Name of the job |
| `name` | String | Name of the need |
### CiConfigStage
......@@ -3958,8 +3960,8 @@ Values for YAML processor result.
| Value | Description |
| ----- | ----------- |
| `INVALID` | Invalid `gitlab-ci.yml` |
| `VALID` | Valid `gitlab-ci.yml` |
| `INVALID` | The configuration file is not valid |
| `VALID` | The configuration file is valid |
### CommitActionMode
......
......@@ -6,49 +6,49 @@ RSpec.describe Resolvers::Ci::ConfigResolver do
include GraphqlHelpers
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
let(:fake_result) do
::Gitlab::Ci::YamlProcessor::Result.new(
ci_config: ::Gitlab::Ci::Config.new(content),
errors: [],
warnings: []
)
end
let_it_be(:content) do
File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci_includes.yml'))
end
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
end
end
it 'returns the correct structure' do
response = resolve(described_class, args: { content: content, include_merged_yaml: false }, ctx: {})
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)
)
context 'with an invalid .gitlab-ci.yml' do
let(:content) { 'invalid' }
expect(response_jobs).to include(
hash_including(group_name: 'rspec', name: :'rspec 0 1', needs: [], stage: 'build'),
hash_including(group_name: 'rspec', name: :'rspec 0 2', needs: [], stage: 'build')
)
expect(response_needs).to include(
hash_including(name: 'rspec 0 1'), hash_including(name: 'spinach')
let(:fake_result) do
Gitlab::Ci::YamlProcessor::Result.new(
ci_config: nil,
errors: ['Invalid configuration format'],
warnings: []
)
end
end
context 'with an invalid .gitlab-ci.yml' 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'])
end
end
......
......@@ -8,6 +8,8 @@ RSpec.describe Types::Ci::Config::JobType do
it 'exposes the expected fields' do
expected_fields = %i[
name
group_name
stage
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 {
name
groups {
name
size
jobs {
name
groupName
stage
needs {
name
}
}
}
}
}
}
)
end
before do
post_graphql_query
end
it_behaves_like 'a working graphql query'
it 'returns the correct structure' do
expect(graphql_data['ciConfig']).to eq(
"status" => "VALID",
"errors" => [],
"stages" =>
[
{
"name" => "build",
"groups" =>
[
{
"name" => "rspec",
"size" => 2,
"jobs" =>
[
{ "name" => "rspec 0 1", "groupName" => "rspec", "stage" => "build", "needs" => [] },
{ "name" => "rspec 0 2", "groupName" => "rspec", "stage" => "build", "needs" => [] }
]
},
{
"name" => "spinach", "size" => 1, "jobs" =>
[
{ "name" => "spinach", "groupName" => "spinach", "stage" => "build", "needs" => [] }
]
}
]
},
{
"name" => "test",
"groups" =>
[
{
"name" => "docker",
"size" => 1,
"jobs" => [
{ "name" => "docker", "groupName" => "docker", "stage" => "test", "needs" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] }
]
}
]
}
]
)
end
end
rspec 0 1:
stage: build
script: "rake spec"
script: 'rake spec'
needs: []
rspec 0 2:
stage: build
script: "rake spec"
script: 'rake spec'
needs: []
spinach:
stage: build
script: "rake spinach"
script: 'rake spinach'
needs: []
docker:
stage: test
script: "curl http://dockerhub/URL"
script: 'curl http://dockerhub/URL'
needs: [spinach, rspec 0 1]
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