Optionally limit github to protected branches

When using a branch and pull model on a shared repository there are
usually one or more protected branches which are gated and a dynamic
number of temporary personal/feature branches which are the source for
the pull requests. These temporary branches while ungated can
potentially include broken zuul config and therefore break the global
tenant wide configuration.

In order to deal with this model add support for excluding unprotected
branches. This can be configured on tenant level and overridden per
project.

Change-Id: I8a45fd41539a3c964a84142f04c1644585c0fdcf

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

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):

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

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

tests/fixtures/config/unprotected-branches/git/org_project1/README

@@ -0,0 +1 @@
+test

tests/fixtures/config/unprotected-branches/git/org_project1/playbooks/project-test.yaml

@@ -0,0 +1,2 @@
+- hosts: all
+  tasks: []

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

tests/fixtures/config/unprotected-branches/git/org_project2/README

@@ -0,0 +1 @@
+test

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.

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

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'
 

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()))

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

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/')]

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)

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()

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)

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
 

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."""

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

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.

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