Merge "Optionally limit github to protected branches" into feature/zuulv3

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

@@ -935,6 +935,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)
@@ -971,7 +972,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
@@ -981,6 +984,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
@@ -999,7 +1006,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,
@@ -1071,6 +1078,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)
@@ -1084,10 +1092,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
 
@@ -1154,7 +1166,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 = []
@@ -1202,7 +1214,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(
@@ -1422,11 +1434,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 = []
@@ -1478,11 +1490,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