Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI status updates sent as direct message to author of commit #136

Merged
merged 8 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion documentation/config_docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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).

Expand All @@ -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"],
Expand All @@ -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
Expand Down
41 changes: 26 additions & 15 deletions lib/action.ml
Original file line number Diff line number Diff line change
Expand Up @@ -99,23 +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 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 []
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
Expand Down
1 change: 1 addition & 0 deletions lib/api.ml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module type Github = sig
end

module type Slack = sig
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
Expand Down
13 changes: 13 additions & 0 deletions lib/api_local.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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:_ ~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"
Expand All @@ -61,6 +62,18 @@ end
module Slack : Api.Slack = struct
include Slack_base

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;
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 =
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;
Expand Down
9 changes: 9 additions & 0 deletions lib/api_remote.ml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,15 @@ module Slack : Api.Slack = struct
(* must read whole response to update lexer state *)
ignore (Slack_j.read_ok_res s l)

(** [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))
~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 *)
let send_notification ~(ctx : Context.t) ~(msg : Slack_t.post_message_req) =
Expand Down
4 changes: 3 additions & 1 deletion lib/config.atd
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ type project_owners_rule <ocaml from="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;
}

Expand Down Expand Up @@ -33,10 +34,11 @@ type project_owners = {
type config = {
prefix_rules : prefix_rules;
label_rules : label_rules;
~status_rules <ocaml default="{allowed_pipelines = Some []; rules = []}"> : status_rules;
~status_rules <ocaml default="{allowed_pipelines = Some []; enable_direct_message = Some false; rules = []}"> : status_rules;
~project_owners <ocaml default="{rules = []}"> : project_owners;
~ignored_users <ocaml default="[]">: 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 <ocaml default="[]">: (string * string) list <json repr="object"> (* list of github to slack profile mappings *)
}

(* This specifies the Slack webhook to query to post to the channel with the given name *)
Expand Down
7 changes: 6 additions & 1 deletion lib/context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions lib/slack.atd
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions test/monorobot.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,5 +95,8 @@
"channel": "frontend-bot"
}
]
},
"user_mappings": {
"[email protected]": "[email protected]"
}
}
89 changes: 89 additions & 0 deletions test/slack_payloads.expected
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,24 @@ will notify #longest-a1
}
===== file ../mock_payloads/status.cancelled_test.json =====
===== file ../mock_payloads/status.failure_test.json =====
will notify #id[[email protected]]
{
"channel": "id[[email protected]]",
"text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: failure",
"attachments": [
{
"fallback": null,
"mrkdwn_in": [ "fields", "text" ],
"color": "danger",
"text": "*Description*: Build #2 failed (20 seconds).",
"fields": [
{
"value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: master"
}
]
}
]
}
will notify #default
{
"channel": "default",
Expand All @@ -518,8 +536,43 @@ 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 =====
will notify #id[[email protected]]
{
"channel": "id[[email protected]]",
"text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
"attachments": [
{
"fallback": null,
"mrkdwn_in": [ "fields", "text" ],
"color": "good",
"text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
"fields": [
{
"value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` 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[[email protected]]
{
"channel": "id[[email protected]]",
"text": "<https://github.com/Codertocat/Hello-World|[Codertocat/Hello-World]> CI Build Status notification: success",
"attachments": [
{
"fallback": null,
"mrkdwn_in": [ "fields", "text" ],
"color": "good",
"fields": [
{
"value": "*Commit*: `<https://github.com/Codertocat/Hello-World/commit/6113728f27ae82c7b1a177c8d03f9e96e0adf246|6113728f>` Initial commit - Codertocat\n*Branches*: master, changes, gh-pages"
}
]
}
]
}
will notify #default
{
"channel": "default",
Expand All @@ -538,6 +591,24 @@ will notify #default
]
}
===== file ../mock_payloads/status.success_test_main_branch.json =====
will notify #id[[email protected]]
{
"channel": "id[[email protected]]",
"text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
"attachments": [
{
"fallback": null,
"mrkdwn_in": [ "fields", "text" ],
"color": "good",
"text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
"fields": [
{
"value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: develop"
}
]
}
]
}
will notify #all-push-events
{
"channel": "all-push-events",
Expand All @@ -557,6 +628,24 @@ will notify #all-push-events
]
}
===== file ../mock_payloads/status.success_test_non_main_branch.json =====
will notify #id[[email protected]]
{
"channel": "id[[email protected]]",
"text": "<https://github.com/ahrefs/monorepo|[ahrefs/monorepo]> CI Build Status notification for <https://buildkite.com/org/pipeline2/builds/2|buildkite/pipeline2>: success",
"attachments": [
{
"fallback": null,
"mrkdwn_in": [ "fields", "text" ],
"color": "good",
"text": "*Description*: Build #2 passed (5 minutes, 19 seconds).",
"fields": [
{
"value": "*Commit*: `<https://github.com/ahrefs/monorepo/commit/0d95302addd66c1816bce1b1d495ed1c93ccd478|0d95302a>` Update README.md - Khady\n*Branch*: master"
}
]
}
]
}
will notify #default
{
"channel": "default",
Expand Down
Loading