diff --git a/doc/source/admin/tenants.rst b/doc/source/admin/tenants.rst index 1f8f7dbd87..b3b2d9cef9 100644 --- a/doc/source/admin/tenants.rst +++ b/doc/source/admin/tenants.rst @@ -29,6 +29,7 @@ configuration. An example tenant definition is:: - tenant: name: my-tenant max-nodes-per-job: 5 + exclude-unprotected-branches: false source: gerrit: config-projects: @@ -39,7 +40,8 @@ configuration. An example tenant definition is:: - zuul-jobs: shadow: common-config - project1 - - project2 + - project2: + exclude-unprotected-branches: true The following attributes are supported: @@ -53,6 +55,16 @@ The following attributes are supported: The maximum number of nodes a job can request, default to 5. A '-1' value removes the limit. +**exclude-unprotected-branches** (optional) + When using a branch and pull model on a shared github repository there are + usually one or more protected branches which are gated and a dynamic number of + personal/feature branches which are the source for the pull requests. These + branches can potentially include broken zuul config and therefore break the + global tenant wide configuration. In order to deal with this zuul's operations + can be limited to the protected branches which are gated. This is a tenant + wide setting and can be overridden per project. If not specified, defaults + to ``false``. + **source** (required) A dictionary of sources to consult for projects. A tenant may contain projects from multiple sources; each of those sources must @@ -104,6 +116,10 @@ The following attributes are supported: "zuul-jobs" projects, the definition in "common-config" will be used. + **exclude-unprotected-branches** + Define if unprotected github branches should be processed. Defaults to the + tenant wide setting of exclude-unprotected-branches. + The order of the projects listed in a tenant is important. A job which is defined in one project may not be redefined in another project; therefore, once a job appears in one project, a project diff --git a/tests/base.py b/tests/base.py index eca31fe52e..0f188bd2f0 100755 --- a/tests/base.py +++ b/tests/base.py @@ -567,7 +567,10 @@ class FakeGithub(object): def __init__(self): self._branches = [FakeGithub.FakeBranch()] - def branches(self): + def branches(self, protected=False): + if protected: + # simulate there is no protected branch + return [] return self._branches def user(self, login): diff --git a/tests/fixtures/config/tenant-parser/unprotected-branches.yaml b/tests/fixtures/config/tenant-parser/unprotected-branches.yaml new file mode 100644 index 0000000000..bf2feefcd3 --- /dev/null +++ b/tests/fixtures/config/tenant-parser/unprotected-branches.yaml @@ -0,0 +1,11 @@ +- tenant: + name: tenant-one + exclude-unprotected-branches: true + source: + gerrit: + config-projects: + - common-config: + exclude-unprotected-branches: false + untrusted-projects: + - org/project1 + - org/project2 diff --git a/tests/fixtures/config/unprotected-branches/git/org_common-config/zuul.yaml b/tests/fixtures/config/unprotected-branches/git/org_common-config/zuul.yaml new file mode 100644 index 0000000000..c0fbf0d28a --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_common-config/zuul.yaml @@ -0,0 +1,19 @@ +- pipeline: + name: check + manager: independent + trigger: + github: + - event: pull_request + action: + - opened + - changed + - reopened + success: + github: + status: 'success' + failure: + github: + status: 'failure' + start: + github: + comment: true diff --git a/tests/fixtures/config/unprotected-branches/git/org_project1/README b/tests/fixtures/config/unprotected-branches/git/org_project1/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_project1/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/unprotected-branches/git/org_project1/playbooks/project-test.yaml b/tests/fixtures/config/unprotected-branches/git/org_project1/playbooks/project-test.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_project1/playbooks/project-test.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/unprotected-branches/git/org_project1/zuul.yaml b/tests/fixtures/config/unprotected-branches/git/org_project1/zuul.yaml new file mode 100644 index 0000000000..31abadf066 --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_project1/zuul.yaml @@ -0,0 +1,8 @@ +- job: + name: project-test + +- project: + name: org/project1 + check: + jobs: + - project-test diff --git a/tests/fixtures/config/unprotected-branches/git/org_project2/README b/tests/fixtures/config/unprotected-branches/git/org_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/unprotected-branches/git/org_project2/zuul.yaml b/tests/fixtures/config/unprotected-branches/git/org_project2/zuul.yaml new file mode 100644 index 0000000000..64d316d31d --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/git/org_project2/zuul.yaml @@ -0,0 +1 @@ +This zuul.yaml is intentionally broken and should not be loaded on startup. diff --git a/tests/fixtures/config/unprotected-branches/main.yaml b/tests/fixtures/config/unprotected-branches/main.yaml new file mode 100644 index 0000000000..8078d37326 --- /dev/null +++ b/tests/fixtures/config/unprotected-branches/main.yaml @@ -0,0 +1,10 @@ +- tenant: + name: tenant-one + source: + github: + config-projects: + - org/common-config + untrusted-projects: + - org/project1 + - org/project2: + exclude-unprotected-branches: true diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py index d08c6a1eda..1ba4ed94be 100644 --- a/tests/unit/test_configloader.py +++ b/tests/unit/test_configloader.py @@ -182,6 +182,7 @@ class TestTenantGroups3(TenantParserTestCase): def test_tenant_groups3(self): tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(False, tenant.exclude_unprotected_branches) self.assertEqual(['common-config'], [x.name for x in tenant.config_projects]) self.assertEqual(['org/project1', 'org/project2'], @@ -212,6 +213,29 @@ class TestTenantGroups3(TenantParserTestCase): project2_config.pipelines['check'].job_list.jobs) +class TestTenantUnprotectedBranches(TenantParserTestCase): + tenant_config_file = 'config/tenant-parser/unprotected-branches.yaml' + + def test_tenant_unprotected_branches(self): + tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(True, tenant.exclude_unprotected_branches) + + self.assertEqual(['common-config'], + [x.name for x in tenant.config_projects]) + self.assertEqual(['org/project1', 'org/project2'], + [x.name for x in tenant.untrusted_projects]) + + tpc = tenant.project_configs + project_name = tenant.config_projects[0].canonical_name + self.assertEqual(False, tpc[project_name].exclude_unprotected_branches) + + project_name = tenant.untrusted_projects[0].canonical_name + self.assertIsNone(tpc[project_name].exclude_unprotected_branches) + + project_name = tenant.untrusted_projects[1].canonical_name + self.assertIsNone(tpc[project_name].exclude_unprotected_branches) + + class TestSplitConfig(ZuulTestCase): tenant_config_file = 'config/split-config/main.yaml' diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 0e199dfa82..68fbe29cb4 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -683,3 +683,20 @@ class TestGithubDriver(ZuulTestCase): self.fake_github.emitEvent, ('ping', pevent), ) + + +class TestGithubUnprotectedBranches(ZuulTestCase): + config_file = 'zuul-github-driver.conf' + tenant_config_file = 'config/unprotected-branches/main.yaml' + + def test_unprotected_branches(self): + tenant = self.sched.abide.tenants.get('tenant-one') + + project1 = tenant.untrusted_projects[0] + project2 = tenant.untrusted_projects[1] + + # project1 should have parsed master + self.assertIn('master', project1.unparsed_branch_config.keys()) + + # project2 should have no parsed branch + self.assertEqual(0, len(project2.unparsed_branch_config.keys())) diff --git a/zuul/configloader.py b/zuul/configloader.py index 4b9b8a0051..26684dfe2c 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -929,6 +929,7 @@ class TenantParser(object): 'include': to_list(classes), 'exclude': to_list(classes), 'shadow': to_list(str), + 'exclude-unprotected-branches': bool, }} project = vs.Any(str, project_dict) @@ -965,7 +966,9 @@ class TenantParser(object): def getSchema(connections=None): tenant = {vs.Required('name'): str, 'max-nodes-per-job': int, - 'source': TenantParser.validateTenantSources(connections)} + 'source': TenantParser.validateTenantSources(connections), + 'exclude-unprotected-branches': bool, + } return vs.Schema(tenant) @staticmethod @@ -975,6 +978,10 @@ class TenantParser(object): tenant = model.Tenant(conf['name']) if conf.get('max-nodes-per-job') is not None: tenant.max_nodes_per_job = conf['max-nodes-per-job'] + if conf.get('exclude-unprotected-branches') is not None: + tenant.exclude_unprotected_branches = \ + conf['exclude-unprotected-branches'] + tenant.unparsed_config = conf unparsed_config = model.UnparsedTenantConfig() # tpcs is TenantProjectConfigs @@ -993,7 +1000,7 @@ class TenantParser(object): TenantParser._loadTenantInRepoLayouts(merger, connections, tenant.config_projects, tenant.untrusted_projects, - cached) + cached, tenant) unparsed_config.extend(tenant.config_projects_config) unparsed_config.extend(tenant.untrusted_projects_config) tenant.layout = TenantParser._parseLayout(base, tenant, @@ -1065,6 +1072,7 @@ class TenantParser(object): project = source.getProject(conf) project_include = current_include shadow_projects = [] + project_exclude_unprotected_branches = None else: project_name = list(conf.keys())[0] project = source.getProject(project_name) @@ -1078,10 +1086,14 @@ class TenantParser(object): as_list(conf[project_name].get('exclude', []))) if project_exclude: project_include = frozenset(project_include - project_exclude) + project_exclude_unprotected_branches = conf[project_name].get( + 'exclude-unprotected-branches', None) tenant_project_config = model.TenantProjectConfig(project) tenant_project_config.load_classes = frozenset(project_include) tenant_project_config.shadow_projects = shadow_projects + tenant_project_config.exclude_unprotected_branches = \ + project_exclude_unprotected_branches return tenant_project_config @@ -1148,7 +1160,7 @@ class TenantParser(object): @staticmethod def _loadTenantInRepoLayouts(merger, connections, config_projects, - untrusted_projects, cached): + untrusted_projects, cached, tenant): config_projects_config = model.UnparsedTenantConfig() untrusted_projects_config = model.UnparsedTenantConfig() jobs = [] @@ -1196,7 +1208,7 @@ class TenantParser(object): # branch. Remember the branch and then implicitly add a # branch selector to each job there. This makes the # in-repo configuration apply only to that branch. - for branch in project.source.getProjectBranches(project): + for branch in project.source.getProjectBranches(project, tenant): project.unparsed_branch_config[branch] = \ model.UnparsedTenantConfig() job = merger.getFiles( @@ -1416,11 +1428,11 @@ class ConfigLoader(object): new_abide.tenants[tenant.name] = new_tenant return new_abide - def _loadDynamicProjectData(self, config, project, files, trusted): + def _loadDynamicProjectData(self, config, project, files, trusted, tenant): if trusted: branches = ['master'] else: - branches = project.source.getProjectBranches(project) + branches = project.source.getProjectBranches(project, tenant) for branch in branches: fns1 = [] @@ -1472,11 +1484,12 @@ class ConfigLoader(object): if include_config_projects: config = model.UnparsedTenantConfig() for project in tenant.config_projects: - self._loadDynamicProjectData(config, project, files, True) + self._loadDynamicProjectData( + config, project, files, True, tenant) else: config = tenant.config_projects_config.copy() for project in tenant.untrusted_projects: - self._loadDynamicProjectData(config, project, files, False) + self._loadDynamicProjectData(config, project, files, False, tenant) layout = model.Layout(tenant) # NOTE: the actual pipeline objects (complete with queues and diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index d23857f2bd..6bf43d64f0 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -616,7 +616,7 @@ class GerritConnection(BaseConnection): (record.get('number'),)) return changes - def getProjectBranches(self, project: Project) -> List[str]: + def getProjectBranches(self, project: Project, tenant) -> List[str]: refs = self.getInfoRefs(project) heads = [str(k[len('refs/heads/'):]) for k in refs.keys() if k.startswith('refs/heads/')] diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index e41859eb8e..7141080ac0 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -54,8 +54,8 @@ class GerritSource(BaseSource): def getProjectOpenChanges(self, project): return self.connection.getProjectOpenChanges(project) - def getProjectBranches(self, project): - return self.connection.getProjectBranches(project) + def getProjectBranches(self, project, tenant): + return self.connection.getProjectBranches(project, tenant) def getGitUrl(self, project): return self.connection.getGitUrl(project) diff --git a/zuul/driver/git/gitconnection.py b/zuul/driver/git/gitconnection.py index f4fe7e5efb..0624088634 100644 --- a/zuul/driver/git/gitconnection.py +++ b/zuul/driver/git/gitconnection.py @@ -48,7 +48,7 @@ class GitConnection(BaseConnection): def addProject(self, project): self.projects[project.name] = project - def getProjectBranches(self, project): + def getProjectBranches(self, project, tenant): # TODO(jeblair): implement; this will need to handle local or # remote git urls. raise NotImplemented() diff --git a/zuul/driver/git/gitsource.py b/zuul/driver/git/gitsource.py index 61a328e650..8d85c082fe 100644 --- a/zuul/driver/git/gitsource.py +++ b/zuul/driver/git/gitsource.py @@ -45,8 +45,8 @@ class GitSource(BaseSource): self.connection.addProject(p) return p - def getProjectBranches(self, project): - return self.connection.getProjectBranches(project) + def getProjectBranches(self, project, tenant): + return self.connection.getProjectBranches(project, tenant) def getGitUrl(self, project): return self.connection.getGitUrl(project) diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 80ac57395e..616e774271 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -698,11 +698,21 @@ class GithubConnection(BaseConnection): def addProject(self, project): self.projects[project.name] = project - def getProjectBranches(self, project): + def getProjectBranches(self, project, tenant): + + # Evaluate if unprotected branches should be excluded or not. The first + # match wins. The order is project -> tenant (default is false). + project_config = tenant.project_configs.get(project.canonical_name) + if project_config.exclude_unprotected_branches is not None: + exclude_unprotected = project_config.exclude_unprotected_branches + else: + exclude_unprotected = tenant.exclude_unprotected_branches + github = self.getGithubClient() owner, proj = project.name.split('/') repository = github.repository(owner, proj) - branches = [branch.name for branch in repository.branches()] + branches = [branch.name for branch in repository.branches( + protected=exclude_unprotected)] log_rate_limit(self.log, github) return branches diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 1bd280fb71..1e7e07a88f 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -68,8 +68,8 @@ class GithubSource(BaseSource): self.connection.addProject(p) return p - def getProjectBranches(self, project): - return self.connection.getProjectBranches(project) + def getProjectBranches(self, project, tenant): + return self.connection.getProjectBranches(project, tenant) def getProjectOpenChanges(self, project): """Get the open changes for a project.""" diff --git a/zuul/driver/timer/__init__.py b/zuul/driver/timer/__init__.py index 4489808f04..69cd508746 100644 --- a/zuul/driver/timer/__init__.py +++ b/zuul/driver/timer/__init__.py @@ -81,7 +81,7 @@ class TimerDriver(Driver, TriggerInterface): def _onTrigger(self, tenant, pipeline_name, timespec): for project_name in tenant.layout.project_configs.keys(): (trusted, project) = tenant.getProject(project_name) - for branch in project.source.getProjectBranches(project): + for branch in project.source.getProjectBranches(project, tenant): event = TimerTriggerEvent() event.type = 'timer' event.timespec = timespec diff --git a/zuul/model.py b/zuul/model.py index 9be8745436..26a79636f4 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -2091,6 +2091,10 @@ class TenantProjectConfig(object): self.load_classes = set() self.shadow_projects = set() + # The tenant's default setting of exclude_unprotected_branches will + # be overridden by this one if not None. + self.exclude_unprotected_branches = None + class ProjectConfig(object): # Represents a project cofiguration @@ -2451,6 +2455,7 @@ class Tenant(object): def __init__(self, name): self.name = name self.max_nodes_per_job = 5 + self.exclude_unprotected_branches = False self.layout = None # The unparsed configuration from the main zuul config for # this tenant. diff --git a/zuul/source/__init__.py b/zuul/source/__init__.py index b37aeb4d1d..0396aff499 100644 --- a/zuul/source/__init__.py +++ b/zuul/source/__init__.py @@ -64,7 +64,7 @@ class BaseSource(object, metaclass=abc.ABCMeta): """Get a project.""" @abc.abstractmethod - def getProjectBranches(self, project): + def getProjectBranches(self, project, tenant): """Get branches for a project""" @abc.abstractmethod