feat: add idnits3 to draft submission checks

dev/build/Dockerfile

@@ -25,6 +25,14 @@ RUN pip3 --disable-pip-version-check --no-cache-dir install -r requirements.txt
     ietf/manage.py patch_libraries && \
     rm -f ietf/settings_local.py
 
+# Install idnits3.  The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+    ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+    idnits3 --version
+
 RUN chmod +x start.sh && \
     chmod +x datatracker-start.sh && \
     chmod +x migration-start.sh && \

dev/diff/settings_local.py

@@ -18,6 +18,7 @@
 
 
 IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
 IDSUBMIT_REPOSITORY_PATH = "test/id/"
 IDSUBMIT_STAGING_PATH = "test/staging/"
 

dev/tests/prepare.sh

@@ -8,6 +8,10 @@ echo "Copying config files..."
 cp ./dev/tests/settings_local.py ./ietf/settings_local.py
 echo "Ensure all requirements.txt packages are installed..."
 pip --disable-pip-version-check --no-cache-dir install -r requirements.txt
+echo "Installing idnits3..."
+npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0
+ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3
+idnits3 --version
 echo "Compiling native node packages..."
 npm ci
 echo "Building static assets..."

dev/tests/settings_local.py

@@ -17,6 +17,7 @@
 }
 
 IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
 IDSUBMIT_REPOSITORY_PATH = "/assets/ietfdata/doc/draft/repository"
 IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"
 

docker/app.Dockerfile

@@ -47,6 +47,14 @@ RUN groupmod --gid $USER_GID $USERNAME \
     && chown -R $USER_UID:$USER_GID /home/$USERNAME \
     || exit 0
 
+# Install idnits3.  The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+    ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+    idnits3 --version
+
 # Switch to local dev user
 USER dev:dev
 

docker/celery.Dockerfile

@@ -41,6 +41,14 @@ RUN groupmod --gid $USER_GID $USERNAME \
     && chown -R $USER_UID:$USER_GID /home/$USERNAME \
     || exit 0
 
+# Install idnits3.  The version is pinned here and in the other container
+# definitions - grep for @ietf-tools/idnits to bump them together.
+# It goes in its own npm prefix so that it can never disturb the idnits2 script
+# at /usr/local/bin/idnits that comes from the base image.
+RUN npm install -g --prefix /usr/local/idnits3 @ietf-tools/idnits@3.1.0 && \
+    ln -sf /usr/local/idnits3/bin/idnits /usr/local/bin/idnits3 && \
+    idnits3 --version
+
 # Switch to local dev user
 USER dev:dev
 

docker/configs/settings_local.py

@@ -24,6 +24,7 @@
 }
 
 IDSUBMIT_IDNITS_BINARY = "/usr/local/bin/idnits"
+IDSUBMIT_IDNITS3_BINARY = "/usr/local/bin/idnits3"
 IDSUBMIT_STAGING_PATH = "/assets/www6s/staging/"
 
 AGENDA_PATH = '/assets/www6s/proceedings/'

ietf/checks.py

@@ -54,18 +54,32 @@ def check_id_submission_files(app_configs, **kwargs):
         return []
     #
     errors = []
+    hint = ("Please either update the local settings to point at the correct\n"
+        "\tfile, or if the setting is correct, make sure the file is in place and\n"
+        "\thas the right permissions.\n")
     for s in ("IDSUBMIT_IDNITS_BINARY", ):
         p = getattr(settings, s)
         if not os.path.exists(p):
             errors.append(checks.Critical(
                 "A file used by the I-D submission tool does not exist\n"
                 "at the path given in the settings file.  The setting is:\n"
                 "    %s = %s" % (s, p),
-                hint = ("Please either update the local settings to point at the correct\n"
-                    "\tfile, or if the setting is correct, make sure the file is in place and\n"
-                    "\thas the right permissions.\n"),
+                hint = hint,
                 id = "datatracker.E0007",
             ))
+    # The idnits3 check is advisory - a submission is not blocked by it, and is
+    # not blocked by its absence either, so only warn if it is not installed.
+    for s in ("IDSUBMIT_IDNITS3_BINARY", ):
+        p = getattr(settings, s)
+        if not os.path.exists(p):
+            errors.append(checks.Warning(
+                "A file used by the I-D submission tool does not exist\n"
+                "at the path given in the settings file.  The advisory checks it\n"
+                "provides will be skipped.  The setting is:\n"
+                "    %s = %s" % (s, p),
+                hint = hint,
+                id = "datatracker.W0007",
+            ))
     return errors
 
 

ietf/settings.py

@@ -954,6 +954,11 @@ def skip_unreadable_post(record):
 IDSUBMIT_STAGING_PATH = '/a/www/www6s/staging/'
 IDSUBMIT_STAGING_URL = '//www.ietf.org/staging/'
 IDSUBMIT_IDNITS_BINARY = '/a/www/ietf-datatracker/scripts/idnits'
+IDSUBMIT_IDNITS3_BINARY = '/usr/local/bin/idnits3'
+# Set True to skip the idnits3 checks that need to fetch remote documents
+IDSUBMIT_IDNITS3_OFFLINE = False
+# Seconds to allow an idnits3 run before giving up on it
+IDSUBMIT_IDNITS3_TIMEOUT = 300
 SUBMIT_PYANG_COMMAND = 'pyang --verbose --ietf -p {libs} {model}'
 SUBMIT_YANGLINT_COMMAND = 'yanglint --verbose -p {tmplib} -p {rfclib} -p {draftlib} -p {ianalib} -p {cataloglib} {model} -i'
 
@@ -967,6 +972,7 @@ def skip_unreadable_post(record):
 
 IDSUBMIT_CHECKER_CLASSES = (
     "ietf.submit.checkers.DraftIdnitsChecker",
+    "ietf.submit.checkers.DraftIdnits3Checker",
     "ietf.submit.checkers.DraftYangChecker",
 #    "ietf.submit.checkers.DraftYangvalidatorChecker",    
 )

ietf/submit/checkers.py

@@ -3,10 +3,12 @@
 
 
 import io
+import json
 import os
 from pathlib import Path
 import re
 import shutil
+import subprocess
 import sys
 import tempfile
 
@@ -311,3 +313,169 @@ def check_file_txt(self, path):
         info['items'] = items
         info['code']['yang'] = model_list
         return passed, message, errors, warnings, info
+
+
+class DraftIdnits3Checker(object):
+    """
+    Draft checker class for idnits3, run in "submission" mode.
+
+    idnits3 understands both text and XML Internet-Drafts, so both
+    check_file_xml() and check_file_txt() are defined; the XML is preferred
+    when the submission includes it, since that is the form the author wrote.
+
+    This checker is advisory: it never fails a submission, even when idnits3
+    reports errors that would block it once idnits3 becomes a required check.
+    It records whether it would have blocked in the check's `items` so that the
+    submitter can be warned.  A run that could not be completed at all (missing
+    or broken binary, unparsable output) returns None for `passed`, which marks
+    the check as "did not apply" and hides it from the submitter.
+    """
+
+    name = "idnits3 check"
+
+    symbol = ""
+
+    _severities = (
+        ("ValidationError", "Errors"),
+        ("ValidationWarning", "Warnings"),
+        ("ValidationComment", "Comments"),
+    )
+
+    def __init__(self, options=None):
+        if options is None:
+            # --mode submission limits the checks to those relevant when a
+            # draft is submitted; --output json gives us reliable per-severity
+            # counts and structured nits to render ourselves.
+            options = ["--mode", "submission", "--output", "json", "--no-color"]
+            if settings.IDSUBMIT_IDNITS3_OFFLINE:
+                options.append("--offline")
+        assert isinstance(options, list)
+        self.options = options
+
+    def _version(self):
+        try:
+            result = subprocess.run(
+                [settings.IDSUBMIT_IDNITS3_BINARY, "--version"],
+                capture_output=True,
+                timeout=settings.IDSUBMIT_IDNITS3_TIMEOUT,
+            )
+        except (OSError, subprocess.SubprocessError):
+            return "unknown version"
+        if result.returncode != 0:
+            return "unknown version"
+        return result.stdout.decode("utf-8", errors="replace").strip()
+
+    def _render(self, nits, counts):
+        """Render the idnits3 nits as the text shown to the submitter"""
+        lines = [
+            "idnits %s (submission mode): %d error%s, %d warning%s, %d comment%s"
+            % (
+                self._version(),
+                counts["error"], "" if counts["error"] == 1 else "s",
+                counts["warning"], "" if counts["warning"] == 1 else "s",
+                counts["comment"], "" if counts["comment"] == 1 else "s",
+            ),
+            "",
+            "These results do not affect this submission.  Errors reported here are",
+            "expected to prevent submission once idnits3 becomes a required check.",
+            "",
+        ]
+        if not nits:
+            lines.append("No nits found.")
+            return "\n".join(lines) + "\n"
+        index = 0
+        for severity, heading in self._severities:
+            of_severity = [n for n in nits if n.get("severity") == severity]
+            if not of_severity:
+                continue
+            lines.append("%s:" % heading)
+            lines.append("")
+            for nit in of_severity:
+                index += 1
+                indent = " " * 6
+                lines.append("%4d. %s" % (index, nit.get("code", "UNKNOWN")))
+                lines.append("%s%s" % (indent, nit.get("desc", "")))
+                if nit.get("text"):
+                    lines.append("%sText: %s" % (indent, nit["text"]))
+                if nit.get("path"):
+                    lines.append("%sPath: %s" % (indent, nit["path"]))
+                if nit.get("line"):
+                    lines.append(
+                        "%sAt: %s"
+                        % (
+                            indent,
+                            ", ".join(
+                                "line %s column %s" % (loc.get("line"), loc.get("pos"))
+                                for loc in nit["line"]
+                            ),
+                        )
+                    )
+                if nit.get("ref"):
+                    lines.append("%sSee %s" % (indent, nit["ref"]))
+                lines.append("")
+        return "\n".join(lines) + "\n"
+
+    def _check_file(self, path):
+        info = {
+            "checker": self.name,
+            "items": [],
+            "code": {},
+            "advisory": True,
+            "would_block_in_future": False,
+        }
+        cmd = [settings.IDSUBMIT_IDNITS3_BINARY] + self.options + [str(path)]
+        try:
+            result = subprocess.run(
+                cmd, capture_output=True, timeout=settings.IDSUBMIT_IDNITS3_TIMEOUT
+            )
+        except (OSError, subprocess.SubprocessError) as err:
+            message = "idnits3 error: %s:\n  %s" % (" ".join(cmd), err)
+            log(message)
+            return None, message, 0, 0, info
+        if result.returncode != 0:
+            message = "idnits3 error: %s:\n  Error %s: %s" % (
+                " ".join(cmd),
+                result.returncode,
+                result.stderr.decode("utf-8", errors="replace"),
+            )
+            log(message)
+            return None, message, 0, 0, info
+        try:
+            output = json.loads(result.stdout.decode("utf-8", errors="replace"))
+            counts = {
+                severity: int(output["nitsBySeverity"].get(severity, 0))
+                for severity in ("error", "warning", "comment")
+            }
+            nits = output.get("nits", [])
+            if not isinstance(nits, list) or not all(
+                isinstance(nit, dict) for nit in nits
+            ):
+                raise ValueError("'nits' is not a list of nits")
+        except (AttributeError, KeyError, TypeError, ValueError) as err:
+            message = "idnits3 error: %s:\n  Could not parse the idnits3 output: %s" % (
+                " ".join(cmd),
+                err,
+            )
+            log(message)
+            return None, message, 0, 0, info
+
+        info["items"] = [
+            (
+                nit["line"][0]["line"] if nit.get("line") else None,
+                None,
+                "%s: %s" % (nit.get("code", "UNKNOWN"), nit.get("desc", "")),
+            )
+            for nit in nits
+        ]
+        info["would_block_in_future"] = counts["error"] > 0
+        # Comments are neither errors nor warnings, but reporting them as
+        # warnings is the only way to get them in front of the submitter.
+        warnings = counts["warning"] + counts["comment"]
+        # Always passes -- idnits3 does not block submission yet.
+        return True, self._render(nits, counts), counts["error"], warnings, info
+
+    def check_file_txt(self, path):
+        return self._check_file(path)
+
+    def check_file_xml(self, path):
+        return self._check_file(path)

ietf/submit/models.py

@@ -130,21 +130,32 @@ def closed_wg_drafts_replaced(self):
 
 class SubmissionCheck(models.Model):
     time = models.DateTimeField(default=timezone.now)
-    submission = ForeignKey(Submission, related_name='checks')
+    submission = ForeignKey(Submission, related_name="checks")
     checker = models.CharField(max_length=256, blank=True)
     passed = models.BooleanField(null=True, default=False)
     message = models.TextField(null=True, blank=True)
     errors = models.IntegerField(null=True, blank=True, default=None)
     warnings = models.IntegerField(null=True, blank=True, default=None)
     items = models.JSONField(null=True, blank=True, default=dict)
-    symbol = models.CharField(max_length=64, default='')
-    #
+    symbol = models.CharField(max_length=64, default="")
+
     def __str__(self):
-        return "%s submission check: %s: %s" % (self.checker, 'Passed' if self.passed else 'Failed', self.message[:48]+'...')
-    def has_warnings(self):
-        return self.warnings != '[]'
-    def has_errors(self):
-        return self.errors != '[]'
+        return (
+            f"{self.checker} submission check: "
+            f"{'Passed' if self.passed else 'Failed'}: {(self.message or '')[:48]}..."
+        )
+
+    @property
+    def is_advisory(self):
+        """Is this a check whose result cannot prevent a submission?"""
+        return bool(isinstance(self.items, dict) and self.items.get("advisory"))
+
+    @property
+    def would_block_in_future(self):
+        """Would this advisory check have blocked the submission if it were required?"""
+        return bool(
+            isinstance(self.items, dict) and self.items.get("would_block_in_future")
+        )
 
 class SubmissionEvent(models.Model):
     submission = ForeignKey(Submission)