Commit fa1e41ea authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'feature/sm/35282-ci_pipeline_variables-2' into 'master'

Follow-up from "Allow to use cross project pipelines" ver 2

Closes #35282, gitlab-ee#2604, and #35584

See merge request !13102
parents 8626cdc3 dca9bd9e
......@@ -219,6 +219,7 @@ module Ci
variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group
variables += secret_variables(environment: environment)
variables += trigger_request.user_variables if trigger_request
variables += pipeline.variables.map(&:to_runner_variable)
variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule
variables += persisted_environment_variables if environment
......
......@@ -15,6 +15,7 @@ module Ci
has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id
has_many :builds, foreign_key: :commit_id
has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
has_many :variables, class_name: 'Ci::PipelineVariable'
# Merge requests for which the current pipeline is running against
# the merge request's latest commit.
......
module Ci
class PipelineVariable < ActiveRecord::Base
extend Ci::Model
include HasVariable
belongs_to :pipeline
validates :key, uniqueness: { scope: :pipeline_id }
end
end
......@@ -21,14 +21,22 @@ module Ci
return result if result
Ci::Pipeline.transaction do
update_merge_requests_head_pipeline if pipeline.save
begin
Ci::Pipeline.transaction do
pipeline.save!
Ci::CreatePipelineStagesService
.new(project, current_user)
.execute(pipeline)
yield(pipeline) if block_given?
Ci::CreatePipelineStagesService
.new(project, current_user)
.execute(pipeline)
end
rescue ActiveRecord::RecordInvalid => e
return error("Failed to persist the pipeline: #{e}")
end
update_merge_requests_head_pipeline
cancel_pending_pipelines if project.auto_cancel_pending_pipelines?
pipeline_created_counter.increment(source: source)
......
# This class is deprecated because we're closing Ci::TriggerRequest.
# New class is PipelineTriggerService (app/services/ci/pipeline_trigger_service.rb)
# which is integrated with Ci::PipelineVariable instaed of Ci::TriggerRequest.
# We remove this class after we removed v1 and v3 API. This class is still being
# referred by such legacy code.
module Ci
module CreateTriggerRequestService
Result = Struct.new(:trigger_request, :pipeline)
......
module Ci
class PipelineTriggerService < BaseService
def execute
if trigger_from_token
create_pipeline_from_trigger(trigger_from_token)
end
end
private
def create_pipeline_from_trigger(trigger)
# this check is to not leak the presence of the project if user cannot read it
return unless trigger.project == project
pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref])
.execute(:trigger, ignore_skip_ci: true) do |pipeline|
trigger.trigger_requests.create!(pipeline: pipeline)
create_pipeline_variables!(pipeline)
end
if pipeline.persisted?
success(pipeline: pipeline)
else
error(pipeline.errors.messages, 400)
end
end
def trigger_from_token
return @trigger if defined?(@trigger)
@trigger = Ci::Trigger.find_by_token(params[:token].to_s)
end
def create_pipeline_variables!(pipeline)
return unless params[:variables]
variables = params[:variables].map do |key, value|
{ key: key, value: value }
end
pipeline.variables.create!(variables)
end
end
end
class CreateCiPipelineVariables < ActiveRecord::Migration
DOWNTIME = false
def up
create_table :ci_pipeline_variables do |t|
t.string :key, null: false
t.text :value
t.text :encrypted_value
t.string :encrypted_value_salt
t.string :encrypted_value_iv
t.integer :pipeline_id, null: false
end
add_index :ci_pipeline_variables, [:pipeline_id, :key], unique: true
end
def down
drop_table :ci_pipeline_variables
end
end
class AddForeignKeyToCiPipelineVariables < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_foreign_key(:ci_pipeline_variables, :ci_pipelines, column: :pipeline_id)
end
def down
remove_foreign_key(:ci_pipeline_variables, column: :pipeline_id)
end
end
......@@ -298,6 +298,17 @@ ActiveRecord::Schema.define(version: 20170725145659) do
add_index "ci_pipeline_schedules", ["next_run_at", "active"], name: "index_ci_pipeline_schedules_on_next_run_at_and_active", using: :btree
add_index "ci_pipeline_schedules", ["project_id"], name: "index_ci_pipeline_schedules_on_project_id", using: :btree
create_table "ci_pipeline_variables", force: :cascade do |t|
t.string "key", null: false
t.text "value"
t.text "encrypted_value"
t.string "encrypted_value_salt"
t.string "encrypted_value_iv"
t.integer "pipeline_id", null: false
end
add_index "ci_pipeline_variables", ["pipeline_id", "key"], name: "index_ci_pipeline_variables_on_pipeline_id_and_key", unique: true, using: :btree
create_table "ci_pipelines", force: :cascade do |t|
t.string "ref"
t.string "sha"
......@@ -1617,6 +1628,7 @@ ActiveRecord::Schema.define(version: 20170725145659) do
add_foreign_key "ci_group_variables", "namespaces", column: "group_id", name: "fk_33ae4d58d8", on_delete: :cascade
add_foreign_key "ci_pipeline_schedules", "projects", name: "fk_8ead60fcc4", on_delete: :cascade
add_foreign_key "ci_pipeline_schedules", "users", column: "owner_id", name: "fk_9ea99f58d2", on_delete: :nullify
add_foreign_key "ci_pipeline_variables", "ci_pipelines", column: "pipeline_id", name: "fk_f29c5f4380", on_delete: :cascade
add_foreign_key "ci_pipelines", "ci_pipeline_schedules", column: "pipeline_schedule_id", name: "fk_3d34ab2e06", on_delete: :nullify
add_foreign_key "ci_pipelines", "ci_pipelines", column: "auto_canceled_by_id", name: "fk_262d4c2d19", on_delete: :nullify
add_foreign_key "ci_pipelines", "projects", name: "fk_86635dbd80", on_delete: :cascade
......
......@@ -15,25 +15,22 @@ module API
optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
end
post ":id/(ref/:ref/)trigger/pipeline", requirements: { ref: /.+/ } do
project = find_project(params[:id])
trigger = Ci::Trigger.find_by_token(params[:token].to_s)
not_found! unless project && trigger
unauthorized! unless trigger.project == project
# validate variables
variables = params[:variables].to_h
unless variables.all? { |key, value| key.is_a?(String) && value.is_a?(String) }
params[:variables] = params[:variables].to_h
unless params[:variables].all? { |key, value| key.is_a?(String) && value.is_a?(String) }
render_api_error!('variables needs to be a map of key-valued strings', 400)
end
# create request and trigger builds
result = Ci::CreateTriggerRequestService.execute(project, trigger, params[:ref].to_s, variables)
pipeline = result.pipeline
project = find_project(params[:id])
not_found! unless project
result = Ci::PipelineTriggerService.new(project, nil, params).execute
not_found! unless result
if pipeline.persisted?
present pipeline, with: Entities::Pipeline
if result[:http_status]
render_api_error!(result[:message], result[:http_status])
else
render_validation_error!(pipeline)
present result[:pipeline], with: Entities::Pipeline
end
end
......
FactoryGirl.define do
factory :ci_pipeline_variable, class: Ci::PipelineVariable do
sequence(:key) { |n| "VARIABLE_#{n}" }
value 'VARIABLE_VALUE'
pipeline factory: :ci_empty_pipeline
end
end
......@@ -102,6 +102,7 @@ pipelines:
- statuses
- builds
- trigger_requests
- variables
- auto_canceled_by
- auto_canceled_pipelines
- auto_canceled_jobs
......@@ -112,6 +113,8 @@ pipelines:
- artifacts
- pipeline_schedule
- merge_requests
pipeline_variables:
- pipeline
stages:
- project
- pipeline
......
......@@ -1468,6 +1468,12 @@ describe Ci::Build do
it { is_expected.to include(predefined_trigger_variable) }
end
context 'when pipeline has a variable' do
let!(:pipeline_variable) { create(:ci_pipeline_variable, pipeline: pipeline) }
it { is_expected.to include(pipeline_variable.to_runner_variable) }
end
context 'when a job was triggered by a pipeline schedule' do
let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
......
......@@ -17,6 +17,7 @@ describe Ci::Pipeline do
it { is_expected.to have_many(:statuses) }
it { is_expected.to have_many(:trigger_requests) }
it { is_expected.to have_many(:variables) }
it { is_expected.to have_many(:builds) }
it { is_expected.to have_many(:auto_canceled_pipelines) }
it { is_expected.to have_many(:auto_canceled_jobs) }
......
require 'spec_helper'
describe Ci::PipelineVariable, models: true do
subject { build(:ci_pipeline_variable) }
it { is_expected.to include_module(HasVariable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:pipeline_id) }
end
......@@ -22,6 +22,7 @@ describe API::Triggers do
before do
stub_ci_pipeline_to_return_yaml_file
trigger.update(owner: user)
end
context 'Handles errors' do
......@@ -36,12 +37,6 @@ describe API::Triggers do
expect(response).to have_http_status(404)
end
it 'returns unauthorized if token is for different project' do
post api("/projects/#{project2.id}/trigger/pipeline"), options.merge(ref: 'master')
expect(response).to have_http_status(401)
end
end
context 'Have a commit' do
......@@ -61,8 +56,7 @@ describe API::Triggers do
post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'other-branch')
expect(response).to have_http_status(400)
expect(json_response['message']['base'])
.to contain_exactly('Reference not found')
expect(json_response['message']).to eq('base' => ["Reference not found"])
end
context 'Validates variables' do
......@@ -88,12 +82,18 @@ describe API::Triggers do
post api("/projects/#{project.id}/trigger/pipeline"), options.merge(variables: variables, ref: 'master')
expect(response).to have_http_status(201)
expect(pipeline.builds.reload.first.trigger_request.variables).to eq(variables)
expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables)
end
end
end
context 'when triggering a pipeline from a trigger token' do
it 'does not leak the presence of project when token is for different project' do
post api("/projects/#{project2.id}/ref/master/trigger/pipeline?token=#{trigger_token}"), { ref: 'refs/heads/other-branch' }
expect(response).to have_http_status(404)
end
it 'creates builds from the ref given in the URL, not in the body' do
expect do
post api("/projects/#{project.id}/ref/master/trigger/pipeline?token=#{trigger_token}"), { ref: 'refs/heads/other-branch' }
......
require 'spec_helper'
describe Ci::PipelineTriggerService, services: true do
let(:project) { create(:project, :repository) }
before do
stub_ci_pipeline_to_return_yaml_file
end
describe '#execute' do
let(:user) { create(:user) }
let(:trigger) { create(:ci_trigger, project: project, owner: user) }
let(:result) { described_class.new(project, user, params).execute }
before do
project.add_developer(user)
end
context 'when trigger belongs to a different project' do
let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
let(:trigger) { create(:ci_trigger, project: create(:empty_project), owner: user) }
it 'does nothing' do
expect { result }.not_to change { Ci::Pipeline.count }
end
end
context 'when params have an existsed trigger token' do
context 'when params have an existsed ref' do
let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
it 'triggers a pipeline' do
expect { result }.to change { Ci::Pipeline.count }.by(1)
expect(result[:pipeline].ref).to eq('master')
expect(result[:pipeline].project).to eq(project)
expect(result[:pipeline].user).to eq(trigger.owner)
expect(result[:status]).to eq(:success)
end
context 'when commit message has [ci skip]' do
before do
allow_any_instance_of(Ci::Pipeline).to receive(:git_commit_message) { '[ci skip]' }
end
it 'ignores [ci skip] and create as general' do
expect { result }.to change { Ci::Pipeline.count }.by(1)
expect(result[:status]).to eq(:success)
end
end
context 'when params have a variable' do
let(:params) { { token: trigger.token, ref: 'master', variables: variables } }
let(:variables) { { 'AAA' => 'AAA123' } }
it 'has a variable' do
expect { result }.to change { Ci::PipelineVariable.count }.by(1)
.and change { Ci::TriggerRequest.count }.by(1)
expect(result[:pipeline].variables.map { |v| { v.key => v.value } }.first).to eq(variables)
expect(result[:pipeline].trigger_requests.last.variables).to be_nil
end
end
end
context 'when params have a non-existsed ref' do
let(:params) { { token: trigger.token, ref: 'invalid-ref', variables: nil } }
it 'does not trigger a pipeline' do
expect { result }.not_to change { Ci::Pipeline.count }
expect(result[:http_status]).to eq(400)
end
end
end
context 'when params have a non-existsed trigger token' do
let(:params) { { token: 'invalid-token', ref: nil, variables: nil } }
it 'does not trigger a pipeline' do
expect { result }.not_to change { Ci::Pipeline.count }
expect(result).to be_nil
end
end
end
end
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