Fix configuration error related to YAML anchors

PyYAML is efficient with YAML anchors and will only construct one
Python object for a YAML mapping that is used multiple times via
anchors.

We copy job variables (a mapping) straight from the YAML to an
attribute of the Job object, then we freeze the Job object which
converts all of its dicts to mappingproxy objects.  This mutates
the contents of the vars dict, and if that is used on another
job via an anchor, we will end up with mappingproxy objects in
what we think is a "raw yaml" configuration dict, and we will not
be able to serialize it in case of errors.

To address this, perform a deep copy of every configuration yaml
blob before parsing it into an object.

Change-Id: Idaa6ff78b1ac5a108fb9f43700cf8e66192c43ce

2 files changed, 73 insertions, 0 deletions

diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index de8b8f3ad..004ede862 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -1557,6 +1557,43 @@ class TestInRepoConfig(ZuulTestCase):
                                     'start_line': 5},
                           })
 
+    def test_dynamic_config_job_anchors(self):
+        # Test the use of anchors in job configuration.  This is a
+        # regression test designed to catch a failure where we freeze
+        # the first job and in doing so, mutate the vars dict.  The
+        # intended behavior is that the two jobs end up with two
+        # separate python objects for their vars dicts.
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: myvars
+                vars: &anchor
+                  plugins:
+                    foo: bar
+
+            - job:
+                name: project-test1
+                timeout: 999999999999
+                vars: *anchor
+
+            - project:
+                name: org/project
+                check:
+                  jobs:
+                    - project-test1
+            """)
+
+        file_dict = {'.zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertEqual(A.reported, 1,
+                         "A should report failure")
+        self.assertEqual(A.patchsets[0]['approvals'][0]['value'], "-1")
+        self.assertIn('max-job-timeout', A.messages[0])
+        self.assertHistory([])
+
     def test_dynamic_config_non_existing_job_in_template(self):
         """Test that requesting a non existent job fails"""
         in_repo_conf = textwrap.dedent(
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 16bff1401..f01be597b 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -437,6 +437,30 @@ def ansible_vars_dict(value):
         ansible_var_name(key)
 
 
+def copy_safe_config(conf):
+    """Return a deep copy of a config dictionary.
+
+    This lets us assign values of a config dictionary to configuration
+    objects, even if those values are nested dictionaries.  This way
+    we can safely freeze the configuration object (the process of
+    which mutates dictionaries) without mutating the original
+    configuration.
+
+    Meanwhile, this does retain the original context information as a
+    single object (some behaviors rely on mutating the source context
+    (e.g., pragma)).
+
+    """
+    ret = copy.deepcopy(conf)
+    for key in (
+            '_source_context',
+            '_start_mark',
+    ):
+        if key in conf:
+            ret[key] = conf[key]
+    return ret
+
+
 class PragmaParser(object):
     pragma = {
         'implied-branch-matchers': bool,
@@ -452,6 +476,7 @@ class PragmaParser(object):
         self.pcontext = pcontext
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
 
         bm = conf.get('implied-branch-matchers')
@@ -512,6 +537,7 @@ class NodeSetParser(object):
         return vs.Schema(nodeset)
 
     def fromYaml(self, conf, anonymous=False):
+        conf = copy_safe_config(conf)
         if anonymous:
             self.anon_schema(conf)
             self.anonymous = True
@@ -599,6 +625,7 @@ class SecretParser(object):
         return vs.Schema(secret)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         s = model.Secret(conf['name'], conf['_source_context'])
         s.source_context = conf['_source_context']
@@ -723,6 +750,7 @@ class JobParser(object):
 
     def fromYaml(self, conf, project_pipeline=False, name=None,
                  validate=True):
+        conf = copy_safe_config(conf)
         if validate:
             self.schema(conf)
 
@@ -1075,6 +1103,7 @@ class ProjectTemplateParser(object):
         return vs.Schema(project)
 
     def fromYaml(self, conf, validate=True, freeze=True):
+        conf = copy_safe_config(conf)
         if validate:
             self.schema(conf)
         source_context = conf['_source_context']
@@ -1165,6 +1194,7 @@ class ProjectParser(object):
         return vs.Schema(project)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
 
         project_name = conf.get('name')
@@ -1328,6 +1358,7 @@ class PipelineParser(object):
         return vs.Schema(pipeline)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         pipeline = model.Pipeline(conf['name'], self.pcontext.tenant)
         pipeline.source_context = conf['_source_context']
@@ -1469,6 +1500,7 @@ class SemaphoreParser(object):
         return vs.Schema(semaphore)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         semaphore = model.Semaphore(conf['name'], conf.get('max', 1))
         semaphore.source_context = conf.get('_source_context')
@@ -1494,6 +1526,7 @@ class QueueParser:
         return vs.Schema(queue)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         queue = model.Queue(
             conf['name'],
@@ -1523,6 +1556,7 @@ class AuthorizationRuleParser(object):
         return vs.Schema(authRule)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         a = model.AuthZRuleTree(conf['name'])
 
@@ -1556,6 +1590,7 @@ class GlobalSemaphoreParser(object):
         return vs.Schema(semaphore)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         semaphore = model.Semaphore(conf['name'], conf.get('max', 1),
                                     global_scope=True)
@@ -1576,6 +1611,7 @@ class ApiRootParser(object):
         return vs.Schema(api_root)
 
     def fromYaml(self, conf):
+        conf = copy_safe_config(conf)
         self.schema(conf)
         api_root = model.ApiRoot(conf.get('authentication-realm'))
         api_root.access_rules = conf.get('access-rules', [])