From 070edf9b2f55bdf50f84d2deb13a214031fce35d Mon Sep 17 00:00:00 2001
From: Lars Wirzenius <liw@liw.fi>
Date: Thu, 26 Jul 2018 10:37:51 +0300
Subject: Change: upon triggering build, check project defines all parameters

---
 NEWS                 |  5 +++++
 ick2/__init__.py     |  1 +
 ick2/apibase.py      |  3 +++
 ick2/exceptions.py   |  7 +++++++
 ick2/projectapi.py   |  9 +++++++++
 yarns/400-build.yarn | 21 ++++++++++++++++++++-
 6 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index ae147a0..c1caa5c 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,11 @@ Version 0.53.2+git, not yet released
   parameter that gives the name of the artifact to use. The defaults
   are `systree_name` and `workspace_name`, respectively.
 
+* The controller now checks, when a project build is triggered, that
+  the project defines all the parameters declared by each pipeline it
+  uses. This doesn't catch when a parameters is used by a pipeline,
+  but not declared by it.
+
 Version 0.53.2, released 2018-07-18
 ------------------------------------
 
diff --git a/ick2/__init__.py b/ick2/__init__.py
index fe8027c..f489396 100644
--- a/ick2/__init__.py
+++ b/ick2/__init__.py
@@ -51,6 +51,7 @@ from .exceptions import (
     IckException,
     MethodNotAllowed,
     ClientIdMissing,
+    ParametersMissing,
 )
 from .responses import (
     OK,
diff --git a/ick2/apibase.py b/ick2/apibase.py
index b629f68..a7b3927 100644
--- a/ick2/apibase.py
+++ b/ick2/apibase.py
@@ -61,6 +61,9 @@ class APIbase:
                 if 'raw_uri_path' in kwargs:
                     del kwargs['raw_uri_path']
                 body = callback(**kwargs)
+            except ick2.ParametersMissing as e:
+                ick2.log.log('error', msg_text=str(e), kwargs=kwargs)
+                return ick2.not_found(str(e))
             except ick2.NotFound as e:
                 ick2.log.log(
                     'error', msg_text='GET Not found', kwargs=kwargs,
diff --git a/ick2/exceptions.py b/ick2/exceptions.py
index 8a5447d..3af7ff0 100644
--- a/ick2/exceptions.py
+++ b/ick2/exceptions.py
@@ -39,3 +39,10 @@ class ClientIdMissing(IckException):
 class MethodNotAllowed(IckException):
 
     pass
+
+
+class ParametersMissing(IckException):
+
+    def __init__(self, names):
+        super().__init__(
+            'Project must define parameters: {}'.format(' '. join(names)))
diff --git a/ick2/projectapi.py b/ick2/projectapi.py
index 90197ae..0d9f8f5 100644
--- a/ick2/projectapi.py
+++ b/ick2/projectapi.py
@@ -103,7 +103,16 @@ class ProjectAPI(ick2.ResourceApiBase):
 
     def _get_actions(self, project):  # pragma: no cover
         actions = []
+        params = project.get('parameters', {})
         for pipeline_name in project.get('pipelines', []):
             pipeline = self._trans.get_resource('pipelines', pipeline_name)
+            wanted = pipeline.get('parameters', [])
+            missing = [
+                name
+                for name in wanted
+                if name not in params
+            ]
+            if missing:
+                raise ick2.ParametersMissing(missing)
             actions.extend(list(pipeline['actions']))
         return actions
diff --git a/yarns/400-build.yarn b/yarns/400-build.yarn
index 1bd52fb..7f261ef 100644
--- a/yarns/400-build.yarn
+++ b/yarns/400-build.yarn
@@ -47,6 +47,7 @@ Add up a project with some named pipelines.
     WHEN user makes request POST /pipelines with a valid token and body
     ... {
     ...     "pipeline": "construct",
+    ...     "parameters": ["foo"],
     ...     "actions": [
     ...         { "where": "host", "shell": "day 1" },
     ...         { "where": "host", "shell": "day 2" }
@@ -63,7 +64,20 @@ Add up a project with some named pipelines.
     ... }
     THEN result has status code 201
 
-Add a second project so we know each project gets its own work steps.
+Define another project that doesn't define the parameter for the
+pipeline. This should succeed, as we don't check parameters until a
+build starts, and the pipeline might not even exist at this time, and
+it may change before the build starts.
+
+    WHEN user makes request POST /projects with a valid token and body
+    ... {
+    ...     "project": "bad_rome",
+    ...     "parameters": {},
+    ...     "pipelines": ["construct"]
+    ... }
+    THEN result has status code 201
+
+Add another project so we know each project gets its own work steps.
 
     WHEN user makes request POST /projects with a valid token and body
     ... {
@@ -97,6 +111,11 @@ Trigger build of project that doesn't exist.
     WHEN user makes request GET /projects/eldorado/+trigger
     THEN result has status code 404
 
+Trigger build of project with missing parameter.
+
+    WHEN user makes request GET /projects/bad_rome/+trigger
+    THEN result has status code 404
+
 Trigger build.
 
     WHEN user makes request GET /projects/rome/+trigger
-- 
cgit v1.2.1