From a557e140b15656d631b930f1d7707bce2b4175db Mon Sep 17 00:00:00 2001
From: Joshua Watt <JPEWhacker@gmail.com>
Date: Thu, 19 Jan 2023 16:20:38 -0600
Subject: [PATCH] Fix include-branches priority

Per the documentation, include-branches should be able to override
exclude-branches, but this was not the case in the way the code was
written. Rework the code to correct this, and also add a test to ensure
it works as documented

Change-Id: I2e23b1533c67ccf84b4d6a36f5a003adc7b3e45a
---
 ...xclude-include-priority-ea4a21ab1e53cb4a.yaml | 12 ++++++++++++
 .../tenant-parser/exclude-include-branches.yaml  | 16 ++++++++++++++++
 tests/unit/test_configloader.py                  |  6 ++++++
 zuul/model.py                                    | 13 ++-----------
 4 files changed, 36 insertions(+), 11 deletions(-)
 create mode 100644 releasenotes/notes/fix-exclude-include-priority-ea4a21ab1e53cb4a.yaml
 create mode 100644 tests/fixtures/config/tenant-parser/exclude-include-branches.yaml

diff --git a/releasenotes/notes/fix-exclude-include-priority-ea4a21ab1e53cb4a.yaml b/releasenotes/notes/fix-exclude-include-priority-ea4a21ab1e53cb4a.yaml
new file mode 100644
index 0000000000..9dd238faf4
--- /dev/null
+++ b/releasenotes/notes/fix-exclude-include-priority-ea4a21ab1e53cb4a.yaml
@@ -0,0 +1,12 @@
+---
+upgrade:
+  - |
+    Fixes `tenant.untrusted-projects.<project>.include-branches` being lower
+    priority than `tenant.untrusted-projects.<project>.exclude-branches` to
+    match the documentation and expected user behavior.
+
+    This only affects projects that are using both `include-branches` and
+    `exclude-branches` at the same time. Now, `include-branches` has priority
+    over `exclude-branches` for any branches that match both. Practically
+    speaking, this means that `exclude-branches` is ignored if
+    `include-branches` is set.
diff --git a/tests/fixtures/config/tenant-parser/exclude-include-branches.yaml b/tests/fixtures/config/tenant-parser/exclude-include-branches.yaml
new file mode 100644
index 0000000000..6d455802a6
--- /dev/null
+++ b/tests/fixtures/config/tenant-parser/exclude-include-branches.yaml
@@ -0,0 +1,16 @@
+- tenant:
+    name: tenant-one
+    source:
+      gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - org/project1:
+              exclude-branches:
+                - foo
+                - bar
+              # Include branches has higher priority than exclude branches
+              include-branches:
+                - foo
+                - bar
+          - org/project2
diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py
index d76eece128..81a05fc372 100644
--- a/tests/unit/test_configloader.py
+++ b/tests/unit/test_configloader.py
@@ -611,6 +611,12 @@ class TestTenantExcludeBranches(TestTenantIncludeBranches):
     # Same test results as include-branches
 
 
+class TestTenantExcludeIncludeBranches(TestTenantIncludeBranches):
+    tenant_config_file = 'config/tenant-parser/exclude-include-branches.yaml'
+
+    # Same test results as include-branches
+
+
 class TestTenantExcludeAll(TenantParserTestCase):
     tenant_config_file = 'config/tenant-parser/exclude-all.yaml'
 
diff --git a/zuul/model.py b/zuul/model.py
index 0d789f9763..0f17e4e2ef 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -7029,24 +7029,15 @@ class TenantProjectConfig(object):
 
     def includesBranch(self, branch):
         if self.include_branches is not None:
-            included = False
             for r in self.include_branches:
                 if r.fullmatch(branch):
-                    included = True
-                    break
-        else:
-            included = True
-        if not included:
+                    return True
             return False
 
-        excluded = False
         if self.exclude_branches is not None:
             for r in self.exclude_branches:
                 if r.fullmatch(branch):
-                    excluded = True
-                    break
-        if excluded:
-            return False
+                    return False
         return True