Merge branch 'validate-dynamic-pipeline-dependencies' into 'master'

Validate dynamic pipeline dependencies

See merge request gitlab-org/gitlab!27916

changelogs/unreleased/validate-dynamic-pipeline-dependencies.yml

+---
+title: Validate dependency on job generating a CI config when using dynamic child pipelines
+merge_request: 27916
+author:
+type: added

doc/ci/parent_child_pipelines.md

@@ -136,12 +136,11 @@ your own script to generate a YAML file, which is then [used to trigger a child
 This technique can be very powerful in generating pipelines targeting content that changed or to
 build a matrix of targets and architectures.
 
+In GitLab 12.9, the child pipeline could fail to be created in certain cases, causing the parent pipeline to fail.
+This is [resolved in GitLab 12.10](https://gitlab.com/gitlab-org/gitlab/-/issues/209070).
+
 ## Limitations
 
 A parent pipeline can trigger many child pipelines, but a child pipeline cannot trigger
 further child pipelines. See the [related issue](https://gitlab.com/gitlab-org/gitlab/issues/29651)
 for discussion on possible future improvements.
-
-When triggering dynamic child pipelines, if the job containing the CI config artifact is not a predecessor of the
-trigger job, the child pipeline will fail to be created, causing also the parent pipeline to fail.
-In the future we want to validate the trigger job's dependencies [at the time the parent pipeline is created](https://gitlab.com/gitlab-org/gitlab/-/issues/209070) rather than when the child pipeline is created.

lib/gitlab/ci/config/entry/include.rb

@@ -15,6 +15,11 @@ module Gitlab
           validations do
             validates :config, hash_or_string: true
             validates :config, allowed_keys: ALLOWED_KEYS
+            validate do
+              if config[:artifact] && config[:job].blank?
+                errors.add(:config, "must specify the job where to fetch the artifact from")
+              end
+            end
           end
         end
       end

lib/gitlab/ci/yaml_processor.rb

@@ -142,6 +142,7 @@ module Gitlab
           validate_job_stage!(name, job)
           validate_job_dependencies!(name, job)
           validate_job_needs!(name, job)
+          validate_dynamic_child_pipeline_dependencies!(name, job)
           validate_job_environment!(name, job)
         end
       end
@@ -163,37 +164,52 @@ module Gitlab
       def validate_job_dependencies!(name, job)
         return unless job[:dependencies]
 
-        stage_index = @stages.index(job[:stage])
-
         job[:dependencies].each do |dependency|
-          raise ValidationError, "#{name} job: undefined dependency: #{dependency}" unless @jobs[dependency.to_sym]
+          validate_job_dependency!(name, dependency)
+        end
+      end
 
-          dependency_stage_index = @stages.index(@jobs[dependency.to_sym][:stage])
+      def validate_dynamic_child_pipeline_dependencies!(name, job)
+        return unless includes = job.dig(:trigger, :include)
 
-          unless dependency_stage_index.present? && dependency_stage_index < stage_index
-            raise ValidationError, "#{name} job: dependency #{dependency} is not defined in prior stages"
-          end
+        includes.each do |included|
+          next unless dependency = included[:job]
+
+          validate_job_dependency!(name, dependency)
         end
       end
 
       def validate_job_needs!(name, job)
-        return unless job.dig(:needs, :job)
-
-        stage_index = @stages.index(job[:stage])
+        return unless needs = job.dig(:needs, :job)
 
-        job.dig(:needs, :job).each do |need|
-          need_job_name = need[:name]
+        needs.each do |need|
+          dependency = need[:name]
+          validate_job_dependency!(name, dependency, 'need')
+        end
+      end
 
-          raise ValidationError, "#{name} job: undefined need: #{need_job_name}" unless @jobs[need_job_name.to_sym]
+      def validate_job_dependency!(name, dependency, dependency_type = 'dependency')
+        unless @jobs[dependency.to_sym]
+          raise ValidationError, "#{name} job: undefined #{dependency_type}: #{dependency}"
+        end
 
-          needs_stage_index = @stages.index(@jobs[need_job_name.to_sym][:stage])
+        job_stage_index = stage_index(name)
+        dependency_stage_index = stage_index(dependency)
 
-          unless needs_stage_index.present? && needs_stage_index < stage_index
-            raise ValidationError, "#{name} job: need #{need_job_name} is not defined in prior stages"
-          end
+        # A dependency might be defined later in the configuration
+        # with a stage that does not exist
+        unless dependency_stage_index.present? && dependency_stage_index < job_stage_index
+          raise ValidationError, "#{name} job: #{dependency_type} #{dependency} is not defined in prior stages"
         end
       end
 
+      def stage_index(name)
+        job = @jobs[name.to_sym]
+        return unless job
+
+        @stages.index(job[:stage])
+      end
+
       def validate_job_environment!(name, job)
         return unless job[:environment]
         return unless job[:environment].is_a?(Hash)

spec/lib/gitlab/ci/yaml_processor_spec.rb

@@ -1647,6 +1647,48 @@ module Gitlab
 
           it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, /is not defined in prior stages/) }
         end
+
+        context 'when trigger job includes artifact generated by a dependency' do
+          context 'when dependency is defined in previous stages' do
+            let(:config) do
+              {
+                build1: { stage: 'build', script: 'test' },
+                test1: { stage: 'test', trigger: {
+                  include: [{ job: 'build1', artifact: 'generated.yml' }]
+                } }
+              }
+            end
+
+            it { expect { subject }.not_to raise_error }
+          end
+
+          context 'when dependency is defined in later stages' do
+            let(:config) do
+              {
+                build1: { stage: 'build', script: 'test' },
+                test1: { stage: 'test', trigger: {
+                  include: [{ job: 'deploy1', artifact: 'generated.yml' }]
+                } },
+                deploy1: { stage: 'deploy', script: 'test' }
+              }
+            end
+
+            it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, /is not defined in prior stages/) }
+          end
+
+          context 'when dependency is not defined' do
+            let(:config) do
+              {
+                build1: { stage: 'build', script: 'test' },
+                test1: { stage: 'test', trigger: {
+                  include: [{ job: 'non-existent', artifact: 'generated.yml' }]
+                } }
+              }
+            end
+
+            it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, /undefined dependency: non-existent/) }
+          end
+        end
       end
 
       describe "Job Needs" do
@@ -2052,6 +2094,34 @@ module Gitlab
         end
       end
 
+      describe 'with trigger:include' do
+        context 'when artifact and job are specified' do
+          let(:config) do
+            YAML.dump({
+              build1: { stage: 'build', script: 'test' },
+              test1: { stage: 'test', trigger: {
+                include: [{ artifact: 'generated.yml', job: 'build1' }]
+              } }
+            })
+          end
+
+          it { expect { subject }.not_to raise_error }
+        end
+
+        context 'when artifact is specified without job' do
+          let(:config) do
+            YAML.dump({
+              build1: { stage: 'build', script: 'test' },
+              test1: { stage: 'test', trigger: {
+                include: [{ artifact: 'generated.yml' }]
+              } }
+            })
+          end
+
+          it { expect { subject }.to raise_error(Gitlab::Ci::YamlProcessor::ValidationError, /must specify the job where to fetch the artifact from/) }
+        end
+      end
+
       describe "Error handling" do
         it "fails to parse YAML" do
           expect do