From fc24f6824b8cbc75b3fda7e7a7b91f83fcf7e161 Mon Sep 17 00:00:00 2001
From: Furkan Ayhan <furkanayhn@gmail.com>
Date: Fri, 16 Oct 2020 08:45:47 +0000
Subject: [PATCH] Implement prefilled variables backend for run pipeline
This is just a backend work for the feature.
This introduces a new endpoint to provide defined variables
in CI config file.
---
.../projects/pipelines_controller.rb | 12 +-
app/models/project.rb | 4 +
.../ci/list_config_variables_service.rb | 13 ++
config/routes/pipelines.rb | 1 +
lib/gitlab/ci/config.rb | 4 +
lib/gitlab/ci/config/entry/variables.rb | 22 ++-
lib/gitlab/ci/yaml_processor/result.rb | 4 +
.../config/entry/legacy_validation_helpers.rb | 14 ++
lib/gitlab/config/entry/validators.rb | 8 ++
.../projects/pipelines_controller_spec.rb | 80 +++++++++++
.../gitlab/ci/config/entry/variables_spec.rb | 127 +++++++++++++-----
spec/lib/gitlab/ci/yaml_processor_spec.rb | 4 +-
.../ci/list_config_variables_service_spec.rb | 77 +++++++++++
13 files changed, 326 insertions(+), 44 deletions(-)
create mode 100644 app/services/ci/list_config_variables_service.rb
create mode 100644 spec/services/ci/list_config_variables_service_spec.rb
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 954b55a821a0..39dd7a9899d9 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -5,11 +5,11 @@ class Projects::PipelinesController < Projects::ApplicationController
include Analytics::UniqueVisitsHelper
before_action :whitelist_query_limiting, only: [:create, :retry]
- before_action :pipeline, except: [:index, :new, :create, :charts]
+ before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables]
before_action :set_pipeline_path, only: [:show]
before_action :authorize_read_pipeline!
before_action :authorize_read_build!, only: [:index]
- before_action :authorize_create_pipeline!, only: [:new, :create]
+ before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables]
before_action :authorize_update_pipeline!, only: [:retry, :cancel]
before_action do
push_frontend_feature_flag(:filter_pipelines_search, project, default_enabled: true)
@@ -209,6 +209,14 @@ def test_report
end
end
+ def config_variables
+ respond_to do |format|
+ format.json do
+ render json: Ci::ListConfigVariablesService.new(@project).execute(params[:sha])
+ end
+ end
+ end
+
private
def serialize_pipelines
diff --git a/app/models/project.rb b/app/models/project.rb
index d7f5254a6e3a..e98bfc2fd210 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -2514,6 +2514,10 @@ def ci_config_path_or_default
ci_config_path.presence || Ci::Pipeline::DEFAULT_CONFIG_PATH
end
+ def ci_config_for(sha)
+ repository.gitlab_ci_yml_for(sha, ci_config_path_or_default)
+ end
+
def enabled_group_deploy_keys
return GroupDeployKey.none unless group
diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb
new file mode 100644
index 000000000000..b5dc192b5125
--- /dev/null
+++ b/app/services/ci/list_config_variables_service.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module Ci
+ class ListConfigVariablesService < ::BaseService
+ def execute(sha)
+ config = project.ci_config_for(sha)
+ return {} unless config
+
+ result = Gitlab::Ci::YamlProcessor.new(config).execute
+ result.valid? ? result.variables_with_data : {}
+ end
+ end
+end
diff --git a/config/routes/pipelines.rb b/config/routes/pipelines.rb
index 605e82af23a4..0fc308b5e650 100644
--- a/config/routes/pipelines.rb
+++ b/config/routes/pipelines.rb
@@ -7,6 +7,7 @@
scope '(*ref)', constraints: { ref: Gitlab::PathRegex.git_reference_regex } do
get :latest, action: :show, defaults: { latest: true }
end
+ get :config_variables
end
member do
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index 9d269831679d..071a8ef830f3 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -54,6 +54,10 @@ def variables
root.variables_value
end
+ def variables_with_data
+ root.variables_entry.value_with_data
+ end
+
def stages
root.stages_value
end
diff --git a/lib/gitlab/ci/config/entry/variables.rb b/lib/gitlab/ci/config/entry/variables.rb
index c9d0c7cb5689..e258f7128fc2 100644
--- a/lib/gitlab/ci/config/entry/variables.rb
+++ b/lib/gitlab/ci/config/entry/variables.rb
@@ -10,16 +10,32 @@ module Entry
class Variables < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
+ ALLOWED_VALUE_DATA = %i[value description].freeze
+
validations do
- validates :config, variables: true
+ validates :config, variables: { allowed_value_data: ALLOWED_VALUE_DATA }
+ end
+
+ def value
+ Hash[@config.map { |key, value| [key.to_s, expand_value(value)[:value]] }]
end
def self.default(**)
{}
end
- def value
- Hash[@config.map { |key, value| [key.to_s, value.to_s] }]
+ def value_with_data
+ Hash[@config.map { |key, value| [key.to_s, expand_value(value)] }]
+ end
+
+ private
+
+ def expand_value(value)
+ if value.is_a?(Hash)
+ { value: value[:value].to_s, description: value[:description] }
+ else
+ { value: value.to_s, description: nil }
+ end
end
end
end
diff --git a/lib/gitlab/ci/yaml_processor/result.rb b/lib/gitlab/ci/yaml_processor/result.rb
index 6c771b220ad7..52a00e41214f 100644
--- a/lib/gitlab/ci/yaml_processor/result.rb
+++ b/lib/gitlab/ci/yaml_processor/result.rb
@@ -99,6 +99,10 @@ def merged_yaml
@ci_config&.to_hash&.to_yaml
end
+ def variables_with_data
+ @ci_config.variables_with_data
+ end
+
private
def variables
diff --git a/lib/gitlab/config/entry/legacy_validation_helpers.rb b/lib/gitlab/config/entry/legacy_validation_helpers.rb
index 415f6f77214c..be7d26fed4e1 100644
--- a/lib/gitlab/config/entry/legacy_validation_helpers.rb
+++ b/lib/gitlab/config/entry/legacy_validation_helpers.rb
@@ -50,6 +50,12 @@ def validate_array_value_variables(variables)
variables.values.flatten(1).all?(&method(:validate_alphanumeric))
end
+ def validate_string_or_hash_value_variables(variables, allowed_value_data)
+ variables.is_a?(Hash) &&
+ variables.keys.all?(&method(:validate_alphanumeric)) &&
+ variables.values.all? { |value| validate_string_or_hash_value_variable(value, allowed_value_data) }
+ end
+
def validate_alphanumeric(value)
validate_string(value) || validate_integer(value)
end
@@ -62,6 +68,14 @@ def validate_string(value)
value.is_a?(String) || value.is_a?(Symbol)
end
+ def validate_string_or_hash_value_variable(value, allowed_value_data)
+ if value.is_a?(Hash)
+ (value.keys - allowed_value_data).empty? && value.values.all?(&method(:validate_alphanumeric))
+ else
+ validate_alphanumeric(value)
+ end
+ end
+
def validate_regexp(value)
Gitlab::UntrustedRegexp::RubySyntax.valid?(value)
end
diff --git a/lib/gitlab/config/entry/validators.rb b/lib/gitlab/config/entry/validators.rb
index a7ec98ace6e7..2a386657e0b1 100644
--- a/lib/gitlab/config/entry/validators.rb
+++ b/lib/gitlab/config/entry/validators.rb
@@ -274,6 +274,8 @@ class VariablesValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
if options[:array_values]
validate_key_array_values(record, attribute, value)
+ elsif options[:allowed_value_data]
+ validate_key_hash_values(record, attribute, value, options[:allowed_value_data])
else
validate_key_values(record, attribute, value)
end
@@ -290,6 +292,12 @@ def validate_key_array_values(record, attribute, value)
record.errors.add(attribute, 'should be a hash of key value pairs, value can be an array')
end
end
+
+ def validate_key_hash_values(record, attribute, value, allowed_value_data)
+ unless validate_string_or_hash_value_variables(value, allowed_value_data)
+ record.errors.add(attribute, 'should be a hash of key value pairs, value can be a hash')
+ end
+ end
end
class ExpressionValidator < ActiveModel::EachValidator
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index c3be7de25a8e..0720124ea57b 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -1148,4 +1148,84 @@ def delete_pipeline
}
end
end
+
+ describe 'GET config_variables.json' do
+ let(:result) { YAML.dump(ci_config) }
+
+ before do
+ stub_gitlab_ci_yml_for_sha(sha, result)
+ end
+
+ context 'when sending a valid sha' do
+ let(:sha) { 'master' }
+ let(:ci_config) do
+ {
+ variables: {
+ KEY1: { value: 'val 1', description: 'description 1' }
+ },
+ test: {
+ stage: 'test',
+ script: 'echo'
+ }
+ }
+ end
+
+ it 'returns variable list' do
+ get_config_variables
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response['KEY1']).to eq({ 'value' => 'val 1', 'description' => 'description 1' })
+ end
+ end
+
+ context 'when sending an invalid sha' do
+ let(:sha) { 'invalid-sha' }
+ let(:ci_config) { nil }
+
+ it 'returns empty json' do
+ get_config_variables
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to eq({})
+ end
+ end
+
+ context 'when sending an invalid config' do
+ let(:sha) { 'master' }
+ let(:ci_config) do
+ {
+ variables: {
+ KEY1: { value: 'val 1', description: 'description 1' }
+ },
+ test: {
+ stage: 'invalid',
+ script: 'echo'
+ }
+ }
+ end
+
+ it 'returns empty result' do
+ get_config_variables
+
+ expect(response).to have_gitlab_http_status(:ok)
+ expect(json_response).to eq({})
+ end
+ end
+
+ private
+
+ def stub_gitlab_ci_yml_for_sha(sha, result)
+ allow_any_instance_of(Repository)
+ .to receive(:gitlab_ci_yml_for)
+ .with(sha, '.gitlab-ci.yml')
+ .and_return(result)
+ end
+
+ def get_config_variables
+ get :config_variables, params: { namespace_id: project.namespace,
+ project_id: project,
+ sha: sha },
+ format: :json
+ end
+ end
end
diff --git a/spec/lib/gitlab/ci/config/entry/variables_spec.rb b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
index d6391092f637..ac33f858f43c 100644
--- a/spec/lib/gitlab/ci/config/entry/variables_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/variables_spec.rb
@@ -3,56 +3,109 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Config::Entry::Variables do
- let(:entry) { described_class.new(config) }
+ subject { described_class.new(config) }
- describe 'validations' do
- context 'when entry config value is correct' do
- let(:config) do
- { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+ shared_examples 'valid config' do
+ describe '#value' do
+ it 'returns hash with key value strings' do
+ expect(subject.value).to eq result
end
+ end
- describe '#value' do
- it 'returns hash with key value strings' do
- expect(entry.value).to eq config
- end
-
- context 'with numeric keys and values in the config' do
- let(:config) { { 10 => 20 } }
+ describe '#errors' do
+ it 'does not append errors' do
+ expect(subject.errors).to be_empty
+ end
+ end
- it 'converts numeric key and numeric value into strings' do
- expect(entry.value).to eq('10' => '20')
- end
- end
+ describe '#valid?' do
+ it 'is valid' do
+ expect(subject).to be_valid
end
+ end
+ end
- describe '#errors' do
- it 'does not append errors' do
- expect(entry.errors).to be_empty
- end
+ shared_examples 'invalid config' do
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(subject).not_to be_valid
end
+ end
- describe '#valid?' do
- it 'is valid' do
- expect(entry).to be_valid
- end
+ describe '#errors' do
+ it 'saves errors' do
+ expect(subject.errors)
+ .to include /should be a hash of key value pairs/
end
end
+ end
- context 'when entry value is not correct' do
- let(:config) { [:VAR, 'test'] }
+ context 'when entry config value has key-value pairs' do
+ let(:config) do
+ { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+ end
- describe '#errors' do
- it 'saves errors' do
- expect(entry.errors)
- .to include /should be a hash of key value pairs/
- end
- end
+ let(:result) do
+ { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+ end
- describe '#valid?' do
- it 'is not valid' do
- expect(entry).not_to be_valid
- end
- end
+ it_behaves_like 'valid config'
+ end
+
+ context 'with numeric keys and values in the config' do
+ let(:config) { { 10 => 20 } }
+ let(:result) do
+ { '10' => '20' }
+ end
+
+ it_behaves_like 'valid config'
+ end
+
+ context 'when entry config value has key-value pair and hash' do
+ let(:config) do
+ { 'VARIABLE_1' => { value: 'value 1', description: 'variable 1' },
+ 'VARIABLE_2' => 'value 2' }
+ end
+
+ let(:result) do
+ { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+ end
+
+ it_behaves_like 'valid config'
+ end
+
+ context 'when entry value is an array' do
+ let(:config) { [:VAR, 'test'] }
+
+ it_behaves_like 'invalid config'
+ end
+
+ context 'when entry value has hash with other key-pairs' do
+ let(:config) do
+ { 'VARIABLE_1' => { value: 'value 1', hello: 'variable 1' },
+ 'VARIABLE_2' => 'value 2' }
end
+
+ it_behaves_like 'invalid config'
+ end
+
+ context 'when entry config value has hash with nil description' do
+ let(:config) do
+ { 'VARIABLE_1' => { value: 'value 1', description: nil } }
+ end
+
+ it_behaves_like 'invalid config'
+ end
+
+ context 'when entry config value has hash without description' do
+ let(:config) do
+ { 'VARIABLE_1' => { value: 'value 1' } }
+ end
+
+ let(:result) do
+ { 'VARIABLE_1' => 'value 1' }
+ end
+
+ it_behaves_like 'valid config'
end
end
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 03579d0936c4..fb6395e888a2 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -2465,13 +2465,13 @@ module Ci
context 'returns errors if variables is not a map' do
let(:config) { YAML.dump({ variables: "test", rspec: { script: "test" } }) }
- it_behaves_like 'returns errors', 'variables config should be a hash of key value pairs'
+ it_behaves_like 'returns errors', 'variables config should be a hash of key value pairs, value can be a hash'
end
context 'returns errors if variables is not a map of key-value strings' do
let(:config) { YAML.dump({ variables: { test: false }, rspec: { script: "test" } }) }
- it_behaves_like 'returns errors', 'variables config should be a hash of key value pairs'
+ it_behaves_like 'returns errors', 'variables config should be a hash of key value pairs, value can be a hash'
end
context 'returns errors if job when is not on_success, on_failure or always' do
diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb
new file mode 100644
index 000000000000..5cc0481768bc
--- /dev/null
+++ b/spec/services/ci/list_config_variables_service_spec.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::ListConfigVariablesService do
+ let_it_be(:project) { create(:project, :repository) }
+ let(:service) { described_class.new(project) }
+ let(:result) { YAML.dump(ci_config) }
+
+ subject { service.execute(sha) }
+
+ before do
+ stub_gitlab_ci_yml_for_sha(sha, result)
+ end
+
+ context 'when sending a valid sha' do
+ let(:sha) { 'master' }
+ let(:ci_config) do
+ {
+ variables: {
+ KEY1: { value: 'val 1', description: 'description 1' },
+ KEY2: { value: 'val 2', description: '' },
+ KEY3: { value: 'val 3' },
+ KEY4: 'val 4'
+ },
+ test: {
+ stage: 'test',
+ script: 'echo'
+ }
+ }
+ end
+
+ it 'returns variable list' do
+ expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
+ expect(subject['KEY2']).to eq({ value: 'val 2', description: '' })
+ expect(subject['KEY3']).to eq({ value: 'val 3', description: nil })
+ expect(subject['KEY4']).to eq({ value: 'val 4', description: nil })
+ end
+ end
+
+ context 'when sending an invalid sha' do
+ let(:sha) { 'invalid-sha' }
+ let(:ci_config) { nil }
+
+ it 'returns empty json' do
+ expect(subject).to eq({})
+ end
+ end
+
+ context 'when sending an invalid config' do
+ let(:sha) { 'master' }
+ let(:ci_config) do
+ {
+ variables: {
+ KEY1: { value: 'val 1', description: 'description 1' }
+ },
+ test: {
+ stage: 'invalid',
+ script: 'echo'
+ }
+ }
+ end
+
+ it 'returns empty result' do
+ expect(subject).to eq({})
+ end
+ end
+
+ private
+
+ def stub_gitlab_ci_yml_for_sha(sha, result)
+ allow_any_instance_of(Repository)
+ .to receive(:gitlab_ci_yml_for)
+ .with(sha, '.gitlab-ci.yml')
+ .and_return(result)
+ end
+end
--
GitLab