From e001353ae97a40e71092a8fd41a917c01ec3c584 Mon Sep 17 00:00:00 2001 From: Brenden Matthews Date: Mon, 10 Dec 2018 19:26:27 -0500 Subject: [PATCH] Add `mergeable` requirement. Introduce new `mergeable` requirement, in similar vein to the `approved` requirement. Ran `make go-generate` to update mocks accordingly. This addresses issue #43. --- cmd/server.go | 6 + cmd/server_test.go | 13 +++ e2e/main.go | 1 - runatlantis.io/docs/apply-requirements.md | 16 +++ .../docs/atlantis-yaml-reference.md | 70 ++++++------ server/events/project_command_builder_test.go | 52 +++++++++ server/events/project_command_runner.go | 49 +++++--- server/events/project_command_runner_test.go | 102 +++++++++++++---- .../mocks/mock_pull_mergeable_checker.go | 106 ++++++++++++++++++ .../events/runtime/pull_mergeable_checker.go | 11 ++ server/events/vcs/bitbucketcloud/client.go | 21 ++++ server/events/vcs/bitbucketcloud/models.go | 5 + server/events/vcs/bitbucketserver/client.go | 24 ++++ server/events/vcs/bitbucketserver/models.go | 5 + server/events/vcs/client.go | 1 + server/events/vcs/github_client.go | 9 ++ server/events/vcs/gitlab_client.go | 12 ++ server/events/vcs/mocks/mock_client.go | 50 +++++++++ server/events/vcs/mocks/mock_proxy.go | 50 +++++++++ .../events/vcs/not_configured_vcs_client.go | 3 + server/events/vcs/proxy.go | 5 + server/events/yaml/parser_validator_test.go | 68 +++++++++++ server/events/yaml/raw/project.go | 9 +- server/events/yaml/raw/project_test.go | 22 +++- server/server.go | 17 ++- 25 files changed, 640 insertions(+), 87 deletions(-) create mode 100644 server/events/runtime/mocks/mock_pull_mergeable_checker.go create mode 100644 server/events/runtime/pull_mergeable_checker.go diff --git a/cmd/server.go b/cmd/server.go index 36bd28e504..06e1390c44 100644 --- a/cmd/server.go +++ b/cmd/server.go @@ -55,6 +55,7 @@ const ( PortFlag = "port" RepoWhitelistFlag = "repo-whitelist" RequireApprovalFlag = "require-approval" + RequireMergeableFlag = "require-mergeable" SilenceWhitelistErrorsFlag = "silence-whitelist-errors" SSLCertFileFlag = "ssl-cert-file" SSLKeyFileFlag = "ssl-key-file" @@ -186,6 +187,11 @@ var boolFlags = []boolFlag{ description: "Require pull requests to be \"Approved\" before allowing the apply command to be run.", defaultValue: false, }, + { + name: RequireMergeableFlag, + description: "Require pull requests to be mergeable before allowing the apply command to be run.", + defaultValue: false, + }, { name: SilenceWhitelistErrorsFlag, description: "Silences the posting of whitelist error comments.", diff --git a/cmd/server_test.go b/cmd/server_test.go index 428ea34591..84c8528b49 100644 --- a/cmd/server_test.go +++ b/cmd/server_test.go @@ -346,6 +346,7 @@ func TestExecute_Defaults(t *testing.T) { Equals(t, "info", passedConfig.LogLevel) Equals(t, 4141, passedConfig.Port) Equals(t, false, passedConfig.RequireApproval) + Equals(t, false, passedConfig.RequireMergeable) Equals(t, "", passedConfig.SSLCertFile) Equals(t, "", passedConfig.SSLKeyFile) } @@ -443,6 +444,7 @@ func TestExecute_Flags(t *testing.T) { cmd.PortFlag: 8181, cmd.RepoWhitelistFlag: "github.com/runatlantis/atlantis", cmd.RequireApprovalFlag: true, + cmd.RequireMergeableFlag: true, cmd.SSLCertFileFlag: "cert-file", cmd.SSLKeyFileFlag: "key-file", }) @@ -469,6 +471,7 @@ func TestExecute_Flags(t *testing.T) { Equals(t, 8181, passedConfig.Port) Equals(t, "github.com/runatlantis/atlantis", passedConfig.RepoWhitelist) Equals(t, true, passedConfig.RequireApproval) + Equals(t, true, passedConfig.RequireMergeable) Equals(t, "cert-file", passedConfig.SSLCertFile) Equals(t, "key-file", passedConfig.SSLKeyFile) } @@ -496,6 +499,7 @@ log-level: "debug" port: 8181 repo-whitelist: "github.com/runatlantis/atlantis" require-approval: true +require-mergeable: true ssl-cert-file: cert-file ssl-key-file: key-file `) @@ -526,6 +530,7 @@ ssl-key-file: key-file Equals(t, 8181, passedConfig.Port) Equals(t, "github.com/runatlantis/atlantis", passedConfig.RepoWhitelist) Equals(t, true, passedConfig.RequireApproval) + Equals(t, true, passedConfig.RequireMergeable) Equals(t, "cert-file", passedConfig.SSLCertFile) Equals(t, "key-file", passedConfig.SSLKeyFile) } @@ -580,6 +585,7 @@ ssl-key-file: key-file "PORT": "8282", "REPO_WHITELIST": "override,override", "REQUIRE_APPROVAL": "false", + "REQUIRE_MERGEABLE": "false", "SSL_CERT_FILE": "override-cert-file", "SSL_KEY_FILE": "override-key-file", } { @@ -610,6 +616,7 @@ ssl-key-file: key-file Equals(t, 8282, passedConfig.Port) Equals(t, "override,override", passedConfig.RepoWhitelist) Equals(t, false, passedConfig.RequireApproval) + Equals(t, false, passedConfig.RequireMergeable) Equals(t, "override-cert-file", passedConfig.SSLCertFile) Equals(t, "override-key-file", passedConfig.SSLKeyFile) } @@ -637,6 +644,7 @@ log-level: "debug" port: 8181 repo-whitelist: "github.com/runatlantis/atlantis" require-approval: true +require-mergeable: true ssl-cert-file: cert-file ssl-key-file: key-file `) @@ -663,6 +671,7 @@ ssl-key-file: key-file cmd.PortFlag: 8282, cmd.RepoWhitelistFlag: "override,override", cmd.RequireApprovalFlag: false, + cmd.RequireMergeableFlag: false, cmd.SSLCertFileFlag: "override-cert-file", cmd.SSLKeyFileFlag: "override-key-file", }) @@ -687,6 +696,7 @@ ssl-key-file: key-file Equals(t, 8282, passedConfig.Port) Equals(t, "override,override", passedConfig.RepoWhitelist) Equals(t, false, passedConfig.RequireApproval) + Equals(t, false, passedConfig.RequireMergeable) Equals(t, "override-cert-file", passedConfig.SSLCertFile) Equals(t, "override-key-file", passedConfig.SSLKeyFile) } @@ -715,6 +725,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { "PORT": "8181", "REPO_WHITELIST": "*", "REQUIRE_APPROVAL": "true", + "REQUIRE_MERGEABLE": "true", "SSL_CERT_FILE": "cert-file", "SSL_KEY_FILE": "key-file", } @@ -749,6 +760,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { cmd.PortFlag: 8282, cmd.RepoWhitelistFlag: "override,override", cmd.RequireApprovalFlag: false, + cmd.RequireMergeableFlag: false, cmd.SSLCertFileFlag: "override-cert-file", cmd.SSLKeyFileFlag: "override-key-file", }) @@ -775,6 +787,7 @@ func TestExecute_FlagEnvVarOverride(t *testing.T) { Equals(t, 8282, passedConfig.Port) Equals(t, "override,override", passedConfig.RepoWhitelist) Equals(t, false, passedConfig.RequireApproval) + Equals(t, false, passedConfig.RequireMergeable) Equals(t, "override-cert-file", passedConfig.SSLCertFile) Equals(t, "override-key-file", passedConfig.SSLKeyFile) } diff --git a/e2e/main.go b/e2e/main.go index 73df2f19bc..01c6f19130 100644 --- a/e2e/main.go +++ b/e2e/main.go @@ -118,7 +118,6 @@ func main() { func createAtlantisWebhook(g *GithubClient, ownerName string, repoName string, hookURL string) (int, error) { // create atlantis hook atlantisHook := &github.Hook{ - Name: github.String("web"), Events: []string{"issue_comment", "pull_request", "push"}, Config: map[string]interface{}{ "url": hookURL, diff --git a/runatlantis.io/docs/apply-requirements.md b/runatlantis.io/docs/apply-requirements.md index 43accb43fb..52f5d7691a 100644 --- a/runatlantis.io/docs/apply-requirements.md +++ b/runatlantis.io/docs/apply-requirements.md @@ -4,6 +4,7 @@ If you'd like to require pull/merge requests to be approved prior to a user running `atlantis apply` simply run Atlantis with the `--require-approval` flag. By default, no approval is required. If you want to configure this on a per-repo/project basis, for example to only require approvals for your production configuration you must use an `atlantis.yaml` file: + ```yaml version: 2 projects: @@ -23,6 +24,21 @@ Atlantis does not count that as an approval and requires at least one user that is not the author of the pull request to approve it. ::: +## Mergeable + +You may also set the `mergeable` requirement for pull/merge requests. A PR/MR must be marked as `mergeable` before a user is able to run `atlantis apply`. This is helpful in prevent certain edge cases, such as applying stale code in an outdated PR which has passed all other checks. You can enable this feature by passing the `--require-mergeable` flag to Atlantis, or by using an `atlantis.yaml` file like this: + +```yaml +version: 2 +projects: +- dir: . + apply_requirements: [mergeable, approved] +``` + +::: tip Note +The meaning of "mergeable" may be slightly different depending on your Git provider. In the case of GitHub and GitLab, it means that a PR/MR has no conflicts and satisfies all required approvals and checks. On BitBucket, it means that merging is possible and there are no conflicts. +::: + ## Next Steps * For more information on GitHub pull request reviews and approvals see: [https://help.github.com/articles/about-pull-request-reviews/](https://help.github.com/articles/about-pull-request-reviews/) * For more information on GitLab merge request reviews and approvals (only supported on GitLab Enterprise) see: [https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html). diff --git a/runatlantis.io/docs/atlantis-yaml-reference.md b/runatlantis.io/docs/atlantis-yaml-reference.md index 3181344358..1a6a8a8075 100644 --- a/runatlantis.io/docs/atlantis-yaml-reference.md +++ b/runatlantis.io/docs/atlantis-yaml-reference.md @@ -24,7 +24,7 @@ projects: autoplan: when_modified: ["*.tf", "../modules/**.tf"] enabled: true - apply_requirements: [approved] + apply_requirements: [mergeable, approved] workflow: myworkflow workflows: myworkflow: @@ -70,11 +70,11 @@ version: projects: workflows: ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| version | int | none | yes | This key is required and must be set to `2`| -| projects | array[[Project](atlantis-yaml-reference.html#project)] | [] | no | Lists the projects in this repo | -| workflows | map[string -> [Workflow](atlantis-yaml-reference.html#workflow)] | {} | no | Custom workflows | +| Key | Type | Default | Required | Description | +| --------- | ---------------------------------------------------------------- | ------- | -------- | ------------------------------------------- | +| version | int | none | yes | This key is required and must be set to `2` | +| projects | array[[Project](atlantis-yaml-reference.html#project)] | [] | no | Lists the projects in this repo | +| workflows | map[string -> [Workflow](atlantis-yaml-reference.html#workflow)] | {} | no | Custom workflows | ### Project ```yaml @@ -87,15 +87,15 @@ apply_requirements: ["approved"] workflow: myworkflow ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| name | string | none | maybe | Required if there is more than one project with the same `dir` and `workspace`. This project name can be used with the `-p` flag.| -| dir | string | none | yes | The directory of this project relative to the repo root. Use `.` for the root. For example if the project was under `./project1` then use `project1`| -| workspace | string| default | no | The [Terraform workspace](https://www.terraform.io/docs/state/workspaces.html) for this project. Atlantis will switch to this workplace when planning/applying and will create it if it doesn't exist.| -| autoplan | [Autoplan](atlantis-yaml-reference.html#autoplan) | none | no | A custom autoplan configuration. If not specified, will use the default algorithm. See [Autoplanning](autoplanning.html).| -| terraform_version | string | none | no | A specific Terraform version to use when running commands for this project. Requires there to be a binary in the Atlantis `PATH` with the name `terraform{VERSION}`, ex. `terraform0.11.0`| -| apply_requirements | array[string] | [] | no | Requirements that must be satisfied before `atlantis apply` can be run. Currently the only supported requirement is `approved`. See [Apply Requirements](apply-requirements.html#approved) for more details.| -| workflow | string | none | no | A custom workflow. If not specified, Atlantis will use its default workflow.| +| Key | Type | Default | Required | Description | +| ------------------ | ------------------------------------------------- | ------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| name | string | none | maybe | Required if there is more than one project with the same `dir` and `workspace`. This project name can be used with the `-p` flag. | +| dir | string | none | yes | The directory of this project relative to the repo root. Use `.` for the root. For example if the project was under `./project1` then use `project1` | +| workspace | string | default | no | The [Terraform workspace](https://www.terraform.io/docs/state/workspaces.html) for this project. Atlantis will switch to this workplace when planning/applying and will create it if it doesn't exist. | +| autoplan | [Autoplan](atlantis-yaml-reference.html#autoplan) | none | no | A custom autoplan configuration. If not specified, will use the default algorithm. See [Autoplanning](autoplanning.html). | +| terraform_version | string | none | no | A specific Terraform version to use when running commands for this project. Requires there to be a binary in the Atlantis `PATH` with the name `terraform{VERSION}`, ex. `terraform0.11.0` | +| apply_requirements | array[string] | [] | no | Requirements that must be satisfied before `atlantis apply` can be run. Currently the only supported requirements are `approved` and `mergeable`. See [Apply Requirements](apply-requirements.html) for more details. | +| workflow | string | none | no | A custom workflow. If not specified, Atlantis will use its default workflow. | ::: tip A project represents a Terraform state. Typically, there is one state per directory and workspace however it's possible to @@ -108,10 +108,10 @@ Atlantis supports this but requires the `name` key to be specified. See [atlanti enabled: true when_modified: ["*.tf"] ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| enabled | boolean | true | no | Whether autoplanning is enabled for this project. | -| when_modified | array[string] | no | no | Uses [.dockerignore](https://docs.docker.com/engine/reference/builder/#dockerignore-file) syntax. If any modified file in the pull request matches, this project will be planned. If not specified, Atlantis will use its own algorithm. See [Autoplanning](autoplanning.html). Paths are relative to the project's dir.| +| Key | Type | Default | Required | Description | +| ------------- | ------------- | ------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| enabled | boolean | true | no | Whether autoplanning is enabled for this project. | +| when_modified | array[string] | no | no | Uses [.dockerignore](https://docs.docker.com/engine/reference/builder/#dockerignore-file) syntax. If any modified file in the pull request matches, this project will be planned. If not specified, Atlantis will use its own algorithm. See [Autoplanning](autoplanning.html). Paths are relative to the project's dir. | ### Workflow ```yaml @@ -119,10 +119,10 @@ plan: apply: ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| plan | [Stage](atlantis-yaml-reference.html#stage) | `steps: [init, plan]` | no | How to plan for this project. | -| apply | [Stage](atlantis-yaml-reference.html#stage) | `steps: [apply]` | no | How to apply for this project. | +| Key | Type | Default | Required | Description | +| ----- | ------------------------------------------- | --------------------- | -------- | ------------------------------ | +| plan | [Stage](atlantis-yaml-reference.html#stage) | `steps: [init, plan]` | no | How to plan for this project. | +| apply | [Stage](atlantis-yaml-reference.html#stage) | `steps: [apply]` | no | How to apply for this project. | ### Stage ```yaml @@ -133,9 +133,9 @@ steps: extra_args: [-lock=false] ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| steps | array[[Step](atlantis-yaml-reference.html#step)] | `[]` | no | List of steps for this stage. If the steps key is empty, no steps will be run for this stage. | +| Key | Type | Default | Required | Description | +| ----- | ------------------------------------------------ | ------- | -------- | --------------------------------------------------------------------------------------------- | +| steps | array[[Step](atlantis-yaml-reference.html#step)] | `[]` | no | List of steps for this stage. If the steps key is empty, no steps will be run for this stage. | ### Step #### Built-In Commands: init, plan, apply @@ -145,9 +145,9 @@ Steps can be a single string for a built-in command. - plan - apply ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| init/plan/apply | string | none | no | Use a built-in command without additional configuration. Only `init`, `plan` and `apply` are supported|| +| Key | Type | Default | Required | Description | +| --------------- | ------ | ------- | -------- | ------------------------------------------------------------------------------------------------------ | +| init/plan/apply | string | none | no | Use a built-in command without additional configuration. Only `init`, `plan` and `apply` are supported | #### Built-In Command With Extra Args A map from string to `extra_args` for a built-in command with extra arguments. @@ -159,17 +159,17 @@ A map from string to `extra_args` for a built-in command with extra arguments. - apply: extra_args: [arg1, arg2] ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| init/plan/apply | map[`extra_args` -> array[string]] | none | no | Use a built-in command and append `extra_args`. Only `init`, `plan` and `apply` are supported as keys and only `extra_args` is supported as a value|| +| Key | Type | Default | Required | Description | +| --------------- | ---------------------------------- | ------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | +| init/plan/apply | map[`extra_args` -> array[string]] | none | no | Use a built-in command and append `extra_args`. Only `init`, `plan` and `apply` are supported as keys and only `extra_args` is supported as a value | #### Custom `run` Command Or a custom command ```yaml - run: custom-command ``` -| Key | Type | Default | Required | Description | -| -------------| --- |-------------| -----|---| -| run | string| none | no | Run a custom command| +| Key | Type | Default | Required | Description | +| --- | ------ | ------- | -------- | -------------------- | +| run | string | none | no | Run a custom command | ::: tip `run` steps are executed with the following environment variables: diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go index a73b6692cb..ed276bfef5 100644 --- a/server/events/project_command_builder_test.go +++ b/server/events/project_command_builder_test.go @@ -350,6 +350,58 @@ projects: ExpWorkspace: "myworkspace", ExpDir: ".", }, + { + Description: "atlantis.yaml with mergeable apply requirement", + Cmd: events.CommentCommand{ + Name: events.PlanCommand, + ProjectName: "myproject", + }, + AtlantisYAML: ` +version: 2 +projects: +- name: myproject + dir: . + workspace: myworkspace + apply_requirements: [mergeable]`, + ExpProjectConfig: &valid.Project{ + Dir: ".", + Workspace: "myworkspace", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + ApplyRequirements: []string{"mergeable"}, + Name: String("myproject"), + }, + ExpWorkspace: "myworkspace", + ExpDir: ".", + }, + { + Description: "atlantis.yaml with mergeable and approved apply requirements", + Cmd: events.CommentCommand{ + Name: events.PlanCommand, + ProjectName: "myproject", + }, + AtlantisYAML: ` +version: 2 +projects: +- name: myproject + dir: . + workspace: myworkspace + apply_requirements: [mergeable, approved]`, + ExpProjectConfig: &valid.Project{ + Dir: ".", + Workspace: "myworkspace", + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: true, + }, + ApplyRequirements: []string{"mergeable", "approved"}, + Name: String("myproject"), + }, + ExpWorkspace: "myworkspace", + ExpDir: ".", + }, { Description: "atlantis.yaml with multiple dir/workspaces matching", Cmd: events.CommentCommand{ diff --git a/server/events/project_command_runner.go b/server/events/project_command_runner.go index 8784e87de6..cea477ddf8 100644 --- a/server/events/project_command_runner.go +++ b/server/events/project_command_runner.go @@ -78,17 +78,19 @@ type ProjectCommandRunner interface { // DefaultProjectCommandRunner implements ProjectCommandRunner. type DefaultProjectCommandRunner struct { - Locker ProjectLocker - LockURLGenerator LockURLGenerator - InitStepRunner StepRunner - PlanStepRunner StepRunner - ApplyStepRunner StepRunner - RunStepRunner StepRunner - PullApprovedChecker runtime.PullApprovedChecker - WorkingDir WorkingDir - Webhooks WebhooksSender - WorkingDirLocker WorkingDirLocker - RequireApprovalOverride bool + Locker ProjectLocker + LockURLGenerator LockURLGenerator + InitStepRunner StepRunner + PlanStepRunner StepRunner + ApplyStepRunner StepRunner + RunStepRunner StepRunner + PullApprovedChecker runtime.PullApprovedChecker + PullMergeableChecker runtime.PullMergeableChecker + WorkingDir WorkingDir + Webhooks WebhooksSender + WorkingDirLocker WorkingDirLocker + RequireApprovalOverride bool + RequireMergeableOverride bool } // Plan runs terraform plan for the project described by ctx. @@ -207,15 +209,20 @@ func (p *DefaultProjectCommandRunner) doApply(ctx models.ProjectCommandContext) } absPath := filepath.Join(repoDir, ctx.RepoRelDir) + // Figure out what our apply requirements are. var applyRequirements []string - if ctx.ProjectConfig != nil { + if p.RequireApprovalOverride || p.RequireMergeableOverride { + // If any server flags are set, they override project config. + if p.RequireMergeableOverride { + applyRequirements = append(applyRequirements, raw.MergeableApplyRequirement) + } + if p.RequireApprovalOverride { + applyRequirements = append(applyRequirements, raw.ApprovedApplyRequirement) + } + } else if ctx.ProjectConfig != nil { + // Else we use the project config if it's set. applyRequirements = ctx.ProjectConfig.ApplyRequirements } - // todo: this class shouldn't know about the server-side approval requirement. - // Instead the project_command_builder should figure this out and store this information in the ctx. # refactor - if p.RequireApprovalOverride { - applyRequirements = []string{raw.ApprovedApplyRequirement} - } for _, req := range applyRequirements { switch req { case raw.ApprovedApplyRequirement: @@ -226,6 +233,14 @@ func (p *DefaultProjectCommandRunner) doApply(ctx models.ProjectCommandContext) if !approved { return "", "Pull request must be approved before running apply.", nil } + case raw.MergeableApplyRequirement: + mergeable, err := p.PullMergeableChecker.PullIsMergeable(ctx.BaseRepo, ctx.Pull) // nolint: vetshadow + if err != nil { + return "", "", errors.Wrap(err, "checking if pull request is mergeable") + } + if !mergeable { + return "", "Pull request must be mergeable before running apply.", nil + } } } // Acquire internal lock for the directory we're going to operate in. diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go index d2d02bfdf6..83ee4b27df 100644 --- a/server/events/project_command_runner_test.go +++ b/server/events/project_command_runner_test.go @@ -134,16 +134,17 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) { mockLocker := mocks.NewMockProjectLocker() runner := events.DefaultProjectCommandRunner{ - Locker: mockLocker, - LockURLGenerator: mockURLGenerator{}, - InitStepRunner: mockInit, - PlanStepRunner: mockPlan, - ApplyStepRunner: mockApply, - RunStepRunner: mockRun, - PullApprovedChecker: nil, - WorkingDir: mockWorkingDir, - Webhooks: nil, - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + Locker: mockLocker, + LockURLGenerator: mockURLGenerator{}, + InitStepRunner: mockInit, + PlanStepRunner: mockPlan, + ApplyStepRunner: mockApply, + RunStepRunner: mockRun, + PullApprovedChecker: nil, + PullMergeableChecker: nil, + WorkingDir: mockWorkingDir, + Webhooks: nil, + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), } repoDir := "/tmp/mydir" @@ -229,6 +230,24 @@ func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) { Equals(t, "Pull request must be approved before running apply.", res.Failure) } +func TestDefaultProjectCommandRunner_ApplyNotMergeable(t *testing.T) { + RegisterMockTestingT(t) + mockWorkingDir := mocks.NewMockWorkingDir() + mockMergeable := mocks2.NewMockPullMergeableChecker() + runner := &events.DefaultProjectCommandRunner{ + WorkingDir: mockWorkingDir, + PullMergeableChecker: mockMergeable, + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + RequireMergeableOverride: true, + } + ctx := models.ProjectCommandContext{} + When(mockWorkingDir.GetWorkingDir(ctx.BaseRepo, ctx.Pull, ctx.Workspace)).ThenReturn("/tmp/mydir", nil) + When(mockMergeable.PullIsMergeable(ctx.BaseRepo, ctx.Pull)).ThenReturn(false, nil) + + res := runner.Apply(ctx) + Equals(t, "Pull request must be mergeable before running apply.", res.Failure) +} + func TestDefaultProjectCommandRunner_Apply(t *testing.T) { cases := []struct { description string @@ -275,7 +294,43 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { }, }, }, - expSteps: []string{"approved", "apply"}, + expSteps: []string{"approve", "apply"}, + expOut: "apply", + }, + { + description: "no workflow, mergeable required, use defaults", + projCfg: &valid.Project{ + Dir: ".", + ApplyRequirements: []string{"mergeable"}, + }, + globalCfg: &valid.Config{ + Version: 2, + Projects: []valid.Project{ + { + Dir: ".", + ApplyRequirements: []string{"mergeable"}, + }, + }, + }, + expSteps: []string{"mergeable", "apply"}, + expOut: "apply", + }, + { + description: "no workflow, mergeable and approved required, use defaults", + projCfg: &valid.Project{ + Dir: ".", + ApplyRequirements: []string{"mergeable", "approved"}, + }, + globalCfg: &valid.Config{ + Version: 2, + Projects: []valid.Project{ + { + Dir: ".", + ApplyRequirements: []string{"mergeable", "approved"}, + }, + }, + }, + expSteps: []string{"mergeable", "approved", "apply"}, expOut: "apply", }, { @@ -349,21 +404,23 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { mockApply := mocks.NewMockStepRunner() mockRun := mocks.NewMockStepRunner() mockApproved := mocks2.NewMockPullApprovedChecker() + mockMergeable := mocks2.NewMockPullMergeableChecker() mockWorkingDir := mocks.NewMockWorkingDir() mockLocker := mocks.NewMockProjectLocker() mockSender := mocks.NewMockWebhooksSender() runner := events.DefaultProjectCommandRunner{ - Locker: mockLocker, - LockURLGenerator: mockURLGenerator{}, - InitStepRunner: mockInit, - PlanStepRunner: mockPlan, - ApplyStepRunner: mockApply, - RunStepRunner: mockRun, - PullApprovedChecker: mockApproved, - WorkingDir: mockWorkingDir, - Webhooks: mockSender, - WorkingDirLocker: events.NewDefaultWorkingDirLocker(), + Locker: mockLocker, + LockURLGenerator: mockURLGenerator{}, + InitStepRunner: mockInit, + PlanStepRunner: mockPlan, + ApplyStepRunner: mockApply, + RunStepRunner: mockRun, + PullApprovedChecker: mockApproved, + PullMergeableChecker: mockMergeable, + WorkingDir: mockWorkingDir, + Webhooks: mockSender, + WorkingDirLocker: events.NewDefaultWorkingDirLocker(), } repoDir := "/tmp/mydir" @@ -385,6 +442,7 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { When(mockApply.Run(ctx, nil, repoDir)).ThenReturn("apply", nil) When(mockRun.Run(ctx, nil, repoDir)).ThenReturn("run", nil) When(mockApproved.PullIsApproved(ctx.BaseRepo, ctx.Pull)).ThenReturn(true, nil) + When(mockMergeable.PullIsMergeable(ctx.BaseRepo, ctx.Pull)).ThenReturn(true, nil) res := runner.Apply(ctx) Equals(t, c.expOut, res.ApplySuccess) @@ -393,6 +451,8 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) { switch step { case "approved": mockApproved.VerifyWasCalledOnce().PullIsApproved(ctx.BaseRepo, ctx.Pull) + case "mergeable": + mockMergeable.VerifyWasCalledOnce().PullIsMergeable(ctx.BaseRepo, ctx.Pull) case "init": mockInit.VerifyWasCalledOnce().Run(ctx, nil, repoDir) case "plan": diff --git a/server/events/runtime/mocks/mock_pull_mergeable_checker.go b/server/events/runtime/mocks/mock_pull_mergeable_checker.go new file mode 100644 index 0000000000..3ef3eb93fe --- /dev/null +++ b/server/events/runtime/mocks/mock_pull_mergeable_checker.go @@ -0,0 +1,106 @@ +// Code generated by pegomock. DO NOT EDIT. +// Source: github.com/runatlantis/atlantis/server/events/runtime (interfaces: PullMergeableChecker) + +package mocks + +import ( + pegomock "github.com/petergtz/pegomock" + models "github.com/runatlantis/atlantis/server/events/models" + "reflect" + "time" +) + +type MockPullMergeableChecker struct { + fail func(message string, callerSkip ...int) +} + +func NewMockPullMergeableChecker() *MockPullMergeableChecker { + return &MockPullMergeableChecker{fail: pegomock.GlobalFailHandler} +} + +func (mock *MockPullMergeableChecker) PullIsMergeable(baseRepo models.Repo, pull models.PullRequest) (bool, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockPullMergeableChecker().") + } + params := []pegomock.Param{baseRepo, pull} + result := pegomock.GetGenericMockFrom(mock).Invoke("PullIsMergeable", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 bool + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(bool) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + +func (mock *MockPullMergeableChecker) VerifyWasCalledOnce() *VerifierPullMergeableChecker { + return &VerifierPullMergeableChecker{ + mock: mock, + invocationCountMatcher: pegomock.Times(1), + } +} + +func (mock *MockPullMergeableChecker) VerifyWasCalled(invocationCountMatcher pegomock.Matcher) *VerifierPullMergeableChecker { + return &VerifierPullMergeableChecker{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + } +} + +func (mock *MockPullMergeableChecker) VerifyWasCalledInOrder(invocationCountMatcher pegomock.Matcher, inOrderContext *pegomock.InOrderContext) *VerifierPullMergeableChecker { + return &VerifierPullMergeableChecker{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + inOrderContext: inOrderContext, + } +} + +func (mock *MockPullMergeableChecker) VerifyWasCalledEventually(invocationCountMatcher pegomock.Matcher, timeout time.Duration) *VerifierPullMergeableChecker { + return &VerifierPullMergeableChecker{ + mock: mock, + invocationCountMatcher: invocationCountMatcher, + timeout: timeout, + } +} + +type VerifierPullMergeableChecker struct { + mock *MockPullMergeableChecker + invocationCountMatcher pegomock.Matcher + inOrderContext *pegomock.InOrderContext + timeout time.Duration +} + +func (verifier *VerifierPullMergeableChecker) PullIsMergeable(baseRepo models.Repo, pull models.PullRequest) *PullMergeableChecker_PullIsMergeable_OngoingVerification { + params := []pegomock.Param{baseRepo, pull} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PullIsMergeable", params, verifier.timeout) + return &PullMergeableChecker_PullIsMergeable_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type PullMergeableChecker_PullIsMergeable_OngoingVerification struct { + mock *MockPullMergeableChecker + methodInvocations []pegomock.MethodInvocation +} + +func (c *PullMergeableChecker_PullIsMergeable_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { + baseRepo, pull := c.GetAllCapturedArguments() + return baseRepo[len(baseRepo)-1], pull[len(pull)-1] +} + +func (c *PullMergeableChecker_PullIsMergeable_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(params[0])) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.PullRequest, len(params[1])) + for u, param := range params[1] { + _param1[u] = param.(models.PullRequest) + } + } + return +} diff --git a/server/events/runtime/pull_mergeable_checker.go b/server/events/runtime/pull_mergeable_checker.go new file mode 100644 index 0000000000..51a9a267a3 --- /dev/null +++ b/server/events/runtime/pull_mergeable_checker.go @@ -0,0 +1,11 @@ +package runtime + +import ( + "github.com/runatlantis/atlantis/server/events/models" +) + +//go:generate pegomock generate -m --use-experimental-model-gen --package mocks -o mocks/mock_pull_mergeable_checker.go PullMergeableChecker + +type PullMergeableChecker interface { + PullIsMergeable(baseRepo models.Repo, pull models.PullRequest) (bool, error) +} diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index 96a3335049..4f5a7fedd6 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -124,6 +124,27 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool return false, nil } +// PullIsMergeable returns true if the merge request has no conflicts and can be merged. +func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + // NOTE: The 1.0 API is deprecated, but the 2.0 API does not provide this endpoint. + path := fmt.Sprintf("%s/1.0/repositories/%s/pullrequests/%d/conflict-status", b.BaseURL, repo.FullName, pull.Num) + resp, err := b.makeRequest("GET", path, nil) + if err != nil { + return false, err + } + var conflictStatus ConflictStatus + if err := json.Unmarshal(resp, &conflictStatus); err != nil { + return false, errors.Wrapf(err, "Could not parse response %q", string(resp)) + } + if err := validator.New().Struct(conflictStatus); err != nil { + return false, errors.Wrapf(err, "API response %q was missing fields", string(resp)) + } + if !*conflictStatus.MergeImpossible && !*conflictStatus.IsConflicted { + return true, nil + } + return false, nil +} + // UpdateStatus updates the status of a commit. func (b *Client) UpdateStatus(repo models.Repo, pull models.PullRequest, status models.CommitStatus, description string) error { bbState := "FAILED" diff --git a/server/events/vcs/bitbucketcloud/models.go b/server/events/vcs/bitbucketcloud/models.go index c40edce778..f4bf7ce75e 100644 --- a/server/events/vcs/bitbucketcloud/models.go +++ b/server/events/vcs/bitbucketcloud/models.go @@ -80,3 +80,8 @@ type Comment struct { type CommentContent struct { Raw *string `json:"raw,omitempty" validate:"required"` } + +type ConflictStatus struct { + MergeImpossible *bool `json:"mergeimpossible,omitempty" validate:"required"` + IsConflicted *bool `json:"isconflicted,omitempty" validate:"required"` +} diff --git a/server/events/vcs/bitbucketserver/client.go b/server/events/vcs/bitbucketserver/client.go index 15a7a45dc5..88d8fc73d9 100644 --- a/server/events/vcs/bitbucketserver/client.go +++ b/server/events/vcs/bitbucketserver/client.go @@ -178,6 +178,30 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool return false, nil } +// PullIsMergeable returns true if the merge request has no conflicts and can be merged. +func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + projectKey, err := b.GetProjectKey(repo.Name, repo.SanitizedCloneURL) + if err != nil { + return false, err + } + path := fmt.Sprintf("%s/rest/api/1.0/projects/%s/repos/%s/pull-requests/%d/merge", b.BaseURL, projectKey, repo.Name, pull.Num) + resp, err := b.makeRequest("GET", path, nil) + if err != nil { + return false, err + } + var mergeStatus MergeStatus + if err := json.Unmarshal(resp, &mergeStatus); err != nil { + return false, errors.Wrapf(err, "Could not parse response %q", string(resp)) + } + if err := validator.New().Struct(mergeStatus); err != nil { + return false, errors.Wrapf(err, "API response %q was missing fields", string(resp)) + } + if *mergeStatus.CanMerge && !*mergeStatus.Conflicted { + return true, nil + } + return false, nil +} + // UpdateStatus updates the status of a commit. func (b *Client) UpdateStatus(repo models.Repo, pull models.PullRequest, status models.CommitStatus, description string) error { bbState := "FAILED" diff --git a/server/events/vcs/bitbucketserver/models.go b/server/events/vcs/bitbucketserver/models.go index ba0ac0e514..5f3b42e231 100644 --- a/server/events/vcs/bitbucketserver/models.go +++ b/server/events/vcs/bitbucketserver/models.go @@ -64,3 +64,8 @@ type Changes struct { NextPageStart *int `json:"nextPageStart,omitempty"` IsLastPage *bool `json:"isLastPage,omitempty" validate:"required"` } + +type MergeStatus struct { + CanMerge *bool `json:"canMerge,omitempty" validate:"required"` + Conflicted *bool `json:"conflicted,omitempty" validate:"required"` +} diff --git a/server/events/vcs/client.go b/server/events/vcs/client.go index 583206bb95..d79bc287ed 100644 --- a/server/events/vcs/client.go +++ b/server/events/vcs/client.go @@ -24,5 +24,6 @@ type Client interface { GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) CreateComment(repo models.Repo, pullNum int, comment string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) + PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error } diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index 1e80b5d668..626c74ebc0 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -121,6 +121,15 @@ func (g *GithubClient) PullIsApproved(repo models.Repo, pull models.PullRequest) return false, nil } +// PullIsMergeable returns true if the pull request is mergeable. +func (g *GithubClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + githubPR, err := g.GetPullRequest(repo, pull.Num) + if err != nil { + return false, errors.Wrap(err, "getting pull request") + } + return githubPR.GetMergeable(), nil +} + // GetPullRequest returns the pull request. func (g *GithubClient) GetPullRequest(repo models.Repo, num int) (*github.PullRequest, error) { pull, _, err := g.client.PullRequests.Get(g.ctx, repo.Owner, repo.Name, num) diff --git a/server/events/vcs/gitlab_client.go b/server/events/vcs/gitlab_client.go index 484b7fba6f..f187410a03 100644 --- a/server/events/vcs/gitlab_client.go +++ b/server/events/vcs/gitlab_client.go @@ -143,6 +143,18 @@ func (g *GitlabClient) PullIsApproved(repo models.Repo, pull models.PullRequest) return true, nil } +// PullIsMergeable returns true if the merge request can be merged. +func (g *GitlabClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + mr, _, err := g.Client.MergeRequests.GetMergeRequest(repo.FullName, pull.Num) + if err != nil { + return false, err + } + if mr.MergeStatus == "can_be_merged" { + return true, nil + } + return false, nil +} + // UpdateStatus updates the build status of a commit. func (g *GitlabClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error { const statusContext = "Atlantis" diff --git a/server/events/vcs/mocks/mock_client.go b/server/events/vcs/mocks/mock_client.go index d2d908a624..e71311fea1 100644 --- a/server/events/vcs/mocks/mock_client.go +++ b/server/events/vcs/mocks/mock_client.go @@ -71,6 +71,25 @@ func (mock *MockClient) PullIsApproved(repo models.Repo, pull models.PullRequest return ret0, ret1 } +func (mock *MockClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClient().") + } + params := []pegomock.Param{repo, pull} + result := pegomock.GetGenericMockFrom(mock).Invoke("PullIsMergeable", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 bool + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(bool) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + func (mock *MockClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClient().") @@ -220,6 +239,37 @@ func (c *Client_PullIsApproved_OngoingVerification) GetAllCapturedArguments() (_ return } +func (verifier *VerifierClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) *Client_PullIsMergeable_OngoingVerification { + params := []pegomock.Param{repo, pull} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PullIsMergeable", params, verifier.timeout) + return &Client_PullIsMergeable_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type Client_PullIsMergeable_OngoingVerification struct { + mock *MockClient + methodInvocations []pegomock.MethodInvocation +} + +func (c *Client_PullIsMergeable_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { + repo, pull := c.GetAllCapturedArguments() + return repo[len(repo)-1], pull[len(pull)-1] +} + +func (c *Client_PullIsMergeable_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(params[0])) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.PullRequest, len(params[1])) + for u, param := range params[1] { + _param1[u] = param.(models.PullRequest) + } + } + return +} + func (verifier *VerifierClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) *Client_UpdateStatus_OngoingVerification { params := []pegomock.Param{repo, pull, state, description} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "UpdateStatus", params, verifier.timeout) diff --git a/server/events/vcs/mocks/mock_proxy.go b/server/events/vcs/mocks/mock_proxy.go index f5f6cb1e13..69c5d98511 100644 --- a/server/events/vcs/mocks/mock_proxy.go +++ b/server/events/vcs/mocks/mock_proxy.go @@ -71,6 +71,25 @@ func (mock *MockClientProxy) PullIsApproved(repo models.Repo, pull models.PullRe return ret0, ret1 } +func (mock *MockClientProxy) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + if mock == nil { + panic("mock must not be nil. Use myMock := NewMockClientProxy().") + } + params := []pegomock.Param{repo, pull} + result := pegomock.GetGenericMockFrom(mock).Invoke("PullIsMergeable", params, []reflect.Type{reflect.TypeOf((*bool)(nil)).Elem(), reflect.TypeOf((*error)(nil)).Elem()}) + var ret0 bool + var ret1 error + if len(result) != 0 { + if result[0] != nil { + ret0 = result[0].(bool) + } + if result[1] != nil { + ret1 = result[1].(error) + } + } + return ret0, ret1 +} + func (mock *MockClientProxy) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error { if mock == nil { panic("mock must not be nil. Use myMock := NewMockClientProxy().") @@ -220,6 +239,37 @@ func (c *ClientProxy_PullIsApproved_OngoingVerification) GetAllCapturedArguments return } +func (verifier *VerifierClientProxy) PullIsMergeable(repo models.Repo, pull models.PullRequest) *ClientProxy_PullIsMergeable_OngoingVerification { + params := []pegomock.Param{repo, pull} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "PullIsMergeable", params, verifier.timeout) + return &ClientProxy_PullIsMergeable_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} +} + +type ClientProxy_PullIsMergeable_OngoingVerification struct { + mock *MockClientProxy + methodInvocations []pegomock.MethodInvocation +} + +func (c *ClientProxy_PullIsMergeable_OngoingVerification) GetCapturedArguments() (models.Repo, models.PullRequest) { + repo, pull := c.GetAllCapturedArguments() + return repo[len(repo)-1], pull[len(pull)-1] +} + +func (c *ClientProxy_PullIsMergeable_OngoingVerification) GetAllCapturedArguments() (_param0 []models.Repo, _param1 []models.PullRequest) { + params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(params) > 0 { + _param0 = make([]models.Repo, len(params[0])) + for u, param := range params[0] { + _param0[u] = param.(models.Repo) + } + _param1 = make([]models.PullRequest, len(params[1])) + for u, param := range params[1] { + _param1[u] = param.(models.PullRequest) + } + } + return +} + func (verifier *VerifierClientProxy) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) *ClientProxy_UpdateStatus_OngoingVerification { params := []pegomock.Param{repo, pull, state, description} methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "UpdateStatus", params, verifier.timeout) diff --git a/server/events/vcs/not_configured_vcs_client.go b/server/events/vcs/not_configured_vcs_client.go index a2381de263..f7abdf76dc 100644 --- a/server/events/vcs/not_configured_vcs_client.go +++ b/server/events/vcs/not_configured_vcs_client.go @@ -35,6 +35,9 @@ func (a *NotConfiguredVCSClient) CreateComment(repo models.Repo, pullNum int, co func (a *NotConfiguredVCSClient) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) { return false, a.err() } +func (a *NotConfiguredVCSClient) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + return false, a.err() +} func (a *NotConfiguredVCSClient) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error { return a.err() } diff --git a/server/events/vcs/proxy.go b/server/events/vcs/proxy.go index e981ee2a4e..91ec6f22c0 100644 --- a/server/events/vcs/proxy.go +++ b/server/events/vcs/proxy.go @@ -25,6 +25,7 @@ type ClientProxy interface { GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) CreateComment(repo models.Repo, pullNum int, comment string) error PullIsApproved(repo models.Repo, pull models.PullRequest) (bool, error) + PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error } @@ -71,6 +72,10 @@ func (d *DefaultClientProxy) PullIsApproved(repo models.Repo, pull models.PullRe return d.clients[repo.VCSHost.Type].PullIsApproved(repo, pull) } +func (d *DefaultClientProxy) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { + return d.clients[repo.VCSHost.Type].PullIsMergeable(repo, pull) +} + func (d *DefaultClientProxy) UpdateStatus(repo models.Repo, pull models.PullRequest, state models.CommitStatus, description string) error { return d.clients[repo.VCSHost.Type].UpdateStatus(repo, pull, state, description) } diff --git a/server/events/yaml/parser_validator_test.go b/server/events/yaml/parser_validator_test.go index a47db640b5..28e7b56dee 100644 --- a/server/events/yaml/parser_validator_test.go +++ b/server/events/yaml/parser_validator_test.go @@ -226,6 +226,74 @@ workflows: }, }, }, + { + description: "project field with mergeable apply requirement", + input: ` +version: 2 +projects: +- dir: . + workspace: myworkspace + terraform_version: v0.11.0 + apply_requirements: [mergeable] + workflow: myworkflow + autoplan: + enabled: false +workflows: + myworkflow: ~`, + exp: valid.Config{ + Version: 2, + Projects: []valid.Project{ + { + Dir: ".", + Workspace: "myworkspace", + Workflow: String("myworkflow"), + TerraformVersion: tfVersion, + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: false, + }, + ApplyRequirements: []string{"mergeable"}, + }, + }, + Workflows: map[string]valid.Workflow{ + "myworkflow": {}, + }, + }, + }, + { + description: "project field with mergeable and approved apply requirements", + input: ` +version: 2 +projects: +- dir: . + workspace: myworkspace + terraform_version: v0.11.0 + apply_requirements: [mergeable, approved] + workflow: myworkflow + autoplan: + enabled: false +workflows: + myworkflow: ~`, + exp: valid.Config{ + Version: 2, + Projects: []valid.Project{ + { + Dir: ".", + Workspace: "myworkspace", + Workflow: String("myworkflow"), + TerraformVersion: tfVersion, + Autoplan: valid.Autoplan{ + WhenModified: []string{"**/*.tf*"}, + Enabled: false, + }, + ApplyRequirements: []string{"mergeable", "approved"}, + }, + }, + Workflows: map[string]valid.Workflow{ + "myworkflow": {}, + }, + }, + }, { description: "project dir with ..", input: ` diff --git a/server/events/yaml/raw/project.go b/server/events/yaml/raw/project.go index 4ba068ccc6..8f58a326ff 100644 --- a/server/events/yaml/raw/project.go +++ b/server/events/yaml/raw/project.go @@ -13,8 +13,9 @@ import ( ) const ( - DefaultWorkspace = "default" - ApprovedApplyRequirement = "approved" + DefaultWorkspace = "default" + ApprovedApplyRequirement = "approved" + MergeableApplyRequirement = "mergeable" ) type Project struct { @@ -37,8 +38,8 @@ func (p Project) Validate() error { validApplyReq := func(value interface{}) error { reqs := value.([]string) for _, r := range reqs { - if r != ApprovedApplyRequirement { - return fmt.Errorf("%q not supported, only %s is supported", r, ApprovedApplyRequirement) + if r != ApprovedApplyRequirement && r != MergeableApplyRequirement { + return fmt.Errorf("%q not supported, only %s and %s are supported", r, ApprovedApplyRequirement, MergeableApplyRequirement) } } return nil diff --git a/server/events/yaml/raw/project_test.go b/server/events/yaml/raw/project_test.go index e6211f585a..cb317d498f 100644 --- a/server/events/yaml/raw/project_test.go +++ b/server/events/yaml/raw/project_test.go @@ -31,7 +31,7 @@ func TestProject_UnmarshalYAML(t *testing.T) { }, }, { - description: "all fields set", + description: "all fields set including mergeable apply requirement", input: ` name: myname dir: mydir @@ -101,16 +101,32 @@ func TestProject_Validate(t *testing.T) { Dir: String("."), ApplyRequirements: []string{"unsupported"}, }, - expErr: "apply_requirements: \"unsupported\" not supported, only approved is supported.", + expErr: "apply_requirements: \"unsupported\" not supported, only approved and mergeable are supported.", }, { - description: "apply reqs with valid", + description: "apply reqs with approved requirement", input: raw.Project{ Dir: String("."), ApplyRequirements: []string{"approved"}, }, expErr: "", }, + { + description: "apply reqs with mergeable requirement", + input: raw.Project{ + Dir: String("."), + ApplyRequirements: []string{"mergeable"}, + }, + expErr: "", + }, + { + description: "apply reqs with mergeable and approved requirements", + input: raw.Project{ + Dir: String("."), + ApplyRequirements: []string{"mergeable", "approved"}, + }, + expErr: "", + }, { description: "empty tf version string", input: raw.Project{ diff --git a/server/server.go b/server/server.go index 5128e9ce65..8fb2c79432 100644 --- a/server/server.go +++ b/server/server.go @@ -102,7 +102,10 @@ type UserConfig struct { RepoWhitelist string `mapstructure:"repo-whitelist"` // RequireApproval is whether to require pull request approval before // allowing terraform apply's to be run. - RequireApproval bool `mapstructure:"require-approval"` + RequireApproval bool `mapstructure:"require-approval"` + // RequireMergeable is whether to require pull requests to be mergeable before + // allowing terraform apply's to run. + RequireMergeable bool `mapstructure:"require-mergeable"` SilenceWhitelistErrors bool `mapstructure:"silence-whitelist-errors"` SlackToken string `mapstructure:"slack-token"` SSLCertFile string `mapstructure:"ssl-cert-file"` @@ -291,11 +294,13 @@ func NewServer(userConfig UserConfig, config Config) (*Server, error) { RunStepRunner: &runtime.RunStepRunner{ DefaultTFVersion: defaultTfVersion, }, - PullApprovedChecker: vcsClient, - WorkingDir: workingDir, - Webhooks: webhooksManager, - WorkingDirLocker: workingDirLocker, - RequireApprovalOverride: userConfig.RequireApproval, + PullApprovedChecker: vcsClient, + PullMergeableChecker: vcsClient, + WorkingDir: workingDir, + Webhooks: webhooksManager, + WorkingDirLocker: workingDirLocker, + RequireApprovalOverride: userConfig.RequireApproval, + RequireMergeableOverride: userConfig.RequireMergeable, }, } repoWhitelist, err := events.NewRepoWhitelistChecker(userConfig.RepoWhitelist)