From 240b1d66963734b566e06c8a10621060b32e2ea9 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Dec 03 2019 10:46:32 +0000
Subject: Expand on the documentation in the code


Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/dist_git_auth.py b/dist_git_auth.py
index 4fde76e..168cfb2 100644
--- a/dist_git_auth.py
+++ b/dist_git_auth.py
@@ -199,20 +199,26 @@ class DistGitAuth(GitAuthHelper):
         self.debug("SIG memberships: %s" % user_sigs)
         self.debug("RCM: %s" % is_rcm)
 
+        # Quick reminder about the protected_namespace: git.centos.org is
+        # hosting in one pagure instance a "regular" git forge as well as
+        # a dist-git instance. So they need to protect the dist-git
+        # namespaces via the dist-git specific checks while the other
+        # namespaces go via the regular checks.
+
         # We have data, start the actual ACL checking
         if (
             repotype == "main"
             and not project.is_fork
             and project.namespace in self.protected_namespaces
         ):
-            # In the protected namespace, we want to make sure we don't
-            # trample on blacklisted content.
+            # In the protected namespace, we want to make sure that we block
+            # blacklisted branches.
             for entry in self.blacklists:
                 if entry.match(refname):
                     self.info("Ref %s is blocked" % refname)
                     return False
 
-            # Allow RCM push
+            # Allow RCM/releng to push regardless
             if is_rcm:
                 for refre in self.rcm_branches:
                     if refre.match(refname):
@@ -228,7 +234,8 @@ class DistGitAuth(GitAuthHelper):
                         self.debug("SIG push")
                         return True
 
-            # For Fedora, allow supported branches
+            # For Fedora, allow supported branches, these are the active
+            # branches in PDC
             is_supported = self.is_supported_branch(project, refname)
             if is_supported is False:
                 self.info("Branch %s is unsupported" % refname)
@@ -238,16 +245,17 @@ class DistGitAuth(GitAuthHelper):
             else:
                 self.debug("No supported status available")
 
+            # This allows to block anything that is not allowed, so no
+            # random branch creation.
             if self.block_unspecified:
                 self.info(
                     "Access to namespace %s is restricted" % project.namespace
                 )
                 return False
 
-            # Block second level blacklists
+            # For branches that are not explicitely active in PDC, check
+            # if the user is allowed to create/push to them.
             if not is_supported:
-                # if a branch is not supported, check if the user is allowed
-                # to push to/create it.
                 for entry in self.unspecified_blacklist:
                     if entry.match(refname):
                         self.info("Unspecified ref %s is blocked" % refname)
@@ -256,7 +264,8 @@ class DistGitAuth(GitAuthHelper):
             # For unspecified refs, they can push if they're a committer
             self.debug("Unspecified branch push")
 
-        # This is outside of the strongly protected namespaces
+        # This is applicable to all namespace, protected or not
+
         if repotype == "main":
             # If this project has PRs only on, or PRs are globally enforced and
             # this is not a fork, only allow pushing if this is a PR merge.
@@ -267,9 +276,9 @@ class DistGitAuth(GitAuthHelper):
                 self.info("A pull request is required for this branch")
                 return False
 
-        # This is an unprotected namespace, let's allow committers
+        # Allow committers to commit
         if is_committer:
-            self.debug("Committer push to unprotected")
+            self.debug("Committer push")
             return True
 
         # If all else fails, deny
diff --git a/dist_git_auth_tests.py b/dist_git_auth_tests.py
index 7092a34..fee227d 100644
--- a/dist_git_auth_tests.py
+++ b/dist_git_auth_tests.py
@@ -288,7 +288,7 @@ class DistGitAuthTestsGeneric(DistGitAuthTests):
             )
         )
 
-        self.expect_info_msg("Committer push to unprotected")
+        self.expect_info_msg("Committer push")
 
     def test_unprotected_non_committer(self):
         project = self.create_namespaced_project('unprotected', 'test')
@@ -329,7 +329,7 @@ class DistGitAuthTestsGeneric(DistGitAuthTests):
             )
         )
 
-        self.expect_info_msg("Committer push to unprotected")
+        self.expect_info_msg("Committer push")
 
     def test_unprotected_pr_required_no_pr(self):
         project = self.create_namespaced_project('unprotected', 'test')
@@ -371,8 +371,8 @@ class DistGitAuthTestsGeneric(DistGitAuthTests):
             )
         )
 
-        self.expect_info_msg("Committer push to unprotected")
-    
+        self.expect_info_msg("Committer push")
+
     def test_unprotected_pr_required_repo_pr_only_no_pr(self):
         settings = {"pull_request_access_only": True}
         project = self.create_namespaced_project('unprotected', 'test', settings=settings)
@@ -393,7 +393,7 @@ class DistGitAuthTestsGeneric(DistGitAuthTests):
         )
 
         self.expect_info_msg("A pull request is required for this branch")
-    
+
     def test_unprotected_pr_required_repo_pr_only(self):
         settings = {"pull_request_access_only": True}
         project = self.create_namespaced_project(
@@ -416,7 +416,8 @@ class DistGitAuthTestsGeneric(DistGitAuthTests):
             )
         )
 
-        self.expect_info_msg("Committer push to unprotected")
+        self.expect_info_msg("Committer push")
+
 
 class DistGitAuthTestsFedora(DistGitAuthTests):
     dga_config = {