Merge pull request mozilla-releng#178 from escapewindow/cotv2-fix-act…

…ions render actions from actions.json; stop multiple jsone.render passes
escapewindow · Jan 22, 2018 · 54892d8 · 54892d8
2 parents 3633ae1 + a7858ac
commit 54892d8
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,18 @@
 All notable changes to this project will be documented in this file.
 This project adheres to [Semantic Versioning](http://semver.org/).
 
+## [8.0.0] - 2018-01-19
+### Added
+- Added `scriptworker.cot.verify.get_jsone_template`, because action tasks use actions.json instead of .taskcluster.yml
+
+### Changed
+- Added a `tasks_for` argument to `populate_jsone_context`.
+- Used `format_json` instead of `pprint.pformat` in most `scriptworker.cot.verify` functions.
+
+### Removed
+- Removed `scriptworker.utils.render_jsone`, since it reduced to a `jsone.render` call.
+- Removed the now-unused `scriptworker.constants.max_jsone_iterations`
+
 ## [7.0.0] - 2018-01-18
 ### Added
 - Added `scriptworker.cot.verify.verify_parent_task_definition`. This is the core change in this release, aka CoT version 2. We now use json-e to rebuild the decision/action task definitions from the tree.

diff --git a/scriptworker/constants.py b/scriptworker/constants.py
@@ -67,7 +67,6 @@
     "cot_version": 2,
     "min_cot_version": 1,
     "max_chain_length": 20,
-    "max_jsone_iterations": 10,
 
     # Specify a default gpg home other than ~/.gnupg
     "gpg_home": None,

diff --git a/scriptworker/cot/verify.py b/scriptworker/cot/verify.py
@@ -61,7 +61,6 @@
     match_url_regex,
     raise_future_exceptions,
     remove_empty_keys,
-    render_jsone,
     rm,
 )
 from taskcluster.exceptions import TaskclusterFailure
@@ -907,7 +906,7 @@ def verify_task_in_task_graph(task_link, graph_defn, level=logging.CRITICAL):
         if value != runtime_defn[key]:
             errors.append("{} {} {} differs!\n graph: {}\n task: {}".format(
                 task_link.name, task_link.task_id, key,
-                pprint.pformat(value), pprint.pformat(runtime_defn[key])
+                format_json(value), format_json(runtime_defn[key])
             ))
     raise_on_errors(errors, level=level)
 
@@ -1114,15 +1113,9 @@ async def get_scm_level(context, project):
 
 # populate_jsone_context {{{1
 async def _get_additional_action_jsone_context(parent_link, decision_link):
-    actions_path = decision_link.get_artifact_full_path('public/actions.json')
     params_path = decision_link.get_artifact_full_path('public/parameters.yml')
-    action_name = get_action_name(parent_link.task)
-    all_actions = load_json_or_yaml(actions_path, is_path=True)['actions']
-    actions_tmpl = [d for d in all_actions if d['name'] == action_name][0]
     parameters = load_json_or_yaml(params_path, is_path=True, file_type='yaml')
     jsone_context = deepcopy(parent_link.task['extra']['action']['context'])
-    jsone_context['push'] = deepcopy(actions_tmpl['task']['$let']['push'])
-    jsone_context['action'] = deepcopy(actions_tmpl['task']['$let']['action'])
     jsone_context['parameters'] = parameters
     jsone_context['task'] = None
     jsone_context['ownTaskId'] = parent_link.task_id
@@ -1139,8 +1132,7 @@ async def _get_additional_hgpush_jsone_context(parent_link, decision_link):
         raise CoTError(
             "Decision task {} comment doesn't match the push comment!\n"
             "Decision comment: \n{}\nPush comment: \n{}".format(
-                decision_link.name, pprint.pformat(decision_comment),
-                pprint.pformat(push_comment)
+                decision_link.name, decision_comment, push_comment
             )
         )
     return {
@@ -1169,13 +1161,15 @@ async def _get_additional_cron_jsone_context(parent_link, decision_link):
     return jsone_context
 
 
-async def populate_jsone_context(chain, parent_link, decision_link):
+async def populate_jsone_context(chain, parent_link, decision_link, tasks_for):
     """Populate the json-e context to rebuild ``parent_link``'s task definition.
 
     Args:
         chain (ChainOfTrust): the chain of trust to add to.
         parent_link (LinkOfTrust): the parent link to test.
         decision_link (LinkOfTrust): the parent link's decision task link.
+        tasks_for (str): the reason the parent link was created (cron,
+            hg-push, action)
 
     Raises:
         CoTError, KeyError, ValueError: on failure.
@@ -1192,9 +1186,6 @@ async def populate_jsone_context(chain, parent_link, decision_link):
     source_url = get_source_url(decision_link)
     project = get_and_check_project(chain.context.config['valid_vcs_rules'], source_url)
     level = await get_scm_level(chain.context, project)
-    tasks_for = get_and_check_tasks_for(
-        parent_link.task, '{} {}: '.format(parent_link.name, parent_link.task_id)
-    )
     log.debug("task_ids: {}".format(task_ids))
     jsone_context = {
         'now': parent_link.task['created'],
@@ -1219,11 +1210,44 @@ async def populate_jsone_context(chain, parent_link, decision_link):
         jsone_context.update(
             await _get_additional_cron_jsone_context(parent_link, decision_link)
         )
-    log.debug("json-e context:")
+    log.debug("{} json-e context:".format(parent_link.name))
     log.debug(pprint.pformat(jsone_context))
     return jsone_context
 
 
+# get_jsone_template {{{1
+async def get_jsone_template(parent_link, decision_link, tasks_for):
+    """Get the appropriate json-e template.
+
+    Args:
+        parent_link (LinkOfTrust): the parent link to test.
+        decision_link (LinkOfTrust): the parent link's decision task link.
+        tasks_for (str): the reason the parent link was created (cron,
+            hg-push, action)
+
+    Returns:
+        dict: the json-e template.
+
+    """
+    if tasks_for == 'action':
+        actions_path = decision_link.get_artifact_full_path('public/actions.json')
+        all_actions = load_json_or_yaml(actions_path, is_path=True)['actions']
+        action_name = get_action_name(parent_link.task)
+        tmpl = [d for d in all_actions if d['name'] == action_name][0]['task']
+    else:
+        context = decision_link.context
+        source_url = get_source_url(decision_link)
+        tmpl = await load_json_or_yaml_from_url(
+            context, source_url, os.path.join(
+                context.config["work_dir"], "{}_taskcluster.yml".format(decision_link.name)
+            )
+        )
+        tmpl = tmpl['tasks'][0]
+    log.debug("{} json-e template:".format(parent_link.name))
+    log.debug(format_json(tmpl))
+    return tmpl
+
+
 # verify_parent_task_definition {{{1
 async def verify_parent_task_definition(chain, parent_link):
     """Rebuild the decision/action/cron task definition via json-e.
@@ -1247,24 +1271,16 @@ async def verify_parent_task_definition(chain, parent_link):
 
     """
     log.info("Verifying {} {} definition...".format(parent_link.name, parent_link.task_id))
-    context = chain.context
     decision_link = chain.get_link(parent_link.decision_task_id)
-    # Download template from the decision task.
-    source_url = get_source_url(decision_link)
-    tmpl = await load_json_or_yaml_from_url(
-        context, source_url, os.path.join(
-            context.config["work_dir"], "{}_taskcluster.yml".format(decision_link.name)
-        )
-    )
-    # Populate template (jsone)
     try:
-        jsone_context = await populate_jsone_context(
-            chain, parent_link, decision_link
+        tasks_for = get_and_check_tasks_for(
+            parent_link.task, '{} {}: '.format(parent_link.name, parent_link.task_id)
         )
-        rebuilt_definition = render_jsone(
-            tmpl, jsone_context, max_iterations=context.config['max_jsone_iterations']
+        tmpl = await get_jsone_template(parent_link, decision_link, tasks_for)
+        jsone_context = await populate_jsone_context(
+            chain, parent_link, decision_link, tasks_for
         )
-        compare_definition = deepcopy(rebuilt_definition["tasks"][0])
+        rebuilt_definition = jsone.render(tmpl, jsone_context)
     except jsone.JSONTemplateError as e:
         log.exception("JSON-e error while rebuilding {} task definition!".format(parent_link.name))
         raise CoTError("JSON-e error while rebuilding {} task definition: {}".format(parent_link.name, str(e)))
@@ -1275,9 +1291,10 @@ async def verify_parent_task_definition(chain, parent_link):
         log.exception(msg)
         raise CoTError(msg + "\n{}".format(str(e)))
 
-    compare_jsone_task_definition(parent_link, compare_definition)
+    compare_jsone_task_definition(parent_link, rebuilt_definition)
 
 
+# compare_jsone_task_definition {{{1
 def compare_jsone_task_definition(parent_link, compare_definition):
     """Compare the json-e rebuilt task definition vs the runtime definition.
 
@@ -1297,11 +1314,12 @@ def compare_jsone_task_definition(parent_link, compare_definition):
         # them instead of keeping them with a None/{}/[] value.
         compare_definition = remove_empty_keys(compare_definition)
         runtime_definition = remove_empty_keys(parent_link.task)
-        log.debug("Compare_definition:\n{}".format(pprint.pformat(compare_definition)))
-        log.debug("Runtime definition:\n{}".format(pprint.pformat(runtime_definition)))
+        log.debug("Compare_definition:\n{}".format(format_json(compare_definition)))
+        log.debug("Runtime definition:\n{}".format(format_json(runtime_definition)))
         diff = list(dictdiffer.diff(compare_definition, runtime_definition))
         if diff:
             raise AssertionError(pprint.pformat(diff))
+        log.info("{}: Good.".format(parent_link.name))
     except (AssertionError, KeyError):
         msg = "{} {}: the rebuilt definition doesn't match the runtime task!".format(
             parent_link.name, parent_link.task_id
@@ -1818,7 +1836,7 @@ def verify_cot_cmdln(args=None):
             })
             cot = ChainOfTrust(context, opts.task_type, task_id=opts.task_id)
             loop.run_until_complete(verify_chain_of_trust(cot))
-            log.info(pprint.pformat(cot.dependent_task_ids()))
+            log.info(format_json(cot.dependent_task_ids()))
             log.info("{} : {}".format(cot.name, cot.task_id))
             for link in cot.links:
                 log.info("{} : {}".format(link.name, link.task_id))

diff --git a/scriptworker/test/test_utils.py b/scriptworker/test/test_utils.py
@@ -482,17 +482,3 @@ def test_add_enumerable_item_to_dict(dict_, key, item, expected):
 )))
 def test_remove_empty_keys(orig, expected):
     assert utils.remove_empty_keys(orig) == expected
-
-
-# render_jsone {{{1
-@pytest.mark.parametrize("tmpl,jsone_context,expected,max_iterations", ((
-    {'foo': '${x}'}, {'x': 'bar'}, {'foo': 'bar'}, 1
-), (
-    {'foo': '$${input.foo}'}, {'input': {'foo': 'bar'}},
-    {'foo': 'bar'}, 1
-), (
-    {'foo': '$${input.foo}'}, {'input': {'foo': 'bar'}},
-    {'foo': '${input.foo}'}, 0
-)))
-def test_render_jsone(tmpl, jsone_context, expected, max_iterations):
-    assert utils.render_jsone(tmpl, jsone_context, max_iterations=max_iterations) == expected
diff --git a/scriptworker/utils.py b/scriptworker/utils.py
@@ -12,7 +12,6 @@
 import functools
 import hashlib
 import json
-import jsone
 import logging
 import os
 import random
@@ -632,27 +631,3 @@ def remove_empty_keys(values, remove=({}, None, [], 'null')):
                 for value in deepcopy(values) if value not in remove]
 
     return values
-
-
-# render_jsone {{{1
-def render_jsone(tmpl, jsone_context, max_iterations=10):
-    """Render json-e from the template and json-e context, up to ``max_iterations`` times.
-
-    Args:
-        tmpl (dict): the json-e template to render.
-        jsone_context (dict): the json-e context to render the template with.
-        max_iterations (int, optional): the maximum number of additional
-            `jsone.render` passes to run through, since templates can define
-            values that get rendered on a pass > 1.
-
-    Returns:
-        dict: the rendered json-e.
-
-    """
-    rebuilt_definition = jsone.render(tmpl, jsone_context)
-    for attempt in range(0, max_iterations):
-        new_rebuild = jsone.render(rebuilt_definition, jsone_context)
-        if new_rebuild == rebuilt_definition:
-            break
-        rebuilt_definition = new_rebuild
-    return rebuilt_definition
diff --git a/scriptworker/version.py b/scriptworker/version.py
@@ -52,7 +52,7 @@ def get_version_string(version):
 
 # 1}}}
 # Semantic versioning 2.0.0  http://semver.org/
-__version__ = (7, 0, 0)
+__version__ = (8, 0, 0)
 __version_string__ = get_version_string(__version__)
 
 

diff --git a/version.json b/version.json
@@ -1,8 +1,8 @@
 {
     "version": [
-        7,
+        8,
         0,
         0
     ],
-    "version_string": "7.0.0"
+    "version_string": "8.0.0"
 }