From f81d5cecff7fddf424cfe446fdcbff813ad2db50 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 7 Jun 2024 18:41:25 -0700 Subject: [PATCH 1/4] allow granular selection of github notification method via destination Signed-off-by: Tony Li --- pkg/services/github.go | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/pkg/services/github.go b/pkg/services/github.go index e25af5f2..b0255a1a 100644 --- a/pkg/services/github.go +++ b/pkg/services/github.go @@ -20,9 +20,7 @@ import ( "github.com/argoproj/notifications-engine/pkg/util/text" ) -var ( - gitSuffix = regexp.MustCompile(`\.git$`) -) +var gitSuffix = regexp.MustCompile(`\.git$`) type GitHubOptions struct { AppID interface{} `json:"appID"` @@ -317,7 +315,7 @@ func fullNameByRepoURL(rawURL string) string { return path } -func (g gitHubService) Send(notification Notification, _ Destination) error { +func (g gitHubService) Send(notification Notification, dest Destination) error { if notification.GitHub == nil { return fmt.Errorf("config is empty") } @@ -326,7 +324,29 @@ func (g gitHubService) Send(notification Notification, _ Destination) error { if len(u) < 2 { return fmt.Errorf("GitHub.repoURL (%s) does not have a `/`", notification.GitHub.repoURL) } - if notification.GitHub.Status != nil { + + var ( + sendStatus bool + sendDeployment bool + sendComment bool + ) + + switch dest.Recipient { + case "status": + sendStatus = true + case "deployment": + sendDeployment = true + case "comment": + sendComment = true + default: + // match previous behavior: send all notifications if a destination is + // not specified + sendStatus = true + sendDeployment = true + sendComment = true + } + + if notification.GitHub.Status != nil && sendStatus { // maximum is 140 characters description := trunc(notification.Message, 140) _, _, err := g.client.Repositories.CreateStatus( @@ -346,7 +366,7 @@ func (g gitHubService) Send(notification Notification, _ Destination) error { } } - if notification.GitHub.Deployment != nil { + if notification.GitHub.Deployment != nil && sendDeployment { // maximum is 140 characters description := trunc(notification.Message, 140) deployments, _, err := g.client.Repositories.ListDeployments( @@ -406,7 +426,7 @@ func (g gitHubService) Send(notification Notification, _ Destination) error { } } - if notification.GitHub.PullRequestComment != nil { + if notification.GitHub.PullRequestComment != nil && sendComment { // maximum is 65536 characters body := trunc(notification.GitHub.PullRequestComment.Content, 65536) comment := &github.IssueComment{ From a7a1ceea628e3d2da0929ce1fe2602f5d0deafcc Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 7 Jun 2024 18:50:13 -0700 Subject: [PATCH 2/4] update docs Signed-off-by: Tony Li --- docs/services/github.md | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/services/github.md b/docs/services/github.md index 4cd25239..70241b4d 100644 --- a/docs/services/github.md +++ b/docs/services/github.md @@ -2,7 +2,13 @@ ## Parameters -The GitHub notification service changes commit status using [GitHub Apps](https://docs.github.com/en/developers/apps) and requires specifying the following settings: +The GitHub notification service can notify the following notification types: + +- Commit statuses +- Deployments +- Pull request comments + +It uses a [GitHub App](https://docs.github.com/en/developers/apps) and requires specifying the following settings: - `appID` - the app id - `installationID` - the app installation id @@ -46,7 +52,7 @@ stringData: -----END RSA PRIVATE KEY----- ``` -6. Create subscription for your GitHub integration +1. Create subscription for your GitHub integration ```yaml apiVersion: argoproj.io/v1alpha1 @@ -56,6 +62,19 @@ metadata: notifications.argoproj.io/subscribe..github: "" ``` +By default, the integration updates all configured notification types. +If you have multiple GitHub notification types configured, you can subscribe to a specific type by setting it as the annotation value: + +```yaml +apiVersion: argoproj.io/v1alpha1 +kind: Application +metadata: + annotations: + notifications.argoproj.io/subscribe..github: "status" + notifications.argoproj.io/subscribe..github: "deployment" + notifications.argoproj.io/subscribe..github: "comment" +``` + ## Templates ![](https://user-images.githubusercontent.com/18019529/108520497-168ce180-730e-11eb-93cb-b0b91f99bdc5.png) From 5e7aa5125692b4885f9dbebf3562a5cd874308a7 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Fri, 7 Jun 2024 19:16:54 -0700 Subject: [PATCH 3/4] update docs Signed-off-by: Tony Li --- docs/services/github.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/services/github.md b/docs/services/github.md index 70241b4d..80ee0de7 100644 --- a/docs/services/github.md +++ b/docs/services/github.md @@ -70,9 +70,8 @@ apiVersion: argoproj.io/v1alpha1 kind: Application metadata: annotations: - notifications.argoproj.io/subscribe..github: "status" - notifications.argoproj.io/subscribe..github: "deployment" - notifications.argoproj.io/subscribe..github: "comment" + notifications.argoproj.io/subscribe..github: "status" # or deployment or comment + notifications.argoproj.io/subscribe..github: "status;deployment" # 2 of the 3 types ``` ## Templates From 56a3811b7b4aac23e5aad8ca7b3be6c7e806f991 Mon Sep 17 00:00:00 2001 From: Tony Li Date: Tue, 11 Jun 2024 14:34:44 -0700 Subject: [PATCH 4/4] refactor based on review Signed-off-by: Tony Li --- pkg/services/github.go | 223 ++++++++++++++++++++++------------------- 1 file changed, 119 insertions(+), 104 deletions(-) diff --git a/pkg/services/github.go b/pkg/services/github.go index b0255a1a..5bc39b68 100644 --- a/pkg/services/github.go +++ b/pkg/services/github.go @@ -325,137 +325,152 @@ func (g gitHubService) Send(notification Notification, dest Destination) error { return fmt.Errorf("GitHub.repoURL (%s) does not have a `/`", notification.GitHub.repoURL) } - var ( - sendStatus bool - sendDeployment bool - sendComment bool - ) + // match previous behavior: + // send all notifications if a destination is not specified + sendAll := dest.Recipient == "" - switch dest.Recipient { - case "status": - sendStatus = true - case "deployment": - sendDeployment = true - case "comment": - sendComment = true - default: - // match previous behavior: send all notifications if a destination is - // not specified - sendStatus = true - sendDeployment = true - sendComment = true + if dest.Recipient == "status" || sendAll { + err := g.sendStatus(u[0], u[1], notification) + if err != nil { + return err + } } - if notification.GitHub.Status != nil && sendStatus { - // maximum is 140 characters - description := trunc(notification.Message, 140) - _, _, err := g.client.Repositories.CreateStatus( - context.Background(), - u[0], - u[1], - notification.GitHub.revision, - &github.RepoStatus{ - State: ¬ification.GitHub.Status.State, - Description: &description, - Context: ¬ification.GitHub.Status.Label, - TargetURL: ¬ification.GitHub.Status.TargetURL, - }, - ) + if dest.Recipient == "deployment" || sendAll { + err := g.sendDeployment(u[0], u[1], notification) if err != nil { return err } } - if notification.GitHub.Deployment != nil && sendDeployment { - // maximum is 140 characters - description := trunc(notification.Message, 140) - deployments, _, err := g.client.Repositories.ListDeployments( - context.Background(), - u[0], - u[1], - &github.DeploymentsListOptions{ - Ref: notification.GitHub.revision, - Environment: notification.GitHub.Deployment.Environment, - }, - ) + if dest.Recipient == "comment" || sendAll { + err := g.sendComment(u[0], u[1], notification) if err != nil { return err } + } - // if no reference is provided, use the revision - ref := notification.GitHub.Deployment.Reference - if ref == "" { - ref = notification.GitHub.revision - } + return nil +} - var deployment *github.Deployment - if len(deployments) != 0 { - deployment = deployments[0] - } else { - deployment, _, err = g.client.Repositories.CreateDeployment( - context.Background(), - u[0], - u[1], - &github.DeploymentRequest{ - Ref: &ref, - Environment: ¬ification.GitHub.Deployment.Environment, - RequiredContexts: ¬ification.GitHub.Deployment.RequiredContexts, - AutoMerge: notification.GitHub.Deployment.AutoMerge, - TransientEnvironment: notification.GitHub.Deployment.TransientEnvironment, - }, - ) - if err != nil { - return err - } - } - _, _, err = g.client.Repositories.CreateDeploymentStatus( +func (g gitHubService) sendStatus(owner, repo string, notification Notification) error { + if notification.GitHub.Status == nil { + return nil + } + + // maximum is 140 characters + description := trunc(notification.Message, 140) + _, _, err := g.client.Repositories.CreateStatus( + context.Background(), + owner, + repo, + notification.GitHub.revision, + &github.RepoStatus{ + State: ¬ification.GitHub.Status.State, + Description: &description, + Context: ¬ification.GitHub.Status.Label, + TargetURL: ¬ification.GitHub.Status.TargetURL, + }, + ) + return err +} + +func (g gitHubService) sendDeployment(owner, repo string, notification Notification) error { + if notification.GitHub.Deployment == nil { + return nil + } + + // maximum is 140 characters + description := trunc(notification.Message, 140) + deployments, _, err := g.client.Repositories.ListDeployments( + context.Background(), + owner, + repo, + &github.DeploymentsListOptions{ + Ref: notification.GitHub.revision, + Environment: notification.GitHub.Deployment.Environment, + }, + ) + if err != nil { + return err + } + + // if no reference is provided, use the revision + ref := notification.GitHub.Deployment.Reference + if ref == "" { + ref = notification.GitHub.revision + } + + var deployment *github.Deployment + if len(deployments) != 0 { + deployment = deployments[0] + } else { + deployment, _, err = g.client.Repositories.CreateDeployment( context.Background(), - u[0], - u[1], - *deployment.ID, - &github.DeploymentStatusRequest{ - State: ¬ification.GitHub.Deployment.State, - LogURL: ¬ification.GitHub.Deployment.LogURL, - Description: &description, - Environment: ¬ification.GitHub.Deployment.Environment, - EnvironmentURL: ¬ification.GitHub.Deployment.EnvironmentURL, + owner, + repo, + &github.DeploymentRequest{ + Ref: &ref, + Environment: ¬ification.GitHub.Deployment.Environment, + RequiredContexts: ¬ification.GitHub.Deployment.RequiredContexts, + AutoMerge: notification.GitHub.Deployment.AutoMerge, + TransientEnvironment: notification.GitHub.Deployment.TransientEnvironment, }, ) if err != nil { return err } } + _, _, err = g.client.Repositories.CreateDeploymentStatus( + context.Background(), + owner, + repo, + *deployment.ID, + &github.DeploymentStatusRequest{ + State: ¬ification.GitHub.Deployment.State, + LogURL: ¬ification.GitHub.Deployment.LogURL, + Description: &description, + Environment: ¬ification.GitHub.Deployment.Environment, + EnvironmentURL: ¬ification.GitHub.Deployment.EnvironmentURL, + }, + ) - if notification.GitHub.PullRequestComment != nil && sendComment { - // maximum is 65536 characters - body := trunc(notification.GitHub.PullRequestComment.Content, 65536) - comment := &github.IssueComment{ - Body: &body, - } + return err +} + +func (g gitHubService) sendComment(owner, repo string, notification Notification) error { + if notification.GitHub.PullRequestComment == nil { + return nil + } + + // maximum is 65536 characters + body := trunc(notification.GitHub.PullRequestComment.Content, 65536) + comment := &github.IssueComment{ + Body: &body, + } + + prs, _, err := g.client.PullRequests.ListPullRequestsWithCommit( + context.Background(), + owner, + repo, + notification.GitHub.revision, + nil, + ) + if err != nil { + return err + } - prs, _, err := g.client.PullRequests.ListPullRequestsWithCommit( + for _, pr := range prs { + _, _, err = g.client.Issues.CreateComment( context.Background(), - u[0], - u[1], - notification.GitHub.revision, - nil, + owner, + repo, + pr.GetNumber(), + comment, ) if err != nil { return err } - - for _, pr := range prs { - _, _, err = g.client.Issues.CreateComment( - context.Background(), - u[0], - u[1], - pr.GetNumber(), - comment, - ) - if err != nil { - return err - } - } } return nil