init

7 files changed, 115 insertions, 13 deletions

diff --git a/spec/factories/ci/pipeline_variable_variables.rb b/spec/factories/ci/pipeline_variable_variables.rb
new file mode 100644
index 00000000000..7c1a7faec08
--- /dev/null
+++ b/spec/factories/ci/pipeline_variable_variables.rb
@@ -0,0 +1,8 @@
+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
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 977174a5fd2..6a41afe0c25 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -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
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index a18da3768d5..86afa856ea7 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -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) }
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 9461905c787..406608256e0 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -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) }
@@ -734,8 +735,6 @@ describe Ci::Pipeline do
 
     context 'on failure and build retry' do
       before do
-        stub_not_protect_default_branch
-
         build.drop
         project.add_developer(user)
 
@@ -1001,8 +1000,6 @@ describe Ci::Pipeline do
     let(:latest_status) { pipeline.statuses.latest.pluck(:status) }
 
     before do
-      stub_not_protect_default_branch
-
       project.add_developer(user)
     end
 
diff --git a/spec/models/ci/pipeline_variable_spec.rb b/spec/models/ci/pipeline_variable_spec.rb
new file mode 100644
index 00000000000..2ce78e34b0c
--- /dev/null
+++ b/spec/models/ci/pipeline_variable_spec.rb
@@ -0,0 +1,8 @@
+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
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index c2636b6614e..0b42a603885 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -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,19 @@ 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(Ci::Pipeline.last.variables.first.key).to eq(variables.keys.first)
+          expect(Ci::Pipeline.last.variables.first.value).to eq(variables.values.first)
         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' }
diff --git a/spec/services/ci/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb
new file mode 100644
index 00000000000..914ec4844d0
--- /dev/null
+++ b/spec/services/ci/pipeline_trigger_service_spec.rb
@@ -0,0 +1,79 @@
+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) }
+
+    before do
+      project.add_developer(user)
+    end
+
+    let(:result) { described_class.new(project, user, params).execute }
+    let(:trigger) { create(:ci_trigger, project: project, owner: user) }
+
+    context 'when params have an existsed trigger token' do
+      let(:token) { trigger.token }
+
+      context 'when params have an existsed ref' do
+        let(:ref) { 'master' }
+
+        it 'triggers a pipeline' do
+          expect { result }.to change { Ci::Pipeline.count }.by(1)
+          expect(result[:pipeline].ref).to eq(ref)
+          expect(result[:pipeline].project).to eq(project)
+          expect(result[:pipeline].user).to eq(trigger.owner)
+          expect(result[:status]).to eq(:success)
+        end
+
+        context 'when params have a variable' do
+          let(:variables) { { 'AAA' => 'AAA123' } }
+
+          it 'has a variable' do
+            expect { result }.to change { Ci::PipelineVariable.count }.by(1)
+            expect(result[:pipeline].variables.first.key).to eq(variables.keys.first)
+            expect(result[:pipeline].variables.first.value).to eq(variables.values.first)
+          end
+        end
+
+        context 'when params have two variables and keys are duplicated' do
+          let(:variables) { [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }] }
+
+          it 'returns error' 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 ref' do
+        let(:ref) { 'invalid-ref' }
+
+        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(:token) { 'invalid-token' }
+
+      it 'does not trigger a pipeline' do
+        expect { result }.not_to change { Ci::Pipeline.count }
+        expect(result).to be_nil
+      end
+    end
+  end
+
+  def params
+    { token: defined?(token) ? token : nil,
+      ref: defined?(ref) ? ref : nil,
+      variables: defined?(variables) ? variables : nil }
+  end
+end