From 9559ef56e3935618d63bc7b96136ec06db7e9bec Mon Sep 17 00:00:00 2001 From: Profpatsch Date: Mon, 13 May 2024 17:54:34 +0200 Subject: [PATCH] feat(fun/clbot,ops/machines/whitby): filter tvix-dev clbot In #tvix-dev, we want to display only CLs that relate to tvix and related projects. So use a pretty dumb allow-list for which CLs to display in that channel. Change-Id: I3ef50b64e3d7fbc27a6690be6a10f1b55c04cd6e Reviewed-on: https://cl.tvl.fyi/c/depot/+/11658 Reviewed-by: flokli Reviewed-by: lukegb Tested-by: BuildkiteCI --- fun/clbot/clbot.go | 22 +++++++++++++++++++--- fun/clbot/clbot_test.go | 24 ++++++++++++++++++++++++ ops/machines/whitby/default.nix | 7 ++++++- ops/modules/clbot.nix | 11 ++++++----- 4 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 fun/clbot/clbot_test.go diff --git a/fun/clbot/clbot.go b/fun/clbot/clbot.go index 72a7f5634..40b044f45 100644 --- a/fun/clbot/clbot.go +++ b/fun/clbot/clbot.go @@ -41,7 +41,8 @@ var ( notifyRepo = flag.String("notify_repo", "depot", "Repo name to notify about") notifyBranches = stringSetFlag{} - neverPing = flag.String("never_ping", "marcus", "Comma-separated terms that should never ping users") + neverPing = flag.String("never_ping", "marcus", "Comma-separated terms that should never ping users") + onlyDisplay = flag.String("only_display", "", "Comma-separated substrings of the gerrit CL Change Subject that should be shown (everything else is dropped)") ) func init() { @@ -193,6 +194,21 @@ func nopingAll(username, message string) string { return strings.ReplaceAll(message, username, noping(username)) } +// changeShouldBeSkipped applies the list of channels in `onlyDisplay` +// to whether we should skip displaying a CL. +func changeShouldBeSkipped(onlyDisplay string, changeSubject string) bool { + // case when we don’t want to filter + if onlyDisplay == "" { + return false + } + for _, needle := range strings.Split(onlyDisplay, ",") { + if strings.Contains(changeSubject, needle) { + return false + } + } + return true +} + func patchSetURL(c gerritevents.Change, p gerritevents.PatchSet) string { return fmt.Sprintf("https://cl.tvl.fyi/%d", c.Number) } @@ -248,13 +264,13 @@ func main() { var parsedMsg string switch e := e.(type) { case *gerritevents.PatchSetCreated: - if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || e.PatchSet.Number != 1 { + if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || e.PatchSet.Number != 1 || changeShouldBeSkipped(*onlyDisplay, e.Change.Subject) { continue } user := username(e.PatchSet.Uploader) parsedMsg = nopingAll(user, fmt.Sprintf("CL/%d proposed by %s - %s - %s", e.Change.Number, user, e.Change.Subject, patchSetURL(e.Change, e.PatchSet))) case *gerritevents.ChangeMerged: - if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] { + if e.Change.Project != *notifyRepo || !notifyBranches[e.Change.Branch] || changeShouldBeSkipped(*onlyDisplay, e.Change.Subject) { continue } owner := username(e.Change.Owner) diff --git a/fun/clbot/clbot_test.go b/fun/clbot/clbot_test.go new file mode 100644 index 000000000..567540c36 --- /dev/null +++ b/fun/clbot/clbot_test.go @@ -0,0 +1,24 @@ +package main + +import ( + "testing" +) + +func TestChangeShouldBeSkipped(t *testing.T) { + dontSkipAny := "" + if changeShouldBeSkipped(dontSkipAny, "mysubject") { + t.Fatal("dontSkipAny should not not be skip any") + } + + showThese := "A,B" + if changeShouldBeSkipped(showThese, "A") { + t.Fatal("A should be shown") + } + if changeShouldBeSkipped(showThese, "B") { + t.Fatal("B should be shown") + } + if !changeShouldBeSkipped(showThese, "C") { + t.Fatal("C should not be shown") + } + +} diff --git a/ops/machines/whitby/default.nix b/ops/machines/whitby/default.nix index 41391c8c0..4c2fd0f2f 100644 --- a/ops/machines/whitby/default.nix +++ b/ops/machines/whitby/default.nix @@ -347,7 +347,12 @@ in # Start the Gerrit->IRC bot services.depot.clbot = { enable = true; - channels = [ "#tvix-dev" "#tvl" ]; + channels = { + "#tvl" = { }; + "#tvix-dev" = { + only_display = "tvix,nix-compat,third_party,third-party,3p"; + }; + }; # See //fun/clbot for details. flags = { diff --git a/ops/modules/clbot.nix b/ops/modules/clbot.nix index bdddff6c8..0a436a874 100644 --- a/ops/modules/clbot.nix +++ b/ops/modules/clbot.nix @@ -7,6 +7,7 @@ let inherit (lib) listToAttrs + mapAttrsToList mkEnableOption mkIf mkOption @@ -25,13 +26,13 @@ let ${pkgs.systemd}/bin/systemd-escape '${name}' >> $out '')); - mkUnit = flags: channel: { + mkUnit = channel: channelFlags: { name = "clbot-${systemdEscape channel}"; value = { description = "${description} to ${channel}"; wantedBy = [ "multi-user.target" ]; - script = "${depot.fun.clbot}/bin/clbot ${mkFlags (cfg.flags // { + script = "${depot.fun.clbot}/bin/clbot ${mkFlags (cfg.flags // channelFlags // { irc_channel = channel; })} -alsologtostderr"; @@ -53,8 +54,8 @@ in }; channels = mkOption { - type = with types; listOf str; - description = "Channels in which to post (generates one unit per channel)"; + type = with types; attrsOf (attrsOf str); + description = "Channels in which to post (generates one unit per channel); nested attrs are used as extra flags to the service, which override the attrs in `flags`"; }; secretsFile = mkOption { @@ -77,6 +78,6 @@ in }; }; - systemd.services = listToAttrs (map (mkUnit cfg.flags) cfg.channels); + systemd.services = listToAttrs (mapAttrsToList mkUnit cfg.channels); }; }