Merge pull request #90 from github/report-as-issue-markdown

Author: zkoppert

README.md

@@ -56,6 +56,7 @@ This action can be configured to authenticate with GitHub App Installation or Pe
 | `REPOSITORY`              | Required to have `ORGANIZATION` or `REPOSITORY` |         | The name of the repository and organization which you want this action to work from. ie. `github/cleanowners` or a comma separated list of multiple repositories `github/cleanowners,super-linter/super-linter` |
 | `EXEMPT_REPOS`            | False    |   ""    | These repositories will be exempt from this action. ex: If my org is set to `github` then I might want to exempt a few of the repos but get the rest by setting `EXEMPT_REPOS` to `github/cleanowners,github/contributors` |
 | `DRY_RUN`                 | False    | False   | If set to true, this action will not create any pull requests. It will only log the repositories that could have the `CODEOWNERS` file updated. This is useful for testing or discovering the scope of this issue in your organization. |
+| `ISSUE_REPORT`            | False    | False   | If set to true, this action will create an issue in the repository with the report on the repositories that had users removed from the `CODEOWNERS` file. |
 
 ### Example workflows
 
@@ -114,6 +115,14 @@ jobs:
           GH_TOKEN: ${{ secrets.GH_TOKEN }}
           ORGANIZATION: <YOUR_ORGANIZATION_GOES_HERE>
           EXEMPT_REPOS: "org_name/repo_name_1, org_name/repo_name_2"
+          ISSUE_REPORT: true
+      - name: Create issue
+        uses: peter-evans/create-issue-from-file@v5
+        with:
+          title: Cleanowners Report
+          content-filepath: ./report.md
+          assignees: <YOUR_GITHUB_HANDLE_HERE>
+          token: ${{ secrets.GITHUB_TOKEN }}
 
 ```
 

cleanowners.py

@@ -5,6 +5,7 @@
 import auth
 import env
 import github3
+from markdown_writer import write_to_markdown
 
 
 def get_org(github_connection, organization):
@@ -33,6 +34,7 @@ def main():  # pragma: no cover
         title,
         body,
         commit_message,
+        issue_report,
     ) = env.get_env_vars()
 
     # Auth to GitHub.com or GHE
@@ -59,6 +61,7 @@ def main():  # pragma: no cover
     # Get the repositories from the organization or list of repositories
     repos = get_repos_iterator(organization, repository_list, github_connection)
 
+    repo_and_users_to_remove = {}
     for repo in repos:
         # Check if the repository is in the exempt_repositories_list
         if repo.full_name in exempt_repositories_list:
@@ -109,6 +112,8 @@ def main():  # pragma: no cover
         # Extract the usernames from the CODEOWNERS file
         usernames = get_usernames_from_codeowners(codeowners_file_contents)
 
+        usernames_to_remove = []
+        codeowners_file_contents_new = None
         for username in usernames:
             org = organization if organization else repo.owner.login
             gh_org = get_org(github_connection, org)
@@ -122,6 +127,7 @@ def main():  # pragma: no cover
                     f"\t{username} is not a member of {org}. Suggest removing them from {repo.full_name}"
                 )
                 users_count += 1
+                usernames_to_remove.append(username)
                 if not dry_run:
                     # Remove that username from the codeowners_file_contents
                     file_changed = True
@@ -130,9 +136,18 @@ def main():  # pragma: no cover
                         codeowners_file_contents.decoded.replace(bytes_username, b"")
                     )
 
+        # Store the repo and users to remove for reporting later
+        if usernames_to_remove:
+            repo_and_users_to_remove[repo] = usernames_to_remove
+
         # Update the CODEOWNERS file if usernames were removed
         if file_changed:
             eligble_for_pr_count += 1
+            new_usernames = get_usernames_from_codeowners(codeowners_file_contents_new)
+            if len(new_usernames) == 0:
+                print(
+                    f"\twarning: All usernames removed from CODEOWNERS in {repo.full_name}."
+                )
             try:
                 pull = commit_changes(
                     title,
@@ -166,6 +181,15 @@ def main():  # pragma: no cover
             f"{round((codeowners_count / (codeowners_count + no_codeowners_count)) * 100, 2)}% of repositories had CODEOWNERS files"
         )
 
+    if issue_report:
+        write_to_markdown(
+            users_count,
+            pull_count,
+            no_codeowners_count,
+            codeowners_count,
+            repo_and_users_to_remove,
+        )
+
 
 def get_repos_iterator(organization, repository_list, github_connection):
     """Get the repositories from the organization or list of repositories"""
@@ -182,7 +206,7 @@ def get_repos_iterator(organization, repository_list, github_connection):
     return repos
 
 
-def get_usernames_from_codeowners(codeowners_file_contents):
+def get_usernames_from_codeowners(codeowners_file_contents, ignore_teams=True):
     """Extract the usernames from the CODEOWNERS file"""
     usernames = []
     for line in codeowners_file_contents.decoded.splitlines():
@@ -201,7 +225,9 @@ def get_usernames_from_codeowners(codeowners_file_contents):
                     handle = handle.split()[0]
                     # Identify team handles by the presence of a slash.
                     # Ignore teams because non-org members cannot be in a team.
-                    if "/" not in handle:
+                    if ignore_teams and "/" not in handle:
+                        usernames.append(handle)
+                    elif not ignore_teams:
                         usernames.append(handle)
     return usernames
 

env.py

@@ -8,6 +8,22 @@
 from dotenv import load_dotenv
 
 
+def get_bool_env_var(env_var_name: str, default: bool = False) -> bool:
+    """Get a boolean environment variable.
+
+    Args:
+        env_var_name: The name of the environment variable to retrieve.
+        default: The default value to return if the environment variable is not set.
+
+    Returns:
+        The value of the environment variable as a boolean.
+    """
+    ev = os.environ.get(env_var_name, "")
+    if ev == "" and default:
+        return default
+    return ev.strip().lower() == "true"
+
+
 def get_int_env_var(env_var_name: str) -> int | None:
     """Get an integer environment variable.
 
@@ -41,6 +57,7 @@ def get_env_vars(
     str,
     str,
     str,
+    bool,
 ]:
     """
     Get the environment variables for use in the action.
@@ -61,6 +78,7 @@ def get_env_vars(
         title (str): The title to use for the pull request
         body (str): The body to use for the pull request
         message (str): Commit message to use
+        issue_report (bool): Whether or not to create an issue report with the results
 
     """
     if not test:
@@ -115,14 +133,7 @@ def get_env_vars(
             repository.strip() for repository in exempt_repos.split(",")
         ]
 
-    dry_run = os.getenv("DRY_RUN")
-    dry_run = dry_run.lower() if dry_run else None
-    if dry_run:
-        if dry_run not in ("true", "false"):
-            raise ValueError("DRY_RUN environment variable not 'true' or 'false'")
-        dry_run_bool = dry_run == "true"
-    else:
-        dry_run_bool = False
+    dry_run = get_bool_env_var("DRY_RUN")
 
     title = os.getenv("TITLE")
     # make sure that title is a string with less than 70 characters
@@ -155,6 +166,8 @@ def get_env_vars(
             "Remove users no longer in this organization from CODEOWNERS file"
         )
 
+    issue_report = get_bool_env_var("ISSUE_REPORT")
+
     return (
         organization,
         repositories_list,
@@ -164,8 +177,9 @@ def get_env_vars(
         token,
         ghe,
         exempt_repositories_list,
-        dry_run_bool,
+        dry_run,
         title,
         body,
         commit_message,
+        issue_report,
     )

markdown_writer.py

@@ -0,0 +1,27 @@
+"""Write the results to a markdown file"""
+
+
+def write_to_markdown(
+    users_count,
+    pull_count,
+    no_codeowners_count,
+    codeowners_count,
+    repo_and_users_to_remove,
+):
+    """Write the results to a markdown file"""
+    with open("report.md", "w", encoding="utf-8") as file:
+        file.write(
+            "# Cleanowners Report\n\n"
+            "## 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"{codeowners_count} Repositories with CODEOWNERS file\n"
+        )
+        if repo_and_users_to_remove:
+            file.write("## Repositories and Users to Remove\n")
+            for repo, users in repo_and_users_to_remove.items():
+                file.write(f"{repo}\n")
+                for user in users:
+                    file.write(f"- {user}\n")
+                file.write("\n")

test_cleanowners.py

@@ -63,7 +63,7 @@ def test_commit_changes(self, mock_uuid):
 class TestGetUsernamesFromCodeowners(unittest.TestCase):
     """Test the get_usernames_from_codeowners function in cleanowners.py"""
 
-    def test_get_usernames_from_codeowners(self):
+    def test_get_usernames_from_codeowners_ignore_teams(self):
         """Test the get_usernames_from_codeowners function."""
         codeowners_file_contents = MagicMock()
         codeowners_file_contents.decoded = """
@@ -82,6 +82,25 @@ def test_get_usernames_from_codeowners(self):
 
         self.assertEqual(result, expected_usernames)
 
+    def test_get_usernames_from_codeowners_with_teams(self):
+        """Test the get_usernames_from_codeowners function."""
+        codeowners_file_contents = MagicMock()
+        codeowners_file_contents.decoded = """
+        # Comment
+        @user1
+        @user2
+        @org/team
+        # Another comment
+        @user3 @user4
+        """.encode(
+            "ASCII"
+        )
+        expected_usernames = ["user1", "user2", "org/team", "user3", "user4"]
+
+        result = get_usernames_from_codeowners(codeowners_file_contents, False)
+
+        self.assertEqual(result, expected_usernames)
+
 
 class TestGetOrganization(unittest.TestCase):
     """Test the get_org function in cleanowners.py"""

test_env.py

@@ -30,6 +30,7 @@ def setUp(self):
             "ORGANIZATION",
             "REPOSITORY",
             "TITLE",
+            "ISSUE_REPORT",
         ]
         for key in env_keys:
             if key in os.environ:
@@ -67,6 +68,7 @@ def test_get_env_vars_with_org(self):
             TITLE,
             BODY,
             COMMIT_MESSAGE,
+            False,
         )
         result = get_env_vars(True)
         self.assertEqual(result, expected_result)
@@ -104,6 +106,7 @@ def test_get_env_vars_with_github_app_and_repos(self):
             TITLE,
             BODY,
             COMMIT_MESSAGE,
+            False,
         )
         result = get_env_vars(True)
         self.assertEqual(result, expected_result)
@@ -141,6 +144,7 @@ def test_get_env_vars_with_token_and_repos(self):
             TITLE,
             BODY,
             COMMIT_MESSAGE,
+            False,
         )
         result = get_env_vars(True)
         self.assertEqual(result, expected_result)
@@ -156,6 +160,7 @@ def test_get_env_vars_with_token_and_repos(self):
             "GH_TOKEN": TOKEN,
             "ORGANIZATION": ORGANIZATION,
             "TITLE": TITLE,
+            "ISSUE_REPORT": "true",
         },
     )
     def test_get_env_vars_optional_values(self):
@@ -173,6 +178,7 @@ def test_get_env_vars_optional_values(self):
             TITLE,
             BODY,
             COMMIT_MESSAGE,
+            True,
         )
         result = get_env_vars(True)
         self.assertEqual(result, expected_result)
@@ -221,6 +227,7 @@ def test_get_env_vars_with_repos_no_dry_run(self):
             "Clean up CODEOWNERS file",
             "Consider these updates to the CODEOWNERS file to remove users no longer in this organization.",
             "Remove users no longer in this organization from CODEOWNERS file",
+            False,
         )
         result = get_env_vars(True)
         self.assertEqual(result, expected_result)

test_env_get_bool.py

@@ -0,0 +1,81 @@
+"""Test the get_bool_env_var function"""
+
+import os
+import unittest
+from unittest.mock import patch
+
+from env import get_bool_env_var
+
+
+class TestEnv(unittest.TestCase):
+    """Test the get_bool_env_var function"""
+
+    @patch.dict(
+        os.environ,
+        {
+            "TEST_BOOL": "true",
+        },
+        clear=True,
+    )
+    def test_get_bool_env_var_that_exists_and_is_true(self):
+        """Test that gets a boolean environment variable that exists and is true"""
+        result = get_bool_env_var("TEST_BOOL", False)
+        self.assertTrue(result)
+
+    @patch.dict(
+        os.environ,
+        {
+            "TEST_BOOL": "false",
+        },
+        clear=True,
+    )
+    def test_get_bool_env_var_that_exists_and_is_false(self):
+        """Test that gets a boolean environment variable that exists and is false"""
+        result = get_bool_env_var("TEST_BOOL", False)
+        self.assertFalse(result)
+
+    @patch.dict(
+        os.environ,
+        {
+            "TEST_BOOL": "nope",
+        },
+        clear=True,
+    )
+    def test_get_bool_env_var_that_exists_and_is_false_due_to_invalid_value(self):
+        """Test that gets a boolean environment variable that exists and is false
+        due to an invalid value
+        """
+        result = get_bool_env_var("TEST_BOOL", False)
+        self.assertFalse(result)
+
+    @patch.dict(
+        os.environ,
+        {
+            "TEST_BOOL": "false",
+        },
+        clear=True,
+    )
+    def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_true(self):
+        """Test that gets a boolean environment variable that does not exist
+        and default value returns: true
+        """
+        result = get_bool_env_var("DOES_NOT_EXIST", True)
+        self.assertTrue(result)
+
+    @patch.dict(
+        os.environ,
+        {
+            "TEST_BOOL": "true",
+        },
+        clear=True,
+    )
+    def test_get_bool_env_var_that_does_not_exist_and_default_value_returns_false(self):
+        """Test that gets a boolean environment variable that does not exist
+        and default value returns: false
+        """
+        result = get_bool_env_var("DOES_NOT_EXIST", False)
+        self.assertFalse(result)
+
+
+if __name__ == "__main__":
+    unittest.main()