Merge pull request 'fix(reporters): restore old behavior for Gerrit reporting' (#27) from bring-back-old-gerrit-reporting into forkos

Reviewed-on: #27
Reviewed-by: jade <jade@noreply.git.lix.systems>

.envrc

@@ -0,0 +1 @@
+use flake

buildbot_nix/__init__.py

@@ -4,42 +4,41 @@ import os
 import sys
 import graphlib
 import base64
-import random
 from collections.abc import Generator
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import TYPE_CHECKING, Any
+from typing import Any
-import tempfile
 
 import buildbot
 from buildbot.configurators import ConfiguratorBase
 from buildbot.plugins import reporters, schedulers, secrets, steps, util, worker
 from buildbot.process import buildstep, logobserver, remotecommand
+from buildbot.process.log import StreamLog
 from buildbot.process.project import Project
 from buildbot.process.properties import Properties
 from buildbot.process.results import ALL_RESULTS, statusToString
 from buildbot.www.auth import AuthBase
 from buildbot.www.oauth2 import OAuth2Auth
 from buildbot.changes.gerritchangesource import GerritChangeSource
-from buildbot.reporters.utils import getURLForBuildrequest
 from buildbot.reporters.generators.build import BuildStatusGenerator
-from buildbot.reporters.message import MessageFormatterFunction
+from buildbot.reporters.utils import getURLForBuildrequest
 from buildbot.process.buildstep import EXCEPTION
 from buildbot.process.buildstep import SUCCESS
-from buildbot.process.buildstep import BuildStepFailed
 from buildbot.process.results import worst_status
 import requests
 
-if TYPE_CHECKING:
-    from buildbot.process.log import Log
-
 from twisted.internet import defer
 from twisted.logger import Logger
 
 from .binary_cache import S3BinaryCacheConfig
+from .message_formatter import ReasonableMessageFormatter, CallbackPayloadBuild, CallbackPayloadBuildSet, CallbackReturn
 
 log = Logger()
 
+# util is a plugin variable, not a module...
+BuilderConfig = util.BuilderConfig
+MasterLock = util.MasterLock
+
 FLAKE_TARGET_ATTRIBUTE_FOR_JOBS = "hydraJobs"
 
 @dataclass
@@ -48,7 +47,7 @@ class EvaluatorSettings:
     worker_count: int
     max_memory_size: int
     gc_roots_dir: str
-    lock: util.MasterLock
+    lock: MasterLock
 
 @dataclass
 class NixBuilder:
@@ -92,6 +91,8 @@ class OAuth2Config:
     debug: bool = False
 
 class KeycloakOAuth2Auth(OAuth2Auth):
+    userinfoUri: str
+
     def __init__(self, *args, debug=False, **kwargs):
         super().__init__(*args, **kwargs)
         self.debug = debug
@@ -255,7 +256,7 @@ class BuildTrigger(steps.BuildStep):
     def run(self):
         self.running = True
         build_props = self.build.getProperties()
-        logs: Log = yield self.addLog("build info")
+        logs: StreamLog = yield self.addLog("build info")
 
         builds_to_schedule = list(self.jobs)
         build_schedule_order = []
@@ -421,7 +422,7 @@ class NixEvalCommand(buildstep.ShellMixin, steps.BuildStep):
             # Filter out failed evaluations
             succeeded_jobs = [job for job in filtered_jobs if job.get('error') is None]
 
-            drv_show_log: Log = yield self.getLog("stdio")
+            drv_show_log: StreamLog = yield self.getLog("stdio")
             all_deps = dict()
 
             def closure_of(key, deps):
@@ -515,9 +516,9 @@ class NixConfigure(buildstep.CommandMixin, steps.BuildStep):
     @defer.inlineCallbacks
     def run(self) -> Generator[Any, object, Any]:
         try:
-            configure_log: Log = yield self.getLog("stdio")
+            configure_log: StreamLog = yield self.getLog("stdio")
         except Exception:
-            configure_log: Log = yield self.addLog("stdio")
+            configure_log: StreamLog = yield self.addLog("stdio")
 
         # Takes precedence.
         configure_log.addStdout("checking if there's a .ci/buildbot.nix...\n")
@@ -565,7 +566,7 @@ class NixBuildCommand(buildstep.ShellMixin, steps.BuildStep):
         if error := self.getProperty("error"):
             attr = self.getProperty("attr")
             # show eval error
-            error_log: Log = yield self.addLog("nix_error")
+            error_log: StreamLog = yield self.addLog("nix_error")
             error_log.addStderr(f"{attr} failed to evaluate:\n{error}")
             return util.FAILURE
 
@@ -589,10 +590,10 @@ def nix_eval_config(
     project: GerritProject,
     worker_names: list[str],
     supported_systems: list[str],
-    eval_lock: util.MasterLock,
+    eval_lock: MasterLock,
     worker_count: int,
     max_memory_size: int,
-) -> util.BuilderConfig:
+) -> BuilderConfig:
     """
     Uses nix-eval-jobs to evaluate the entrypoint of this project.
     For each evaluated attribute a new build pipeline is started.
@@ -676,7 +677,7 @@ def nix_build_config(
     build_stores: list[str],
     signing_keyfile: str | None = None,
     binary_cache_config: S3BinaryCacheConfig | None = None
-) -> util.BuilderConfig:
+) -> BuilderConfig:
     """Builds one nix flake attribute."""
     factory = util.BuildFactory()
 
@@ -806,11 +807,11 @@ def config_for_project(
     nix_supported_systems: list[str],
     nix_eval_worker_count: int,
     nix_eval_max_memory_size: int,
-    eval_lock: util.MasterLock,
+    eval_lock: MasterLock,
     nix_builders: list[NixBuilder],
     signing_keyfile: str | None = None,
     binary_cache_config: S3BinaryCacheConfig | None = None
-) -> Project:
+) -> None:
     config["projects"].append(Project(project.name))
     config["schedulers"].extend(
         [
@@ -885,22 +886,25 @@ class PeriodicWithStartup(schedulers.Periodic):
             yield self.setState("last_build", None)
         yield super().activate()
 
-def gerritReviewFmt(url, data):
+def gerritReviewFmt(url: str, payload: CallbackPayloadBuild | CallbackPayloadBuildSet) -> CallbackReturn:
-    if 'build' not in data:
+    assert isinstance(payload, CallbackPayloadBuild), "BuildSet are not handled yet!"
-        raise ValueError('`build` is supposed to be present to format a build')
 
-    build = data['build']
+    build = payload.build
     if 'builder' not in build and 'name' not in build['builder']:
         raise ValueError('either `builder` or `builder.name` is not present in the build dictionary, unexpected format request')
 
     builderName = build['builder']['name']
 
     result = build['results']
+    log.info("Formatting a message for a Gerrit build: {} -- result is {}".format(builderName, result))
     if result == util.RETRY:
-        return dict()
+        return CallbackReturn()
 
-    if builderName != f'{build["properties"].get("event.project")}/nix-eval':
+    expectedBuilderName = f'{build["properties"].get("event.project")[0]}/nix-eval'
-        return dict()
+
+    if builderName != expectedBuilderName:
+        log.info("Passing {} builder which is not of the form '{}'".format(builderName, expectedBuilderName))
+        return CallbackReturn()
 
     failed = build['properties'].get('failed_builds', [[]])[0]
 
@@ -924,7 +928,8 @@ def gerritReviewFmt(url, data):
         message += "\nFor more details visit:\n"
         message += build['url'] + "\n"
 
-    return dict(message=message, labels=labels)
+    log.info("Message formatted: {}, labels: Verified={}".format(message, labels['Verified']))
+    return CallbackReturn(body=message, extra_info={'labels': labels})
 
 class GerritNixConfigurator(ConfiguratorBase):
     """Janitor is a configurator which create a Janitor Builder with all needed Janitor steps"""
@@ -982,32 +987,32 @@ class GerritNixConfigurator(ConfiguratorBase):
 
         self.auth_method = auth_method
 
-    def configure(self, config: dict[str, Any]) -> None:
+    def configure(self, config_dict: dict[str, Any]) -> None:
-        worker_config = json.loads(read_secret_file(self.nix_workers_secret_name))
+        worker_config_dict = json.loads(read_secret_file(self.nix_workers_secret_name))
         worker_names = []
 
         if self.manhole is not None:
-            config["manhole"] = self.manhole
+            config_dict["manhole"] = self.manhole
 
-        config.setdefault("projects", [])
+        config_dict.setdefault("projects", [])
-        config.setdefault("secretsProviders", [])
+        config_dict.setdefault("secretsProviders", [])
-        config.setdefault("www", {
-            'allowed_origins': self.allowed_origins
-        })
 
-        for item in worker_config:
+        print('Default allowed origins for this Buildbot server: {}'.format(', '.join(self.allowed_origins)))
+        config_dict["www"]["allowed_origins"] = self.allowed_origins
+
+        for item in worker_config_dict:
             cores = item.get("cores", 0)
             for i in range(cores):
                 for arch in self.nix_supported_systems + ["other"]:
                     worker_name = f"{item['name']}-{i:03}"
-                    config["workers"].append(worker.Worker(f"{worker_name}-{arch}", item["pass"]))
+                    config_dict["workers"].append(worker.Worker(f"{worker_name}-{arch}", item["pass"]))
                     worker_names.append(worker_name)
 
         eval_lock = util.MasterLock("nix-eval")
 
         for project in self.projects:
             config_for_project(
-                config,
+                config_dict,
                 self.gerrit_config,
                 GerritProject(name=project, private_sshkey_path=self.gerrit_sshkey_path),
                 worker_names,
@@ -1020,15 +1025,15 @@ class GerritNixConfigurator(ConfiguratorBase):
                 binary_cache_config=self.binary_cache_config
             )
 
-        config["change_source"] = self.gerrit_change_source
+        config_dict["change_source"] = self.gerrit_change_source
-        config["services"].append(
+        config_dict["services"].append(
             reporters.GerritStatusPush(self.gerrit_server, self.gerrit_user,
                                        port=self.gerrit_port,
                                        identity_file=self.gerrit_sshkey_path,
                                        generators=[
-                                           # gerritReviewCB / self.url
                                            BuildStatusGenerator(
-                                               message_formatter=MessageFormatterFunction(
+                                               mode='all',
+                                               message_formatter=ReasonableMessageFormatter(
                                                    lambda data: gerritReviewFmt(self.url, data),
                                                    "plain",
                                                    want_properties=True,
@@ -1036,11 +1041,10 @@ class GerritNixConfigurator(ConfiguratorBase):
                                                ),
                                            ),
                                        ])
-            # startCB, summaryCB are too noisy, we won't use them.
         )
 
         if self.prometheus_config is not None:
-            config['services'].append(reporters.Prometheus(port=self.prometheus_config.get('port', 9100), interface=self.prometheus_config.get('address', '')))
+            config_dict['services'].append(reporters.Prometheus(port=self.prometheus_config.get('port', 9100), interface=self.prometheus_config.get('address', '')))
 
         # Upstream defaults pretend they already do something similar
         # but they didn't work, hence the custom function.
@@ -1050,7 +1054,7 @@ class GerritNixConfigurator(ConfiguratorBase):
                 return ref
             return ref.rsplit('/', 1)[0]
 
-        config["services"].append(
+        config_dict["services"].append(
             util.OldBuildCanceller(
                 "build_canceller",
                 filters=[
@@ -1073,12 +1077,12 @@ class GerritNixConfigurator(ConfiguratorBase):
         systemd_secrets = secrets.SecretInAFile(
             dirname=os.environ["CREDENTIALS_DIRECTORY"],
         )
-        config["secretsProviders"].append(systemd_secrets)
+        config_dict["secretsProviders"].append(systemd_secrets)
 
-        config["www"].setdefault("plugins", {})
+        config_dict["www"].setdefault("plugins", {})
 
-        if "authz" not in config["www"]:
+        if "authz" not in config_dict["www"]:
-            config["www"]["authz"] = util.Authz(
+            config_dict["www"]["authz"] = util.Authz(
                 allowRules=[
                     util.AnyEndpointMatcher(role="admin", defaultDeny=False),
                     util.StopBuildEndpointMatcher(role="owner"),
@@ -1092,5 +1096,5 @@ class GerritNixConfigurator(ConfiguratorBase):
                 ],
             )
 
-        if "auth" not in config["www"] and self.auth_method is not None:
+        if "auth" not in config_dict["www"] and self.auth_method is not None:
-            config["www"]["auth"] = self.auth_method
+            config_dict["www"]["auth"] = self.auth_method

buildbot_nix/message_formatter.py

@@ -0,0 +1,46 @@
+from typing import Any, Callable, Literal
+from buildbot.reporters.message import MessageFormatterBase
+import dataclasses
+
+@dataclasses.dataclass
+class CallbackPayloadBuild:
+    # buddy i have no idea what the fuck is in this
+    build: dict[str, Any]
+
+@dataclasses.dataclass
+class CallbackPayloadBuildSet:
+    buildset: dict[str, Any]
+    # i have no idea what the fuck is in this honestly
+    builds: Any
+
+@dataclasses.dataclass
+class CallbackReturn:
+    body: str | None = None
+    subject: str | None = None
+    type: Literal['plain'] | Literal['html'] | Literal['json'] = 'plain'
+    extra_info: dict[str, Any] | None = None
+    # FIXME: support other template types, if they actually become necessary
+    template_type: Literal['plain'] = 'plain'
+
+class ReasonableMessageFormatter(MessageFormatterBase):
+    """
+    Message formatter which uses strongly typed data classes to reduce suffering slightly.
+    """
+    CallbackFunc = Callable[[CallbackPayloadBuild | CallbackPayloadBuildSet], CallbackReturn]
+    def __init__(self, function: CallbackFunc, template_type: str, **kwargs):
+        super().__init__(**kwargs)
+        self.template_type = template_type
+        self._function = function
+
+    def format_message_for_build(self, master, build, **kwargs):
+        return dataclasses.asdict(self._function(CallbackPayloadBuild(build=build)))
+
+    def format_message_for_buildset(self, master, buildset, builds, **kwargs):
+        return dataclasses.asdict(self._function(CallbackPayloadBuildSet(buildset=buildset, builds=builds)))
+
+    # These only exist as callbacks, the only one actually used is render_message_dict
+    def render_message_body(self, context):
+        return None
+
+    def render_message_subject(self, context):
+        return None

default.nix

@@ -1,4 +1,5 @@
 { setuptools, buildPythonPackage }:
+# TODO: figure out how to make this depend upon buildbot without leaking it further.
 buildPythonPackage {
   name = "buildbot-nix";
   format = "pyproject";

flake.nix

@@ -42,6 +42,11 @@
               pkgs.ruff
             ];
           };
+          devShells.default = pkgs.mkShell {
+            packages = [
+              self'.packages.buildbot-nix
+            ];
+          };
           packages.buildbot-nix = pkgs.python3.pkgs.callPackage ./default.nix { };
           checks =
             let