feat(3486): validate that requires only reference existing stages

lib/phase/flatten.js

@@ -3,6 +3,7 @@
 const Hoek = require('@hapi/hoek');
 const clone = require('clone');
 const { STAGE_TRIGGER } = require('screwdriver-data-schema/config/regex');
+const { normalizeRequires } = require('./requires');
 
 const STAGE_PREFIX = 'stage@';
 const STAGE_SETUP_TEARDOWN_PATTERN = /stage@[\w-]+:(?:setup|teardown)$/;
@@ -628,11 +629,7 @@ function getTeardownRequires(jobs, stageName) {
             jobs[jobName].stage.name === stageName &&
             !STAGE_SETUP_TEARDOWN_PATTERN.test(jobName)
         ) {
-            if (Array.isArray(jobs[jobName].requires)) {
-                stageJobsRequires = stageJobsRequires.concat(jobs[jobName].requires);
-            } else {
-                stageJobsRequires.push(jobs[jobName].requires);
-            }
+            stageJobsRequires = stageJobsRequires.concat(normalizeRequires(jobs[jobName].requires));
             stageJobNames.push(jobName);
         }
     });

lib/phase/functional.js

@@ -3,9 +3,11 @@
 const Joi = require('joi');
 const Hoek = require('@hapi/hoek');
 const clone = require('clone');
+const { STAGE_TRIGGER, STAGE_SETUP_TEARDOWN_JOB_NAME } = require('screwdriver-data-schema/config/regex');
 const WorkflowParser = require('screwdriver-workflow-parser');
 const SlackValidation = require('screwdriver-notifications-slack');
 const EmailValidation = require('screwdriver-notifications-email');
+const { normalizeRequires } = require('./requires');
 
 // Actual environment size is limited by space, not quantity.
 // We do this to force sd.yaml to be simpler.
@@ -242,19 +244,78 @@ async function generateWorkflowGraph(doc, triggerFactory, pipelineId) {
     return WorkflowParser.getWorkflow(cloneDoc, triggerFactory, pipelineId);
 }
 
+/**
+ * Validate that requires fields only reference existing stages
+ * @method validateRequiresStagesExist
+ * @param  {Object} config
+ * @param  {Object} config.jobs    Map of job name to job configuration
+ * @param  {Object} config.stages  Map of stage name to stage configuration
+ * @return {Array}                 List of errors
+ */
+function validateRequiresStagesExist({ jobs, stages }) {
+    const errors = [];
+    const stageNames = Object.keys(stages || {});
+    const stageNameSet = new Set(stageNames);
+
+    // Validate job-level requires that reference stages.
+    Object.entries(jobs || {}).forEach(([jobName, job]) => {
+        const setupTeardownMatched = STAGE_SETUP_TEARDOWN_JOB_NAME.exec(jobName);
+
+        // Stage-level requires are copied to setup jobs during flattening, so skip them here.
+        if (setupTeardownMatched && setupTeardownMatched[2] === 'setup') {
+            return;
+        }
+
+        const requiresList = normalizeRequires(job.requires);
+
+        requiresList.forEach(requires => {
+            const matched = STAGE_TRIGGER.exec(requires);
+
+            if (matched && !stageNameSet.has(matched[1])) {
+                errors.push(
+                    `${jobName} job has invalid requires: ${requires}. Cannot find stage ${matched[1]} in stages.`
+                );
+            }
+        });
+    });
+
+    // Validate stage-level requires that reference stages.
+    Object.entries(stages || {}).forEach(([stageName, stage]) => {
+        const stageRequiresList = normalizeRequires(stage.requires);
+
+        stageRequiresList.forEach(requires => {
+            const matched = STAGE_TRIGGER.exec(requires);
+
+            if (matched && !stageNameSet.has(matched[1])) {
+                errors.push(
+                    `${stageName} stage has invalid requires: ${requires}. Cannot find stage ${matched[1]} in stages.`
+                );
+            }
+        });
+    });
+
+    return errors;
+}
+
 /**
  * Ensure the workflowGraph is a valid one
+ *  - Stage references point to existing stages
  *  - No cycles in workflowGraph
  * @method validateWorkflowGraph
  * @param  {Object}          doc Document that went through flattening
  * @return {Array}               List of errors
  */
 function validateWorkflowGraph(doc) {
+    const errors = validateRequiresStagesExist({
+        jobs: doc.jobs,
+        stages: doc.stages
+    });
+
     if (WorkflowParser.hasCycle(doc.workflowGraph)) {
-        return ['Jobs: should not have circular dependency in jobs'];
+        errors.push('Jobs: should not have circular dependency in jobs');
     }
 
-    return [];
+    return errors;
 }
 
 /**

lib/phase/requires.js

@@ -0,0 +1,23 @@
+'use strict';
+
+/**
+ * Normalize requires value to array
+ * @method normalizeRequires
+ * @param  {String|Array|undefined|null} requiresValue Value to normalize
+ * @return {Array}
+ */
+function normalizeRequires(requiresValue) {
+    if (Array.isArray(requiresValue)) {
+        return requiresValue;
+    }
+
+    if (!requiresValue) {
+        return [];
+    }
+
+    return [requiresValue];
+}
+
+module.exports = {
+    normalizeRequires
+};

test/data/pipeline-with-requires-existing-stages.yaml

@@ -0,0 +1,25 @@
+jobs:
+  job1:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job1
+  job2:
+    image: node:18
+    requires: [~job1]
+    steps:
+      - echo: echo job2
+  job3:
+    image: node:18
+    requires: [stage@stg1]
+    steps:
+      - echo: echo job3
+  job4:
+    image: node:18
+    requires: [~stage@stg1]
+    steps:
+      - echo: echo job4
+stages:
+  stg1:
+    jobs: [job1, job2]
+    requires: [~commit]

test/data/pipeline-with-requires-invalid-stages.yaml

@@ -0,0 +1,25 @@
+jobs:
+  job1:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job1
+  job2:
+    image: node:18
+    requires: [~job1]
+    steps:
+      - echo: echo job2
+  job3:
+    image: node:18
+    requires: [~stage@stg2]
+    steps:
+      - echo: echo job3
+  job4:
+    image: node:18
+    requires: [stage@stg1, stage@stg2]
+    steps:
+      - echo: echo job4
+stages:
+  stg1:
+    jobs: [job1, job2]
+    requires: [~commit]

test/data/pipeline-with-requires-nonexistent-stage.yaml

@@ -0,0 +1,16 @@
+jobs:
+  job1:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job1
+  job2:
+    image: node:18
+    requires: [~job1]
+    steps:
+      - echo: echo job2
+  job3:
+    image: node:18
+    requires: [stage@stg1]
+    steps:
+      - echo: echo job3

test/data/pipeline-with-stage-requires-existing-stages.yaml

@@ -0,0 +1,26 @@
+jobs:
+  job1:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job1
+  job2:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job2
+  job3:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job3
+stages:
+  stg1:
+    jobs: [job1]
+    requires: [~commit]
+  stg2:
+    jobs: [job2]
+    requires: [stage@stg1]
+  stg3:
+    jobs: [job3]
+    requires: [~stage@stg1]

test/data/pipeline-with-stage-requires-invalid-stages.yaml

@@ -0,0 +1,34 @@
+jobs:
+  job1:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job1
+  job2:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job2
+  job3:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job3
+  job4:
+    image: node:18
+    requires: []
+    steps:
+      - echo: echo job4
+stages:
+  stg1:
+    jobs: [job1]
+    requires: [~commit]
+  stg2:
+    jobs: [job2]
+    requires: [stage@stg1]
+  stg3:
+    jobs: [job3]
+    requires: [stage@stg1, stage@missing1]
+  stg4:
+    jobs: [job4]
+    requires: [~stage@missing2]

test/index.test.js

@@ -285,6 +285,68 @@ describe('config parser', () => {
                     }).then(data => {
                         assert.match(data.errors[0], /Error: Job foo in stage cannot have sourcePaths defined./);
                     }));
+
+                it('returns an error if a job requires a nonexistent stage and no stages are defined', () =>
+                    parser({
+                        yaml: loadData('pipeline-with-requires-nonexistent-stage.yaml'),
+                        triggerFactory,
+                        pipelineId
+                    }).then(data => {
+                        assert.match(
+                            data.errors[0],
+                            /Error: job3 job has invalid requires: stage@stg1\. Cannot find stage stg1 in stages\./
+                        );
+                    }));
+
+                it('returns errors if jobs require nonexistent stages when stages are defined', () =>
+                    parser({
+                        yaml: loadData('pipeline-with-requires-invalid-stages.yaml'),
+                        triggerFactory,
+                        pipelineId
+                    }).then(data => {
+                        assert.match(
+                            data.errors[0],
+                            /Error: job3 job has invalid requires: ~stage@stg2:teardown\. Cannot find stage stg2 in stages\./
+                        );
+                        assert.match(
+                            data.errors[0],
+                            /job4 job has invalid requires: stage@stg2:teardown\. Cannot find stage stg2 in stages\./
+                        );
+                    }));
+
+                it('does not return an error if jobs require existing stages', () =>
+                    parser({
+                        yaml: loadData('pipeline-with-requires-existing-stages.yaml'),
+                        triggerFactory,
+                        pipelineId
+                    }).then(data => {
+                        assert.notOk(data.errors);
+                    }));
+
+                it('returns errors if stages require nonexistent stages', () =>
+                    parser({
+                        yaml: loadData('pipeline-with-stage-requires-invalid-stages.yaml'),
+                        triggerFactory,
+                        pipelineId
+                    }).then(data => {
+                        assert.match(
+                            data.errors[0],
+                            /Error: stg3 stage has invalid requires: stage@missing1:teardown\. Cannot find stage missing1 in stages\./
+                        );
+                        assert.match(
+                            data.errors[0],
+                            /stg4 stage has invalid requires: ~stage@missing2:teardown\. Cannot find stage missing2 in stages\./
+                        );
+                    }));
+
+                it('does not return an error if stages require existing stages', () =>
+                    parser({
+                        yaml: loadData('pipeline-with-stage-requires-existing-stages.yaml'),
+                        triggerFactory,
+                        pipelineId
+                    }).then(data => {
+                        assert.notOk(data.errors);
+                    }));
             });
 
             describe('subscribe', () => {