Check dco_signoffs files for missing DCO hashes

When checking for missing DCOs, any hashes found to be documented in
the dco_signoffs (or a user-specified alternative) directory will no
longer be included in the missing list.

Some formatting improvements have also been made to the relevant
files, particularly removing unnecessary #noqa and #nosec directives,
which were not affecting pylint scores.

Issue: RELENG-3396
Change-Id: I7cdac03fb406c6ce862b997c01fb0fab7cfecd76
Signed-off-by: Eric Ball <eball@linuxfoundation.org>

diff --git a/lftools/cli/dco.py b/lftools/cli/dco.py
index e6181cd..aa36c40 100644 (file)
--- a/lftools/cli/dco.py
+++ b/lftools/cli/dco.py
@@ -27,37 +27,61 @@ def dco(ctx):
 
 @click.command()
 @click.argument("repo-path", required=False)
+@click.option(
+    "--signoffs",
+    type=str,
+    required=False,
+    default="dco_signoffs",
+    help="Specify a directory to check for DCO signoff text files",
+)
 @click.pass_context
-def check(ctx, repo_path):
+def check(ctx, repo_path, signoffs):
     """Check repository for commits missing DCO.
 
     This check will exclude merge commits and empty commits.
     It operates in your current working directory which has to
     be a git repository.  Alternatively, you can opt to pass in the
     path to a git repo.
+
+    By default, this will also check for DCO signoff files in a directory
+    named "dco_signoffs".  To check in a different directory, use the
+    --signoffs option.  To ignore signoff files, an empty string can be passed.
+
     Refer to https://developercertificate.org/
     """
     if not repo_path:
         repo_path = "."
-    status = dco_checker.check(repo_path)
+    status = dco_checker.check(repo_path, signoffs)
     sys.exit(status)
 
 
 @click.command()
 @click.argument("repo-path", required=False)
+@click.option(
+    "--signoffs",
+    type=str,
+    required=False,
+    default="dco_signoffs",
+    help="Specify a directory to check for DCO signoff text files",
+)
 @click.pass_context
-def match(ctx, repo_path):
+def match(ctx, repo_path, signoffs):
     """Check for commits whose DCO does not match the commit author's email.
 
     This check will exclude merge commits and empty commits.
     It operates in your current working directory which has to
     be a git repository.  Alternatively, you can opt to pass in the
     path to a git repo.
+
+    By default, this will also check for DCO signoff files in a directory
+    named "dco_signoffs".  To check in a different directory, use the
+    --signoffs option.  To ignore signoff files, an empty string can be passed.
+
     Refer to https://developercertificate.org/
     """
     if not repo_path:
         repo_path = "."
-    status = dco_checker.match(repo_path)
+    status = dco_checker.match(repo_path, signoffs)
     sys.exit(status)
 
 

diff --git a/lftools/shell/dco.py b/lftools/shell/dco.py
index 5377d56..1a1a06c 100644 (file)
--- a/lftools/shell/dco.py
+++ b/lftools/shell/dco.py
@@ -13,10 +13,11 @@
 __author__ = "DW Talton"
 
 import logging
+import os
 from os import chdir
 from os import getcwd
 import re
-import subprocess  # nosec
+import subprocess
 
 log = logging.getLogger(__name__)
 
@@ -30,7 +31,7 @@ def get_branches(path=getcwd(), invert=False):
     chdir(path)
     try:
         branches = (
-            subprocess.check_output("git branch -r | grep -v origin/HEAD", shell=True)  # nosec
+            subprocess.check_output("git branch -r | grep -v origin/HEAD", shell=True)
             .decode(encoding="UTF-8")
             .splitlines()
         )
@@ -38,8 +39,8 @@ def get_branches(path=getcwd(), invert=False):
         for branch in branches:
             branch = branch.strip()
             hashes = (
-                subprocess.check_output(  # nosec
-                    'git log {} --no-merges --pretty="%H %ae" --grep "Signed-off-by" {}'.format(branch, invert),  # noqa
+                subprocess.check_output(
+                    'git log {} --no-merges --pretty="%H %ae" --grep "Signed-off-by" {}'.format(branch, invert),
                     shell=True,
                 )
                 .decode(encoding="UTF-8")
@@ -57,7 +58,7 @@ def get_branches(path=getcwd(), invert=False):
         exit(1)
 
 
-def check(path=getcwd()):
+def check(path=getcwd(), signoffs_dir="dco_signoffs"):
     """Check repository for commits missing DCO."""
     chdir(path)
     try:
@@ -70,9 +71,21 @@ def check(path=getcwd()):
                 if commit:
                     missing.append(commit.split(" ")[0])
 
-            if missing:
-                # de-dupe the commit list
-                missing_list = list(dict.fromkeys(missing))
+            if not missing:
+                exit(0)
+
+            # de-dupe the commit list
+            missing_list = list(dict.fromkeys(missing))
+
+            # Check for dco_signoffs file
+            if os.path.isdir(signoffs_dir):
+                missing_list = [
+                    commit
+                    for commit in missing_list
+                    if subprocess.run(["grep", "-Ri", commit, signoffs_dir], capture_output=True).returncode
+                ]
+
+            if missing_list:
                 for commit in missing_list:
                     log.info("{}".format(commit))
                 exit(1)
@@ -81,7 +94,7 @@ def check(path=getcwd()):
         exit(1)
 
 
-def match(path=getcwd()):
+def match(path=getcwd(), signoffs_dir="dco_signoffs"):
     """Check for commits where DCO does not match the commit author's email."""
     chdir(path)
     try:
@@ -89,30 +102,44 @@ def match(path=getcwd()):
         exit_code = 0
         if not hashes:
             exit(exit_code)
-        else:
-            for commit in hashes:
-                commit_id = commit.split(" ")[0]
-                if commit_id:
-                    commit_log_message = subprocess.check_output(  # nosec
-                        "git log --format=%B -n 1 {}".format(commit_id), shell=True
-                    ).decode(encoding="UTF-8")
-                    commit_author_email = (
-                        subprocess.check_output("git log --format='%ae' {}^!".format(commit_id), shell=True)  # nosec
-                        .decode(encoding="UTF-8")
-                        .strip()
-                    )
-                    sob_email_regex = "(?=Signed\-off\-by: )*[\<](.*)[\>]"  # noqa
-                    sob_results = re.findall(sob_email_regex, commit_log_message)
-
-                    if commit_author_email in sob_results:
+
+        # de-dupe the commit list
+        hashes_list = list(dict.fromkeys(hashes))
+
+        for commit in hashes_list:
+            commit_id = commit.split(" ")[0]
+            if commit_id:
+                commit_log_message = subprocess.check_output(
+                    "git log --format=%B -n 1 {}".format(commit_id), shell=True
+                ).decode(encoding="UTF-8")
+                commit_author_email = (
+                    subprocess.check_output("git log --format='%ae' {}^!".format(commit_id), shell=True)
+                    .decode(encoding="UTF-8")
+                    .strip()
+                )
+                sob_email_regex = r"(?=Signed\-off\-by: ).*[\<](.*)[\>]"
+                sob_results = re.findall(sob_email_regex, commit_log_message)
+
+                if not commit_author_email in sob_results and os.path.isdir(signoffs_dir):
+                    dco_file = subprocess.run(
+                        ["grep", "-Rli", commit_author_email, signoffs_dir], capture_output=True
+                    ).stdout
+                    # If commit_author_email and commit_id are in the same file, do not print info
+                    if (
+                        dco_file
+                        and not subprocess.run(
+                            ["grep", "-li", commit_id, dco_file.strip()], capture_output=True
+                        ).returncode
+                    ):
                         continue
-                    else:
-                        log.info(
-                            "For commit ID {}: \n\tCommitter is {}"
-                            "\n\tbut commit is signed off by {}\n".format(commit_id, commit_author_email, sob_results)
-                        )
-                        exit_code = 1
-            exit(exit_code)
+
+                log.info(
+                    "For commit ID {}: \n\tCommitter is {}"
+                    "\n\tbut commit is signed off by {}\n".format(commit_id, commit_author_email, sob_results)
+                )
+                exit_code = 1
+
+        exit(exit_code)
     except subprocess.CalledProcessError as e:
         log.error(e)
         exit(1)

diff --git a/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml b/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml
new file mode 100644 (file)
index 0000000..eb47980
--- /dev/null
+++ b/releasenotes/notes/dco-signoffs-feature-e0f65249153942fb.yaml
@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    ``lftools dco check`` will now include a check for signoff files to remove
+    commits from the "missing DCO" list. By default, this will check the
+    directory "dco_signoffs", but the ``--signoffs`` option can be used to
+    specify a different directory.

diff --git a/requirements.txt b/requirements.txt
index 64823cf..0d02590 100644 (file)
--- a/requirements.txt
+++ b/requirements.txt
@@ -31,7 +31,7 @@ PyGithub==1.46
 PyJWT==1.7.1
 pyrsistent==0.15.7
 python-jenkins==1.7.0
-PyYAML==5.3
+PyYAML
 requests==2.23.0
 rsa==4.0
 ruamel.yaml==0.16.10