Merge pull request screwdriver-cd#8 from screwdriver-cd/RestrictedNames

Restricting sd- from the step names

Author: nkatzman

lib/phase/functional.js

@@ -30,10 +30,10 @@ function getDefaultWorkflow(jobs) {
  * Make sure the Matrix they specified is valid
  * @method validateJobMatrix
  * @param  {Object}      job     Job to inspect
- * @param  {String}      jobName Name of job (for error reporting)
+ * @param  {String}      prefix  Prefix before reporting errors
  * @return {Array}               List of errors
  */
-function validateJobMatrix(job, jobName) {
+function validateJobMatrix(job, prefix) {
     let matrixSize = 1;
     const errors = [];
     const matrix = Hoek.reach(job, 'matrix', {
@@ -45,7 +45,7 @@ function validateJobMatrix(job, jobName) {
     const environmentSize = Object.keys(matrix).length + Object.keys(environment).length;
 
     if (environmentSize > MAX_ENVIRONMENT_VARS) {
-        errors.push(`Job ${jobName}: "environment" and "matrix" can only have a combined ` +
+        errors.push(`${prefix}: "environment" and "matrix" can only have a combined ` +
             ` maxiumum of ${MAX_ENVIRONMENT_VARS} environment variables defined ` +
             `(currently ${environmentSize})`);
     }
@@ -55,13 +55,37 @@ function validateJobMatrix(job, jobName) {
     });
 
     if (matrixSize > MAX_PERMUTATIONS) {
-        errors.push(`Job ${jobName}: Matrix cannot contain >${MAX_PERMUTATIONS} permutations ` +
+        errors.push(`${prefix}: "matrix" cannot contain >${MAX_PERMUTATIONS} permutations ` +
             `(currently ${matrixSize})`);
     }
 
     return errors;
 }
 
+/**
+ * Make sure the Steps are possible
+ *  - Check it doesn't start with sd-
+ * @method validateJobSteps
+ * @param  {Object}      job     Job to inspect
+ * @param  {String}      prefix  Prefix before reporting errors
+ * @return {Array}               List of errors
+ */
+function validateJobSteps(job, prefix) {
+    const errors = [];
+    const steps = Hoek.reach(job, 'commands', {
+        default: []
+    });
+
+    steps.forEach((step) => {
+        if (step.name.indexOf('sd-') === 0) {
+            errors.push(`${prefix}: Step "${step.name}": `
+                + 'cannot use a restricted prefix "sd-"');
+        }
+    });
+
+    return errors;
+}
+
 /**
  * Ensure the job is something that can be run
  *  - has at least one step
@@ -94,13 +118,16 @@ function validateJobSchema(doc) {
 
     // Validate jobs contain required minimum fields
     Object.keys(doc.jobs).forEach((jobName) => {
+        const prefix = `Job "${jobName}"`;
+
         try {
-            Joi.attempt(doc.jobs[jobName], SCHEMA_JOB, `Job ${jobName}`);
+            Joi.attempt(doc.jobs[jobName], SCHEMA_JOB, prefix);
         } catch (err) {
             errors.push(err.message);
         }
 
-        errors = errors.concat(validateJobMatrix(doc.jobs[jobName], jobName));
+        errors = errors.concat(validateJobMatrix(doc.jobs[jobName], prefix));
+        errors = errors.concat(validateJobSteps(doc.jobs[jobName], prefix));
     });
 
     return errors;

test/data/restricted-step-name.yaml

@@ -0,0 +1,5 @@
+jobs:
+    main:
+        image: node:4
+        steps:
+            - sd-setup: woot

test/index.test.js

@@ -140,7 +140,16 @@ describe('config parser', () => {
         it('returns an error if matrix is too big', (done) => {
             parser(loadData('too-big-matrix.yaml'), (err) => {
                 assert.isNotNull(err);
-                assert.match(err.toString(), /Job main: Matrix cannot contain >25 permutations/);
+                assert.match(err.toString(), /Job "main": "matrix" cannot contain >25 perm/);
+                done();
+            });
+        });
+
+        it('returns an error if using restricted step names', (done) => {
+            parser(loadData('restricted-step-name.yaml'), (err) => {
+                assert.isNotNull(err);
+                assert.match(err.toString(),
+                    /Job "main": Step "sd-setup": cannot use a restricted prefix "sd-"/);
                 done();
             });
         });