From b5199b258c27eef2f9900a20f2a7cfea4974d823 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 20 Mar 2023 16:58:49 -0700 Subject: [PATCH] Add implied-branch-matchers to tenant config Options such as always-dynamic-branches or exclude-unprotected-branches can cause Zuul to toggle between using or not using implied branch matchers as projects move between having one or more than one branch. Let operators avoid that by specifying the intended behavior if necessary. Change-Id: Ib8efa224fc396220ae85896845be4a908ac1008d --- doc/source/config/job.rst | 4 +- doc/source/config/pragma.rst | 7 ++- doc/source/tenants.rst | 21 ++++++++ ...lied-branch-matchers-721614143082f33b.yaml | 10 ++++ .../git/common-config/zuul.yaml | 53 +++++++++++++++++++ .../git/org_project/README | 1 + .../git/org_project/playbooks/test-job.yaml | 2 + .../git/org_project/zuul.yaml | 2 + .../tenant-implied-branch-matchers/main.yaml | 9 ++++ tests/unit/test_v3.py | 42 +++++++++++++++ zuul/configloader.py | 14 +++-- zuul/model.py | 9 ++-- 12 files changed, 166 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/tenant-implied-branch-matchers-721614143082f33b.yaml create mode 100644 tests/fixtures/config/tenant-implied-branch-matchers/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/README create mode 100644 tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/playbooks/test-job.yaml create mode 100644 tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/zuul.yaml create mode 100644 tests/fixtures/config/tenant-implied-branch-matchers/main.yaml diff --git a/doc/source/config/job.rst b/doc/source/config/job.rst index f9e7b7b71b..da9ce23ce9 100644 --- a/doc/source/config/job.rst +++ b/doc/source/config/job.rst @@ -1036,7 +1036,9 @@ Here is an example of two job definitions: only affect it. See :attr:`pragma.implied-branch-matchers` for how to override - this behavior on a per-file basis. + this behavior on a per-file basis. The behavior may also be + configured by a Zuul administrator using + :attr:`tenant.untrusted-projects..implied-branch-matchers`. .. attr:: files diff --git a/doc/source/config/pragma.rst b/doc/source/config/pragma.rst index 6b5eefb3c0..30a39ac691 100644 --- a/doc/source/config/pragma.rst +++ b/doc/source/config/pragma.rst @@ -25,7 +25,7 @@ pragma directives may not be set and then unset within the same file. .. attr:: implied-branch-matchers This is a boolean, which, if set, may be used to enable - (``True``) or disable (``False``) the addition of implied branch + (``true``) or disable (``false``) the addition of implied branch matchers to job and project-template definitions. Normally Zuul decides whether to add these based on heuristics described in :attr:`job.branches`. This attribute overrides that behavior. @@ -33,6 +33,11 @@ pragma directives may not be set and then unset within the same file. This can be useful if a project has multiple branches, yet the jobs defined in the master branch should apply to all branches. + The behavior may also be configured by a Zuul administrator + using + :attr:`tenant.untrusted-projects..implied-branch-matchers`. + This pragma overrides that setting if both are present. + Note that if a job contains an explicit branch matcher, it will be used regardless of the value supplied here. diff --git a/doc/source/tenants.rst b/doc/source/tenants.rst index b43b0bedfa..476670b85c 100644 --- a/doc/source/tenants.rst +++ b/doc/source/tenants.rst @@ -271,6 +271,27 @@ configuration. Some examples of tenant definitions are: circumstances and only calculate the configuration of a single additional branch when it is used. + .. attr:: implied-branch-matchers + + This is a boolean, which, if set, may be used to enable + (``true``) or disable (``false``) the addition of implied + branch matchers to job and project-template definitions. + Normally Zuul decides whether to add these based on + heuristics described in :attr:`job.branches`. This + attribute overrides that behavior. + + This can be useful if branch settings for this project may + produce an unpredictable number of branches to load from. + Setting this value explicitly here can avoid unexpected + behavior changes as branches are added or removed from the + load set. + + The :attr:`pragma.implied-branch-matchers` pragma will + override the setting here if present. + + Note that if a job contains an explicit branch matcher, it + will be used regardless of the value supplied here. + .. attr:: extra-config-paths Normally Zuul loads in-repo configuration from the first diff --git a/releasenotes/notes/tenant-implied-branch-matchers-721614143082f33b.yaml b/releasenotes/notes/tenant-implied-branch-matchers-721614143082f33b.yaml new file mode 100644 index 0000000000..b0a4f1d7ce --- /dev/null +++ b/releasenotes/notes/tenant-implied-branch-matchers-721614143082f33b.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + A new tenant project configuration option, + `implied-branch-matchers` has been added. This is useful in + conjunction with the branch exclusion options which may cause Zuul + to change behavior as the set of load branches switches between + one and more than one. The operator can fix the appropriate + behavior in the tenant config file rather than relying on the + heuristic in these cases. diff --git a/tests/fixtures/config/tenant-implied-branch-matchers/git/common-config/zuul.yaml b/tests/fixtures/config/tenant-implied-branch-matchers/git/common-config/zuul.yaml new file mode 100644 index 0000000000..7a8b45e34a --- /dev/null +++ b/tests/fixtures/config/tenant-implied-branch-matchers/git/common-config/zuul.yaml @@ -0,0 +1,53 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- pipeline: + name: gate + manager: dependent + post-review: True + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + success: + gerrit: + Verified: 2 + submit: true + failure: + gerrit: + Verified: -2 + start: + gerrit: + Verified: 0 + precedence: high + +- job: + name: base + parent: null + +- project: + name: common-config + check: + jobs: [] + gate: + jobs: + - noop + +- project: + name: org/project + check: + jobs: [] + gate: + jobs: + - noop diff --git a/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/README b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/playbooks/test-job.yaml b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/playbooks/test-job.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/playbooks/test-job.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/zuul.yaml b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/zuul.yaml new file mode 100644 index 0000000000..95a306b6e0 --- /dev/null +++ b/tests/fixtures/config/tenant-implied-branch-matchers/git/org_project/zuul.yaml @@ -0,0 +1,2 @@ +- job: + name: test-job diff --git a/tests/fixtures/config/tenant-implied-branch-matchers/main.yaml b/tests/fixtures/config/tenant-implied-branch-matchers/main.yaml new file mode 100644 index 0000000000..668884590d --- /dev/null +++ b/tests/fixtures/config/tenant-implied-branch-matchers/main.yaml @@ -0,0 +1,9 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project: + implied-branch-matchers: true diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 22d9518a93..86aac97767 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -6093,6 +6093,48 @@ class TestPragmaMultibranch(ZuulTestCase): ], ordered=False) +class TestTenantImpliedBranchMatchers(ZuulTestCase): + tenant_config_file = 'config/tenant-implied-branch-matchers/main.yaml' + + def test_tenant_implied_branch_matchers(self): + # Test that we can force implied branch matchers in the tenant + # config even in the case where a project only has one branch. + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + jobs = tenant.layout.getJobs('test-job') + self.assertEqual(len(jobs), 1) + for job in tenant.layout.getJobs('test-job'): + self.assertIsNotNone(job.branch_matcher) + + def test_pragma_overrides_tenant_implied_branch_matchers(self): + # Test that we can force implied branch matchers off with a pragma + # even if the tenant config has it set on. + config = textwrap.dedent( + """ + - job: + name: test-job + - pragma: + implied-branch-matchers: False + """) + file_dict = {'zuul.yaml': config} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.setMerged() + self.fake_gerrit.addEvent(A.getChangeMergedEvent()) + self.waitUntilSettled() + + self.create_branch('org/project', 'stable/pike') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable/pike')) + self.waitUntilSettled() + + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + jobs = tenant.layout.getJobs('test-job') + self.assertEqual(len(jobs), 2) + for job in tenant.layout.getJobs('test-job'): + self.assertIsNone(job.branch_matcher) + + class TestBaseJobs(ZuulTestCase): tenant_config_file = 'config/base-jobs/main.yaml' diff --git a/zuul/configloader.py b/zuul/configloader.py index fe22fe0f85..e60cadadc5 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1639,7 +1639,7 @@ class ParseContext(object): def getImpliedBranches(self, source_context): # If the user has set a pragma directive for this, use the - # value (if unset, the value is None). + # value (ixf unset, the value is None). if source_context.implied_branch_matchers is True: if source_context.implied_branches is not None: return source_context.implied_branches @@ -1691,6 +1691,7 @@ class TenantParser(object): 'exclude-branches': to_list(str), 'always-dynamic-branches': to_list(str), 'allow-circular-dependencies': bool, + 'implied-branch-matchers': bool, }} project = vs.Any(str, project_dict) @@ -1934,6 +1935,7 @@ class TenantParser(object): project_exclude_branches = None project_always_dynamic_branches = None project_load_branch = None + project_implied_branch_matchers = None else: project_name = list(conf.keys())[0] project = source.getProject(project_name) @@ -1988,6 +1990,8 @@ class TenantParser(object): if x.endswith('/')]) project_load_branch = conf[project_name].get( 'load-branch', None) + project_implied_branch_matchers = conf[project_name].get( + 'implied-branch-matchers', None) tenant_project_config = model.TenantProjectConfig(project) tenant_project_config.load_classes = frozenset(project_include) @@ -2001,6 +2005,8 @@ class TenantParser(object): tenant_project_config.extra_config_files = extra_config_files tenant_project_config.extra_config_dirs = extra_config_dirs tenant_project_config.load_branch = project_load_branch + tenant_project_config.implied_branch_matchers = \ + project_implied_branch_matchers return tenant_project_config @@ -2175,7 +2181,8 @@ class TenantParser(object): # accumulates a list of all merger jobs submitted. source_context = model.SourceContext( project.canonical_name, project.name, - project.connection_name, branch, '', False) + project.connection_name, branch, '', False, + tpc.implied_branch_matchers) if min_ltimes is not None: files_cache = self.unparsed_config_cache.getFilesCache( project.canonical_name, branch) @@ -2915,7 +2922,8 @@ class ConfigLoader(object): if data: source_context = model.SourceContext( project.canonical_name, project.name, - project.connection_name, branch, fn, trusted) + project.connection_name, branch, fn, trusted, + tpc.implied_branch_matchers) # Prevent mixing configuration source conf_root = fn.split('/')[0] diff --git a/zuul/model.py b/zuul/model.py index 5be5923a5d..a0f7f14397 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1890,7 +1890,8 @@ class SourceContext(ConfigObject): originate.""" def __init__(self, project_canonical_name, project_name, - project_connection_name, branch, path, trusted): + project_connection_name, branch, path, trusted, + implied_branch_matchers=None): super(SourceContext, self).__init__() self.project_canonical_name = project_canonical_name self.project_name = project_name @@ -1898,7 +1899,7 @@ class SourceContext(ConfigObject): self.branch = branch self.path = path self.trusted = trusted - self.implied_branch_matchers = None + self.implied_branch_matchers = implied_branch_matchers self.implied_branches = None def __str__(self): @@ -1915,7 +1916,8 @@ class SourceContext(ConfigObject): def copy(self): return self.__class__( self.project_canonical_name, self.project_name, - self.project_connection_name, self.branch, self.path, self.trusted) + self.project_connection_name, self.branch, self.path, self.trusted, + self.implied_branch_matchers) def isSameProject(self, other): if not isinstance(other, SourceContext): @@ -7179,6 +7181,7 @@ class TenantProjectConfig(object): # Load config from a different branch if this is a config project self.load_branch = None self.merge_modes = None + self.implied_branch_matchers = None def isAlwaysDynamicBranch(self, branch): if self.always_dynamic_branches is None: