Commit 992e3497 authored by Shinya Maeda's avatar Shinya Maeda

Squashed commit of the following:

commit dca9bd9e
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Fri Jul 28 18:46:04 2017 +0900

    Expand pipeline_trigger_service_spec by godfat request

commit 2ea6915e
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Fri Jul 28 18:16:46 2017 +0900

    Fix revert miss

commit 276c44bf
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Fri Jul 28 00:38:26 2017 +0900

    Fix static snalysys

commit 8c4c310d
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Fri Jul 28 00:33:41 2017 +0900

    Revert "Move validate to begin/rescue block"

    This reverts commit 5ec3b63bfc6f341df040ae08be4858c7181bcacf.

commit b59abf5b
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Fri Jul 28 00:31:24 2017 +0900

    Use let(:params) instead of def param

commit 34d2b8e7
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Thu Jul 27 23:09:30 2017 +0900

    Use let(:pipeline) for variables spec in triggers_spec.rb

commit a2a7fc1f
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Thu Jul 27 22:47:18 2017 +0900

    Remove is_a?(Hash) from unless params[:variables] in create_pipeline_variables

commit 445f2135
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Thu Jul 27 22:44:53 2017 +0900

    Use bang for trigger.trigger_requests.create

commit b3b8b595
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Thu Jul 27 22:41:21 2017 +0900

    Remove if pipeline.variables in Ci::Build#variables

commit 48389e99
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Thu Jul 27 15:52:55 2017 +0900

    Fix pipeline

commit 19789154
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Wed Jul 26 22:58:31 2017 +0900

    Move validate to begin/rescue block

commit 1ad4efe6
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Wed Jul 26 18:44:49 2017 +0900

    fix merge miss

commit 56418e85
Author: Shinya Maeda <shinya@gitlab.com>
Date:   Wed Jul 26 18:31:09 2017 +0900

    init
parent c4499a01
......@@ -223,6 +223,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
......
......@@ -27,6 +27,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
......@@ -2,7 +2,7 @@ module Ci
class CreatePipelineService < BaseService
attr_reader :pipeline
def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, mirror_update: false, &block)
def execute(source, ignore_skip_ci: false, save_on_errors: true, trigger_request: nil, schedule: nil, mirror_update: false)
@pipeline = Ci::Pipeline.new(
source: source,
project: project,
......@@ -22,7 +22,27 @@ module Ci
return result if result
_create_pipeline(source, &block)
begin
Ci::Pipeline.transaction do
pipeline.save!
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)
pipeline.tap(&:process!)
end
private
......@@ -69,24 +89,6 @@ module Ci
end
end
def _create_pipeline(source)
Ci::Pipeline.transaction do
update_merge_requests_head_pipeline if pipeline.save
yield(pipeline) if block_given?
Ci::CreatePipelineStagesService
.new(project, current_user)
.execute(pipeline)
end
cancel_pending_pipelines if project.auto_cancel_pending_pipelines?
pipeline_created_counter.increment(source: source)
pipeline.tap(&:process!)
end
def allowed_to_trigger_pipeline?(triggering_user)
if triggering_user
allowed_to_create?(triggering_user)
......
# 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)
......
......@@ -14,9 +14,11 @@ module Ci
# this check is to not leak the presence of the project if user cannot read it
return unless trigger.project == project
trigger_request = trigger.trigger_requests.create(variables: params[:variables])
pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref])
.execute(:trigger, ignore_skip_ci: true, trigger_request: trigger_request)
.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)
......@@ -47,17 +49,26 @@ module Ci
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 job_from_token
return @job if defined?(@job)
@job = Ci::Build.find_by_token(params[:token].to_s)
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
......@@ -357,6 +357,17 @@ ActiveRecord::Schema.define(version: 20170726111039) 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"
......@@ -1907,6 +1918,7 @@ ActiveRecord::Schema.define(version: 20170726111039) 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
......
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
......@@ -106,6 +106,7 @@ pipelines:
- statuses
- builds
- trigger_requests
- variables
- auto_canceled_by
- auto_canceled_pipelines
- auto_canceled_jobs
......@@ -120,6 +121,8 @@ pipelines:
- sourced_pipelines
- triggered_by_pipeline
- triggered_pipelines
pipeline_variables:
- pipeline
stages:
- project
- pipeline
......
......@@ -1487,6 +1487,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
......@@ -55,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
......@@ -82,7 +82,7 @@ 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
......
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