From 9f6746c9faf3880060245a5e1a2f6f3947b0812e Mon Sep 17 00:00:00 2001 From: d-lau Date: Tue, 5 May 2020 17:14:12 +0900 Subject: [PATCH 1/4] added feature to label issues with label on no changes --- README.md | 24 +++++++++++++ config/config.go | 10 ++++-- config/config_test.go | 7 ++-- ...-with-destroy-and-no-changes.tfnotify.yaml | 2 ++ main.go | 1 + notifier/github/client.go | 2 ++ notifier/github/github.go | 6 ++++ notifier/github/github_test.go | 8 +++++ notifier/github/notify.go | 11 ++++++ notifier/github/notify_test.go | 20 +++++++++++ terraform/parser.go | 29 ++++++++------- terraform/parser_test.go | 36 ++++++++++--------- 12 files changed, 124 insertions(+), 32 deletions(-) rename example-with-destroy.tfnotify.yaml => example-with-destroy-and-no-changes.tfnotify.yaml (91%) diff --git a/README.md b/README.md index e53603f..6340d54 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,30 @@ terraform: # ... ``` +You can also let tfnotify add a label to PRs whose `terraform plan` output result in no change to the current infrastructure. + +```yaml +--- +# ... +terraform: + # ... + plan: + template: | + {{ .Title }} [CI link]( {{ .Link }} ) + {{ .Message }} + {{if .Result}} + 
{{ .Result }}
+
+ {{end}} +
Details (Click me) + + 
{{ .Body }}
+
+ when_no_changes: + github_label: "no-changes" + # ... +``` + Sometimes you may want not to HTML-escape Terraform command outputs. For example, when you use code block to print command output, it's better to use raw characters instead of character references (e.g. `-/+` -> `-/+`, `"` -> `"`). diff --git a/config/config.go b/config/config.go index ad9e62f..c75a183 100644 --- a/config/config.go +++ b/config/config.go @@ -81,8 +81,9 @@ type Fmt struct { // Plan is a terraform plan config type Plan struct { - Template string `yaml:"template"` - WhenDestroy WhenDestroy `yaml:"when_destroy,omitempty"` + Template string `yaml:"template"` + WhenDestroy WhenDestroy `yaml:"when_destroy,omitempty"` + WhenNoChanges WhenNoChanges `yaml:"when_no_changes,omitempty"` } // WhenDestroy is a configuration to notify the plan result contains destroy operation @@ -90,6 +91,11 @@ type WhenDestroy struct { Template string `yaml:"template"` } +// WhenNoChange is a configuration to add a label when the plan result contains no change +type WhenNoChanges struct { + GithubLabel string `yaml:"github_label,omitempty"` +} + // Apply is a terraform apply config type Apply struct { Template string `yaml:"template"` diff --git a/config/config_test.go b/config/config_test.go index 5d37c8a..e90a954 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -63,7 +63,7 @@ func TestLoadFile(t *testing.T) { ok: true, }, { - file: "../example-with-destroy.tfnotify.yaml", + file: "../example-with-destroy-and-no-changes.tfnotify.yaml", cfg: Config{ CI: "circleci", Notifier: Notifier{ @@ -96,13 +96,16 @@ func TestLoadFile(t *testing.T) { WhenDestroy: WhenDestroy{ Template: "## :warning: WARNING: Resource Deletion will happen :warning:\n\nThis plan contains **resource deletion**. Please check the plan result very carefully!\n", }, + WhenNoChanges: WhenNoChanges{ + GithubLabel: "no-changes", + }, }, Apply: Apply{ Template: "", }, UseRawOutput: false, }, - path: "../example-with-destroy.tfnotify.yaml", + path: "../example-with-destroy-and-no-changes.tfnotify.yaml", }, ok: true, }, diff --git a/example-with-destroy.tfnotify.yaml b/example-with-destroy-and-no-changes.tfnotify.yaml similarity index 91% rename from example-with-destroy.tfnotify.yaml rename to example-with-destroy-and-no-changes.tfnotify.yaml index 56d8570..410df8d 100644 --- a/example-with-destroy.tfnotify.yaml +++ b/example-with-destroy-and-no-changes.tfnotify.yaml @@ -18,6 +18,8 @@ terraform: 
{{ .Body }}
+ when_no_changes: + github_label: "no-changes" when_destroy: template: | ## :warning: WARNING: Resource Deletion will happen :warning: diff --git a/main.go b/main.go index e031976..ac755e7 100644 --- a/main.go +++ b/main.go @@ -115,6 +115,7 @@ func (t *tfnotify) Run() error { Template: t.template, DestroyWarningTemplate: t.destroyWarningTemplate, WarnDestroy: t.warnDestroy, + NoChangesLabel: t.config.Terraform.Plan.WhenNoChanges.GithubLabel, }) if err != nil { return err diff --git a/notifier/github/client.go b/notifier/github/client.go index 2d4586b..cea8a5f 100644 --- a/notifier/github/client.go +++ b/notifier/github/client.go @@ -48,6 +48,8 @@ type Config struct { // DestroyWarningTemplate is used only for additional warning // the plan result contains destroy operation DestroyWarningTemplate terraform.Template + // NoChangesLabel is a label to add to PRs when terraform output contains no changes + NoChangesLabel string } // PullRequest represents GitHub Pull Request metadata diff --git a/notifier/github/github.go b/notifier/github/github.go index db71ffd..e5cde0a 100644 --- a/notifier/github/github.go +++ b/notifier/github/github.go @@ -11,6 +11,7 @@ type API interface { IssuesCreateComment(ctx context.Context, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) IssuesDeleteComment(ctx context.Context, commentID int64) (*github.Response, error) IssuesListComments(ctx context.Context, number int, opt *github.IssueListCommentsOptions) ([]*github.IssueComment, *github.Response, error) + IssuesAddLabels(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) RepositoriesListCommits(ctx context.Context, opt *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) RepositoriesGetCommit(ctx context.Context, sha string) (*github.RepositoryCommit, *github.Response, error) @@ -37,6 +38,11 @@ func (g *GitHub) IssuesListComments(ctx context.Context, number int, opt *github return g.Client.Issues.ListComments(ctx, g.owner, g.repo, number, opt) } +// IssuesAddLabels is a wrapper of https://godoc.org/github.com/google/go-github/github#IssuesService.AddLabelsToIssue +func (g *GitHub) IssuesAddLabels(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) { + return g.Client.Issues.AddLabelsToIssue(ctx, g.owner, g.repo, number, labels) +} + // RepositoriesCreateComment is a wrapper of https://godoc.org/github.com/google/go-github/github#RepositoriesService.CreateComment func (g *GitHub) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return g.Client.Repositories.CreateComment(ctx, g.owner, g.repo, sha, comment) diff --git a/notifier/github/github_test.go b/notifier/github/github_test.go index 982eeb6..4a17c68 100644 --- a/notifier/github/github_test.go +++ b/notifier/github/github_test.go @@ -12,6 +12,7 @@ type fakeAPI struct { FakeIssuesCreateComment func(ctx context.Context, number int, comment *github.IssueComment) (*github.IssueComment, *github.Response, error) FakeIssuesDeleteComment func(ctx context.Context, commentID int64) (*github.Response, error) FakeIssuesListComments func(ctx context.Context, number int, opt *github.IssueListCommentsOptions) ([]*github.IssueComment, *github.Response, error) + FakeIssuesAddLabels func(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) FakeRepositoriesCreateComment func(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) FakeRepositoriesListCommits func(ctx context.Context, opt *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) FakeRepositoriesGetCommit func(ctx context.Context, sha string) (*github.RepositoryCommit, *github.Response, error) @@ -29,6 +30,10 @@ func (g *fakeAPI) IssuesListComments(ctx context.Context, number int, opt *githu return g.FakeIssuesListComments(ctx, number, opt) } +func (g *fakeAPI) IssuesAddLabels(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) { + return g.FakeIssuesAddLabels(ctx, number, labels) +} + func (g *fakeAPI) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return g.FakeRepositoriesCreateComment(ctx, sha, comment) } @@ -66,6 +71,9 @@ func newFakeAPI() fakeAPI { } return comments, nil, nil }, + FakeIssuesAddLabels: func(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) { + return nil, nil, nil + }, FakeRepositoriesCreateComment: func(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return &github.RepositoryComment{ ID: github.Int64(28427394), diff --git a/notifier/github/notify.go b/notifier/github/notify.go index 8cd74cf..565c412 100644 --- a/notifier/github/notify.go +++ b/notifier/github/notify.go @@ -1,6 +1,7 @@ package github import ( + "context" "github.com/mercari/tfnotify/terraform" ) @@ -30,6 +31,16 @@ func (g *NotifyService) Notify(body string) (exit int, err error) { return result.ExitCode, err } } + if result.HasNoChanges && cfg.PR.IsNumber() && cfg.NoChangesLabel != "" { + _, _, err = g.client.API.IssuesAddLabels( + context.Background(), + cfg.PR.Number, + []string{cfg.NoChangesLabel}, + ) + if err != nil { + return result.ExitCode, err + } + } } template.SetValue(terraform.CommonTemplate{ diff --git a/notifier/github/notify_test.go b/notifier/github/notify_test.go index 9394991..2cc317b 100644 --- a/notifier/github/notify_test.go +++ b/notifier/github/notify_test.go @@ -124,6 +124,26 @@ func TestNotifyNotify(t *testing.T) { ok: true, exitCode: 0, }, + { + // valid with no changes + // TODO(drlau): check that the label was actually added + config: Config{ + Token: "token", + Owner: "owner", + Repo: "repo", + PR: PullRequest{ + Revision: "", + Number: 1, + Message: "message", + }, + Parser: terraform.NewPlanParser(), + Template: terraform.NewPlanTemplate(terraform.DefaultPlanTemplate), + NoChangesLabel: "terraform/no-changes", + }, + body: "No changes. Infrastructure is up-to-date.", + ok: true, + exitCode: 0, + }, { // valid, contains destroy, but not to notify config: Config{ diff --git a/terraform/parser.go b/terraform/parser.go index 1c6926a..b7cabea 100644 --- a/terraform/parser.go +++ b/terraform/parser.go @@ -13,10 +13,11 @@ type Parser interface { // ParseResult represents the result of parsed terraform execution type ParseResult struct { - Result string - HasDestroy bool - ExitCode int - Error error + Result string + HasDestroy bool + HasNoChanges bool + ExitCode int + Error error } // DefaultParser is a parser for terraform commands @@ -31,9 +32,10 @@ type FmtParser struct { // PlanParser is a parser for terraform plan type PlanParser struct { - Pass *regexp.Regexp - Fail *regexp.Regexp - HasDestroy *regexp.Regexp + Pass *regexp.Regexp + Fail *regexp.Regexp + HasDestroy *regexp.Regexp + HasNoChanges *regexp.Regexp } // ApplyParser is a parser for terraform apply @@ -60,7 +62,8 @@ func NewPlanParser() *PlanParser { Pass: regexp.MustCompile(`(?m)^(Plan: \d|No changes.)`), Fail: regexp.MustCompile(`(?m)^(Error: )`), // "0 to destroy" should be treated as "no destroy" - HasDestroy: regexp.MustCompile(`(?m)([1-9][0-9]* to destroy.)`), + HasDestroy: regexp.MustCompile(`(?m)([1-9][0-9]* to destroy.)`), + HasNoChanges: regexp.MustCompile(`(?m)^(No changes. Infrastructure is up-to-date.)`), } } @@ -122,12 +125,14 @@ func (p *PlanParser) Parse(body string) ParseResult { } hasDestroy := p.HasDestroy.MatchString(line) + hasNoChanges := p.HasNoChanges.MatchString(line) return ParseResult{ - Result: result, - HasDestroy: hasDestroy, - ExitCode: exitCode, - Error: nil, + Result: result, + HasDestroy: hasDestroy, + HasNoChanges: hasNoChanges, + ExitCode: exitCode, + Error: nil, } } diff --git a/terraform/parser_test.go b/terraform/parser_test.go index c7f964e..5988b81 100644 --- a/terraform/parser_test.go +++ b/terraform/parser_test.go @@ -307,20 +307,22 @@ func TestPlanParserParse(t *testing.T) { name: "plan ok pattern", body: planSuccessResult, result: ParseResult{ - Result: "Plan: 1 to add, 0 to change, 0 to destroy.", - HasDestroy: false, - ExitCode: 0, - Error: nil, + Result: "Plan: 1 to add, 0 to change, 0 to destroy.", + HasDestroy: false, + HasNoChanges: false, + ExitCode: 0, + Error: nil, }, }, { name: "no stdin", body: "", result: ParseResult{ - Result: "", - HasDestroy: false, - ExitCode: 1, - Error: errors.New("cannot parse plan result"), + Result: "", + HasDestroy: false, + HasNoChanges: false, + ExitCode: 1, + Error: errors.New("cannot parse plan result"), }, }, { @@ -341,20 +343,22 @@ func TestPlanParserParse(t *testing.T) { name: "plan no changes", body: planNoChanges, result: ParseResult{ - Result: "No changes. Infrastructure is up-to-date.", - HasDestroy: false, - ExitCode: 0, - Error: nil, + Result: "No changes. Infrastructure is up-to-date.", + HasDestroy: false, + HasNoChanges: true, + ExitCode: 0, + Error: nil, }, }, { name: "plan has destroy", body: planHasDestroy, result: ParseResult{ - Result: "Plan: 0 to add, 0 to change, 1 to destroy.", - HasDestroy: true, - ExitCode: 0, - Error: nil, + Result: "Plan: 0 to add, 0 to change, 1 to destroy.", + HasDestroy: true, + HasNoChanges: false, + ExitCode: 0, + Error: nil, }, }, } From c911703395d7cc8df7c5b2db8c3955e67893424a Mon Sep 17 00:00:00 2001 From: d-lau Date: Mon, 11 May 2020 10:10:50 +0900 Subject: [PATCH 2/4] mention no change label is github only for now --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 6340d54..87c3e05 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ terraform: # ... ``` -You can also let tfnotify add a label to PRs whose `terraform plan` output result in no change to the current infrastructure. +You can also let tfnotify add a label to PRs whose `terraform plan` output result in no change to the current infrastructure. Currently, this feature is for Github labels only. ```yaml --- From 72b43569bbdbd63c53775eaae00156997b491bf9 Mon Sep 17 00:00:00 2001 From: d-lau Date: Mon, 11 May 2020 12:54:10 +0900 Subject: [PATCH 3/4] always attempt to remove the label if defined --- notifier/github/github.go | 6 ++++++ notifier/github/github_test.go | 8 ++++++++ notifier/github/notify.go | 22 ++++++++++++++++++---- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/notifier/github/github.go b/notifier/github/github.go index e5cde0a..3738af8 100644 --- a/notifier/github/github.go +++ b/notifier/github/github.go @@ -12,6 +12,7 @@ type API interface { IssuesDeleteComment(ctx context.Context, commentID int64) (*github.Response, error) IssuesListComments(ctx context.Context, number int, opt *github.IssueListCommentsOptions) ([]*github.IssueComment, *github.Response, error) IssuesAddLabels(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) + IssuesRemoveLabel(ctx context.Context, number int, label string) (*github.Response, error) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) RepositoriesListCommits(ctx context.Context, opt *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) RepositoriesGetCommit(ctx context.Context, sha string) (*github.RepositoryCommit, *github.Response, error) @@ -43,6 +44,11 @@ func (g *GitHub) IssuesAddLabels(ctx context.Context, number int, labels []strin return g.Client.Issues.AddLabelsToIssue(ctx, g.owner, g.repo, number, labels) } +// IssuesAddLabels is a wrapper of https://godoc.org/github.com/google/go-github/github#IssuesService.RemoveLabelForIssue +func (g *GitHub) IssuesRemoveLabel(ctx context.Context, number int, label string) (*github.Response, error) { + return g.Client.Issues.RemoveLabelForIssue(ctx, g.owner, g.repo, number, label) +} + // RepositoriesCreateComment is a wrapper of https://godoc.org/github.com/google/go-github/github#RepositoriesService.CreateComment func (g *GitHub) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return g.Client.Repositories.CreateComment(ctx, g.owner, g.repo, sha, comment) diff --git a/notifier/github/github_test.go b/notifier/github/github_test.go index 4a17c68..e84a78e 100644 --- a/notifier/github/github_test.go +++ b/notifier/github/github_test.go @@ -13,6 +13,7 @@ type fakeAPI struct { FakeIssuesDeleteComment func(ctx context.Context, commentID int64) (*github.Response, error) FakeIssuesListComments func(ctx context.Context, number int, opt *github.IssueListCommentsOptions) ([]*github.IssueComment, *github.Response, error) FakeIssuesAddLabels func(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) + FakeIssuesRemoveLabel func(ctx context.Context, number int, label string) (*github.Response, error) FakeRepositoriesCreateComment func(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) FakeRepositoriesListCommits func(ctx context.Context, opt *github.CommitsListOptions) ([]*github.RepositoryCommit, *github.Response, error) FakeRepositoriesGetCommit func(ctx context.Context, sha string) (*github.RepositoryCommit, *github.Response, error) @@ -34,6 +35,10 @@ func (g *fakeAPI) IssuesAddLabels(ctx context.Context, number int, labels []stri return g.FakeIssuesAddLabels(ctx, number, labels) } +func (g *fakeAPI) IssuesRemoveLabel(ctx context.Context, number int, label string) (*github.Response, error) { + return g.FakeIssuesRemoveLabel(ctx, number, label) +} + func (g *fakeAPI) RepositoriesCreateComment(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return g.FakeRepositoriesCreateComment(ctx, sha, comment) } @@ -74,6 +79,9 @@ func newFakeAPI() fakeAPI { FakeIssuesAddLabels: func(ctx context.Context, number int, labels []string) ([]*github.Label, *github.Response, error) { return nil, nil, nil }, + FakeIssuesRemoveLabel: func(ctx context.Context, number int, label string) (*github.Response, error) { + return nil, nil + }, FakeRepositoriesCreateComment: func(ctx context.Context, sha string, comment *github.RepositoryComment) (*github.RepositoryComment, *github.Response, error) { return &github.RepositoryComment{ ID: github.Int64(28427394), diff --git a/notifier/github/notify.go b/notifier/github/notify.go index 565c412..e631b56 100644 --- a/notifier/github/notify.go +++ b/notifier/github/notify.go @@ -3,6 +3,7 @@ package github import ( "context" "github.com/mercari/tfnotify/terraform" + "net/http" ) // NotifyService handles communication with the notification related @@ -31,15 +32,28 @@ func (g *NotifyService) Notify(body string) (exit int, err error) { return result.ExitCode, err } } - if result.HasNoChanges && cfg.PR.IsNumber() && cfg.NoChangesLabel != "" { - _, _, err = g.client.API.IssuesAddLabels( + if cfg.PR.IsNumber() && cfg.NoChangesLabel != "" { + // Always attempt to remove the label first so that an IssueLabeled event is created + resp, err := g.client.API.IssuesRemoveLabel( context.Background(), cfg.PR.Number, - []string{cfg.NoChangesLabel}, + cfg.NoChangesLabel, ) - if err != nil { + // Ignore 404 errors, which are from the PR not having the label + if err != nil && resp.StatusCode != http.StatusNotFound { return result.ExitCode, err } + + if result.HasNoChanges { + _, _, err = g.client.API.IssuesAddLabels( + context.Background(), + cfg.PR.Number, + []string{cfg.NoChangesLabel}, + ) + if err != nil { + return result.ExitCode, err + } + } } } From 3c91c66ecd93de11f9829f81bdbee119a8003e97 Mon Sep 17 00:00:00 2001 From: d-lau Date: Mon, 11 May 2020 14:41:34 +0900 Subject: [PATCH 4/4] github(_)label -> label --- README.md | 2 +- config/config.go | 2 +- config/config_test.go | 2 +- example-with-destroy-and-no-changes.tfnotify.yaml | 2 +- main.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 87c3e05..6fc228d 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ terraform: 
{{ .Body }}
when_no_changes: - github_label: "no-changes" + label: "no-changes" # ... ``` diff --git a/config/config.go b/config/config.go index c75a183..003e91f 100644 --- a/config/config.go +++ b/config/config.go @@ -93,7 +93,7 @@ type WhenDestroy struct { // WhenNoChange is a configuration to add a label when the plan result contains no change type WhenNoChanges struct { - GithubLabel string `yaml:"github_label,omitempty"` + Label string `yaml:"label,omitempty"` } // Apply is a terraform apply config diff --git a/config/config_test.go b/config/config_test.go index e90a954..4bc5ac2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -97,7 +97,7 @@ func TestLoadFile(t *testing.T) { Template: "## :warning: WARNING: Resource Deletion will happen :warning:\n\nThis plan contains **resource deletion**. Please check the plan result very carefully!\n", }, WhenNoChanges: WhenNoChanges{ - GithubLabel: "no-changes", + Label: "no-changes", }, }, Apply: Apply{ diff --git a/example-with-destroy-and-no-changes.tfnotify.yaml b/example-with-destroy-and-no-changes.tfnotify.yaml index 410df8d..75ee6d3 100644 --- a/example-with-destroy-and-no-changes.tfnotify.yaml +++ b/example-with-destroy-and-no-changes.tfnotify.yaml @@ -19,7 +19,7 @@ terraform: 
{{ .Body }}
when_no_changes: - github_label: "no-changes" + label: "no-changes" when_destroy: template: | ## :warning: WARNING: Resource Deletion will happen :warning: diff --git a/main.go b/main.go index ac755e7..0d6ff13 100644 --- a/main.go +++ b/main.go @@ -115,7 +115,7 @@ func (t *tfnotify) Run() error { Template: t.template, DestroyWarningTemplate: t.destroyWarningTemplate, WarnDestroy: t.warnDestroy, - NoChangesLabel: t.config.Terraform.Plan.WhenNoChanges.GithubLabel, + NoChangesLabel: t.config.Terraform.Plan.WhenNoChanges.Label, }) if err != nil { return err