check generic-worker interactive tasks. raise on docker-worker interactive tasks. (#452)

* check generic-worker interactive tasks. raise on docker-worker interactive tasks.

* address review comment

* 34.0.0

Author: escapewindow

HISTORY.rst

@@ -4,6 +4,20 @@ Change Log
 All notable changes to this project will be documented in this file.
 This project adheres to `Semantic Versioning <http://semver.org/>`__.
 
+[34.0.0] - 2020-04-17
+---------------------
+
+Added
+~~~~~
+- added ``check_interactive_generic_worker``
+
+Changed
+~~~~~~~
+- ``check_interactive_docker_worker`` now raises ``CoTError`` on errors, rather
+    than returning the list of error messages
+- ``check_interactive_docker_worker`` now also runs against the chain task, if it's
+    docker-worker
+
 [33.1.1] - 2020-04-09
 ---------------------
 

src/scriptworker/cot/verify.py

@@ -365,7 +365,6 @@ def get_valid_worker_impls():
             validation functions.
 
     """
-    # TODO support taskcluster worker
     return immutabledict({"docker-worker": verify_docker_worker_task, "generic-worker": verify_generic_worker_task, "scriptworker": verify_scriptworker_task})
 
 
@@ -436,8 +435,8 @@ def check_interactive_docker_worker(link):
     Args:
         link (LinkOfTrust): the task link we're checking.
 
-    Returns:
-        list: the list of error errors.  Success is an empty list.
+    Raises:
+        CoTError: on interactive.
 
     """
     errors = []
@@ -449,7 +448,34 @@ def check_interactive_docker_worker(link):
             errors.append("{} is interactive: task.payload.env.TASKCLUSTER_INTERACTIVE!".format(link.name))
     except KeyError:
         errors.append("check_interactive_docker_worker: {} task definition is malformed!".format(link.name))
-    return errors
+    raise_on_errors(errors)
+
+
+# check_interactive_generic_worker {{{1
+def check_interactive_generic_worker(link):
+    """Given a task, make sure the task was not defined as interactive.
+
+    * ``task.payload.rdpInfo`` must be absent or False.
+    * ``task.payload.scopes`` must not contain a scope starting with ``generic-worker:allow-rdp:``
+
+    Args:
+        link (LinkOfTrust): the task link we're checking.
+
+    Raises:
+        CoTError: on interactive.
+
+    """
+    errors = []
+    log.info("Checking for {} {} interactive generic-worker".format(link.name, link.task_id))
+    try:
+        if link.task["payload"].get("rdpInfo"):
+            errors.append("{} is interactive: task.payload.rdpInfo!".format(link.name))
+        for scope in link.task["scopes"]:
+            if scope.startswith("generic-worker:allow-rdp:"):
+                errors.append("{} is interactive: {}!".format(link.name, scope))
+    except KeyError:
+        errors.append("check_interactive_generic_worker: {} task definition is malformed!".format(link.name))
+    raise_on_errors(errors)
 
 
 # verify_docker_image_sha {{{1
@@ -1679,12 +1705,11 @@ async def verify_docker_worker_task(chain, link):
         CoTError: on failure.
 
     """
+    check_interactive_docker_worker(link)
     if chain != link:
-        # These two checks will die on `link.cot` if `link` is a ChainOfTrust
-        # object (e.g., the task we're running `verify_cot` against is a
-        # docker-worker task). So only run these tests if they are not the chain
-        # object.
-        check_interactive_docker_worker(link)
+        # This check will die on `link.cot` if `link` is a ChainOfTrust object
+        # (e.g., the task we're running `verify_cot` against is a docker-worker
+        # task). So only run this test if the link is not the chain object.
         verify_docker_image_sha(chain, link)
 
 
@@ -1700,7 +1725,9 @@ async def verify_generic_worker_task(chain, link):
         CoTError: on failure.
 
     """
-    pass
+    check_interactive_generic_worker(link)
+    # XXX once generic-worker supports docker, verify the docker image sha
+    #     if applicable.
 
 
 # verify_scriptworker_task {{{1

src/scriptworker/version.py

@@ -54,7 +54,7 @@ def get_version_string(version: Union[ShortVerType, LongVerType]) -> str:
 
 # 1}}}
 # Semantic versioning 2.0.0  http://semver.org/
-__version__ = (33, 1, 1)
+__version__ = (34, 0, 0)
 __version_string__ = get_version_string(__version__)
 
 

tests/test_cot_verify.py

@@ -464,23 +464,44 @@ def test_guess_task_type():
 
 # check_interactive_docker_worker {{{1
 @pytest.mark.parametrize(
-    "task,has_errors",
+    "task,raises",
     (
         ({"payload": {"features": {}, "env": {}}}, False),
         ({"payload": {"features": {"interactive": True}, "env": {}}}, True),
         ({"payload": {"features": {}, "env": {"TASKCLUSTER_INTERACTIVE": "x"}}}, True),
         ({}, True),
     ),
 )
-def test_check_interactive_docker_worker(task, has_errors):
+def test_check_interactive_docker_worker(task, raises):
     link = MagicMock()
     link.name = "foo"
     link.task = task
-    result = cotverify.check_interactive_docker_worker(link)
-    if has_errors:
-        assert len(result) >= 1
+    if raises:
+        with pytest.raises(CoTError):
+            cotverify.check_interactive_docker_worker(link)
+    else:
+        cotverify.check_interactive_docker_worker(link)
+
+
+# check_interactive_generic_worker {{{1
+@pytest.mark.parametrize(
+    "task,raises",
+    (
+        ({"payload": {}, "scopes": []}, False),
+        ({"payload": {"rdpInfo": {"foo": "bar"}}, "scopes": []}, True),
+        ({"payload": {}, "scopes": ["foo", "generic-worker:allow-rdp:foo:bar"]}, True),
+        ({}, True),
+    ),
+)
+def test_check_interactive_generic_worker(task, raises):
+    link = MagicMock()
+    link.name = "foo"
+    link.task = task
+    if raises:
+        with pytest.raises(CoTError):
+            cotverify.check_interactive_generic_worker(link)
     else:
-        assert result == []
+        cotverify.check_interactive_generic_worker(link)
 
 
 # verify_docker_image_sha {{{1
@@ -1892,17 +1913,24 @@ async def test_verify_docker_worker_task(mocker):
     mocker.patch.object(cotverify, "check_interactive_docker_worker", new=check.method1)
     mocker.patch.object(cotverify, "verify_docker_image_sha", new=check.method2)
     await cotverify.verify_docker_worker_task(chain, chain)
-    check.method1.assert_not_called()
+    check.method1.assert_called_once_with(chain)
     check.method2.assert_not_called()
     await cotverify.verify_docker_worker_task(chain, link)
-    check.method1.assert_called_once_with(link)
+    check.method1.assert_called_with(link)
     check.method2.assert_called_once_with(chain, link)
 
 
 # verify_generic_worker_task {{{1
 @pytest.mark.asyncio
 async def test_verify_generic_worker_task(mocker):
-    await cotverify.verify_generic_worker_task(MagicMock(), MagicMock())
+    chain = MagicMock()
+    link = MagicMock()
+    check = MagicMock()
+    mocker.patch.object(cotverify, "check_interactive_generic_worker", new=check.method1)
+    await cotverify.verify_generic_worker_task(chain, chain)
+    check.method1.assert_called_once_with(chain)
+    await cotverify.verify_generic_worker_task(chain, link)
+    check.method1.assert_called_with(link)
 
 
 # verify_worker_impls {{{1

version.json

@@ -1,8 +1,8 @@
 {
     "version":[
-        33,
-        1,
-        1
+        34,
+        0,
+        0
     ],
-    "version_string":"33.1.1"
+    "version_string":"34.0.0"
 }