feat(fun/clbot,ops/machines/whitby): filter tvix-dev clbot r/8212

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 <flokli@flokli.de>
Reviewed-by: lukegb <lukegb@tvl.fyi>
Tested-by: BuildkiteCI

4 files changed, 55 insertions, 9 deletions

diff --git a/fun/clbot/clbot.go b/fun/clbot/clbot.go
index 72a7f5634332..40b044f45d39 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 000000000000..567540c364f7
--- /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 41391c8c0b87..4c2fd0f2f533 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 bdddff6c810b..0a436a8749d0 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);
   };
 }