diff --git a/CHANGELOG.md b/CHANGELOG.md index 93117bf0df..ec1f7caab6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,31 @@ +# v0.17.0-beta +Feature release. Due to a sizeable refactor and the number of configuration settings supported in Atlantis, this is a pre-release and should not be considered fully stable. + +## Features +* Add `--enable-policy-checks` which adds a policy checking step to the Atlantis workflow and runs server-side conftest policies on the terraform plan output. ([#1317](https://github.com/runatlantis/atlantis/pull/1317) by @msarvar and @nishkrishnan) + - Supports `atlantis approve_policies` which allows a set of blessed github users to approve failing policies. +* Support pre-workflow hooks on all comment/auto triggered commands ([#1418](https://github.com/runatlantis/atlantis/pull/1418) by @nishkrishnan) +* Add `HEAD_COMMIT` to run steps +* Update terraform version to 0.14.7 + +## Backwards Incompatibilities/Notes +* If you're using the Atlantis Docker image and aren't setting the `--default-tf-version` flag + then the default version of Terraform will now be 0.14.7. Simply set the above + flag to your desired default version to avoid any issues. + +## Downloads +* [atlantis_darwin_amd64.zip](https://github.com/runatlantis/atlantis/releases/download/v0.17.0-beta/atlantis_darwin_amd64.zip) +* [atlantis_linux_386.zip](https://github.com/runatlantis/atlantis/releases/download/v0.17.0-beta/atlantis_linux_386.zip) +* [atlantis_linux_amd64.zip](https://github.com/runatlantis/atlantis/releases/download/v0.17.0-beta/atlantis_linux_amd64.zip) +* [atlantis_linux_arm.zip](https://github.com/runatlantis/atlantis/releases/download/v0.17.0-beta/atlantis_linux_arm.zip) + +## Docker +[`runatlantis/atlantis:v0.17.0-beta`](https://hub.docker.com/r/runatlantis/atlantis/tags/) + +## Diff v0.16.1..v0.17.0-beta +https://github.com/runatlantis/atlantis/compare/v0.16.1...v0.17.0-beta + + # v0.16.1 Few improvements and a number of bug fixes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 767afeb29a..2747177a9f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -71,6 +71,11 @@ docker-compose up --detach --build docker run --rm -v $(pwd):/go/src/github.com/runatlantis/atlantis -w /go/src/github.com/runatlantis/atlantis runatlantis/testing-env make test ``` +Or to run the integration tests +``` +docker run --rm -v $(pwd):/go/src/github.com/runatlantis/atlantis -w /go/src/github.com/runatlantis/atlantis runatlantis/testing-env make test-all +``` + ## Calling Your Local Atlantis From GitHub - Create a test terraform repository in your GitHub. - Create a personal access token for Atlantis. See [Create a GitHub token](https://github.com/runatlantis/atlantis/tree/master/runatlantis.io/docs/access-credentials.md#generating-an-access-token). diff --git a/kustomize/bundle.yaml b/kustomize/bundle.yaml index 131877e6aa..3fe6ae8acf 100644 --- a/kustomize/bundle.yaml +++ b/kustomize/bundle.yaml @@ -22,7 +22,7 @@ spec: fsGroup: 1000 # Atlantis group (1000) read/write access to volumes. containers: - name: atlantis - image: runatlantis/atlantis:v0.16.1 + image: runatlantis/atlantis:v0.17.0-beta env: - name: ATLANTIS_DATA_DIR value: /atlantis diff --git a/main.go b/main.go index 45722def92..dc4307941a 100644 --- a/main.go +++ b/main.go @@ -20,7 +20,7 @@ import ( "github.com/spf13/viper" ) -const atlantisVersion = "0.16.1" +const atlantisVersion = "0.17.0-beta" func main() { v := viper.New() diff --git a/runatlantis.io/docs/custom-workflows.md b/runatlantis.io/docs/custom-workflows.md index 2e0f8b61fb..db04ef7c1b 100644 --- a/runatlantis.io/docs/custom-workflows.md +++ b/runatlantis.io/docs/custom-workflows.md @@ -349,6 +349,7 @@ Or a custom command * `HEAD_REPO_NAME` - Name of the repository that is getting merged into the base repository, ex. `atlantis`. * `HEAD_REPO_OWNER` - Owner of the repository that is getting merged into the base repository, ex. `acme-corp`. * `HEAD_BRANCH_NAME` - Name of the head branch of the pull request (the branch that is getting merged into the base) + * `HEAD_COMMIT` - The sha256 that points to the head of the branch that is being pull requested into the base. If the pull request is from Bitbucket Cloud the string will only be 12 characters long because Bitbucket Cloud truncates its commit IDs. * `BASE_BRANCH_NAME` - Name of the base branch of the pull request (the branch that the pull request is getting merged into) * `PROJECT_NAME` - Name of the project configured in `atlantis.yaml`. If no project name is configured this will be an empty string. * `PULL_NUM` - Pull request number or ID, ex. `2`. diff --git a/runatlantis.io/docs/locking.md b/runatlantis.io/docs/locking.md index b723163c73..75680e12aa 100644 --- a/runatlantis.io/docs/locking.md +++ b/runatlantis.io/docs/locking.md @@ -64,5 +64,5 @@ Atlantis is doing is running `terraform plan` and `apply` and so all of the locking built in to those commands by Terraform isn't affected. In more detail, Terraform state locking locks the state while you run `terraform apply` -so that multiple apply's can't run concurrently. Atlantis's locking is at a higher +so that multiple applies can't run concurrently. Atlantis's locking is at a higher level because it prevents multiple pull requests from working on the same state. diff --git a/runatlantis.io/docs/pre-workflow-hooks.md b/runatlantis.io/docs/pre-workflow-hooks.md index d71069fb53..dee974e17f 100644 --- a/runatlantis.io/docs/pre-workflow-hooks.md +++ b/runatlantis.io/docs/pre-workflow-hooks.md @@ -50,6 +50,7 @@ command](custom-workflows.html#custom-run-command). * `HEAD_REPO_NAME` - Name of the repository that is getting merged into the base repository, ex. `atlantis`. * `HEAD_REPO_OWNER` - Owner of the repository that is getting merged into the base repository, ex. `acme-corp`. * `HEAD_BRANCH_NAME` - Name of the head branch of the pull request (the branch that is getting merged into the base) + * `HEAD_COMMIT` - The sha256 that points to the head of the branch that is being pull requested into the base. If the pull request is from Bitbucket Cloud the string will only be 12 characters long because Bitbucket Cloud truncates its commit IDs. * `BASE_BRANCH_NAME` - Name of the base branch of the pull request (the branch that the pull request is getting merged into) * `PULL_NUM` - Pull request number or ID, ex. `2`. * `PULL_AUTHOR` - Username of the pull request author, ex. `acme-user`. diff --git a/runatlantis.io/docs/server-configuration.md b/runatlantis.io/docs/server-configuration.md index 20ea7bf7fc..8cf09da191 100644 --- a/runatlantis.io/docs/server-configuration.md +++ b/runatlantis.io/docs/server-configuration.md @@ -226,6 +226,13 @@ Values are chosen in this order: ``` Stops atlantis locking projects and or workspaces when running terraform +* ### `--enable-policy-checks` + + ```bash + atlantis server --enable-policy-checks + ``` + Enables atlantis to run server side policies on the result of a terraform plan. Policies are defined in [server side repo config](https://www.runatlantis.io/docs/server-side-repo-config.html#reference). + * ### `--enable-regexp-cmd` ```bash atlantis server --enable-regexp-cmd diff --git a/runatlantis.io/docs/server-side-repo-config.md b/runatlantis.io/docs/server-side-repo-config.md index 76c6c8bd62..9dbb4e93ac 100644 --- a/runatlantis.io/docs/server-side-repo-config.md +++ b/runatlantis.io/docs/server-side-repo-config.md @@ -28,6 +28,10 @@ repos: # Repo ID's are of the form {VCS hostname}/{org}/{repo name}, ex. # github.com/runatlantis/atlantis. - id: /.*/ + # branch is an regex matching pull requests by base branch + # (the branch the pull request is getting merged into). + # By default, all branches are matched + branch: /.*/ # apply_requirements sets the Apply Requirements for all repos that match. apply_requirements: [approved, mergeable] @@ -331,6 +335,7 @@ custom workflows. |-----------|---------------------------------------------------------|-----------|----------|---------------------------------------------------------------------------------------| | repos | array[[Repo](#repo)] | see below | no | List of repos to apply settings to. | | workflows | map[string: [Workflow](custom-workflows.html#workflow)] | see below | no | Map from workflow name to workflow. Workflows override the default Atlantis commands. | +| policies | Policies. | none | no | List of policy sets to run and associated metadata | ::: tip A Note On Defaults @@ -355,6 +360,7 @@ workflows: apply: steps: [apply] ``` + This gets merged with whatever config you write. If you set a workflow with the key `default`, it will override this. ::: @@ -400,3 +406,24 @@ If you set a workflow with the key `default`, it will override this. * `allow_custom_workflows` is set from the `id: /.*/` config and isn't unset by the `id: github.com/owner/repo` config because it didn't define that key. ::: + +### Policies + +| Key | Type | Default | Required | Description | +|------------------------|-----------------|---------|-----------|------------------------------------------| +| conftest_version | string | none | no | conftest version to run all policy sets | +| owners | Owners(#Owners) | none | yes | owners that can approve failing policies | +| policy_sets | []PolicySet | none | yes | set of policies to run on a plan output | + +### Owners +| Key | Type | Default | Required | Description | +|-------------|-------------------|---------|------------|---------------------------------------------------------| +| users | []string | none | yes | list of github users that can approve failing policies | + +### PolicySet +| Key | Type | Default | Required | Description | +|------------------------|-----------------|---------|-----------|------------------------------------------| +| name | string | none | yes | unique name for the policy set | +| path | string | none | yes | path to the rego policies | +| source | string | none | yes | only `local` is supported at this time | + diff --git a/server/events/command_runner.go b/server/events/command_runner.go index e7cf6bd2f3..5d78eebe3f 100644 --- a/server/events/command_runner.go +++ b/server/events/command_runner.go @@ -108,9 +108,10 @@ type DefaultCommandRunner struct { // SilenceForkPRErrorsFlag is the name of the flag that controls fork PR's. We use // this in our error message back to the user on a forked PR so they know // how to disable error comment - SilenceForkPRErrorsFlag string - CommentCommandRunnerByCmd map[models.CommandName]CommentCommandRunner - Drainer *Drainer + SilenceForkPRErrorsFlag string + CommentCommandRunnerByCmd map[models.CommandName]CommentCommandRunner + Drainer *Drainer + PreWorkflowHooksCommandRunner PreWorkflowHooksCommandRunner } // RunAutoplanCommand runs plan and policy_checks when a pull request is opened or updated. @@ -139,12 +140,14 @@ func (c *DefaultCommandRunner) RunAutoplanCommand(baseRepo models.Repo, headRepo return } - autoPlanRunner := buildCommentCommandRunner(c, models.PlanCommand) - if autoPlanRunner == nil { - ctx.Log.Err("invalid autoplan command") - return + err := c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx) + + if err != nil { + ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, models.PlanCommand) } + autoPlanRunner := buildCommentCommandRunner(c, models.PlanCommand) + autoPlanRunner.Run(ctx, nil) } @@ -182,12 +185,14 @@ func (c *DefaultCommandRunner) RunCommentCommand(baseRepo models.Repo, maybeHead return } - cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) - if cmdRunner == nil { - ctx.Log.Err("command %s is not supported", cmd.Name.String()) - return + err = c.PreWorkflowHooksCommandRunner.RunPreHooks(ctx) + + if err != nil { + ctx.Log.Err("Error running pre-workflow hooks %s. Proceeding with %s command.", err, cmd.Name.String()) } + cmdRunner := buildCommentCommandRunner(c, cmd.CommandName()) + cmdRunner.Run(ctx, cmd) } diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index 47aba3c1d5..6ca31c1563 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -61,6 +61,7 @@ var approvePoliciesCommandRunner *events.ApprovePoliciesCommandRunner var planCommandRunner *events.PlanCommandRunner var applyCommandRunner *events.ApplyCommandRunner var unlockCommandRunner *events.UnlockCommandRunner +var preWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner func setup(t *testing.T) *vcsmocks.MockClient { RegisterMockTestingT(t) @@ -161,17 +162,22 @@ func setup(t *testing.T) *vcsmocks.MockClient { models.UnlockCommand: unlockCommandRunner, } + preWorkflowHooksCommandRunner = mocks.NewMockPreWorkflowHooksCommandRunner() + + When(preWorkflowHooksCommandRunner.RunPreHooks(matchers.AnyPtrToEventsCommandContext())).ThenReturn(nil) + ch = events.DefaultCommandRunner{ - VCSClient: vcsClient, - CommentCommandRunnerByCmd: commentCommandRunnerByCmd, - EventParser: eventParsing, - GithubPullGetter: githubGetter, - GitlabMergeRequestGetter: gitlabGetter, - AzureDevopsPullGetter: azuredevopsGetter, - Logger: logger, - AllowForkPRs: false, - AllowForkPRsFlag: "allow-fork-prs-flag", - Drainer: drainer, + VCSClient: vcsClient, + CommentCommandRunnerByCmd: commentCommandRunnerByCmd, + EventParser: eventParsing, + GithubPullGetter: githubGetter, + GitlabMergeRequestGetter: gitlabGetter, + AzureDevopsPullGetter: azuredevopsGetter, + Logger: logger, + AllowForkPRs: false, + AllowForkPRsFlag: "allow-fork-prs-flag", + Drainer: drainer, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, } return vcsClient } @@ -645,7 +651,7 @@ func TestRunCommentCommand_DrainOngoing(t *testing.T) { } func TestRunCommentCommand_DrainNotOngoing(t *testing.T) { - t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occured") + t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occurred") setup(t) When(githubGetter.GetPullRequest(fixtures.GithubRepo, fixtures.Pull.Num)).ThenPanic("panic test - if you're seeing this in a test failure this isn't the failing test") ch.RunCommentCommand(fixtures.GithubRepo, &fixtures.GithubRepo, nil, fixtures.User, fixtures.Pull.Num, nil) @@ -662,7 +668,7 @@ func TestRunAutoplanCommand_DrainOngoing(t *testing.T) { } func TestRunAutoplanCommand_DrainNotOngoing(t *testing.T) { - t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occured") + t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occurred") setup(t) fixtures.Pull.BaseRepo = fixtures.GithubRepo When(projectCommandBuilder.BuildAutoplanCommands(matchers.AnyPtrToEventsCommandContext())).ThenPanic("panic test - if you're seeing this in a test failure this isn't the failing test") diff --git a/server/events/delete_lock_command.go b/server/events/delete_lock_command.go index 7033017db4..5ed05f0bc4 100644 --- a/server/events/delete_lock_command.go +++ b/server/events/delete_lock_command.go @@ -45,6 +45,7 @@ func (l *DefaultDeleteLockCommand) DeleteLocksByPull(repoFullName string, pullNu return err } if len(locks) == 0 { + l.Logger.Debug("No locks found for pull") return nil } @@ -61,6 +62,7 @@ func (l *DefaultDeleteLockCommand) deleteWorkingDir(lock models.ProjectLock) { // installations of Atlantis will have locks in their DB that do not have // this field on PullRequest. We skip deleting the working dir in this case. if lock.Pull.BaseRepo == (models.Repo{}) { + l.Logger.Debug("Not deleting the working dir.") return } unlock, err := l.WorkingDirLocker.TryLock(lock.Pull.BaseRepo.FullName, lock.Pull.Num, lock.Workspace) diff --git a/server/events/mocks/matchers/ptr_to_events_commandcontext.go b/server/events/mocks/matchers/ptr_to_events_commandcontext.go index 896b495636..60fe569249 100644 --- a/server/events/mocks/matchers/ptr_to_events_commandcontext.go +++ b/server/events/mocks/matchers/ptr_to_events_commandcontext.go @@ -3,8 +3,9 @@ package matchers import ( "github.com/petergtz/pegomock" - events "github.com/runatlantis/atlantis/server/events" "reflect" + + events "github.com/runatlantis/atlantis/server/events" ) func AnyPtrToEventsCommandContext() *events.CommandContext { @@ -18,3 +19,15 @@ func EqPtrToEventsCommandContext(value *events.CommandContext) *events.CommandCo var nullValue *events.CommandContext return nullValue } + +func NotEqPtrToEventsCommandContext(value *events.CommandContext) *events.CommandContext { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue *events.CommandContext + return nullValue +} + +func PtrToEventsCommandContextThat(matcher pegomock.ArgumentMatcher) *events.CommandContext { + pegomock.RegisterMatcher(matcher) + var nullValue *events.CommandContext + return nullValue +} diff --git a/server/events/mocks/mock_pre_workflows_hooks_command_runner.go b/server/events/mocks/mock_pre_workflows_hooks_command_runner.go index 3019cc1a79..ebbb9f8766 100644 --- a/server/events/mocks/mock_pre_workflows_hooks_command_runner.go +++ b/server/events/mocks/mock_pre_workflows_hooks_command_runner.go @@ -5,7 +5,7 @@ package mocks import ( pegomock "github.com/petergtz/pegomock" - models "github.com/runatlantis/atlantis/server/events/models" + events "github.com/runatlantis/atlantis/server/events" "reflect" "time" ) @@ -27,12 +27,19 @@ func (mock *MockPreWorkflowHooksCommandRunner) SetFailHandler(fh pegomock.FailHa } func (mock *MockPreWorkflowHooksCommandRunner) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockPreWorkflowHooksCommandRunner) RunPreHooks(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) { +func (mock *MockPreWorkflowHooksCommandRunner) RunPreHooks(ctx *events.CommandContext) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockPreWorkflowHooksCommandRunner().") } - params := []pegomock.Param{baseRepo, headRepo, pull, user} - pegomock.GetGenericMockFrom(mock).Invoke("RunPreHooks", params, []reflect.Type{}) + params := []pegomock.Param{ctx} + result := pegomock.GetGenericMockFrom(mock).Invoke("RunPreHooks", params, []reflect.Type{reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(error) + } + } + return ret0 } func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledOnce() *VerifierMockPreWorkflowHooksCommandRunner { @@ -42,14 +49,14 @@ func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledOnce() *VerifierMo } } -func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalled(invocationCountMatcher pegomock.Matcher) *VerifierMockPreWorkflowHooksCommandRunner { +func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockPreWorkflowHooksCommandRunner { return &VerifierMockPreWorkflowHooksCommandRunner{ mock: mock, invocationCountMatcher: invocationCountMatcher, } } -func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledInOrder(invocationCountMatcher pegomock.Matcher, inOrderContext *pegomock.InOrderContext) *VerifierMockPreWorkflowHooksCommandRunner { +func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockPreWorkflowHooksCommandRunner { return &VerifierMockPreWorkflowHooksCommandRunner{ mock: mock, invocationCountMatcher: invocationCountMatcher, @@ -57,7 +64,7 @@ func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledInOrder(invocation } } -func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledEventually(invocationCountMatcher pegomock.Matcher, timeout time.Duration) *VerifierMockPreWorkflowHooksCommandRunner { +func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockPreWorkflowHooksCommandRunner { return &VerifierMockPreWorkflowHooksCommandRunner{ mock: mock, invocationCountMatcher: invocationCountMatcher, @@ -67,13 +74,13 @@ func (mock *MockPreWorkflowHooksCommandRunner) VerifyWasCalledEventually(invocat type VerifierMockPreWorkflowHooksCommandRunner struct { mock *MockPreWorkflowHooksCommandRunner - invocationCountMatcher pegomock.Matcher + invocationCountMatcher pegomock.InvocationCountMatcher inOrderContext *pegomock.InOrderContext timeout time.Duration } -func (verifier *VerifierMockPreWorkflowHooksCommandRunner) RunPreHooks(baseRepo models.Repo, headRepo models.Repo, pull models.PullRequest, user models.User) *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification { - params := []pegomock.Param{baseRepo, headRepo, pull, user} +func (verifier *VerifierMockPreWorkflowHooksCommandRunner) RunPreHooks(ctx *events.CommandContext) *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification { + params := []pegomock.Param{ctx} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "RunPreHooks", params, verifier.timeout) return &MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -83,29 +90,17 @@ type MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification) GetCapturedArguments() (models.Repo, models.Repo, models.PullRequest, models.User) { - baseRepo, headRepo, pull, user := c.GetAllCapturedArguments() - return baseRepo[len(baseRepo)-1], headRepo[len(headRepo)-1], pull[len(pull)-1], user[len(user)-1] +func (c *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification) GetCapturedArguments() *events.CommandContext { + ctx := c.GetAllCapturedArguments() + return ctx[len(ctx)-1] } -func (c *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.Repo, _param2 []models.PullRequest, _param3 []models.User) { +func (c *MockPreWorkflowHooksCommandRunner_RunPreHooks_OngoingVerification) GetAllCapturedArguments() (_param0 []*events.CommandContext) { params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) if len(params) > 0 { - _param0 = make([]models.Repo, len(c.methodInvocations)) + _param0 = make([]*events.CommandContext, len(c.methodInvocations)) for u, param := range params[0] { - _param0[u] = param.(models.Repo) - } - _param1 = make([]models.Repo, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(models.Repo) - } - _param2 = make([]models.PullRequest, len(c.methodInvocations)) - for u, param := range params[2] { - _param2[u] = param.(models.PullRequest) - } - _param3 = make([]models.User, len(c.methodInvocations)) - for u, param := range params[3] { - _param3[u] = param.(models.User) + _param0[u] = param.(*events.CommandContext) } } return diff --git a/server/events/pre_workflow_hooks_command_runner.go b/server/events/pre_workflow_hooks_command_runner.go index 739359b408..cd5ae6eed1 100644 --- a/server/events/pre_workflow_hooks_command_runner.go +++ b/server/events/pre_workflow_hooks_command_runner.go @@ -1,93 +1,79 @@ package events import ( - "fmt" - "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/runtime" "github.com/runatlantis/atlantis/server/events/vcs" "github.com/runatlantis/atlantis/server/events/yaml/valid" - "github.com/runatlantis/atlantis/server/logging" - "github.com/runatlantis/atlantis/server/recovery" ) //go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_pre_workflows_hooks_command_runner.go PreWorkflowHooksCommandRunner type PreWorkflowHooksCommandRunner interface { - RunPreHooks( - baseRepo models.Repo, - headRepo models.Repo, - pull models.PullRequest, - user models.User, - ) + RunPreHooks(ctx *CommandContext) error } // DefaultPreWorkflowHooksCommandRunner is the first step when processing a workflow hook commands. type DefaultPreWorkflowHooksCommandRunner struct { VCSClient vcs.Client - Logger logging.SimpleLogging WorkingDirLocker WorkingDirLocker WorkingDir WorkingDir GlobalCfg valid.GlobalCfg - Drainer *Drainer - PreWorkflowHookRunner *runtime.PreWorkflowHookRunner + PreWorkflowHookRunner runtime.PreWorkflowHookRunner } // RunPreHooks runs pre_workflow_hooks when PR is opened or updated. func (w *DefaultPreWorkflowHooksCommandRunner) RunPreHooks( - baseRepo models.Repo, - headRepo models.Repo, - pull models.PullRequest, - user models.User, -) { - if opStarted := w.Drainer.StartOp(); !opStarted { - if commentErr := w.VCSClient.CreateComment(baseRepo, pull.Num, ShutdownComment, "pre_workflow_hooks"); commentErr != nil { - w.Logger.Log(logging.Error, "unable to comment that Atlantis is shutting down: %s", commentErr) + ctx *CommandContext, +) error { + pull := ctx.Pull + baseRepo := pull.BaseRepo + headRepo := ctx.HeadRepo + user := ctx.User + log := ctx.Log + + preWorkflowHooks := make([]*valid.PreWorkflowHook, 0) + for _, repo := range w.GlobalCfg.Repos { + if repo.IDMatches(baseRepo.ID()) && repo.BranchMatches(pull.BaseBranch) && len(repo.PreWorkflowHooks) > 0 { + preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...) } - return } - defer w.Drainer.OpDone() - log := w.buildLogger(baseRepo.FullName, pull.Num) - defer w.logPanics(baseRepo, pull.Num, log) + // short circuit any other calls if there are no pre-hooks configured + if len(preWorkflowHooks) == 0 { + return nil + } - log.Info("running pre hooks") + log.Debug("pre-hooks configured, running...") unlockFn, err := w.WorkingDirLocker.TryLock(baseRepo.FullName, pull.Num, DefaultWorkspace) if err != nil { - log.Warn("workspace is locked") - return + return err } log.Debug("got workspace lock") defer unlockFn() repoDir, _, err := w.WorkingDir.Clone(log, headRepo, pull, DefaultWorkspace) if err != nil { - log.Err("unable to run pre workflow hooks: %s", err) - return + return err } - preWorkflowHooks := make([]*valid.PreWorkflowHook, 0) - for _, repo := range w.GlobalCfg.Repos { - if repo.IDMatches(baseRepo.ID()) && len(repo.PreWorkflowHooks) > 0 { - preWorkflowHooks = append(preWorkflowHooks, repo.PreWorkflowHooks...) - } - } - - ctx := models.PreWorkflowHookCommandContext{ - BaseRepo: baseRepo, - HeadRepo: headRepo, - Log: log, - Pull: pull, - User: user, - Verbose: false, - } - - err = w.runHooks(ctx, preWorkflowHooks, repoDir) + err = w.runHooks( + models.PreWorkflowHookCommandContext{ + BaseRepo: baseRepo, + HeadRepo: headRepo, + Log: log, + Pull: pull, + User: user, + Verbose: false, + }, + preWorkflowHooks, repoDir) if err != nil { - log.Err("pre workflow hook run error results: %s", err) + return err } + + return nil } func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( @@ -100,30 +86,9 @@ func (w *DefaultPreWorkflowHooksCommandRunner) runHooks( _, err := w.PreWorkflowHookRunner.Run(ctx, hook.RunCommand, repoDir) if err != nil { - return nil + return err } } return nil } - -func (w *DefaultPreWorkflowHooksCommandRunner) buildLogger(repoFullName string, pullNum int) *logging.SimpleLogger { - src := fmt.Sprintf("%s#%d", repoFullName, pullNum) - return w.Logger.NewLogger(src, true, w.Logger.GetLevel()) -} - -// logPanics logs and creates a comment on the pull request for panics. -func (w *DefaultPreWorkflowHooksCommandRunner) logPanics(baseRepo models.Repo, pullNum int, logger logging.SimpleLogging) { - if err := recover(); err != nil { - stack := recovery.Stack(3) - logger.Err("PANIC: %s\n%s", err, stack) - if commentErr := w.VCSClient.CreateComment( - baseRepo, - pullNum, - fmt.Sprintf("**Error: goroutine panic. This is a bug.**\n```\n%s\n%s```", err, stack), - "", - ); commentErr != nil { - logger.Err("unable to comment: %s", commentErr) - } - } -} diff --git a/server/events/pre_workflow_hooks_command_runner_test.go b/server/events/pre_workflow_hooks_command_runner_test.go index e8290359c9..b0c3a7dc98 100644 --- a/server/events/pre_workflow_hooks_command_runner_test.go +++ b/server/events/pre_workflow_hooks_command_runner_test.go @@ -1,87 +1,221 @@ package events_test import ( - "fmt" - "strings" + "errors" "testing" . "github.com/petergtz/pegomock" "github.com/runatlantis/atlantis/server/events" - "github.com/runatlantis/atlantis/server/events/matchers" "github.com/runatlantis/atlantis/server/events/mocks" + "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/models/fixtures" - "github.com/runatlantis/atlantis/server/events/runtime" + runtime_mocks "github.com/runatlantis/atlantis/server/events/runtime/mocks" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" + "github.com/runatlantis/atlantis/server/events/yaml/valid" "github.com/runatlantis/atlantis/server/logging" - logmocks "github.com/runatlantis/atlantis/server/logging/mocks" . "github.com/runatlantis/atlantis/testing" ) var wh events.DefaultPreWorkflowHooksCommandRunner var whWorkingDir *mocks.MockWorkingDir var whWorkingDirLocker *mocks.MockWorkingDirLocker -var whDrainer *events.Drainer +var whPreWorkflowHookRunner *runtime_mocks.MockPreWorkflowHookRunner -func preWorkflowHooksSetup(t *testing.T) *vcsmocks.MockClient { +func preWorkflowHooksSetup(t *testing.T) { RegisterMockTestingT(t) vcsClient := vcsmocks.NewMockClient() - logger := logmocks.NewMockSimpleLogging() whWorkingDir = mocks.NewMockWorkingDir() whWorkingDirLocker = mocks.NewMockWorkingDirLocker() - whDrainer = &events.Drainer{} + whPreWorkflowHookRunner = runtime_mocks.NewMockPreWorkflowHookRunner() wh = events.DefaultPreWorkflowHooksCommandRunner{ VCSClient: vcsClient, - Logger: logger, WorkingDirLocker: whWorkingDirLocker, WorkingDir: whWorkingDir, - Drainer: whDrainer, - PreWorkflowHookRunner: &runtime.PreWorkflowHookRunner{}, + PreWorkflowHookRunner: whPreWorkflowHookRunner, } - return vcsClient } -func TestPreWorkflowHooksCommand_LogPanics(t *testing.T) { - t.Log("if there is a panic it is commented back on the pull request") - vcsClient := preWorkflowHooksSetup(t) - logger := wh.Logger.NewLogger("log", false, logging.LogLevel(1)) - - When(whWorkingDir.Clone( - logger, - fixtures.GithubRepo, - fixtures.Pull, - events.DefaultWorkspace, - )).ThenPanic("panic test - if you're seeing this in a test failure this isn't the failing test") - - wh.RunPreHooks(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) - _, _, comment, _ := vcsClient.VerifyWasCalledOnce().CreateComment(matchers.AnyModelsRepo(), AnyInt(), AnyString(), AnyString()).GetCapturedArguments() - Assert(t, strings.Contains(comment, "Error: goroutine panic"), fmt.Sprintf("comment should be about a goroutine panic but was %q", comment)) +func newBool(b bool) *bool { + return &b } -// Test that if one plan fails and we are using automerge, that -// we delete the plans. func TestRunPreHooks_Clone(t *testing.T) { - preWorkflowHooksSetup(t) - logger := wh.Logger.NewLogger("log", false, logging.LogLevel(1)) - When(whWorkingDir.Clone(logger, fixtures.GithubRepo, fixtures.Pull, events.DefaultWorkspace)). - ThenReturn("path/to/repo", false, nil) - wh.RunPreHooks(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) -} + log := logging.NewNoopLogger() -func TestRunPreHooks_DrainOngoing(t *testing.T) { - t.Log("if drain is ongoing then a message should be displayed") - vcsClient := preWorkflowHooksSetup(t) - whDrainer.ShutdownBlocking() - wh.RunPreHooks(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) - vcsClient.VerifyWasCalledOnce().CreateComment(fixtures.GithubRepo, fixtures.Pull.Num, "Atlantis server is shutting down, please try again later.", "pre_workflow_hooks") -} + var newPull = fixtures.Pull + newPull.BaseRepo = fixtures.GithubRepo + + ctx := &events.CommandContext{ + Pull: newPull, + HeadRepo: fixtures.GithubRepo, + User: fixtures.User, + Log: log, + } + + testHook := valid.PreWorkflowHook{ + StepName: "test", + RunCommand: "some command", + } + + pCtx := models.PreWorkflowHookCommandContext{ + BaseRepo: fixtures.GithubRepo, + HeadRepo: fixtures.GithubRepo, + Pull: newPull, + Log: log, + User: fixtures.User, + Verbose: false, + } + + repoDir := "path/to/repo" + result := "some result" + + t.Run("success hooks in cfg", func(t *testing.T) { + preWorkflowHooksSetup(t) + + var unlockCalled *bool = newBool(false) + unlockFn := func() { + unlockCalled = newBool(true) + } + + globalCfg := valid.GlobalCfg{ + Repos: []valid.Repo{ + { + ID: fixtures.GithubRepo.ID(), + PreWorkflowHooks: []*valid.PreWorkflowHook{ + &testHook, + }, + }, + }, + } + + wh.GlobalCfg = globalCfg + + When(whWorkingDirLocker.TryLock(fixtures.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace)).ThenReturn(unlockFn, nil) + When(whWorkingDir.Clone(log, fixtures.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(pCtx, testHook.RunCommand, repoDir)).ThenReturn(result, nil) + + err := wh.RunPreHooks(ctx) + + Ok(t, err) + whPreWorkflowHookRunner.VerifyWasCalledOnce().Run(pCtx, testHook.RunCommand, repoDir) + Assert(t, *unlockCalled == true, "unlock function called") + }) + t.Run("success hooks not in cfg", func(t *testing.T) { + preWorkflowHooksSetup(t) + globalCfg := valid.GlobalCfg{ + Repos: []valid.Repo{ + // one with hooks but mismatched id + { + ID: "id1", + PreWorkflowHooks: []*valid.PreWorkflowHook{ + &testHook, + }, + }, + // one with the correct id but no hooks + { + ID: fixtures.GithubRepo.ID(), + PreWorkflowHooks: []*valid.PreWorkflowHook{}, + }, + }, + } + + wh.GlobalCfg = globalCfg + + err := wh.RunPreHooks(ctx) + + Ok(t, err) + + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(pCtx, testHook.RunCommand, repoDir) + whWorkingDirLocker.VerifyWasCalled(Never()).TryLock(fixtures.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace) + whWorkingDir.VerifyWasCalled(Never()).Clone(log, fixtures.GithubRepo, newPull, events.DefaultWorkspace) + }) + t.Run("error locking work dir", func(t *testing.T) { + preWorkflowHooksSetup(t) + + globalCfg := valid.GlobalCfg{ + Repos: []valid.Repo{ + { + ID: fixtures.GithubRepo.ID(), + PreWorkflowHooks: []*valid.PreWorkflowHook{ + &testHook, + }, + }, + }, + } + + wh.GlobalCfg = globalCfg + + When(whWorkingDirLocker.TryLock(fixtures.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace)).ThenReturn(func() {}, errors.New("some error")) + + err := wh.RunPreHooks(ctx) + + Assert(t, err != nil, "error not nil") + whWorkingDir.VerifyWasCalled(Never()).Clone(log, fixtures.GithubRepo, newPull, events.DefaultWorkspace) + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(pCtx, testHook.RunCommand, repoDir) + }) + + t.Run("error cloning", func(t *testing.T) { + preWorkflowHooksSetup(t) + + var unlockCalled *bool = newBool(false) + unlockFn := func() { + unlockCalled = newBool(true) + } + + globalCfg := valid.GlobalCfg{ + Repos: []valid.Repo{ + { + ID: fixtures.GithubRepo.ID(), + PreWorkflowHooks: []*valid.PreWorkflowHook{ + &testHook, + }, + }, + }, + } + + wh.GlobalCfg = globalCfg + + When(whWorkingDirLocker.TryLock(fixtures.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace)).ThenReturn(unlockFn, nil) + When(whWorkingDir.Clone(log, fixtures.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, errors.New("some error")) + + err := wh.RunPreHooks(ctx) + + Assert(t, err != nil, "error not nil") + + whPreWorkflowHookRunner.VerifyWasCalled(Never()).Run(pCtx, testHook.RunCommand, repoDir) + Assert(t, *unlockCalled == true, "unlock function called") + }) + + t.Run("error running pre hook", func(t *testing.T) { + preWorkflowHooksSetup(t) + + var unlockCalled *bool = newBool(false) + unlockFn := func() { + unlockCalled = newBool(true) + } + + globalCfg := valid.GlobalCfg{ + Repos: []valid.Repo{ + { + ID: fixtures.GithubRepo.ID(), + PreWorkflowHooks: []*valid.PreWorkflowHook{ + &testHook, + }, + }, + }, + } + + wh.GlobalCfg = globalCfg + + When(whWorkingDirLocker.TryLock(fixtures.GithubRepo.FullName, newPull.Num, events.DefaultWorkspace)).ThenReturn(unlockFn, nil) + When(whWorkingDir.Clone(log, fixtures.GithubRepo, newPull, events.DefaultWorkspace)).ThenReturn(repoDir, false, nil) + When(whPreWorkflowHookRunner.Run(pCtx, testHook.RunCommand, repoDir)).ThenReturn(result, errors.New("some error")) + + err := wh.RunPreHooks(ctx) -func TestRunPreHooks_DrainNotOngoing(t *testing.T) { - t.Log("if drain is not ongoing then remove ongoing operation must be called even if panic occured") - preWorkflowHooksSetup(t) - When(whWorkingDir.Clone(logger, fixtures.GithubRepo, fixtures.Pull, events.DefaultWorkspace)).ThenPanic("panic test - if you're seeing this in a test failure this isn't the failing test") - wh.RunPreHooks(fixtures.GithubRepo, fixtures.GithubRepo, fixtures.Pull, fixtures.User) - whWorkingDir.VerifyWasCalledOnce().Clone(logger, fixtures.GithubRepo, fixtures.Pull, events.DefaultWorkspace) - Equals(t, 0, whDrainer.GetStatus().InProgressOps) + Assert(t, err != nil, "error not nil") + Assert(t, *unlockCalled == true, "unlock function called") + }) } diff --git a/server/events/runtime/mocks/matchers/models_preworkflowhookcommandcontext.go b/server/events/runtime/mocks/matchers/models_preworkflowhookcommandcontext.go new file mode 100644 index 0000000000..8a57120c7c --- /dev/null +++ b/server/events/runtime/mocks/matchers/models_preworkflowhookcommandcontext.go @@ -0,0 +1,33 @@ +// Code generated by pegomock. DO NOT EDIT. +package matchers + +import ( + "github.com/petergtz/pegomock" + "reflect" + + models "github.com/runatlantis/atlantis/server/events/models" +) + +func AnyModelsPreWorkflowHookCommandContext() models.PreWorkflowHookCommandContext { + pegomock.RegisterMatcher(pegomock.NewAnyMatcher(reflect.TypeOf((*(models.PreWorkflowHookCommandContext))(nil)).Elem())) + var nullValue models.PreWorkflowHookCommandContext + return nullValue +} + +func EqModelsPreWorkflowHookCommandContext(value models.PreWorkflowHookCommandContext) models.PreWorkflowHookCommandContext { + pegomock.RegisterMatcher(&pegomock.EqMatcher{Value: value}) + var nullValue models.PreWorkflowHookCommandContext + return nullValue +} + +func NotEqModelsPreWorkflowHookCommandContext(value models.PreWorkflowHookCommandContext) models.PreWorkflowHookCommandContext { + pegomock.RegisterMatcher(&pegomock.NotEqMatcher{Value: value}) + var nullValue models.PreWorkflowHookCommandContext + return nullValue +} + +func ModelsPreWorkflowHookCommandContextThat(matcher pegomock.ArgumentMatcher) models.PreWorkflowHookCommandContext { + pegomock.RegisterMatcher(matcher) + var nullValue models.PreWorkflowHookCommandContext + return nullValue +} diff --git a/server/events/runtime/mocks/mock_pre_workflows_hook_runner.go b/server/events/runtime/mocks/mock_pre_workflows_hook_runner.go new file mode 100644 index 0000000000..dcb5c32935 --- /dev/null +++ b/server/events/runtime/mocks/mock_pre_workflows_hook_runner.go @@ -0,0 +1,117 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/events/runtime (interfaces: PreWorkflowHookRunner) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockPreWorkflowHookRunner struct { + fail func(message string, callerSkip ...int) +} + +func NewMockPreWorkflowHookRunner(options ...pegomock.Option) *MockPreWorkflowHookRunner { + mock := &MockPreWorkflowHookRunner{} + for _, option := range options { + option.Apply(mock) + } + return mock +} + +func (mock *MockPreWorkflowHookRunner) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } +func (mock *MockPreWorkflowHookRunner) FailHandler() pegomock.FailHandler { return mock.fail } + +func (mock *MockPreWorkflowHookRunner) Run(ctx models.PreWorkflowHookCommandContext, command string, path string) (string, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockPreWorkflowHookRunner().") + } + params := []pegomock.Param{ctx, command, path} + result := pegomock.GetGenericMockFrom(mock).Invoke("Run", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 string + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(string) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + +func (mock *MockPreWorkflowHookRunner) VerifyWasCalledOnce() *VerifierMockPreWorkflowHookRunner { + return &VerifierMockPreWorkflowHookRunner{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockPreWorkflowHookRunner) VerifyWasCalled(invocationCountMatcher pegomock.InvocationCountMatcher) *VerifierMockPreWorkflowHookRunner { + return &VerifierMockPreWorkflowHookRunner{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockPreWorkflowHookRunner) VerifyWasCalledInOrder(invocationCountMatcher pegomock.InvocationCountMatcher, inOrderContext *pegomock.InOrderContext) *VerifierMockPreWorkflowHookRunner { + return &VerifierMockPreWorkflowHookRunner{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockPreWorkflowHookRunner) VerifyWasCalledEventually(invocationCountMatcher pegomock.InvocationCountMatcher, timeout time.Duration) *VerifierMockPreWorkflowHookRunner { + return &VerifierMockPreWorkflowHookRunner{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierMockPreWorkflowHookRunner struct { + mock *MockPreWorkflowHookRunner + invocationCountMatcher pegomock.InvocationCountMatcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierMockPreWorkflowHookRunner) Run(ctx models.PreWorkflowHookCommandContext, command string, path string) *MockPreWorkflowHookRunner_Run_OngoingVerification { + params := []pegomock.Param{ctx, command, path} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "Run", params, verifier.timeout) + return &MockPreWorkflowHookRunner_Run_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type MockPreWorkflowHookRunner_Run_OngoingVerification struct { + mock *MockPreWorkflowHookRunner + methodInvocations []pegomock.MethodInvocation +} + +func (c *MockPreWorkflowHookRunner_Run_OngoingVerification) GetCapturedArguments() (models.PreWorkflowHookCommandContext, string, string) { + ctx, command, path := c.GetAllCapturedArguments() + return ctx[len(ctx)-1], command[len(command)-1], path[len(path)-1] +} + +func (c *MockPreWorkflowHookRunner_Run_OngoingVerification) GetAllCapturedArguments() (_param0 []models.PreWorkflowHookCommandContext, _param1 []string, _param2 []string) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.PreWorkflowHookCommandContext, len(c.methodInvocations)) + for u, param := range params[0] { + _param0[u] = param.(models.PreWorkflowHookCommandContext) + } + _param1 = make([]string, len(c.methodInvocations)) + for u, param := range params[1] { + _param1[u] = param.(string) + } + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range params[2] { + _param2[u] = param.(string) + } + } + return +} diff --git a/server/events/runtime/pre_workflow_hook_runner.go b/server/events/runtime/pre_workflow_hook_runner.go index fbe6ee0007..df0c694463 100644 --- a/server/events/runtime/pre_workflow_hook_runner.go +++ b/server/events/runtime/pre_workflow_hook_runner.go @@ -8,9 +8,14 @@ import ( "github.com/runatlantis/atlantis/server/events/models" ) -type PreWorkflowHookRunner struct{} +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_pre_workflows_hook_runner.go PreWorkflowHookRunner +type PreWorkflowHookRunner interface { + Run(ctx models.PreWorkflowHookCommandContext, command string, path string) (string, error) +} + +type DefaultPreWorkflowHookRunner struct{} -func (wh *PreWorkflowHookRunner) Run(ctx models.PreWorkflowHookCommandContext, command string, path string) (string, error) { +func (wh DefaultPreWorkflowHookRunner) Run(ctx models.PreWorkflowHookCommandContext, command string, path string) (string, error) { cmd := exec.Command("sh", "-c", command) // #nosec cmd.Dir = path @@ -21,6 +26,7 @@ func (wh *PreWorkflowHookRunner) Run(ctx models.PreWorkflowHookCommandContext, c "BASE_REPO_OWNER": ctx.BaseRepo.Owner, "DIR": path, "HEAD_BRANCH_NAME": ctx.Pull.HeadBranch, + "HEAD_COMMIT": ctx.Pull.HeadCommit, "HEAD_REPO_NAME": ctx.HeadRepo.Name, "HEAD_REPO_OWNER": ctx.HeadRepo.Owner, "PULL_AUTHOR": ctx.Pull.Author, diff --git a/server/events/runtime/pre_workflow_hook_runner_test.go b/server/events/runtime/pre_workflow_hook_runner_test.go index 19f59b9994..70c092a564 100644 --- a/server/events/runtime/pre_workflow_hook_runner_test.go +++ b/server/events/runtime/pre_workflow_hook_runner_test.go @@ -49,8 +49,8 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { ExpErr: "exit status 127: running \"lkjlkj\" in", }, { - Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_author=$PULL_AUTHOR", - ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat base_branch_name=master pull_num=2 pull_author=acme\n", + Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME head_commit=$HEAD_COMMIT base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_author=$PULL_AUTHOR", + ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=master pull_num=2 pull_author=acme\n", }, { Command: "echo user_name=$USER_NAME", @@ -70,7 +70,7 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { logger := logging.NewNoopLogger() - r := runtime.PreWorkflowHookRunner{} + r := runtime.DefaultPreWorkflowHookRunner{} t.Run(c.Command, func(t *testing.T) { tmpDir, cleanup := TempDir(t) defer cleanup() @@ -86,6 +86,7 @@ func TestPreWorkflowHookRunner_Run(t *testing.T) { Pull: models.PullRequest{ Num: 2, HeadBranch: "add-feat", + HeadCommit: "12345abcdef", BaseBranch: "master", Author: "acme", }, diff --git a/server/events/runtime/run_step_runner.go b/server/events/runtime/run_step_runner.go index e4d0015b93..266f39eb2e 100644 --- a/server/events/runtime/run_step_runner.go +++ b/server/events/runtime/run_step_runner.go @@ -44,6 +44,7 @@ func (r *RunStepRunner) Run(ctx models.ProjectCommandContext, command string, pa "COMMENT_ARGS": strings.Join(ctx.EscapedCommentArgs, ","), "DIR": path, "HEAD_BRANCH_NAME": ctx.Pull.HeadBranch, + "HEAD_COMMIT": ctx.Pull.HeadCommit, "HEAD_REPO_NAME": ctx.HeadRepo.Name, "HEAD_REPO_OWNER": ctx.HeadRepo.Owner, "PATH": fmt.Sprintf("%s:%s", os.Getenv("PATH"), r.TerraformBinDir), diff --git a/server/events/runtime/run_step_runner_test.go b/server/events/runtime/run_step_runner_test.go index f662fbf1cd..04528113c3 100644 --- a/server/events/runtime/run_step_runner_test.go +++ b/server/events/runtime/run_step_runner_test.go @@ -65,8 +65,8 @@ func TestRunStepRunner_Run(t *testing.T) { ExpOut: "workspace=myworkspace version=0.11.0 dir=$DIR planfile=$DIR/my::project::name-myworkspace.tfplan project=my/project/name\n", }, { - Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_author=$PULL_AUTHOR repo_rel_dir=$REPO_REL_DIR", - ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat base_branch_name=master pull_num=2 pull_author=acme repo_rel_dir=mydir\n", + Command: "echo base_repo_name=$BASE_REPO_NAME base_repo_owner=$BASE_REPO_OWNER head_repo_name=$HEAD_REPO_NAME head_repo_owner=$HEAD_REPO_OWNER head_branch_name=$HEAD_BRANCH_NAME head_commit=$HEAD_COMMIT base_branch_name=$BASE_BRANCH_NAME pull_num=$PULL_NUM pull_author=$PULL_AUTHOR repo_rel_dir=$REPO_REL_DIR", + ExpOut: "base_repo_name=basename base_repo_owner=baseowner head_repo_name=headname head_repo_owner=headowner head_branch_name=add-feat head_commit=12345abcdef base_branch_name=master pull_num=2 pull_author=acme repo_rel_dir=mydir\n", }, { Command: "echo user_name=$USER_NAME", @@ -124,6 +124,7 @@ func TestRunStepRunner_Run(t *testing.T) { Pull: models.PullRequest{ Num: 2, HeadBranch: "add-feat", + HeadCommit: "12345abcdef", BaseBranch: "master", Author: "acme", }, diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 1c78248845..a0b1db6a46 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -361,7 +361,7 @@ func (g *GithubClient) MergePull(pull models.PullRequest) error { pull.BaseRepo.Owner, pull.BaseRepo.Name, pull.Num, - // NOTE: Using the emtpy string here causes GitHub to autogenerate + // NOTE: Using the empty string here causes GitHub to autogenerate // the commit message as it normally would. "", options) diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index 92a1bf3470..8968c92c70 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -1014,11 +1014,17 @@ func TestParseGlobalCfg(t *testing.T) { - apply_requirements: []`, expErr: "repos: (0: (id: cannot be blank.).).", }, - "invalid regex": { + "invalid id regex": { input: `repos: - id: /?/`, expErr: "repos: (0: (id: parsing: /?/: error parsing regexp: missing argument to repetition operator: `?`.).).", }, + "invalid branch regex": { + input: `repos: +- id: /.*/ + branch: /?/`, + expErr: "repos: (0: (branch: parsing: /?/: error parsing regexp: missing argument to repetition operator: `?`.).).", + }, "workflow doesn't exist": { input: `repos: - id: /.*/ @@ -1115,6 +1121,7 @@ repos: allowed_overrides: [apply_requirements, workflow] allow_custom_workflows: true - id: /.*/ + branch: /(master|main)/ pre_workflow_hooks: - run: custom workflow command workflows: @@ -1155,6 +1162,7 @@ policies: }, { IDRegex: regexp.MustCompile(".*"), + BranchRegex: regexp.MustCompile("(master|main)"), PreWorkflowHooks: preWorkflowHooks, }, }, @@ -1228,6 +1236,7 @@ workflows: Repos: []valid.Repo{ { IDRegex: regexp.MustCompile(".*"), + BranchRegex: regexp.MustCompile(".*"), PreWorkflowHooks: []*valid.PreWorkflowHook{}, ApplyRequirements: []string{}, Workflow: &valid.Workflow{ @@ -1301,6 +1310,10 @@ workflows: Assert(t, expRepo.IDRegex.String() == actRepo.IDRegex.String(), "%q != %q for repos[%d]", expRepo.IDRegex.String(), actRepo.IDRegex.String(), i) } + if expRepo.BranchRegex != nil { + Assert(t, expRepo.BranchRegex.String() == actRepo.BranchRegex.String(), + "%q != %q for repos[%d]", expRepo.BranchRegex.String(), actRepo.BranchRegex.String(), i) + } } }) } diff --git a/server/events/yaml/raw/global_cfg.go b/server/events/yaml/raw/global_cfg.go index 7d5157a757..b48111505a 100644 --- a/server/events/yaml/raw/global_cfg.go +++ b/server/events/yaml/raw/global_cfg.go @@ -20,6 +20,7 @@ type GlobalCfg struct { // Repo is the raw schema for repos in the server-side repo config. type Repo struct { ID string `yaml:"id" json:"id"` + Branch string `yaml:"branch" json:"branch"` ApplyRequirements []string `yaml:"apply_requirements" json:"apply_requirements"` PreWorkflowHooks []PreWorkflowHook `yaml:"pre_workflow_hooks" json:"pre_workflow_hooks"` Workflow *string `yaml:"workflow,omitempty" json:"workflow,omitempty"` @@ -121,6 +122,11 @@ func (r Repo) HasRegexID() bool { return strings.HasPrefix(r.ID, "/") && strings.HasSuffix(r.ID, "/") } +// HasRegexBranch returns true if a branch regex was set. +func (r Repo) HasRegexBranch() bool { + return strings.HasPrefix(r.Branch, "/") && strings.HasSuffix(r.Branch, "/") +} + func (r Repo) Validate() error { idValid := func(value interface{}) error { id := value.(string) @@ -131,6 +137,15 @@ func (r Repo) Validate() error { return errors.Wrapf(err, "parsing: %s", id) } + branchValid := func(value interface{}) error { + branch := value.(string) + if !r.HasRegexBranch() { + return nil + } + _, err := regexp.Compile(branch[1 : len(branch)-1]) + return errors.Wrapf(err, "parsing: %s", branch) + } + overridesValid := func(value interface{}) error { overrides := value.([]string) for _, o := range overrides { @@ -149,6 +164,7 @@ func (r Repo) Validate() error { return validation.ValidateStruct(&r, validation.Field(&r.ID, validation.Required, validation.By(idValid)), + validation.Field(&r.Branch, validation.By(branchValid)), validation.Field(&r.AllowedOverrides, validation.By(overridesValid)), validation.Field(&r.ApplyRequirements, validation.By(validApplyReq)), validation.Field(&r.Workflow, validation.By(workflowExists)), @@ -166,6 +182,13 @@ func (r Repo) ToValid(workflows map[string]valid.Workflow) valid.Repo { id = r.ID } + var branchRegex *regexp.Regexp + if r.HasRegexBranch() { + withoutSlashes := r.Branch[1 : len(r.Branch)-1] + // Safe to use MustCompile because we test it in Validate(). + branchRegex = regexp.MustCompile(withoutSlashes) + } + var workflow *valid.Workflow if r.Workflow != nil { // This key is guaranteed to exist because we test for it in @@ -184,6 +207,7 @@ func (r Repo) ToValid(workflows map[string]valid.Workflow) valid.Repo { return valid.Repo{ ID: id, IDRegex: idRegex, + BranchRegex: branchRegex, ApplyRequirements: r.ApplyRequirements, PreWorkflowHooks: preWorkflowHooks, Workflow: workflow, diff --git a/server/events/yaml/valid/global_cfg.go b/server/events/yaml/valid/global_cfg.go index dac71e19b2..fe6af76f82 100644 --- a/server/events/yaml/valid/global_cfg.go +++ b/server/events/yaml/valid/global_cfg.go @@ -34,6 +34,7 @@ type Repo struct { // IDRegex is the regex match for this config. // If ID is set then this will be nil. IDRegex *regexp.Regexp + BranchRegex *regexp.Regexp ApplyRequirements []string PreWorkflowHooks []*PreWorkflowHook Workflow *Workflow @@ -94,13 +95,7 @@ var DefaultPlanStage = Stage{ }, } -// NewGlobalCfg returns a global config that respects the parameters. -// allowRepoCfg is true if users want to allow repos full config functionality. -// mergeableReq is true if users want to set the mergeable apply requirement -// for all repos. -// approvedReq is true if users want to set the approved apply requirement -// for all repos. -func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) GlobalCfg { +func NewGlobalCfgWithHooks(allowRepoCfg bool, mergeableReq bool, approvedReq bool, preWorkflowHooks []*PreWorkflowHook) GlobalCfg { defaultWorkflow := Workflow{ Name: DefaultWorkflowName, Apply: DefaultApplyStage, @@ -112,7 +107,6 @@ func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) Global applyReqs := []string{} allowedOverrides := []string{} allowedWorkflows := []string{} - preWorkflowHooks := make([]*PreWorkflowHook, 0) if mergeableReq { applyReqs = append(applyReqs, MergeableApplyReq) } @@ -130,6 +124,7 @@ func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) Global Repos: []Repo{ { IDRegex: regexp.MustCompile(".*"), + BranchRegex: regexp.MustCompile(".*"), ApplyRequirements: applyReqs, PreWorkflowHooks: preWorkflowHooks, Workflow: &defaultWorkflow, @@ -144,6 +139,19 @@ func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) Global } } +// NewGlobalCfg returns a global config that respects the parameters. +// allowRepoCfg is true if users want to allow repos full config functionality. +// mergeableReq is true if users want to set the mergeable apply requirement +// for all repos. +// approvedReq is true if users want to set the approved apply requirement +// for all repos. + +func NewGlobalCfg(allowRepoCfg bool, mergeableReq bool, approvedReq bool) GlobalCfg { + preWorkflowHooks := make([]*PreWorkflowHook, 0) + + return NewGlobalCfgWithHooks(allowRepoCfg, mergeableReq, approvedReq, preWorkflowHooks) +} + // IDMatches returns true if the repo ID otherID matches this config. func (r Repo) IDMatches(otherID string) bool { if r.ID != "" { @@ -152,6 +160,14 @@ func (r Repo) IDMatches(otherID string) bool { return r.IDRegex.MatchString(otherID) } +// BranchMatches returns true if the branch other matches a branch regex (if preset). +func (r Repo) BranchMatches(other string) bool { + if r.BranchRegex == nil { + return true + } + return r.BranchRegex.MatchString(other) +} + // IDString returns a string representation of this config. func (r Repo) IDString() string { if r.ID != "" { diff --git a/server/events/yaml/valid/global_cfg_test.go b/server/events/yaml/valid/global_cfg_test.go index 2c85205e25..5fbf759ce0 100644 --- a/server/events/yaml/valid/global_cfg_test.go +++ b/server/events/yaml/valid/global_cfg_test.go @@ -50,6 +50,7 @@ func TestNewGlobalCfg(t *testing.T) { Repos: []valid.Repo{ { IDRegex: regexp.MustCompile(".*"), + BranchRegex: regexp.MustCompile(".*"), ApplyRequirements: []string{}, Workflow: &expDefaultWorkflow, AllowedWorkflows: []string{}, @@ -108,6 +109,7 @@ func TestNewGlobalCfg(t *testing.T) { // For each test, we change our expected cfg based on the parameters. exp := deepcopy.Copy(baseCfg).(valid.GlobalCfg) exp.Repos[0].IDRegex = regexp.MustCompile(".*") // deepcopy doesn't copy the regex. + exp.Repos[0].BranchRegex = regexp.MustCompile(".*") if c.allowRepoCfg { exp.Repos[0].AllowCustomWorkflows = Bool(true) @@ -132,6 +134,10 @@ func TestNewGlobalCfg(t *testing.T) { Assert(t, expRepo.IDRegex.String() == actRepo.IDRegex.String(), "%q != %q for repos[%d]", expRepo.IDRegex.String(), actRepo.IDRegex.String(), i) } + if expRepo.BranchRegex != nil { + Assert(t, expRepo.BranchRegex.String() == actRepo.BranchRegex.String(), + "%q != %q for repos[%d]", expRepo.BranchRegex.String(), actRepo.BranchRegex.String(), i) + } } }) } @@ -726,6 +732,21 @@ func TestRepo_IDString(t *testing.T) { Equals(t, "/regex.*/", (valid.Repo{IDRegex: regexp.MustCompile("regex.*")}).IDString()) } +func TestRepo_BranchMatches(t *testing.T) { + // Test matches when no branch regex is set. + Equals(t, true, (valid.Repo{}).BranchMatches("main")) + + // Test regexes. + Equals(t, true, (valid.Repo{BranchRegex: regexp.MustCompile(".*")}).BranchMatches("main")) + Equals(t, true, (valid.Repo{BranchRegex: regexp.MustCompile("main")}).BranchMatches("main")) + Equals(t, false, (valid.Repo{BranchRegex: regexp.MustCompile("^main$")}).BranchMatches("foo-main")) + Equals(t, false, (valid.Repo{BranchRegex: regexp.MustCompile("^main$")}).BranchMatches("main-foo")) + Equals(t, true, (valid.Repo{BranchRegex: regexp.MustCompile("(main|master)")}).BranchMatches("main")) + Equals(t, true, (valid.Repo{BranchRegex: regexp.MustCompile("(main|master)")}).BranchMatches("master")) + Equals(t, true, (valid.Repo{BranchRegex: regexp.MustCompile("release")}).BranchMatches("release-stage")) + Equals(t, false, (valid.Repo{BranchRegex: regexp.MustCompile("release")}).BranchMatches("main")) +} + // String is a helper routine that allocates a new string value // to store v and returns a pointer to it. func String(v string) *string { return &v } diff --git a/server/events_controller.go b/server/events_controller.go index 5dca5efaee..de08599083 100644 --- a/server/events_controller.go +++ b/server/events_controller.go @@ -45,13 +45,12 @@ const bitbucketServerSignatureHeader = "X-Hub-Signature" // EventsController handles all webhook requests which signify 'events' in the // VCS host, ex. GitHub. type EventsController struct { - PreWorkflowHooksCommandRunner events.PreWorkflowHooksCommandRunner - CommandRunner events.CommandRunner - PullCleaner events.PullCleaner - Logger *logging.SimpleLogger - Parser events.EventParsing - CommentParser events.CommentParsing - ApplyDisabled bool + CommandRunner events.CommandRunner + PullCleaner events.PullCleaner + Logger *logging.SimpleLogger + Parser events.EventParsing + CommentParser events.CommentParsing + ApplyDisabled bool // GithubWebhookSecret is the secret added to this webhook via the GitHub // UI that identifies this call as coming from GitHub. If empty, no // request validation is done. @@ -344,9 +343,6 @@ func (e *EventsController) handlePullRequestEvent(w http.ResponseWriter, baseRep // closed. fmt.Fprintln(w, "Processing...") - e.Logger.Info("running pre workflow hooks if present") - e.PreWorkflowHooksCommandRunner.RunPreHooks(baseRepo, headRepo, pull, user) - e.Logger.Info("executing autoplan") if !e.TestingMode { go e.CommandRunner.RunAutoplanCommand(baseRepo, headRepo, pull, user) diff --git a/server/events_controller_e2e_test.go b/server/events_controller_e2e_test.go index 4d21789b27..823cf5945a 100644 --- a/server/events_controller_e2e_test.go +++ b/server/events_controller_e2e_test.go @@ -25,6 +25,8 @@ import ( "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/runtime" + runtimemocks "github.com/runatlantis/atlantis/server/events/runtime/mocks" + runtimematchers "github.com/runatlantis/atlantis/server/events/runtime/mocks/matchers" "github.com/runatlantis/atlantis/server/events/runtime/policy" "github.com/runatlantis/atlantis/server/events/terraform" vcsmocks "github.com/runatlantis/atlantis/server/events/vcs/mocks" @@ -37,6 +39,8 @@ import ( type NoopTFDownloader struct{} +var mockPreWorkflowHookRunner *runtimemocks.MockPreWorkflowHookRunner + func (m *NoopTFDownloader) GetFile(dst, src string, opts ...getter.ClientOption) error { return nil } @@ -368,6 +372,9 @@ func TestGitHubWorkflow(t *testing.T) { ctrl.Post(w, pullClosedReq) responseContains(t, w, 200, "Pull request cleaned successfully") + // Let's verify the pre-workflow hook was called for each comment including the pull request opened event + mockPreWorkflowHookRunner.VerifyWasCalled(Times(len(c.Comments)+1)).Run(runtimematchers.AnyModelsPreWorkflowHookCommandContext(), EqString("some dummy command"), AnyString()) + // Now we're ready to verify Atlantis made all the comments back (or // replies) that we expect. We expect each plan to have 1 comment, // and apply have 1 for each comment plus one for the locks deleted at the @@ -599,7 +606,12 @@ func setupE2E(t *testing.T, repoDir string, policyChecksEnabled bool) (server.Ev defaultTFVersion := terraformClient.DefaultVersion() locker := events.NewDefaultWorkingDirLocker() parser := &yaml.ParserValidator{} - globalCfg := valid.NewGlobalCfg(true, false, false) + globalCfg := valid.NewGlobalCfgWithHooks(true, false, false, []*valid.PreWorkflowHook{ + { + StepName: "global_hook", + RunCommand: "some dummy command", + }, + }) expCfgPath := filepath.Join(absRepoPath(t, repoDir), "repos.yaml") if _, err := os.Stat(expCfgPath); err == nil { globalCfg, err = parser.ParseGlobalCfg(expCfgPath, globalCfg) @@ -609,14 +621,13 @@ func setupE2E(t *testing.T, repoDir string, policyChecksEnabled bool) (server.Ev parallelPoolSize := 1 + mockPreWorkflowHookRunner = runtimemocks.NewMockPreWorkflowHookRunner() preWorkflowHooksCommandRunner := &events.DefaultPreWorkflowHooksCommandRunner{ VCSClient: e2eVCSClient, GlobalCfg: globalCfg, - Logger: logger, WorkingDirLocker: locker, WorkingDir: workingDir, - Drainer: drainer, - PreWorkflowHookRunner: &runtime.PreWorkflowHookRunner{}, + PreWorkflowHookRunner: mockPreWorkflowHookRunner, } projectCommandBuilder := events.NewProjectCommandBuilder( policyChecksEnabled, @@ -750,24 +761,24 @@ func setupE2E(t *testing.T, repoDir string, policyChecksEnabled bool) (server.Ev } commandRunner := &events.DefaultCommandRunner{ - EventParser: eventParser, - VCSClient: e2eVCSClient, - GithubPullGetter: e2eGithubGetter, - GitlabMergeRequestGetter: e2eGitlabGetter, - Logger: logger, - AllowForkPRs: allowForkPRs, - AllowForkPRsFlag: "allow-fork-prs", - CommentCommandRunnerByCmd: commentCommandRunnerByCmd, - Drainer: drainer, + EventParser: eventParser, + VCSClient: e2eVCSClient, + GithubPullGetter: e2eGithubGetter, + GitlabMergeRequestGetter: e2eGitlabGetter, + Logger: logger, + AllowForkPRs: allowForkPRs, + AllowForkPRsFlag: "allow-fork-prs", + CommentCommandRunnerByCmd: commentCommandRunnerByCmd, + Drainer: drainer, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, } repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") Ok(t, err) ctrl := server.EventsController{ - TestingMode: true, - PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, - CommandRunner: commandRunner, + TestingMode: true, + CommandRunner: commandRunner, PullCleaner: &events.PullClosedExecutor{ Locker: lockingClient, VCSClient: e2eVCSClient, diff --git a/server/events_controller_test.go b/server/events_controller_test.go index ebfa917ad7..af24490eb8 100644 --- a/server/events_controller_test.go +++ b/server/events_controller_test.go @@ -45,7 +45,7 @@ var secret = []byte("secret") func TestPost_NotGithubOrGitlab(t *testing.T) { t.Log("when the request is not for gitlab or github a 400 is returned") - e, _, _, _, _, _, _, _, _ := setup(t) + e, _, _, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) e.Post(w, req) @@ -54,7 +54,7 @@ func TestPost_NotGithubOrGitlab(t *testing.T) { func TestPost_UnsupportedVCSGithub(t *testing.T) { t.Log("when the request is for an unsupported vcs a 400 is returned") - e, _, _, _, _, _, _, _, _ := setup(t) + e, _, _, _, _, _, _, _ := setup(t) e.SupportedVCSHosts = nil req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "value") @@ -65,7 +65,7 @@ func TestPost_UnsupportedVCSGithub(t *testing.T) { func TestPost_UnsupportedVCSGitlab(t *testing.T) { t.Log("when the request is for an unsupported vcs a 400 is returned") - e, _, _, _, _, _, _, _, _ := setup(t) + e, _, _, _, _, _, _, _ := setup(t) e.SupportedVCSHosts = nil req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") @@ -76,7 +76,7 @@ func TestPost_UnsupportedVCSGitlab(t *testing.T) { func TestPost_InvalidGithubSecret(t *testing.T) { t.Log("when the github payload can't be validated a 400 is returned") - e, v, _, _, _, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "value") @@ -87,7 +87,7 @@ func TestPost_InvalidGithubSecret(t *testing.T) { func TestPost_InvalidGitlabSecret(t *testing.T) { t.Log("when the gitlab payload can't be validated a 400 is returned") - e, _, gl, _, _, _, _, _, _ := setup(t) + e, _, gl, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") @@ -98,7 +98,7 @@ func TestPost_InvalidGitlabSecret(t *testing.T) { func TestPost_UnsupportedGithubEvent(t *testing.T) { t.Log("when the event type is an unsupported github event we ignore it") - e, v, _, _, _, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "value") @@ -109,7 +109,7 @@ func TestPost_UnsupportedGithubEvent(t *testing.T) { func TestPost_UnsupportedGitlabEvent(t *testing.T) { t.Log("when the event type is an unsupported gitlab event we ignore it") - e, _, gl, _, _, _, _, _, _ := setup(t) + e, _, gl, _, _, _, _, _ := setup(t) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") @@ -121,7 +121,7 @@ func TestPost_UnsupportedGitlabEvent(t *testing.T) { // Test that if the comment comes from a commit rather than a merge request, // we give an error and ignore it. func TestPost_GitlabCommentOnCommit(t *testing.T) { - e, _, gl, _, _, _, _, _, _ := setup(t) + e, _, gl, _, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) w := httptest.NewRecorder() req.Header.Set(gitlabHeader, "value") @@ -132,7 +132,7 @@ func TestPost_GitlabCommentOnCommit(t *testing.T) { func TestPost_GithubCommentNotCreated(t *testing.T) { t.Log("when the event is a github comment but it's not a created event we ignore it") - e, v, _, _, _, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "issue_comment") // comment action is deleted, not created @@ -145,7 +145,7 @@ func TestPost_GithubCommentNotCreated(t *testing.T) { func TestPost_GithubInvalidComment(t *testing.T) { t.Log("when the event is a github comment without all expected data we return a 400") - e, v, _, p, _, _, _, _, _ := setup(t) + e, v, _, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` @@ -158,7 +158,7 @@ func TestPost_GithubInvalidComment(t *testing.T) { func TestPost_GitlabCommentInvalidCommand(t *testing.T) { t.Log("when the event is a gitlab comment with an invalid command we ignore it") - e, _, gl, _, _, _, _, _, cp := setup(t) + e, _, gl, _, _, _, _, cp := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) @@ -170,7 +170,7 @@ func TestPost_GitlabCommentInvalidCommand(t *testing.T) { func TestPost_GithubCommentInvalidCommand(t *testing.T) { t.Log("when the event is a github comment with an invalid command we ignore it") - e, v, _, p, _, _, _, _, cp := setup(t) + e, v, _, p, _, _, _, cp := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` @@ -299,7 +299,7 @@ func TestPost_GithubCommentNotAllowlistedWithSilenceErrors(t *testing.T) { func TestPost_GitlabCommentResponse(t *testing.T) { // When the event is a gitlab comment that warrants a comment response we comment back. - e, _, gl, _, _, _, _, vcsClient, cp := setup(t) + e, _, gl, _, _, _, vcsClient, cp := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) @@ -312,7 +312,7 @@ func TestPost_GitlabCommentResponse(t *testing.T) { func TestPost_GithubCommentResponse(t *testing.T) { t.Log("when the event is a github comment that warrants a comment response we comment back") - e, v, _, p, _, _, _, vcsClient, cp := setup(t) + e, v, _, p, _, _, vcsClient, cp := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` @@ -330,7 +330,7 @@ func TestPost_GithubCommentResponse(t *testing.T) { func TestPost_GitlabCommentSuccess(t *testing.T) { t.Log("when the event is a gitlab comment with a valid command we call the command handler") - e, _, gl, _, cr, _, _, _, _ := setup(t) + e, _, gl, _, cr, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeCommentEvent{}, nil) @@ -343,7 +343,7 @@ func TestPost_GitlabCommentSuccess(t *testing.T) { func TestPost_GithubCommentSuccess(t *testing.T) { t.Log("when the event is a github comment with a valid command we call the command handler") - e, v, _, p, cr, _, _, _, cp := setup(t) + e, v, _, p, cr, _, _, cp := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "issue_comment") event := `{"action": "created"}` @@ -362,7 +362,7 @@ func TestPost_GithubCommentSuccess(t *testing.T) { func TestPost_GithubPullRequestInvalid(t *testing.T) { t.Log("when the event is a github pull request with invalid data we return a 400") - e, v, _, p, _, _, _, _, _ := setup(t) + e, v, _, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "pull_request") @@ -376,7 +376,7 @@ func TestPost_GithubPullRequestInvalid(t *testing.T) { func TestPost_GitlabMergeRequestInvalid(t *testing.T) { t.Log("when the event is a gitlab merge request with invalid data we return a 400") - e, _, gl, p, _, _, _, _, _ := setup(t) + e, _, gl, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeEvent{}, nil) @@ -390,7 +390,7 @@ func TestPost_GitlabMergeRequestInvalid(t *testing.T) { func TestPost_GithubPullRequestNotAllowlisted(t *testing.T) { t.Log("when the event is a github pull request to a non-allowlisted repo we return a 400") - e, v, _, _, _, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) var err error e.RepoAllowlistChecker, err = events.NewRepoAllowlistChecker("github.com/nevermatch") Ok(t, err) @@ -406,7 +406,7 @@ func TestPost_GithubPullRequestNotAllowlisted(t *testing.T) { func TestPost_GitlabMergeRequestNotAllowlisted(t *testing.T) { t.Log("when the event is a gitlab merge request to a non-allowlisted repo we return a 400") - e, _, gl, p, _, _, _, _, _ := setup(t) + e, _, gl, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") @@ -425,7 +425,7 @@ func TestPost_GitlabMergeRequestNotAllowlisted(t *testing.T) { func TestPost_GithubPullRequestUnsupportedAction(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") - e, v, _, _, _, _, _, _, _ := setup(t) + e, v, _, _, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "pull_request") @@ -440,7 +440,7 @@ func TestPost_GithubPullRequestUnsupportedAction(t *testing.T) { func TestPost_GitlabMergeRequestUnsupportedAction(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a gitlab merge request to a non-allowlisted repo we return a 400") - e, _, gl, p, _, _, _, _, _ := setup(t) + e, _, gl, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") var event gitlab.MergeEvent @@ -537,7 +537,7 @@ func TestPost_GithubPullRequestClosedErrCleaningPull(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a closed pull request and we have an error calling CleanUpPull we return a 503") RegisterMockTestingT(t) - e, v, _, p, _, _, c, _, _ := setup(t) + e, v, _, p, _, c, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "pull_request") @@ -555,7 +555,7 @@ func TestPost_GithubPullRequestClosedErrCleaningPull(t *testing.T) { func TestPost_GitlabMergeRequestClosedErrCleaningPull(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a closed gitlab merge request and an error occurs calling CleanUpPull we return a 500") - e, _, gl, p, _, _, c, _, _ := setup(t) + e, _, gl, p, _, c, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") var event gitlab.MergeEvent @@ -573,7 +573,7 @@ func TestPost_GitlabMergeRequestClosedErrCleaningPull(t *testing.T) { func TestPost_GithubClosedPullRequestSuccess(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a pull request and everything works we return a 200") - e, v, _, p, _, _, c, _, _ := setup(t) + e, v, _, p, _, c, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(githubHeader, "pull_request") @@ -591,7 +591,7 @@ func TestPost_GithubClosedPullRequestSuccess(t *testing.T) { func TestPost_GitlabMergeRequestSuccess(t *testing.T) { t.Skip("relies too much on mocks, should use real event parser") t.Log("when the event is a gitlab merge request and the cleanup works we return a 200") - e, _, gl, p, _, _, _, _, _ := setup(t) + e, _, gl, p, _, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) req.Header.Set(gitlabHeader, "value") When(gl.ParseAndValidate(req, secret)).ThenReturn(gitlab.MergeEvent{}, nil) @@ -710,7 +710,7 @@ func TestPost_PullOpenedOrUpdated(t *testing.T) { for _, c := range cases { t.Run(c.Description, func(t *testing.T) { - e, v, gl, p, cr, wh, _, _, _ := setup(t) + e, v, gl, p, cr, _, _, _ := setup(t) req, _ := http.NewRequest("GET", "", bytes.NewBuffer(nil)) var pullRequest models.PullRequest var repo models.Repo @@ -736,39 +736,36 @@ func TestPost_PullOpenedOrUpdated(t *testing.T) { w := httptest.NewRecorder() e.Post(w, req) responseContains(t, w, http.StatusOK, "Processing...") - wh.VerifyWasCalledOnce().RunPreHooks(models.Repo{}, models.Repo{}, models.PullRequest{State: models.ClosedPullState}, models.User{}) cr.VerifyWasCalledOnce().RunAutoplanCommand(models.Repo{}, models.Repo{}, models.PullRequest{State: models.ClosedPullState}, models.User{}) }) } } -func setup(t *testing.T) (server.EventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPreWorkflowHooksCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing) { +func setup(t *testing.T) (server.EventsController, *mocks.MockGithubRequestValidator, *mocks.MockGitlabRequestParserValidator, *emocks.MockEventParsing, *emocks.MockCommandRunner, *emocks.MockPullCleaner, *vcsmocks.MockClient, *emocks.MockCommentParsing) { RegisterMockTestingT(t) v := mocks.NewMockGithubRequestValidator() gl := mocks.NewMockGitlabRequestParserValidator() p := emocks.NewMockEventParsing() cp := emocks.NewMockCommentParsing() - wh := emocks.NewMockPreWorkflowHooksCommandRunner() cr := emocks.NewMockCommandRunner() c := emocks.NewMockPullCleaner() vcsmock := vcsmocks.NewMockClient() repoAllowlistChecker, err := events.NewRepoAllowlistChecker("*") Ok(t, err) e := server.EventsController{ - TestingMode: true, - Logger: logging.NewNoopLogger(), - GithubRequestValidator: v, - Parser: p, - CommentParser: cp, - CommandRunner: cr, - PreWorkflowHooksCommandRunner: wh, - PullCleaner: c, - GithubWebhookSecret: secret, - SupportedVCSHosts: []models.VCSHostType{models.Github, models.Gitlab}, - GitlabWebhookSecret: secret, - GitlabRequestParserValidator: gl, - RepoAllowlistChecker: repoAllowlistChecker, - VCSClient: vcsmock, + TestingMode: true, + Logger: logging.NewNoopLogger(), + GithubRequestValidator: v, + Parser: p, + CommentParser: cp, + CommandRunner: cr, + PullCleaner: c, + GithubWebhookSecret: secret, + SupportedVCSHosts: []models.VCSHostType{models.Github, models.Gitlab}, + GitlabWebhookSecret: secret, + GitlabRequestParserValidator: gl, + RepoAllowlistChecker: repoAllowlistChecker, + VCSClient: vcsmock, } - return e, v, gl, p, cr, wh, c, vcsmock, cp + return e, v, gl, p, cr, c, vcsmock, cp } diff --git a/server/server.go b/server/server.go index 7535b767b4..a0eb0e694c 100644 --- a/server/server.go +++ b/server/server.go @@ -397,11 +397,9 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { preWorkflowHooksCommandRunner := &events.DefaultPreWorkflowHooksCommandRunner{ VCSClient: vcsClient, GlobalCfg: globalCfg, - Logger: logger, WorkingDirLocker: workingDirLocker, WorkingDir: workingDir, - Drainer: drainer, - PreWorkflowHookRunner: &runtime.PreWorkflowHookRunner{}, + PreWorkflowHookRunner: runtime.DefaultPreWorkflowHookRunner{}, } projectCommandBuilder := events.NewProjectCommandBuilder( policyChecksEnabled, @@ -535,19 +533,20 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { } commandRunner := &events.DefaultCommandRunner{ - VCSClient: vcsClient, - GithubPullGetter: githubClient, - GitlabMergeRequestGetter: gitlabClient, - AzureDevopsPullGetter: azuredevopsClient, - CommentCommandRunnerByCmd: commentCommandRunnerByCmd, - EventParser: eventParser, - Logger: logger, - AllowForkPRs: userConfig.AllowForkPRs, - AllowForkPRsFlag: config.AllowForkPRsFlag, - SilenceForkPRErrors: userConfig.SilenceForkPRErrors, - SilenceForkPRErrorsFlag: config.SilenceForkPRErrorsFlag, - DisableAutoplan: userConfig.DisableAutoplan, - Drainer: drainer, + VCSClient: vcsClient, + GithubPullGetter: githubClient, + GitlabMergeRequestGetter: gitlabClient, + AzureDevopsPullGetter: azuredevopsClient, + CommentCommandRunnerByCmd: commentCommandRunnerByCmd, + EventParser: eventParser, + Logger: logger, + AllowForkPRs: userConfig.AllowForkPRs, + AllowForkPRsFlag: config.AllowForkPRsFlag, + SilenceForkPRErrors: userConfig.SilenceForkPRErrors, + SilenceForkPRErrorsFlag: config.SilenceForkPRErrorsFlag, + DisableAutoplan: userConfig.DisableAutoplan, + Drainer: drainer, + PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, } repoAllowlist, err := events.NewRepoAllowlistChecker(userConfig.RepoAllowlist) if err != nil { @@ -566,7 +565,6 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { DeleteLockCommand: deleteLockCommand, } eventsController := &EventsController{ - PreWorkflowHooksCommandRunner: preWorkflowHooksCommandRunner, CommandRunner: commandRunner, PullCleaner: pullClosedExecutor, Parser: eventParser, diff --git a/yarn.lock b/yarn.lock index 242e769ef7..f3d1dd1eac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4582,8 +4582,8 @@ pretty-time@^1.0.0: nanoseconds "^1.0.0" prismjs@^1.13.0: - version "1.21.0" - resolved "https://registry.yarnpkg.com/prismjs/-/prismjs-1.21.0.tgz#36c086ec36b45319ec4218ee164c110f9fc015a3" + version "1.23.0" + resolved "https://registry.yarnpkg.com/prismjs/-/prismjs-1.23.0.tgz#d3b3967f7d72440690497652a9d40ff046067f33" optionalDependencies: clipboard "^2.0.0"