Add --silence-whitelist-errors server flag #313
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #313 +/- ##
==========================================
+ Coverage 70.54% 70.56% +0.02%
==========================================
Files 61 61
Lines 3639 3642 +3
==========================================
+ Hits 2567 2570 +3
Misses 893 893
Partials 179 179
Continue to review full report at Codecov.
|
vanetix
force-pushed
the
f/silence-whitelist-errors
branch
from
73165a7
to
23b5623
Compare
vanetix
changed the title
lkysow
requested changes
Oct 16, 2018
server/events_controller.go
Outdated
| @@ -57,6 +57,9 @@ type EventsController struct { | |||
| // request validation is done. | |||
| GitlabWebhookSecret []byte | |||
| RepoWhitelistChecker *events.RepoWhitelistChecker | |||
| // Signifies if we post a comment when an event comes in for a non-whitelisted | |||
There was a problem hiding this comment.
Comment needs to start with // SilenceWhitelistErrors as per golang comment style guide:
Notice this comment is a complete sentence that begins with the name of the element it describes...
https://blog.golang.org/godoc-documenting-go-code
ex:
// SilenceWhitelistErrors controls whether we write an error comment on pull requests from non-whitelisted repos.
server/events_controller.go
Outdated
| @@ -418,6 +421,10 @@ func (e *EventsController) respond(w http.ResponseWriter, lvl logging.LogLevel, | |||
| // commentNotWhitelisted comments on the pull request that the repo is not | |||
| // whitelisted. | |||
There was a problem hiding this comment.
need to update this comment to read:
// commentNotWhitelisted comments on the pull request that the repo is not
// whitelisted unless whitelist error comments are disabled.
server/events_controller_test.go
Outdated
| @@ -198,6 +198,35 @@ func TestPost_GitlabCommentNotWhitelisted(t *testing.T) { | |||
| vcsClient.VerifyWasCalledOnce().CreateComment(expRepo, 1, "```\nError: This repo is not whitelisted for Atlantis.\n```") | |||
| } | |||
|
|
|||
| func TestPost_GitlabCommentNotWhitelistedWithSilenceErrors(t *testing.T) { | |||
| t.Log("when the event is a gitlab comment from a repo that isn't whitelisted and we are silencing errors, do no comment with an error") | |||
server/events_controller_test.go
Outdated
| exp := "Repo not whitelisted" | ||
| Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) | ||
| expRepo, _ := models.NewRepo(models.Gitlab, "gitlabhq/gitlab-test", "https://example.com/gitlabhq/gitlab-test.git", "", "") | ||
| vcsClient.VerifyWasCalled(Never()).CreateComment(expRepo, 1, "```\nError: This repo is not whitelisted for Atlantis.\n```") |
There was a problem hiding this comment.
use
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString())
Instead. This way we can ensure we never comment and the test won't pass even if we change the comment format.
server/events_controller_test.go
Outdated
| @@ -227,6 +256,36 @@ func TestPost_GithubCommentNotWhitelisted(t *testing.T) { | |||
| vcsClient.VerifyWasCalledOnce().CreateComment(expRepo, 2, "```\nError: This repo is not whitelisted for Atlantis.\n```") | |||
| } | |||
|
|
|||
| func TestPost_GithubCommentNotWhitelistedWithSilenceErrors(t *testing.T) { | |||
| t.Log("when the event is a github comment from a repo that isn't whitelisted and we are silencing errors, do no comment with an error") | |||
server/events_controller_test.go
Outdated
| exp := "Repo not whitelisted" | ||
| Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) | ||
| expRepo, _ := models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "") | ||
| vcsClient.VerifyWasCalled(Never()).CreateComment(expRepo, 2, "```\nError: This repo is not whitelisted for Atlantis.\n```") |
There was a problem hiding this comment.
vcsClient.VerifyWasCalled(Never()).CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString())
lkysow
reviewed
Oct 16, 2018
server/events_controller_test.go
Outdated
| @@ -223,8 +223,9 @@ func TestPost_GitlabCommentNotWhitelistedWithSilenceErrors(t *testing.T) { | |||
| body, _ := ioutil.ReadAll(w.Result().Body) | |||
| exp := "Repo not whitelisted" | |||
| Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) | |||
| expRepo, _ := models.NewRepo(models.Gitlab, "gitlabhq/gitlab-test", "https://example.com/gitlabhq/gitlab-test.git", "", "") | |||
| vcsClient.VerifyWasCalled(Never()).CreateComment(expRepo, 1, "```\nError: This repo is not whitelisted for Atlantis.\n```") | |||
| models.NewRepo(models.Gitlab, "gitlabhq/gitlab-test", "https://example.com/gitlabhq/gitlab-test.git", "", "") | |||
There was a problem hiding this comment.
I think you can delete this line.
lkysow
reviewed
Oct 16, 2018
server/events_controller_test.go
Outdated
| @@ -282,8 +283,8 @@ func TestPost_GithubCommentNotWhitelistedWithSilenceErrors(t *testing.T) { | |||
| body, _ := ioutil.ReadAll(w.Result().Body) | |||
| exp := "Repo not whitelisted" | |||
| Assert(t, strings.Contains(string(body), exp), "exp %q to be contained in %q", exp, string(body)) | |||
| expRepo, _ := models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "") | |||
| vcsClient.VerifyWasCalled(Never()).CreateComment(expRepo, 2, "```\nError: This repo is not whitelisted for Atlantis.\n```") | |||
| models.NewRepo(models.Github, "baxterthehacker/public-repo", "https://github.com/baxterthehacker/public-repo.git", "", "") | |||
|
Thanks! Totally forgot about the golang comment style. Also went ahead and removed the |
|
Removed, I thought it could probably be - just not really familiar with pegomock. |
lkysow
changed the title
|
No problem, thanks again for all your work! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #312
Some users would like to add the Atlantis webhook at the org level and then use the whitelist to configure which repos Atlantis actually runs on. In this configuration, it's annoying that Atlantis continually comments on pull requests "Repo is not whitelisted".
This PR adds a flag
--silence-whitelist-errorswhich will prevent commenting back on the pull request.