From 314bd3511ab13d104184072dfff7730e68c188ed Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Tue, 8 Aug 2023 14:20:42 +0800 Subject: [PATCH 1/8] api* : add lookup user by email API --- lib/api.ml | 2 ++ lib/api_local.ml | 10 ++++++++++ lib/api_remote.ml | 17 +++++++++++++++++ lib/slack.atd | 14 ++++++++++++++ 4 files changed, 43 insertions(+) diff --git a/lib/api.ml b/lib/api.ml index c1be62a0..28c6778c 100644 --- a/lib/api.ml +++ b/lib/api.ml @@ -18,6 +18,8 @@ module type Github = sig end module type Slack = sig + val lookup_user : ctx:Context.t -> email:string -> lookup_user_res slack_response Lwt.t + val send_notification : ctx:Context.t -> msg:post_message_req -> unit slack_response Lwt.t val send_chat_unfurl diff --git a/lib/api_local.ml b/lib/api_local.ml index 524c241d..4a566434 100644 --- a/lib/api_local.ml +++ b/lib/api_local.ml @@ -52,6 +52,7 @@ end (** The base implementation for local check payload debugging and mocking tests *) module Slack_base : Api.Slack = struct + let lookup_user ~ctx:_ ~email:_ = Lwt.return @@ Error "undefined for local setup" let send_notification ~ctx:_ ~msg:_ = Lwt.return @@ Error "undefined for local setup" let send_chat_unfurl ~ctx:_ ~channel:_ ~ts:_ ~unfurls:_ () = Lwt.return @@ Error "undefined for local setup" let send_auth_test ~ctx:_ () = Lwt.return @@ Error "undefined for local setup" @@ -61,6 +62,15 @@ end module Slack : Api.Slack = struct include Slack_base + let lookup_user ~ctx:_ ~email = + let msg = {Slack_t.email = email} in + let json = msg |> Slack_j.string_of_lookup_user_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string in + Stdio.printf "looking up slack user by email %s\n" email; + Stdio.printf "%s\n" json; + let mock_user = {Slack_t.id = "mock_id"; name="mock_name"; real_name="mock_real_name"} in + let mock_response = {Slack_t.user = mock_user} in + Lwt.return @@ Ok mock_response + let send_notification ~ctx:_ ~msg = let json = msg |> Slack_j.string_of_post_message_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string in Stdio.printf "will notify #%s\n" msg.channel; diff --git a/lib/api_remote.ml b/lib/api_remote.ml index 525d527f..ff54f9ca 100644 --- a/lib/api_remote.ml +++ b/lib/api_remote.ml @@ -117,6 +117,23 @@ module Slack : Api.Slack = struct (* must read whole response to update lexer state *) ignore (Slack_j.read_ok_res s l) + (** [lookup_user ctx email] queries slack for a user profile with [email] *) + let lookup_user ~(ctx: Context.t) ~email = + log#info "looking up %s\n" email; + let build_error e = fmt_error "%s\nfailed to lookup Slack user" e in + let secrets = Context.get_secrets_exn ctx in + match secrets.slack_access_token with + | None -> Lwt.return @@ build_error @@ sprintf "no token configured to lookup user email %s" email + | Some access_token -> + let headers = [ bearer_token_header access_token ] in + let data = {Slack_t.email = email} |> Slack_j.string_of_lookup_user_req in + let body = `Raw ("application/json", data) in + let url ="https://slack.com/api/users.lookupByEmail" in + log#info "data: %s" data; + match%lwt slack_api_request ~body ~headers `GET url Slack_j.read_lookup_user_res with + | Ok res -> Lwt.return @@ Ok res + | Error e -> Lwt.return @@ build_error e + (** [send_notification ctx msg] notifies [msg.channel] with the payload [msg]; uses web API with access token if available, or with webhook otherwise *) let send_notification ~(ctx : Context.t) ~(msg : Slack_t.post_message_req) = diff --git a/lib/slack.atd b/lib/slack.atd index 285a12b8..dab01ffa 100644 --- a/lib/slack.atd +++ b/lib/slack.atd @@ -67,6 +67,20 @@ type post_message_res = { channel: string; } +type lookup_user_req = { + email: string; +} + +type lookup_user_res = { + user: user; +} + +type user = { + id: string; + name: string; + real_name: string; +} + type link_shared_link = { domain: string; url: string; From d57a7a488da8e86b6fe71aeebde338e30ec62c5d Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Tue, 8 Aug 2023 17:05:25 +0800 Subject: [PATCH 2/8] action: reroute status updates to commit author --- lib/action.ml | 8 +++++++- test/slack_payloads.expected | 22 ++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 9cc90492..2f718e7b 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -99,7 +99,13 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let current_status = n.state in let rules = cfg.status_rules.rules in let action_on_match (branches : branch list) = - let default = Option.to_list cfg.prefix_rules.default_channel in + let%lwt default = + match%lwt Slack_api.lookup_user ~ctx ~email:n.commit.commit.author.email with + (* To send a DM, channel parameter is set to the user id of the recipient *) + | Ok res -> Lwt.return [ res.user.id ] + | Error e -> log#error "%s: nowhere to send status update" e; + Lwt.return [] + in let%lwt () = State.set_repo_pipeline_status ctx.state repo.url ~pipeline ~branches ~status:current_status in match List.is_empty branches with | true -> Lwt.return [] diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 1e3c84fe..f60cbc73 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -497,9 +497,11 @@ will notify #longest-a1 } ===== file ../mock_payloads/status.cancelled_test.json ===== ===== file ../mock_payloads/status.failure_test.json ===== -will notify #default +looking up slack user by email mail@example.org +{ "email": "mail@example.org" } +will notify #mock_id { - "channel": "default", + "channel": "mock_id", "text": " CI Build Status notification for : failure", "attachments": [ { @@ -518,11 +520,15 @@ will notify #default ===== file ../mock_payloads/status.merge_develop.json ===== ===== file ../mock_payloads/status.pending_test.json ===== ===== file ../mock_payloads/status.state_hide_success_test.json ===== +looking up slack user by email mail@example.org +{ "email": "mail@example.org" } ===== file ../mock_payloads/status.state_hide_success_test_disallowed_pipeline.json ===== ===== file ../mock_payloads/status.success_public_repo_no_buildkite.json ===== -will notify #default +looking up slack user by email 21031067+Codertocat@users.noreply.github.com +{ "email": "21031067+Codertocat@users.noreply.github.com" } +will notify #mock_id { - "channel": "default", + "channel": "mock_id", "text": " CI Build Status notification: success", "attachments": [ { @@ -538,6 +544,8 @@ will notify #default ] } ===== file ../mock_payloads/status.success_test_main_branch.json ===== +looking up slack user by email mail@example.org +{ "email": "mail@example.org" } will notify #all-push-events { "channel": "all-push-events", @@ -557,9 +565,11 @@ will notify #all-push-events ] } ===== file ../mock_payloads/status.success_test_non_main_branch.json ===== -will notify #default +looking up slack user by email mail@example.org +{ "email": "mail@example.org" } +will notify #mock_id { - "channel": "default", + "channel": "mock_id", "text": " CI Build Status notification for : success", "attachments": [ { From 19ca9a4dca1f8feeafcfe5d69c7081eb379b2cc6 Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Wed, 9 Aug 2023 09:20:39 +0800 Subject: [PATCH 3/8] api_remote: update lookup_user to be more terse --- lib/api_remote.ml | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/lib/api_remote.ml b/lib/api_remote.ml index ff54f9ca..21b0bc3d 100644 --- a/lib/api_remote.ml +++ b/lib/api_remote.ml @@ -118,21 +118,11 @@ module Slack : Api.Slack = struct ignore (Slack_j.read_ok_res s l) (** [lookup_user ctx email] queries slack for a user profile with [email] *) - let lookup_user ~(ctx: Context.t) ~email = - log#info "looking up %s\n" email; - let build_error e = fmt_error "%s\nfailed to lookup Slack user" e in - let secrets = Context.get_secrets_exn ctx in - match secrets.slack_access_token with - | None -> Lwt.return @@ build_error @@ sprintf "no token configured to lookup user email %s" email - | Some access_token -> - let headers = [ bearer_token_header access_token ] in - let data = {Slack_t.email = email} |> Slack_j.string_of_lookup_user_req in - let body = `Raw ("application/json", data) in - let url ="https://slack.com/api/users.lookupByEmail" in - log#info "data: %s" data; - match%lwt slack_api_request ~body ~headers `GET url Slack_j.read_lookup_user_res with - | Ok res -> Lwt.return @@ Ok res - | Error e -> Lwt.return @@ build_error e + let lookup_user ~(ctx : Context.t) ~email = + let data = Slack_j.string_of_lookup_user_req { Slack_t.email } in + request_token_auth ~name:"lookup user by email" + ~body:(`Raw ("application/json", data)) + ~ctx `GET "users.lookupByEmail" Slack_j.read_lookup_user_res (** [send_notification ctx msg] notifies [msg.channel] with the payload [msg]; uses web API with access token if available, or with webhook otherwise *) From 1964e0a3b641e1689d168e2b9a883120b7560fd3 Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Wed, 9 Aug 2023 09:57:26 +0800 Subject: [PATCH 4/8] Apply suggestions from code review Co-authored-by: Yasunari Watanabe --- lib/action.ml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index 2f718e7b..d2fc0492 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -103,8 +103,9 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct match%lwt Slack_api.lookup_user ~ctx ~email:n.commit.commit.author.email with (* To send a DM, channel parameter is set to the user id of the recipient *) | Ok res -> Lwt.return [ res.user.id ] - | Error e -> log#error "%s: nowhere to send status update" e; - Lwt.return [] + | Error e -> + log#warn "couldn't match commit email to slack profile: %s" e; + Lwt.return (Option.to_list cfg.prefix_rules.default_channel) in let%lwt () = State.set_repo_pipeline_status ctx.state repo.url ~pipeline ~branches ~status:current_status in match List.is_empty branches with From 2ebb4bd2c65466f8e2faf314067f6678579e433d Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Wed, 9 Aug 2023 10:30:05 +0800 Subject: [PATCH 5/8] api*: fmt and update logging to clearer --- lib/api.ml | 1 - lib/api_local.ml | 14 ++++++++------ test/slack_payloads.expected | 22 ++++++---------------- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/lib/api.ml b/lib/api.ml index 28c6778c..f1d22890 100644 --- a/lib/api.ml +++ b/lib/api.ml @@ -19,7 +19,6 @@ end module type Slack = sig val lookup_user : ctx:Context.t -> email:string -> lookup_user_res slack_response Lwt.t - val send_notification : ctx:Context.t -> msg:post_message_req -> unit slack_response Lwt.t val send_chat_unfurl diff --git a/lib/api_local.ml b/lib/api_local.ml index 4a566434..cac9f0d3 100644 --- a/lib/api_local.ml +++ b/lib/api_local.ml @@ -63,12 +63,14 @@ module Slack : Api.Slack = struct include Slack_base let lookup_user ~ctx:_ ~email = - let msg = {Slack_t.email = email} in - let json = msg |> Slack_j.string_of_lookup_user_req |> Yojson.Basic.from_string |> Yojson.Basic.pretty_to_string in - Stdio.printf "looking up slack user by email %s\n" email; - Stdio.printf "%s\n" json; - let mock_user = {Slack_t.id = "mock_id"; name="mock_name"; real_name="mock_real_name"} in - let mock_response = {Slack_t.user = mock_user} in + let mock_user = + { + Slack_t.id = sprintf "id[%s]" email; + name = sprintf "name[%s]" email; + real_name = sprintf "real_name[%s]" email; + } + in + let mock_response = { Slack_t.user = mock_user } in Lwt.return @@ Ok mock_response let send_notification ~ctx:_ ~msg = diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index f60cbc73..ad0ded2b 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -497,11 +497,9 @@ will notify #longest-a1 } ===== file ../mock_payloads/status.cancelled_test.json ===== ===== file ../mock_payloads/status.failure_test.json ===== -looking up slack user by email mail@example.org -{ "email": "mail@example.org" } -will notify #mock_id +will notify #id[mail@example.org] { - "channel": "mock_id", + "channel": "id[mail@example.org]", "text": " CI Build Status notification for : failure", "attachments": [ { @@ -520,15 +518,11 @@ will notify #mock_id ===== file ../mock_payloads/status.merge_develop.json ===== ===== file ../mock_payloads/status.pending_test.json ===== ===== file ../mock_payloads/status.state_hide_success_test.json ===== -looking up slack user by email mail@example.org -{ "email": "mail@example.org" } ===== file ../mock_payloads/status.state_hide_success_test_disallowed_pipeline.json ===== ===== file ../mock_payloads/status.success_public_repo_no_buildkite.json ===== -looking up slack user by email 21031067+Codertocat@users.noreply.github.com -{ "email": "21031067+Codertocat@users.noreply.github.com" } -will notify #mock_id +will notify #id[21031067+Codertocat@users.noreply.github.com] { - "channel": "mock_id", + "channel": "id[21031067+Codertocat@users.noreply.github.com]", "text": " CI Build Status notification: success", "attachments": [ { @@ -544,8 +538,6 @@ will notify #mock_id ] } ===== file ../mock_payloads/status.success_test_main_branch.json ===== -looking up slack user by email mail@example.org -{ "email": "mail@example.org" } will notify #all-push-events { "channel": "all-push-events", @@ -565,11 +557,9 @@ will notify #all-push-events ] } ===== file ../mock_payloads/status.success_test_non_main_branch.json ===== -looking up slack user by email mail@example.org -{ "email": "mail@example.org" } -will notify #mock_id +will notify #id[mail@example.org] { - "channel": "mock_id", + "channel": "id[mail@example.org]", "text": " CI Build Status notification for : success", "attachments": [ { From 70bd5cf8dae41ec6d56b0e7903dcd79f15604864 Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Wed, 9 Aug 2023 11:05:50 +0800 Subject: [PATCH 6/8] config_docs: update docs --- documentation/config_docs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/config_docs.md b/documentation/config_docs.md index 2bff4870..c2a3cc5e 100644 --- a/documentation/config_docs.md +++ b/documentation/config_docs.md @@ -163,7 +163,7 @@ The following takes place when a status notification is received. - `failure`: `allow` - `error`: `allow` - `success`: `allow_once` -1. For those payloads allowed by step 1, if it isn't a main branch build notification, route to the default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules. +1. For those payloads allowed by step 1, if it isn't a main branch build notification, query for a slack profile that matches the author of the commit and direct message them. If no profile is found, route to default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules. Internally, the bot keeps track of the status of the last allowed payload, for a given pipeline and branch. This information is used to evaluate the status rules (see below). From e6b2544e7b8049c9154c886ba119ebc358cebaf4 Mon Sep 17 00:00:00 2001 From: Lee Koon Wen Date: Thu, 14 Sep 2023 22:57:32 +0100 Subject: [PATCH 7/8] api: inject list of manual mappings of github to slack emails --- lib/action.ml | 2 +- lib/api.ml | 2 +- lib/api_local.ml | 5 +++-- lib/api_remote.ml | 6 ++++-- lib/config.atd | 1 + test/monorobot.json | 3 +++ test/slack_payloads.expected | 8 ++++---- 7 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/action.ml b/lib/action.ml index d2fc0492..4a2aff12 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -100,7 +100,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let rules = cfg.status_rules.rules in let action_on_match (branches : branch list) = let%lwt default = - match%lwt Slack_api.lookup_user ~ctx ~email:n.commit.commit.author.email with + match%lwt Slack_api.lookup_user ~ctx ~cfg ~email:n.commit.commit.author.email with (* To send a DM, channel parameter is set to the user id of the recipient *) | Ok res -> Lwt.return [ res.user.id ] | Error e -> diff --git a/lib/api.ml b/lib/api.ml index f1d22890..ae73d058 100644 --- a/lib/api.ml +++ b/lib/api.ml @@ -18,7 +18,7 @@ module type Github = sig end module type Slack = sig - val lookup_user : ctx:Context.t -> email:string -> lookup_user_res slack_response Lwt.t + val lookup_user : ctx:Context.t -> cfg:Config_t.config -> email:string -> lookup_user_res slack_response Lwt.t val send_notification : ctx:Context.t -> msg:post_message_req -> unit slack_response Lwt.t val send_chat_unfurl diff --git a/lib/api_local.ml b/lib/api_local.ml index cac9f0d3..e1cf376c 100644 --- a/lib/api_local.ml +++ b/lib/api_local.ml @@ -52,7 +52,7 @@ end (** The base implementation for local check payload debugging and mocking tests *) module Slack_base : Api.Slack = struct - let lookup_user ~ctx:_ ~email:_ = Lwt.return @@ Error "undefined for local setup" + let lookup_user ~ctx:_ ~cfg:_ ~email:_ = Lwt.return @@ Error "undefined for local setup" let send_notification ~ctx:_ ~msg:_ = Lwt.return @@ Error "undefined for local setup" let send_chat_unfurl ~ctx:_ ~channel:_ ~ts:_ ~unfurls:_ () = Lwt.return @@ Error "undefined for local setup" let send_auth_test ~ctx:_ () = Lwt.return @@ Error "undefined for local setup" @@ -62,7 +62,8 @@ end module Slack : Api.Slack = struct include Slack_base - let lookup_user ~ctx:_ ~email = + let lookup_user ~ctx:_ ~(cfg : Config_t.config) ~email = + let email = List.Assoc.find cfg.user_mappings ~equal:String.equal email |> Option.value ~default:email in let mock_user = { Slack_t.id = sprintf "id[%s]" email; diff --git a/lib/api_remote.ml b/lib/api_remote.ml index 21b0bc3d..a66b3dc7 100644 --- a/lib/api_remote.ml +++ b/lib/api_remote.ml @@ -117,8 +117,10 @@ module Slack : Api.Slack = struct (* must read whole response to update lexer state *) ignore (Slack_j.read_ok_res s l) - (** [lookup_user ctx email] queries slack for a user profile with [email] *) - let lookup_user ~(ctx : Context.t) ~email = + (** [lookup_user cfg email] queries slack for a user profile with [email] *) + let lookup_user ~(ctx : Context.t) ~(cfg : Config_t.config) ~email = + (* Check if config holds the Github to Slack email mapping *) + let email = List.Assoc.find cfg.user_mappings ~equal:String.equal email |> Option.value ~default:email in let data = Slack_j.string_of_lookup_user_req { Slack_t.email } in request_token_auth ~name:"lookup user by email" ~body:(`Raw ("application/json", data)) diff --git a/lib/config.atd b/lib/config.atd index f1a1d68d..b818145f 100644 --- a/lib/config.atd +++ b/lib/config.atd @@ -37,6 +37,7 @@ type config = { ~project_owners : project_owners; ~ignored_users : string list; (* list of ignored users *) ?main_branch_name : string nullable; (* the name of the main branch; used to filter out notifications about merges of main branch into other branches *) + ~user_mappings : (string * string) list (* list of github to slack profile mappings *) } (* This specifies the Slack webhook to query to post to the channel with the given name *) diff --git a/test/monorobot.json b/test/monorobot.json index 111c01fc..f6eedadc 100644 --- a/test/monorobot.json +++ b/test/monorobot.json @@ -95,5 +95,8 @@ "channel": "frontend-bot" } ] + }, + "user_mappings": { + "mail@example.org": "slack_mail@example.com" } } diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index ad0ded2b..5875bb05 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -497,9 +497,9 @@ will notify #longest-a1 } ===== file ../mock_payloads/status.cancelled_test.json ===== ===== file ../mock_payloads/status.failure_test.json ===== -will notify #id[mail@example.org] +will notify #id[slack_mail@example.com] { - "channel": "id[mail@example.org]", + "channel": "id[slack_mail@example.com]", "text": " CI Build Status notification for : failure", "attachments": [ { @@ -557,9 +557,9 @@ will notify #all-push-events ] } ===== file ../mock_payloads/status.success_test_non_main_branch.json ===== -will notify #id[mail@example.org] +will notify #id[slack_mail@example.com] { - "channel": "id[mail@example.org]", + "channel": "id[slack_mail@example.com]", "text": " CI Build Status notification for : success", "attachments": [ { From 6a4ac5ad19d9cafbb45d7f7d8bec101346e3b8ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis=20Roch=C3=A9=20=28Ahrefs=29?= Date: Wed, 14 Feb 2024 05:51:49 +0000 Subject: [PATCH 8/8] action: always direct message commit author on status event regardless of the branch --- documentation/config_docs.md | 4 +- lib/action.ml | 38 ++++++++------- lib/config.atd | 3 +- lib/context.ml | 7 ++- test/slack_payloads.expected | 89 ++++++++++++++++++++++++++++++++++++ 5 files changed, 121 insertions(+), 20 deletions(-) diff --git a/documentation/config_docs.md b/documentation/config_docs.md index c2a3cc5e..7a8e4e0b 100644 --- a/documentation/config_docs.md +++ b/documentation/config_docs.md @@ -163,7 +163,7 @@ The following takes place when a status notification is received. - `failure`: `allow` - `error`: `allow` - `success`: `allow_once` -1. For those payloads allowed by step 1, if it isn't a main branch build notification, query for a slack profile that matches the author of the commit and direct message them. If no profile is found, route to default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules. +1. For those payloads allowed by step 1, if it isn't a main branch build notification, route to the default channel to reduce spam in topic channels. Otherwise, check the notification commit's files according to the prefix rules. In addition, query for a slack profile that matches the author of the commit and direct message them. Internally, the bot keeps track of the status of the last allowed payload, for a given pipeline and branch. This information is used to evaluate the status rules (see below). @@ -175,6 +175,7 @@ Internally, the bot keeps track of the status of the last allowed payload, for a "default", "buildkite/monorobot-test" ], + "enable_direct_message": true, "rules": [ { "on": ["failure"], @@ -196,6 +197,7 @@ Internally, the bot keeps track of the status of the last allowed payload, for a | value | description | default | |-|-|-| | `allowed_pipelines` | a list of pipeline names; if specified, payloads whose pipeline name is not in the list will be ignored immediately, without checking the **status rules** | all pipelines included in the status rule check | +| `enable_direct_message` | control direct message sent to notify about build status | false | | `rules` | a list of **status rules** to determine whether to *allow* or *ignore* a payload for further processing | required field | ### Status Rules diff --git a/lib/action.ml b/lib/action.ml index 4a2aff12..8d0d6b8c 100644 --- a/lib/action.ml +++ b/lib/action.ml @@ -99,30 +99,34 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct let current_status = n.state in let rules = cfg.status_rules.rules in let action_on_match (branches : branch list) = - let%lwt default = + let%lwt direct_message = match%lwt Slack_api.lookup_user ~ctx ~cfg ~email:n.commit.commit.author.email with (* To send a DM, channel parameter is set to the user id of the recipient *) | Ok res -> Lwt.return [ res.user.id ] | Error e -> log#warn "couldn't match commit email to slack profile: %s" e; - Lwt.return (Option.to_list cfg.prefix_rules.default_channel) + Lwt.return [] in + let default = Option.to_list cfg.prefix_rules.default_channel in let%lwt () = State.set_repo_pipeline_status ctx.state repo.url ~pipeline ~branches ~status:current_status in - match List.is_empty branches with - | true -> Lwt.return [] - | false -> - match cfg.main_branch_name with - | None -> Lwt.return default - | Some main_branch_name -> - (* non-main branch build notifications go to default channel to reduce spam in topic channels *) - match List.exists branches ~f:(fun { name } -> String.equal name main_branch_name) with - | false -> Lwt.return default - | true -> - let sha = n.commit.sha in - ( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with - | Error e -> action_error e - | Ok commit -> Lwt.return @@ partition_commit cfg commit.files - ) + let%lwt chans = + match List.is_empty branches with + | true -> Lwt.return [] + | false -> + match cfg.main_branch_name with + | None -> Lwt.return default + | Some main_branch_name -> + (* non-main branch build notifications go to default channel to reduce spam in topic channels *) + match List.exists branches ~f:(fun { name } -> String.equal name main_branch_name) with + | false -> Lwt.return default + | true -> + let sha = n.commit.sha in + ( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with + | Error e -> action_error e + | Ok commit -> Lwt.return @@ partition_commit cfg commit.files + ) + in + Lwt.return (direct_message @ chans) in if Context.is_pipeline_allowed ctx repo.url ~pipeline then begin let%lwt repo_state = State.find_or_add_repo ctx.state repo.url in diff --git a/lib/config.atd b/lib/config.atd index b818145f..5e8cd6ed 100644 --- a/lib/config.atd +++ b/lib/config.atd @@ -6,6 +6,7 @@ type project_owners_rule = abstract (* This type of rule is used for CI build notifications. *) type status_rules = { ?allowed_pipelines : string list nullable; (* keep only status events with a title matching this list *) + ?enable_direct_message: bool nullable; rules: status_rule list; } @@ -33,7 +34,7 @@ type project_owners = { type config = { prefix_rules : prefix_rules; label_rules : label_rules; - ~status_rules : status_rules; + ~status_rules : status_rules; ~project_owners : project_owners; ~ignored_users : string list; (* list of ignored users *) ?main_branch_name : string nullable; (* the name of the main branch; used to filter out notifications about merges of main branch into other branches *) diff --git a/lib/context.ml b/lib/context.ml index 1df57c1a..a6f6c746 100644 --- a/lib/context.ml +++ b/lib/context.ml @@ -2,6 +2,8 @@ open Base open Common open Devkit +let log = Log.from "context" + exception Context_error of string let context_error fmt = Printf.ksprintf (fun msg -> raise (Context_error msg)) fmt @@ -69,7 +71,10 @@ let is_pipeline_allowed ctx repo_url ~pipeline = | Some allowed_pipelines when not @@ List.exists allowed_pipelines ~f:(String.equal pipeline) -> false | _ -> true -let log = Log.from "context" +let is_status_direct_message_enabled ctx repo_url = + match find_repo_config ctx repo_url with + | None -> true + | Some config -> Option.value config.status_rules.enable_direct_message ~default:false let refresh_secrets ctx = let path = ctx.secrets_filepath in diff --git a/test/slack_payloads.expected b/test/slack_payloads.expected index 5875bb05..3529b45f 100644 --- a/test/slack_payloads.expected +++ b/test/slack_payloads.expected @@ -515,9 +515,45 @@ will notify #id[slack_mail@example.com] } ] } +will notify #default +{ + "channel": "default", + "text": " CI Build Status notification for : failure", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "fields", "text" ], + "color": "danger", + "text": "*Description*: Build #2 failed (20 seconds).", + "fields": [ + { + "value": "*Commit*: `` Update README.md - Khady\n*Branch*: master" + } + ] + } + ] +} ===== file ../mock_payloads/status.merge_develop.json ===== ===== file ../mock_payloads/status.pending_test.json ===== ===== file ../mock_payloads/status.state_hide_success_test.json ===== +will notify #id[slack_mail@example.com] +{ + "channel": "id[slack_mail@example.com]", + "text": " CI Build Status notification for : success", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "fields", "text" ], + "color": "good", + "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).", + "fields": [ + { + "value": "*Commit*: `` Update README.md - Khady\n*Branch*: master" + } + ] + } + ] +} ===== file ../mock_payloads/status.state_hide_success_test_disallowed_pipeline.json ===== ===== file ../mock_payloads/status.success_public_repo_no_buildkite.json ===== will notify #id[21031067+Codertocat@users.noreply.github.com] @@ -537,7 +573,42 @@ will notify #id[21031067+Codertocat@users.noreply.github.com] } ] } +will notify #default +{ + "channel": "default", + "text": " CI Build Status notification: success", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "fields", "text" ], + "color": "good", + "fields": [ + { + "value": "*Commit*: `` Initial commit - Codertocat\n*Branches*: master, changes, gh-pages" + } + ] + } + ] +} ===== file ../mock_payloads/status.success_test_main_branch.json ===== +will notify #id[slack_mail@example.com] +{ + "channel": "id[slack_mail@example.com]", + "text": " CI Build Status notification for : success", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "fields", "text" ], + "color": "good", + "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).", + "fields": [ + { + "value": "*Commit*: `` Update README.md - Khady\n*Branch*: develop" + } + ] + } + ] +} will notify #all-push-events { "channel": "all-push-events", @@ -575,6 +646,24 @@ will notify #id[slack_mail@example.com] } ] } +will notify #default +{ + "channel": "default", + "text": " CI Build Status notification for : success", + "attachments": [ + { + "fallback": null, + "mrkdwn_in": [ "fields", "text" ], + "color": "good", + "text": "*Description*: Build #2 passed (5 minutes, 19 seconds).", + "fields": [ + { + "value": "*Commit*: `` Update README.md - Khady\n*Branch*: master" + } + ] + } + ] +} ===== file ../mock_slack_events/commits.one_file_modified.json ===== will unfurl in #C047QTRD1CH {