Trigger child pipeline via YAML syntax

Define `trigger:include` to spawn a child
pipeline inside the same project.

changelogs/unreleased/add-trigger-include-in-ci-yaml-syntax.yml

+---
+title: Introduce "trigger:include" CI YAML syntax to create a sub-pipeline
+merge_request: 21041
+author:
+type: added

ee/app/models/ee/ci/bridge.rb

@@ -98,29 +98,41 @@ module EE
         self.user
       end
 
-      def target_project_path
+      def target_project
         downstream_project || upstream_project
       end
 
       def triggers_child_pipeline?
-        same_project? && yaml_for_downstream.present?
+        yaml_for_downstream.present?
+      end
+
+      def downstream_pipeline_params
+        return child_params if triggers_child_pipeline?
+        return cross_project_params if downstream_project.present?
+
+        {}
       end
 
       def downstream_project
         strong_memoize(:downstream_project) do
-          options&.dig(:trigger, :project)
+          if downstream_project_path
+            ::Project.find_by_full_path(downstream_project_path)
+          elsif triggers_child_pipeline?
+            project
+          end
         end
       end
 
       def yaml_for_downstream
         strong_memoize(:yaml_for_downstream) do
-          options&.dig(:trigger, :yaml)
+          includes = options&.dig(:trigger, :include)
+          YAML.dump('include' => includes) if includes
         end
       end
 
       def upstream_project
         strong_memoize(:upstream_project) do
-          options&.dig(:bridge_needs, :pipeline)
+          upstream_project_path && ::Project.find_by_full_path(upstream_project_path)
         end
       end
 
@@ -149,10 +161,47 @@ module EE
         end
       end
 
+      def downstream_project_path
+        strong_memoize(:downstream_project_path) do
+          options&.dig(:trigger, :project)
+        end
+      end
+
+      def upstream_project_path
+        strong_memoize(:upstream_project_path) do
+          options&.dig(:bridge_needs, :pipeline)
+        end
+      end
+
       private
 
-      def same_project?
-        ::Project.find_by_full_path(downstream_project) == project
+      def cross_project_params
+        {
+          project: downstream_project,
+          source: :cross_project_pipeline,
+          target_revision: {
+            ref: target_ref || downstream_project.default_branch
+          }
+        }
+      end
+
+      def child_params
+        parent_pipeline = pipeline
+
+        {
+          project: project,
+          source: :parent_pipeline,
+          target_revision: {
+            ref: parent_pipeline.ref,
+            checkout_sha: parent_pipeline.sha,
+            before: parent_pipeline.before_sha,
+            source_sha: parent_pipeline.source_sha,
+            target_sha: parent_pipeline.target_sha
+          },
+          other_execute_params: {
+            config_content: yaml_for_downstream
+          }
+        }
       end
     end
   end

ee/app/services/ci/create_cross_project_pipeline_service.rb

@@ -8,124 +8,70 @@ module Ci
     def execute(bridge)
       @bridge = bridge
 
-      unless target_project_exists?
-        return bridge.drop!(:downstream_bridge_project_not_found)
-      end
-
-      if target_project == project && !bridge.triggers_child_pipeline?
-        return bridge.drop!(:invalid_bridge_trigger)
-      end
-
-      unless can_create_cross_pipeline?
-        return bridge.drop!(:insufficient_bridge_permissions)
-      end
+      pipeline_params = @bridge.downstream_pipeline_params
+      target_ref = pipeline_params.dig(:target_revision, :ref)
 
-      if bridge.triggers_child_pipeline?
-        create_child_pipeline!
-      else
-        create_cross_project_pipeline!
-      end
-    end
+      return unless ensure_preconditions!(target_ref)
 
-    private
-
-    def target_project_exists?
-      target_project.present? &&
-        can?(current_user, :read_project, target_project)
-    end
-
-    def can_create_cross_pipeline?
-      can?(current_user, :update_pipeline, project) &&
-        can?(target_user, :create_pipeline, target_project) &&
-          can_update_branch?
-    end
-
-    def can_update_branch?
-      ::Gitlab::UserAccess.new(target_user, project: target_project).can_update_branch?(target_ref)
-    end
+      service = ::Ci::CreatePipelineService.new(
+        pipeline_params.fetch(:project),
+        current_user,
+        pipeline_params.fetch(:target_revision))
 
-    def create_cross_project_pipeline!
-      ::Ci::CreatePipelineService
-        .new(target_project, target_user, ref: target_ref)
-        .execute(:cross_project_pipeline, ignore_skip_ci: true) do |pipeline|
+      service.execute(
+        pipeline_params.fetch(:source),
+        { ignore_skip_ci: true }.merge(pipeline_params[:other_execute_params] || {})) do |pipeline|
           @bridge.sourced_pipelines.build(
             source_pipeline: @bridge.pipeline,
             source_project: @bridge.project,
-            project: target_project,
+            project: @bridge.downstream_project,
             pipeline: pipeline)
 
           pipeline.variables.build(@bridge.downstream_variables)
         end
     end
 
-    def create_child_pipeline!
-      return unless @bridge.triggers_child_pipeline?
+    private
 
-      parent_pipeline = @bridge.pipeline
+    def ensure_preconditions!(target_ref)
+      unless downstream_project_accessible?
+        @bridge.drop!(:downstream_bridge_project_not_found)
+        return false
+      end
 
-      ::Ci::CreatePipelineService
-        .new(@bridge.project, @bridge.user,
-          ref: parent_pipeline.ref,
-          checkout_sha: parent_pipeline.sha,
-          before: parent_pipeline.before_sha,
-          source_sha: parent_pipeline.source_sha,
-          target_sha: parent_pipeline.target_sha
-        )
-        .execute(:parent_pipeline, ignore_skip_ci: true, config_content: @bridge.yaml_for_downstream) do |pipeline|
-          @bridge.sourced_pipelines.build(
-            source_pipeline: @bridge.pipeline,
-            source_project: @bridge.project,
-            project: target_project,
-            pipeline: pipeline)
+      # TODO: Remove this condition if favour of model validation
+      # https://gitlab.com/gitlab-org/gitlab/issues/38338
+      if downstream_project == project && !@bridge.triggers_child_pipeline?
+        @bridge.drop!(:invalid_bridge_trigger)
+        return false
+      end
 
-          pipeline.variables.build(@bridge.downstream_variables)
-        end
-    end
+      unless can_create_downstream_pipeline?(target_ref)
+        @bridge.drop!(:insufficient_bridge_permissions)
+        return false
+      end
 
-    def create_child_pipeline!
-      parent_pipeline = @bridge.pipeline
-
-      ::Ci::CreatePipelineService
-        .new(@bridge.project, @bridge.user,
-          ref: parent_pipeline.ref,
-          checkout_sha: parent_pipeline.sha,
-          before: parent_pipeline.before_sha,
-          source_sha: parent_pipeline.source_sha,
-          target_sha: parent_pipeline.target_sha
-        )
-        .execute(:pipeline,
-          ignore_skip_ci: true,
-          config_content: downstream_yaml,
-          schedule: parent_pipeline.pipeline_schedule) do |pipeline|
-            @bridge.sourced_pipelines.build(
-              source_pipeline: @bridge.pipeline,
-              source_project: @bridge.project,
-              project: target_project,
-              pipeline: pipeline)
-
-            pipeline.variables.build(@bridge.downstream_variables)
-          end
+      true
     end
 
-    def downstream_yaml
-      return unless @bridge.triggers_child_pipeline?
-
-      @bridge.downstream_yaml
+    def downstream_project_accessible?
+      downstream_project.present? &&
+        can?(current_user, :read_project, downstream_project)
     end
 
-    def target_user
-      strong_memoize(:target_user) { @bridge.target_user }
+    def can_create_downstream_pipeline?(target_ref)
+      can?(current_user, :update_pipeline, project) &&
+        can?(current_user, :create_pipeline, downstream_project) &&
+          can_update_branch?(target_ref)
     end
 
-    def target_ref
-      strong_memoize(:target_ref) do
-        @bridge.target_ref || target_project.default_branch
-      end
+    def can_update_branch?(target_ref)
+      ::Gitlab::UserAccess.new(current_user, project: downstream_project).can_update_branch?(target_ref)
     end
 
-    def target_project
-      strong_memoize(:target_project) do
-        Project.find_by_full_path(@bridge.target_project_path)
+    def downstream_project
+      strong_memoize(:downstream_project) do
+        @bridge.downstream_project
       end
     end
   end

ee/app/services/ci/subscribe_bridge_service.rb

@@ -5,7 +5,7 @@ module Ci
     include ::Gitlab::Utils::StrongMemoize
 
     def execute(bridge)
-      return unless bridge.upstream_project
+      return unless bridge.upstream_project_path
 
       @bridge = bridge
 
@@ -29,7 +29,7 @@ module Ci
 
     def upstream_project
       strong_memoize(:upstream_project) do
-        ::Project.find_by_full_path(@bridge.target_project_path)
+        @bridge.upstream_project
       end
     end
 

ee/lib/ee/gitlab/ci/config/entry/trigger.rb

@@ -22,19 +22,57 @@ module EE
               end
             end
 
-            class ComplexTrigger < ::Gitlab::Config::Entry::Node
-              include ::Gitlab::Config::Entry::Validatable
-              include ::Gitlab::Config::Entry::Attributable
+            class ComplexTrigger < ::Gitlab::Config::Entry::Simplifiable
+              strategy :CrossProjectTrigger, if: -> (config) { !config.key?(:include) }
+
+              strategy :SameProjectTrigger, if: -> (config) do
+                ::Feature.enabled?(:ci_parent_child_pipeline) &&
+                  config.key?(:include)
+              end
 
-              ALLOWED_KEYS = %i[project branch strategy].freeze
-              attributes :project, :branch, :strategy
+              class CrossProjectTrigger < ::Gitlab::Config::Entry::Node
+                include ::Gitlab::Config::Entry::Validatable
+                include ::Gitlab::Config::Entry::Attributable
+
+                ALLOWED_KEYS = %i[project branch strategy].freeze
+                attributes :project, :branch, :strategy
+
+                validations do
+                  validates :config, presence: true
+                  validates :config, allowed_keys: ALLOWED_KEYS
+                  validates :project, presence: true
+                  validates :branch, type: String, allow_nil: true
+                  validates :strategy, type: String, inclusion: { in: %w[depend], message: 'should be depend' }, allow_nil: true
+                end
+              end
+
+              class SameProjectTrigger < ::Gitlab::Config::Entry::Node
+                include ::Gitlab::Config::Entry::Validatable
+                include ::Gitlab::Config::Entry::Attributable
+                include ::Gitlab::Config::Entry::Configurable
+
+                ALLOWED_KEYS = %i[strategy include].freeze
+                attributes :strategy
+
+                validations do
+                  validates :config, presence: true
+                  validates :config, allowed_keys: ALLOWED_KEYS
+                  validates :strategy, type: String, inclusion: { in: %w[depend], message: 'should be depend' }, allow_nil: true
+                end
+
+                entry :include, ::Gitlab::Ci::Config::Entry::Includes,
+                  description: 'List of external YAML files to include.',
+                  reserved: true
+              end
 
-              validations do
-                validates :config, presence: true
-                validates :config, allowed_keys: ALLOWED_KEYS
-                validates :project, presence: true
-                validates :branch, type: String, allow_nil: true
-                validates :strategy, type: String, inclusion: { in: %w[depend], message: 'should be depend' }, allow_nil: true
+              class UnknownStrategy < ::Gitlab::Config::Entry::Node
+                def errors
+                  if ::Feature.enabled?(:ci_parent_child_pipeline)
+                    ['config must specify either project or include']
+                  else
+                    ['config must specify project']
+                  end
+                end
               end
             end
 

ee/spec/lib/ee/gitlab/ci/config/entry/trigger_spec.rb

 # frozen_string_literal: true
 
-require 'fast_spec_helper'
-require_dependency 'active_model'
+require 'spec_helper'
 
 describe EE::Gitlab::Ci::Config::Entry::Trigger do
   subject { described_class.new(config) }
@@ -83,6 +82,53 @@ describe EE::Gitlab::Ci::Config::Entry::Trigger do
       end
     end
 
+    context '#include' do
+      context 'with simple include' do
+        let(:config) { { include: 'path/to/config.yml' } }
+
+        it { is_expected.to be_valid }
+
+        it 'returns a trigger configuration hash' do
+          expect(subject.value).to eq(include: 'path/to/config.yml' )
+        end
+      end
+
+      context 'with project' do
+        let(:config) { { project: 'some/project', include: 'path/to/config.yml' } }
+
+        it { is_expected.not_to be_valid }
+
+        it 'is returns an error' do
+          expect(subject.errors.first)
+            .to match /config contains unknown keys: project/
+        end
+      end
+
+      context 'with branch' do
+        let(:config) { { branch: 'feature', include: 'path/to/config.yml' } }
+
+        it { is_expected.not_to be_valid }
+
+        it 'is returns an error' do
+          expect(subject.errors.first)
+            .to match /config contains unknown keys: branch/
+        end
+      end
+
+      context 'when feature flag is off' do
+        before do
+          stub_feature_flags(ci_parent_child_pipeline: false)
+        end
+
+        let(:config) { { include: 'path/to/config.yml' } }
+
+        it 'is returns an error if include is used' do
+          expect(subject.errors.first)
+            .to match /config must specify project/
+        end
+      end
+    end
+
     context 'when config contains unknown keys' do
       let(:config) { { project: 'some/project', unknown: 123 } }
 

ee/spec/models/ci/bridge_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe Ci::Bridge do
   set(:project) { create(:project) }
+  set(:target_project) { create(:project, name: 'project', namespace: create(:namespace, name: 'my')) }
   set(:pipeline) { create(:ci_pipeline, project: project) }
 
   let(:bridge) do
@@ -157,10 +158,10 @@ describe Ci::Bridge do
     end
   end
 
-  describe '#target_project_path' do
+  describe '#target_project' do
     context 'when trigger is defined' do
       it 'returns a full path of a project' do
-        expect(bridge.target_project_path).to eq 'my/project'
+        expect(bridge.target_project).to eq target_project
       end
     end
 
@@ -168,7 +169,7 @@ describe Ci::Bridge do
       let(:options) { { trigger: {} } }
 
       it 'returns nil' do
-        expect(bridge.target_project_path).to be_nil
+        expect(bridge.target_project).to be_nil
       end
     end
   end
@@ -293,39 +294,23 @@ describe Ci::Bridge do
   describe '#triggers_child_pipeline?' do
     subject { bridge.triggers_child_pipeline? }
 
-    context 'when downstream project is same as the bridge project' do
-      context 'when bridge defines a downstream YAML' do
-        let(:options) do
-          {
-            trigger: {
-              project: project.full_path,
-              yaml: YAML.dump(rspec: { script: 'rspec' })
-            }
+    context 'when bridge defines a downstream YAML' do
+      let(:options) do
+        {
+          trigger: {
+            include: 'path/to/child.yml'
           }
-        end
-
-        it { is_expected.to be_truthy }
+        }
       end
 
-      context 'when bridge does not define a downstream YAML' do
-        let(:options) do
-          {
-            trigger: {
-              project: project.full_path
-            }
-          }
-        end
-
-        it { is_expected.to be_falsey }
-      end
+      it { is_expected.to be_truthy }
     end
 
-    context 'when downstream project is different than bridge project' do
+    context 'when bridge does not define a downstream YAML' do
       let(:options) do
         {
           trigger: {
-            project: 'my/project',
-            yaml: YAML.dump(rspec: { script: 'rspec' })
+            project: project.full_path
           }
         }
       end

ee/spec/services/ci/create_cross_project_pipeline_service_spec.rb

@@ -148,7 +148,7 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
         end
       end
 
-      context 'when a custom YAML is provided' do
+      context 'when "include" is provided' do
         shared_examples 'creates a child pipeline' do
           it 'creates only one new pipeline' do
             expect { service.execute(bridge) }
@@ -201,10 +201,7 @@ describe Ci::CreateCrossProjectPipelineService, '#execute' do
 
         let(:trigger) do
           {
-            trigger: {
-              project: upstream_project.full_path,
-              yaml: YAML.dump({ include: 'child-pipeline.yml' })
-            }
+            trigger: { include: 'child-pipeline.yml' }
           }
         end
 

ee/spec/services/ci/create_pipeline_service/needs_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 describe Ci::CreatePipelineService do
   subject(:execute) { service.execute(:push) }
 
+  set(:downstream_project) { create(:project, name: 'project', namespace: create(:namespace, name: 'some'))}
   let(:project) { create(:project, :repository) }
   let(:user) { create(:admin) }
   let(:service) { described_class.new(project, user, { ref: 'refs/heads/master' }) }
@@ -46,6 +47,6 @@ describe Ci::CreatePipelineService do
   it 'persists bridge target project' do
     bridge = execute.stages.last.bridges.first
 
-    expect(bridge.downstream_project).to eq('some/project')
+    expect(bridge.downstream_project).to eq downstream_project
   end
 end

ee/spec/services/ci/create_pipeline_service_spec.rb

@@ -73,8 +73,6 @@ describe Ci::CreatePipelineService, '#execute' do
 
   describe 'cross-project pipeline triggers' do
     before do
-      stub_feature_flags(cross_project_pipeline_triggers: true)
-
       stub_ci_pipeline_yaml_file <<~YAML
         test:
           script: rspec
@@ -144,6 +142,41 @@ describe Ci::CreatePipelineService, '#execute' do
     end
   end
 
+  describe 'child pipeline triggers' do
+    before do
+      stub_ci_pipeline_yaml_file <<~YAML
+        test:
+          script: rspec
+
+        deploy:
+          variables:
+            CROSS: downstream
+          stage: deploy
+          trigger:
+            include:
+              - local: path/to/child.yml
+      YAML
+    end
+
+    it 'creates bridge jobs correctly' do
+      pipeline = create_pipeline!
+
+      test = pipeline.statuses.find_by(name: 'test')
+      bridge = pipeline.statuses.find_by(name: 'deploy')
+
+      expect(pipeline).to be_persisted
+      expect(test).to be_a Ci::Build
+      expect(bridge).to be_a Ci::Bridge
+      expect(bridge.stage).to eq 'deploy'
+      expect(pipeline.statuses).to match_array [test, bridge]
+      expect(bridge.options).to eq(
+        'trigger' => { 'include' => { '0' => { 'local' => 'path/to/child.yml' } } }
+      )
+      expect(bridge.yaml_variables)
+        .to include(key: 'CROSS', value: 'downstream', public: true)
+    end
+  end
+
   def create_pipeline!
     service.execute(:push)
   end