From abf973e324f3b596202af82d13e1d2cd77624df4 Mon Sep 17 00:00:00 2001 From: Tobias Henkel Date: Fri, 28 Jul 2017 10:07:34 +0200 Subject: [PATCH] Fix dynamic layout with regex approval filters In case the layout includes some regex approval filters (like username) the dynamic layout creation fails during deepcopy of the pipelines [1]. This is caused by Github/GerritApprovalFilter overwriting stuff in the original data structure. Fix this by using the already existing deepcopied data structures. To test this an unused pipeline with approval filters for Github and Gerrit is added to the dynamic layout test cases. This triggers the deepcopy error without the fix in every test_dynamic_* test case. [1] Traceback (most recent call last): File "/usr/lib/python3.6/site-packages/zuul/manager/__init__.py", line 453, in _loadDynamicLayout include_config_projects=False) File "/usr/lib/python3.6/site-packages/zuul/configloader.py", line 1478, in createDynamicLayout config = tenant.config_projects_config.copy() File "/usr/lib/python3.6/site-packages/zuul/model.py", line 2150, in copy r.pipelines = copy.deepcopy(self.pipelines) File "/usr/lib/python3.6/copy.py", line 150, in deepcopy y = copier(x, memo) ... File "/usr/lib/python3.6/copy.py", line 240, in _deepcopy_dict y[deepcopy(key, memo)] = deepcopy(value, memo) File "/usr/lib/python3.6/copy.py", line 161, in deepcopy y = copier(memo) TypeError: cannot deepcopy this pattern object Change-Id: I4f7f74787aa91e938d0e2f07bd15b4e21d49cb88 --- .../config/in-repo/git/common-config/zuul.yaml | 17 +++++++++++++++++ tests/unit/test_scheduler.py | 1 + tests/unit/test_v3.py | 2 ++ zuul/driver/gerrit/gerritmodel.py | 5 +++-- zuul/driver/github/githubmodel.py | 4 ++-- 5 files changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/fixtures/config/in-repo/git/common-config/zuul.yaml b/tests/fixtures/config/in-repo/git/common-config/zuul.yaml index d4ffad6ab1..bfd61992f0 100644 --- a/tests/fixtures/config/in-repo/git/common-config/zuul.yaml +++ b/tests/fixtures/config/in-repo/git/common-config/zuul.yaml @@ -52,6 +52,23 @@ Verified: 0 precedence: high +# This pipeline is there to ensure that dynamic pipeline copy operations also +# work with regex approval filters. +- pipeline: + name: pipeline-with-regex + manager: independent + require: + gerrit: + approval: + - Code-Review: 2 + username: maintainer + require: + github: + review: + - username: '^(herp|derp)$' + type: approved + trigger: {} + - job: name: common-config-test diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index e52eea9a85..5dd3f4e63c 100755 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -5293,6 +5293,7 @@ class TestSemaphoreMultiTenant(ZuulTestCase): class TestSemaphoreInRepo(ZuulTestCase): + config_file = 'zuul-connections-gerrit-and-github.conf' tenant_config_file = 'config/in-repo/main.yaml' def test_semaphore_in_repo(self): diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 7dcb4ae32c..aa091e58e9 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -70,6 +70,7 @@ class TestMultipleTenants(AnsibleZuulTestCase): class TestInRepoConfig(ZuulTestCase): # A temporary class to hold new tests while others are disabled + config_file = 'zuul-connections-gerrit-and-github.conf' tenant_config_file = 'config/in-repo/main.yaml' def test_in_repo_config(self): @@ -712,6 +713,7 @@ class TestProjectKeys(ZuulTestCase): # sure we exercise that code, in this test we allow Zuul to create # keys for the project on startup. create_project_keys = True + config_file = 'zuul-connections-gerrit-and-github.conf' tenant_config_file = 'config/in-repo/main.yaml' def test_key_generation(self): diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py index f35c3e7159..7c1bb5a723 100644 --- a/zuul/driver/gerrit/gerritmodel.py +++ b/zuul/driver/gerrit/gerritmodel.py @@ -61,9 +61,10 @@ class GerritTriggerEvent(TriggerEvent): class GerritApprovalFilter(object): def __init__(self, required_approvals=[], reject_approvals=[]): self._required_approvals = copy.deepcopy(required_approvals) - self.required_approvals = self._tidy_approvals(required_approvals) + self.required_approvals = self._tidy_approvals( + self._required_approvals) self._reject_approvals = copy.deepcopy(reject_approvals) - self.reject_approvals = self._tidy_approvals(reject_approvals) + self.reject_approvals = self._tidy_approvals(self._reject_approvals) def _tidy_approvals(self, approvals): for a in approvals: diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py index db119f0fd6..ffd1c3f944 100644 --- a/zuul/driver/github/githubmodel.py +++ b/zuul/driver/github/githubmodel.py @@ -68,8 +68,8 @@ class GithubCommonFilter(object): reject_reviews=[]): self._required_reviews = copy.deepcopy(required_reviews) self._reject_reviews = copy.deepcopy(reject_reviews) - self.required_reviews = self._tidy_reviews(required_reviews) - self.reject_reviews = self._tidy_reviews(reject_reviews) + self.required_reviews = self._tidy_reviews(self._required_reviews) + self.reject_reviews = self._tidy_reviews(self._reject_reviews) self.required_statuses = required_statuses def _tidy_reviews(self, reviews):