Merge pull request #325 from vchrombie/feat/suggest-codeowners-pr-23

feat: propose CODEOWNERS file when missing

Author: zkoppert

README.md

@@ -4,6 +4,8 @@
 
 Cleanowners is a GitHub Action that is designed to help keep `CODEOWNERS` files current by removing users that are no longer a part of the organization. This is helpful for companies that are looking to remove outdated information in the `CODEOWNERS` file. This action can be paired with other `CODEOWNERS` related actions to suggest new owners or lint `CODEOWNERS` files to ensure accuracy.
 
+If a repository is missing a `CODEOWNERS` file (or it is empty), the action will open a pull request that adds a placeholder `CODEOWNERS` file for maintainers to update.
+
 This action was developed by the GitHub OSPO for our own use and developed in a way that we could open source it that it might be useful to you as well! If you want to know more about how we use it, reach out in an issue in this repository.
 
 ## Support

cleanowners.py

@@ -84,11 +84,42 @@ def main():  # pragma: no cover
         # Check to see if repository has a CODEOWNERS file
         file_changed = False
         codeowners_file_contents, codeowners_filepath = get_codeowners_file(repo)
+        has_codeowners = codeowners_file_contents is not None
+        codeowners_size = (
+            getattr(codeowners_file_contents, "size", None) if has_codeowners else None
+        )
+        is_empty_codeowners = has_codeowners and codeowners_size == 0
 
-        if not codeowners_file_contents:
-            print(f"Skipping {repo.full_name} as it does not have a CODEOWNERS file")
+        if not has_codeowners or is_empty_codeowners:
+            repo_name = repo.full_name
             no_codeowners_count += 1
-            repos_missing_codeowners.append(repo)
+            repos_missing_codeowners.append(repo_name)
+
+            if not has_codeowners:
+                print(f"{repo_name} does not have a CODEOWNERS file")
+            else:
+                print(f"{repo_name} has an empty CODEOWNERS file")
+
+            if dry_run:
+                continue
+
+            suggested_codeowners = build_default_codeowners(repo)
+            target_path = codeowners_filepath or ".github/CODEOWNERS"
+            eligble_for_pr_count += 1
+            try:
+                pull = commit_changes(
+                    title,
+                    body,
+                    repo,
+                    suggested_codeowners,
+                    commit_message,
+                    target_path,
+                    create_new=not has_codeowners,
+                )
+                pull_count += 1
+                print(f"\tCreated pull request {pull.html_url}")
+            except github3.exceptions.NotFoundError:
+                print("\tFailed to create pull request. Check write permissions.")
             continue
 
         codeowners_count += 1
@@ -179,35 +210,14 @@ def get_codeowners_file(repo):
     the file contents and file path or None if it doesn't exist
     """
     codeowners_file_contents = None
-    codeowners_filepath = None
-    try:
-        if (
-            repo.file_contents(".github/CODEOWNERS")
-            and repo.file_contents(".github/CODEOWNERS").size > 0
-        ):
-            codeowners_file_contents = repo.file_contents(".github/CODEOWNERS")
-            codeowners_filepath = ".github/CODEOWNERS"
-    except github3.exceptions.NotFoundError:
-        pass
-    try:
-        if (
-            repo.file_contents("CODEOWNERS")
-            and repo.file_contents("CODEOWNERS").size > 0
-        ):
-            codeowners_file_contents = repo.file_contents("CODEOWNERS")
-            codeowners_filepath = "CODEOWNERS"
-    except github3.exceptions.NotFoundError:
-        pass
-    try:
-        if (
-            repo.file_contents("docs/CODEOWNERS")
-            and repo.file_contents("docs/CODEOWNERS").size > 0
-        ):
-            codeowners_file_contents = repo.file_contents("docs/CODEOWNERS")
-            codeowners_filepath = "docs/CODEOWNERS"
-    except github3.exceptions.NotFoundError:
-        pass
-    return codeowners_file_contents, codeowners_filepath
+    for path in (".github/CODEOWNERS", "CODEOWNERS", "docs/CODEOWNERS"):
+        try:
+            codeowners_file_contents = repo.file_contents(path)
+            if codeowners_file_contents:
+                return codeowners_file_contents, path
+        except github3.exceptions.NotFoundError:
+            continue
+    return None, None
 
 
 def print_stats(
@@ -216,7 +226,7 @@ def print_stats(
     """Print the statistics from this run to the terminal output"""
     print(f"Found {users_count} users to remove")
     print(f"Created {pull_count} pull requests successfully")
-    print(f"Skipped {no_codeowners_count} repositories without a CODEOWNERS file")
+    print(f"Found {no_codeowners_count} repositories missing or empty CODEOWNERS files")
     print(f"Processed {codeowners_count} repositories with a CODEOWNERS file")
     if eligble_for_pr_count == 0:
         print("No pull requests were needed")
@@ -273,13 +283,31 @@ def get_usernames_from_codeowners(codeowners_file_contents, ignore_teams=True):
     return usernames
 
 
+def build_default_codeowners(repo):
+    """Build a placeholder CODEOWNERS file for repositories without one."""
+    owner_login = repo.owner.login
+    owner_type = getattr(repo.owner, "type", "")
+    if owner_type == "Organization":
+        owner_handle = f"{owner_login}/REPLACE_WITH_TEAM"
+    else:
+        owner_handle = owner_login
+
+    contents = (
+        "# CODEOWNERS\n"
+        "# Replace the placeholder with the appropriate owner(s) for this repository.\n"
+        f"* @{owner_handle}\n"
+    )
+    return contents.encode("ASCII")
+
+
 def commit_changes(
     title,
     body,
     repo,
     codeowners_file_contents_new,
     commit_message,
     codeowners_filepath,
+    create_new=False,
 ):
     """Commit the changes to the repo and open a pull request and return the pull request object"""
     default_branch = repo.default_branch
@@ -288,11 +316,19 @@ def commit_changes(
     front_matter = "refs/heads/"
     branch_name = f"codeowners-{str(uuid.uuid4())}"
     repo.create_ref(front_matter + branch_name, default_branch_commit)
-    repo.file_contents(codeowners_filepath).update(
-        message=commit_message,
-        content=codeowners_file_contents_new,
-        branch=branch_name,
-    )
+    if create_new:
+        repo.create_file(
+            codeowners_filepath,
+            commit_message,
+            codeowners_file_contents_new,
+            branch=branch_name,
+        )
+    else:
+        repo.file_contents(codeowners_filepath).update(
+            message=commit_message,
+            content=codeowners_file_contents_new,
+            branch=branch_name,
+        )
 
     pull = repo.create_pull(
         title=title, body=body, head=branch_name, base=repo.default_branch

markdown_writer.py

@@ -16,7 +16,7 @@ def write_to_markdown(
             "## Overall Stats\n"
             f"{users_count} Users to Remove\n"
             f"{pull_count} Pull Requests created\n"
-            f"{no_codeowners_count} Repositories with no CODEOWNERS file\n"
+            f"{no_codeowners_count} Repositories missing or empty CODEOWNERS files\n"
             f"{codeowners_count} Repositories with CODEOWNERS file\n"
         )
         if repo_and_users_to_remove:
@@ -27,7 +27,7 @@ def write_to_markdown(
                     file.write(f"- {user}\n")
                 file.write("\n")
         if repos_missing_codeowners:
-            file.write("## Repositories Missing CODEOWNERS\n")
+            file.write("## Repositories Missing or Empty CODEOWNERS\n")
             for repo in repos_missing_codeowners:
                 file.write(f"- {repo}\n")
             file.write("\n")

test_cleanowners.py

@@ -7,6 +7,7 @@
 
 import github3
 from cleanowners import (
+    build_default_codeowners,
     commit_changes,
     get_codeowners_file,
     get_org,
@@ -62,6 +63,40 @@ def test_commit_changes(self, mock_uuid):
         # Assert that the function returned the expected result
         self.assertEqual(result, "MockPullRequest")
 
+    @patch("uuid.uuid4")
+    def test_commit_changes_create_new_file(self, mock_uuid):
+        """Test the commit_changes function when creating a new file."""
+        mock_uuid.return_value = uuid.UUID("12345678123456781234567812345678")
+        mock_repo = MagicMock()
+        mock_repo.default_branch = "main"
+        mock_repo.ref.return_value.object.sha = "abc123"
+        mock_repo.create_ref.return_value = True
+        mock_repo.create_file.return_value = True
+        mock_repo.create_pull.return_value = "MockPullRequest"
+
+        result = commit_changes(
+            "Test Title",
+            "Test Body",
+            mock_repo,
+            b"new content",
+            "Test commit message",
+            "CODEOWNERS",
+            create_new=True,
+        )
+
+        branch_name = "codeowners-12345678-1234-5678-1234-567812345678"
+        mock_repo.create_ref.assert_called_once_with(
+            f"refs/heads/{branch_name}", "abc123"
+        )
+        mock_repo.create_file.assert_called_once_with(
+            "CODEOWNERS",
+            "Test commit message",
+            b"new content",
+            branch=branch_name,
+        )
+        mock_repo.file_contents.assert_not_called()
+        self.assertEqual(result, "MockPullRequest")
+
 
 class TestGetUsernamesFromCodeowners(unittest.TestCase):
     """Test the get_usernames_from_codeowners function in cleanowners.py"""
@@ -197,7 +232,7 @@ def test_print_stats_all_counts(self, mock_stdout):
         expected_output = (
             "Found 4 users to remove\n"
             "Created 5 pull requests successfully\n"
-            "Skipped 2 repositories without a CODEOWNERS file\n"
+            "Found 2 repositories missing or empty CODEOWNERS files\n"
             "Processed 3 repositories with a CODEOWNERS file\n"
             "50.0% of eligible repositories had pull requests created\n"
             "60.0% of repositories had CODEOWNERS files\n"
@@ -211,7 +246,7 @@ def test_print_stats_no_pull_requests_needed(self, mock_stdout):
         expected_output = (
             "Found 4 users to remove\n"
             "Created 0 pull requests successfully\n"
-            "Skipped 2 repositories without a CODEOWNERS file\n"
+            "Found 2 repositories missing or empty CODEOWNERS files\n"
             "Processed 3 repositories with a CODEOWNERS file\n"
             "No pull requests were needed\n"
             "60.0% of repositories had CODEOWNERS files\n"
@@ -225,7 +260,7 @@ def test_print_stats_no_repositories_processed(self, mock_stdout):
         expected_output = (
             "Found 0 users to remove\n"
             "Created 0 pull requests successfully\n"
-            "Skipped 0 repositories without a CODEOWNERS file\n"
+            "Found 0 repositories missing or empty CODEOWNERS files\n"
             "Processed 0 repositories with a CODEOWNERS file\n"
             "No pull requests were needed\n"
             "No repositories were processed\n"
@@ -274,8 +309,40 @@ def test_codeowners_not_found(self):
         self.assertIsNone(path)
 
     def test_codeowners_empty_file(self):
-        """Test that an empty CODEOWNERS file is not considered valid because it is empty."""
+        """Test that an empty CODEOWNERS file is returned for further handling."""
         self.repo.file_contents.side_effect = lambda path: MagicMock(size=0)
         contents, path = get_codeowners_file(self.repo)
-        self.assertIsNone(contents)
-        self.assertIsNone(path)
+        self.assertIsNotNone(contents)
+        self.assertEqual(path, ".github/CODEOWNERS")
+
+    def test_codeowners_not_found_then_found(self):
+        """Test that a later path is used when earlier ones are not found."""
+        not_found = github3.exceptions.NotFoundError(resp=MagicMock(status_code=404))
+        self.repo.file_contents.side_effect = [not_found, MagicMock(size=1)]
+        contents, path = get_codeowners_file(self.repo)
+        self.assertIsNotNone(contents)
+        self.assertEqual(path, "CODEOWNERS")
+
+
+class TestBuildDefaultCodeowners(unittest.TestCase):
+    """Test the build_default_codeowners function in cleanowners.py"""
+
+    def test_build_default_codeowners_for_org(self):
+        """Test placeholder uses org team handle."""
+        repo = MagicMock()
+        repo.owner.login = "my-org"
+        repo.owner.type = "Organization"
+
+        result = build_default_codeowners(repo)
+
+        self.assertIn(b"@my-org/REPLACE_WITH_TEAM", result)
+
+    def test_build_default_codeowners_for_user(self):
+        """Test placeholder uses user handle."""
+        repo = MagicMock()
+        repo.owner.login = "my-user"
+        repo.owner.type = "User"
+
+        result = build_default_codeowners(repo)
+
+        self.assertIn(b"@my-user", result)

test_markdown_writer.py

@@ -19,7 +19,7 @@ def test_write_with_all_counts_and_no_users_to_remove(self):
                 "## Overall Stats\n"
                 "0 Users to Remove\n"
                 "0 Pull Requests created\n"
-                "2 Repositories with no CODEOWNERS file\n"
+                "2 Repositories missing or empty CODEOWNERS files\n"
                 "3 Repositories with CODEOWNERS file\n"
             )
 
@@ -35,7 +35,7 @@ def test_write_with_repos_and_users_with_users_to_remove(self):
                     "## Overall Stats\n"
                     "1 Users to Remove\n"
                     "2 Pull Requests created\n"
-                    "3 Repositories with no CODEOWNERS file\n"
+                    "3 Repositories missing or empty CODEOWNERS files\n"
                     "4 Repositories with CODEOWNERS file\n"
                 ),
                 call("## Repositories and Users to Remove\n"),
@@ -61,10 +61,10 @@ def test_write_with_repos_missing_codeowners(self):
                     "## Overall Stats\n"
                     "0 Users to Remove\n"
                     "0 Pull Requests created\n"
-                    "2 Repositories with no CODEOWNERS file\n"
+                    "2 Repositories missing or empty CODEOWNERS files\n"
                     "0 Repositories with CODEOWNERS file\n"
                 ),
-                call("## Repositories Missing CODEOWNERS\n"),
+                call("## Repositories Missing or Empty CODEOWNERS\n"),
                 call("- repo1\n"),
                 call("- repo2\n"),
                 call("\n"),
@@ -81,7 +81,7 @@ def test_write_with_empty_inputs(self):
                 "## Overall Stats\n"
                 "0 Users to Remove\n"
                 "0 Pull Requests created\n"
-                "0 Repositories with no CODEOWNERS file\n"
+                "0 Repositories missing or empty CODEOWNERS files\n"
                 "0 Repositories with CODEOWNERS file\n"
             )