From 13fa63552387fbfe96689c7245536c08911a194d Mon Sep 17 00:00:00 2001 From: Alexey Kosenko Date: Sat, 2 Nov 2024 23:09:17 +0300 Subject: [PATCH] feat(automerge): implement GitHub --auto-merge-method flag for apply command (#4895) Signed-off-by: a1k0u Signed-off-by: X-Guardian --- runatlantis.io/docs/automerging.md | 17 ++ runatlantis.io/docs/using-atlantis.md | 1 + server/core/config/valid/global_cfg.go | 1 + server/events/apply_command_runner.go | 2 +- server/events/automerger.go | 3 +- server/events/comment_parser.go | 36 +++- server/events/comment_parser_test.go | 33 +++- server/events/event_parser.go | 7 +- server/events/event_parser_test.go | 8 +- server/events/mocks/mock_comment_building.go | 172 ++++++++++-------- server/events/models/models.go | 3 + .../events/project_command_context_builder.go | 4 +- .../project_command_context_builder_test.go | 8 +- server/events/vcs/github_client.go | 42 ++++- server/events/vcs/github_client_test.go | 70 ++++++- 15 files changed, 292 insertions(+), 115 deletions(-) diff --git a/runatlantis.io/docs/automerging.md b/runatlantis.io/docs/automerging.md index 2716a572ee..5c2f96d34e 100644 --- a/runatlantis.io/docs/automerging.md +++ b/runatlantis.io/docs/automerging.md @@ -29,6 +29,23 @@ Automerging can be enabled either by: If automerge is enabled, you can disable it for a single `atlantis apply` command with the `--auto-merge-disabled` option. +## How to set the merge method for automerge + +If automerge is enabled, you can use the `--auto-merge-method` option +for the `atlantis apply` command to specify which merge method use. + +```shell +atlantis apply --auto-merge-method +``` + +The `method` must be one of: + +- merge +- rebase +- squash + +This is currently only implemented for the GitHub VCS. + ## Requirements ### All Plans Must Succeed diff --git a/runatlantis.io/docs/using-atlantis.md b/runatlantis.io/docs/using-atlantis.md index 61c06e1a21..aa0a372231 100644 --- a/runatlantis.io/docs/using-atlantis.md +++ b/runatlantis.io/docs/using-atlantis.md @@ -149,6 +149,7 @@ atlantis apply -w staging * `-p project` Apply the plan for this project. Refers to the name of the project configured in the repo's [`atlantis.yaml` file](repo-level-atlantis-yaml.md). Cannot be used at same time as `-d` or `-w`. * `-w workspace` Apply the plan for this [Terraform workspace](https://developer.hashicorp.com/terraform/language/state/workspaces). Ignore this if Terraform workspaces are unused. * `--auto-merge-disabled` Disable [automerge](automerging.md) for this apply command. +* `--auto-merge-method method` Specify which [merge method](automerging.md#how-to-set-merge-method-for-automerge) use for the apply command if [automerge](automerging.md) is enabled. Implemented only for GitHub. * `--verbose` Append Atlantis log to comment. ### Additional Terraform flags diff --git a/server/core/config/valid/global_cfg.go b/server/core/config/valid/global_cfg.go index 11a267c151..a930ef22bc 100644 --- a/server/core/config/valid/global_cfg.go +++ b/server/core/config/valid/global_cfg.go @@ -104,6 +104,7 @@ type MergedProjectCfg struct { Name string AutoplanEnabled bool AutoMergeDisabled bool + AutoMergeMethod string TerraformVersion *version.Version RepoCfgVersion int PolicySets PolicySets diff --git a/server/events/apply_command_runner.go b/server/events/apply_command_runner.go index ee6bf8ab1f..6c69032910 100644 --- a/server/events/apply_command_runner.go +++ b/server/events/apply_command_runner.go @@ -181,7 +181,7 @@ func (a *ApplyCommandRunner) Run(ctx *command.Context, cmd *CommentCommand) { a.updateCommitStatus(ctx, pullStatus) if a.autoMerger.automergeEnabled(projectCmds) && !cmd.AutoMergeDisabled { - a.autoMerger.automerge(ctx, pullStatus, a.autoMerger.deleteSourceBranchOnMergeEnabled(projectCmds)) + a.autoMerger.automerge(ctx, pullStatus, a.autoMerger.deleteSourceBranchOnMergeEnabled(projectCmds), cmd.AutoMergeMethod) } } diff --git a/server/events/automerger.go b/server/events/automerger.go index fa74beac0f..1d19964076 100644 --- a/server/events/automerger.go +++ b/server/events/automerger.go @@ -13,7 +13,7 @@ type AutoMerger struct { GlobalAutomerge bool } -func (c *AutoMerger) automerge(ctx *command.Context, pullStatus models.PullStatus, deleteSourceBranchOnMerge bool) { +func (c *AutoMerger) automerge(ctx *command.Context, pullStatus models.PullStatus, deleteSourceBranchOnMerge bool, mergeMethod string) { // We only automerge if all projects have been successfully applied. for _, p := range pullStatus.Projects { if p.Status != models.AppliedPlanStatus { @@ -32,6 +32,7 @@ func (c *AutoMerger) automerge(ctx *command.Context, pullStatus models.PullStatu ctx.Log.Info("automerging pull request") var pullOptions models.PullRequestOptions pullOptions.DeleteSourceBranchOnMerge = deleteSourceBranchOnMerge + pullOptions.MergeMethod = mergeMethod err := c.VCSClient.MergePull(ctx.Log, ctx.Pull, pullOptions) if err != nil { diff --git a/server/events/comment_parser.go b/server/events/comment_parser.go index 3b3d2d3b0a..829c15ced9 100644 --- a/server/events/comment_parser.go +++ b/server/events/comment_parser.go @@ -41,6 +41,8 @@ const ( policySetFlagShort = "" autoMergeDisabledFlagLong = "auto-merge-disabled" autoMergeDisabledFlagShort = "" + autoMergeMethodFlagLong = "auto-merge-method" + autoMergeMethodFlagShort = "" verboseFlagLong = "verbose" verboseFlagShort = "" clearPolicyApprovalFlagLong = "clear-policy-approval" @@ -70,7 +72,7 @@ type CommentBuilder interface { // BuildPlanComment builds a plan comment for the specified args. BuildPlanComment(repoRelDir string, workspace string, project string, commentArgs []string) string // BuildApplyComment builds an apply comment for the specified args. - BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string + BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool, autoMergeMethod string) string // BuildApprovePoliciesComment builds an approve_policies comment for the specified args. BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) string } @@ -226,7 +228,9 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com var project string var policySet string var clearPolicyApproval bool - var verbose, autoMergeDisabled bool + var verbose bool + var autoMergeDisabled bool + var autoMergeMethod string var flagSet *pflag.FlagSet var name command.Name @@ -248,6 +252,7 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com flagSet.StringVarP(&dir, dirFlagLong, dirFlagShort, "", "Apply the plan for this directory, relative to root of repo, ex. 'child/dir'.") flagSet.StringVarP(&project, projectFlagLong, projectFlagShort, "", "Apply the plan for this project. Refers to the name of the project configured in a repo config file. Cannot be used at same time as workspace or dir flags.") flagSet.BoolVarP(&autoMergeDisabled, autoMergeDisabledFlagLong, autoMergeDisabledFlagShort, false, "Disable automerge after apply.") + flagSet.StringVarP(&autoMergeMethod, autoMergeMethodFlagLong, autoMergeMethodFlagShort, "", "Specifies the merge method for the VCS if automerge is enabled. (Currently only implemented for GitHub)") flagSet.BoolVarP(&verbose, verboseFlagLong, verboseFlagShort, false, "Append Atlantis log to comment.") case command.ApprovePolicies.String(): name = command.ApprovePolicies @@ -317,8 +322,20 @@ func (e *CommentParser) Parse(rawComment string, vcsHost models.VCSHostType) Com return CommentParseResult{CommentResponse: e.errMarkdown(err, cmd, flagSet)} } + if autoMergeMethod != "" { + if autoMergeDisabled { + err := fmt.Sprintf("cannot use --%s at the same time as --%s", autoMergeMethodFlagLong, autoMergeDisabledFlagLong) + return CommentParseResult{CommentResponse: e.errMarkdown(err, cmd, flagSet)} + } + + if vcsHost != models.Github { + err := fmt.Sprintf("--%s is not currently implemented for %s", autoMergeMethodFlagLong, vcsHost.String()) + return CommentParseResult{CommentResponse: e.errMarkdown(err, cmd, flagSet)} + } + } + return CommentParseResult{ - Command: NewCommentCommand(dir, extraArgs, name, subName, verbose, autoMergeDisabled, workspace, project, policySet, clearPolicyApproval), + Command: NewCommentCommand(dir, extraArgs, name, subName, verbose, autoMergeDisabled, autoMergeMethod, workspace, project, policySet, clearPolicyApproval), } } @@ -387,7 +404,7 @@ func (e *CommentParser) parseArgs(name command.Name, args []string, flagSet *pfl // BuildPlanComment builds a plan comment for the specified args. func (e *CommentParser) BuildPlanComment(repoRelDir string, workspace string, project string, commentArgs []string) string { - flags := e.buildFlags(repoRelDir, workspace, project, false) + flags := e.buildFlags(repoRelDir, workspace, project, false, "") commentFlags := "" if len(commentArgs) > 0 { var flagsWithoutQuotes []string @@ -402,18 +419,18 @@ func (e *CommentParser) BuildPlanComment(repoRelDir string, workspace string, pr } // BuildApplyComment builds an apply comment for the specified args. -func (e *CommentParser) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string { - flags := e.buildFlags(repoRelDir, workspace, project, autoMergeDisabled) +func (e *CommentParser) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool, autoMergeMethod string) string { + flags := e.buildFlags(repoRelDir, workspace, project, autoMergeDisabled, autoMergeMethod) return fmt.Sprintf("%s %s%s", e.ExecutableName, command.Apply.String(), flags) } // BuildApprovePoliciesComment builds an apply comment for the specified args. func (e *CommentParser) BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) string { - flags := e.buildFlags(repoRelDir, workspace, project, false) + flags := e.buildFlags(repoRelDir, workspace, project, false, "") return fmt.Sprintf("%s %s%s", e.ExecutableName, command.ApprovePolicies.String(), flags) } -func (e *CommentParser) buildFlags(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string { +func (e *CommentParser) buildFlags(repoRelDir string, workspace string, project string, autoMergeDisabled bool, autoMergeMethod string) string { // Add quotes if dir has spaces. if strings.Contains(repoRelDir, " ") { repoRelDir = fmt.Sprintf("%q", repoRelDir) @@ -441,6 +458,9 @@ func (e *CommentParser) buildFlags(repoRelDir string, workspace string, project if autoMergeDisabled { flags = fmt.Sprintf("%s --%s", flags, autoMergeDisabledFlagLong) } + if autoMergeMethod != "" { + flags = fmt.Sprintf("%s --%s %s", flags, autoMergeMethodFlagLong, autoMergeMethod) + } return flags } diff --git a/server/events/comment_parser_test.go b/server/events/comment_parser_test.go index 45c22e7e5f..88ededcfff 100644 --- a/server/events/comment_parser_test.go +++ b/server/events/comment_parser_test.go @@ -729,6 +729,7 @@ func TestBuildPlanApplyVersionComment(t *testing.T) { workspace string project string autoMergeDisabled bool + autoMergeMethod string commentArgs []string expPlanFlags string expApplyFlags string @@ -824,6 +825,16 @@ func TestBuildPlanApplyVersionComment(t *testing.T) { expApplyFlags: "-d dir -w workspace --auto-merge-disabled", expVersionFlags: "-d dir -w workspace", }, + { + repoRelDir: "dir", + workspace: "workspace", + project: "", + autoMergeMethod: "squash", + commentArgs: []string{`"arg1"`, `"arg2"`, `arg3`}, + expPlanFlags: "-d dir -w workspace -- arg1 arg2 arg3", + expApplyFlags: "-d dir -w workspace --auto-merge-method squash", + expVersionFlags: "-d dir -w workspace", + }, } for _, c := range cases { @@ -834,7 +845,7 @@ func TestBuildPlanApplyVersionComment(t *testing.T) { actComment := commentParser.BuildPlanComment(c.repoRelDir, c.workspace, c.project, c.commentArgs) Equals(t, fmt.Sprintf("atlantis plan %s", c.expPlanFlags), actComment) case command.Apply: - actComment := commentParser.BuildApplyComment(c.repoRelDir, c.workspace, c.project, c.autoMergeDisabled) + actComment := commentParser.BuildApplyComment(c.repoRelDir, c.workspace, c.project, c.autoMergeDisabled, c.autoMergeMethod) Equals(t, fmt.Sprintf("atlantis apply %s", c.expApplyFlags), actComment) } } @@ -1020,14 +1031,18 @@ var PlanUsage = `Usage of plan: ` var ApplyUsage = `Usage of apply: - --auto-merge-disabled Disable automerge after apply. - -d, --dir string Apply the plan for this directory, relative to root of - repo, ex. 'child/dir'. - -p, --project string Apply the plan for this project. Refers to the name of - the project configured in a repo config file. Cannot - be used at same time as workspace or dir flags. - --verbose Append Atlantis log to comment. - -w, --workspace string Apply the plan for this Terraform workspace. + --auto-merge-disabled Disable automerge after apply. + --auto-merge-method string Specifies the merge method for the VCS if + automerge is enabled. (Currently only implemented + for GitHub) + -d, --dir string Apply the plan for this directory, relative to + root of repo, ex. 'child/dir'. + -p, --project string Apply the plan for this project. Refers to the + name of the project configured in a repo config + file. Cannot be used at same time as workspace or + dir flags. + --verbose Append Atlantis log to comment. + -w, --workspace string Apply the plan for this Terraform workspace. ` var ApprovePolicyUsage = `Usage of approve_policies: diff --git a/server/events/event_parser.go b/server/events/event_parser.go index 62dd634a18..a6b4b363ac 100644 --- a/server/events/event_parser.go +++ b/server/events/event_parser.go @@ -128,6 +128,8 @@ type CommentCommand struct { SubName string // AutoMergeDisabled is true if the command should not automerge after apply. AutoMergeDisabled bool + // AutoMergeMethod specified the merge method for the VCS if automerge enabled. + AutoMergeMethod string // Verbose is true if the command should output verbosely. Verbose bool // Workspace is the name of the Terraform workspace to run the command in. @@ -177,11 +179,11 @@ func (c CommentCommand) IsAutoplan() bool { // String returns a string representation of the command. func (c CommentCommand) String() string { - return fmt.Sprintf("command=%q verbose=%t dir=%q workspace=%q project=%q policyset=%q, clear-policy-approval=%t, flags=%q", c.Name.String(), c.Verbose, c.RepoRelDir, c.Workspace, c.ProjectName, c.PolicySet, c.ClearPolicyApproval, strings.Join(c.Flags, ",")) + return fmt.Sprintf("command=%q, verbose=%t, dir=%q, workspace=%q, project=%q, policyset=%q, auto-merge-disabled=%t, auto-merge-method=%s, clear-policy-approval=%t, flags=%q", c.Name.String(), c.Verbose, c.RepoRelDir, c.Workspace, c.ProjectName, c.PolicySet, c.AutoMergeDisabled, c.AutoMergeMethod, c.ClearPolicyApproval, strings.Join(c.Flags, ",")) } // NewCommentCommand constructs a CommentCommand, setting all missing fields to defaults. -func NewCommentCommand(repoRelDir string, flags []string, name command.Name, subName string, verbose, autoMergeDisabled bool, workspace string, project string, policySet string, clearPolicyApproval bool) *CommentCommand { +func NewCommentCommand(repoRelDir string, flags []string, name command.Name, subName string, verbose, autoMergeDisabled bool, autoMergeMethod string, workspace string, project string, policySet string, clearPolicyApproval bool) *CommentCommand { // If repoRelDir was empty we want to keep it that way to indicate that it // wasn't specified in the comment. if repoRelDir != "" { @@ -198,6 +200,7 @@ func NewCommentCommand(repoRelDir string, flags []string, name command.Name, sub Verbose: verbose, Workspace: workspace, AutoMergeDisabled: autoMergeDisabled, + AutoMergeMethod: autoMergeMethod, ProjectName: project, PolicySet: policySet, ClearPolicyApproval: clearPolicyApproval, diff --git a/server/events/event_parser_test.go b/server/events/event_parser_test.go index 3b7b206a7d..fffe30e3eb 100644 --- a/server/events/event_parser_test.go +++ b/server/events/event_parser_test.go @@ -750,14 +750,14 @@ func TestNewCommand_CleansDir(t *testing.T) { for _, c := range cases { t.Run(c.RepoRelDir, func(t *testing.T) { - cmd := events.NewCommentCommand(c.RepoRelDir, nil, command.Plan, "", false, false, "workspace", "", "", false) + cmd := events.NewCommentCommand(c.RepoRelDir, nil, command.Plan, "", false, false, "", "workspace", "", "", false) Equals(t, c.ExpDir, cmd.RepoRelDir) }) } } func TestNewCommand_EmptyDirWorkspaceProject(t *testing.T) { - cmd := events.NewCommentCommand("", nil, command.Plan, "", false, false, "", "", "", false) + cmd := events.NewCommentCommand("", nil, command.Plan, "", false, false, "", "", "", "", false) Equals(t, events.CommentCommand{ RepoRelDir: "", Flags: nil, @@ -769,7 +769,7 @@ func TestNewCommand_EmptyDirWorkspaceProject(t *testing.T) { } func TestNewCommand_AllFieldsSet(t *testing.T) { - cmd := events.NewCommentCommand("dir", []string{"a", "b"}, command.Plan, "", true, false, "workspace", "project", "policyset", false) + cmd := events.NewCommentCommand("dir", []string{"a", "b"}, command.Plan, "", true, false, "", "workspace", "project", "policyset", false) Equals(t, events.CommentCommand{ Workspace: "workspace", RepoRelDir: "dir", @@ -816,7 +816,7 @@ func TestCommentCommand_IsAutoplan(t *testing.T) { } func TestCommentCommand_String(t *testing.T) { - exp := `command="plan" verbose=true dir="mydir" workspace="myworkspace" project="myproject" policyset="", clear-policy-approval=false, flags="flag1,flag2"` + exp := `command="plan", verbose=true, dir="mydir", workspace="myworkspace", project="myproject", policyset="", auto-merge-disabled=false, auto-merge-method=, clear-policy-approval=false, flags="flag1,flag2"` Equals(t, exp, (events.CommentCommand{ RepoRelDir: "mydir", Flags: []string{"flag1", "flag2"}, diff --git a/server/events/mocks/mock_comment_building.go b/server/events/mocks/mock_comment_building.go index 1e461a07ee..1d25d4eacd 100644 --- a/server/events/mocks/mock_comment_building.go +++ b/server/events/mocks/mock_comment_building.go @@ -24,49 +24,49 @@ func NewMockCommentBuilder(options ...pegomock.Option) *MockCommentBuilder { func (mock *MockCommentBuilder) SetFailHandler(fh pegomock.FailHandler) { mock.fail = fh } func (mock *MockCommentBuilder) FailHandler() pegomock.FailHandler { return mock.fail } -func (mock *MockCommentBuilder) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool) string { +func (mock *MockCommentBuilder) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool, mergeMethod string) string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockCommentBuilder().") } - params := []pegomock.Param{repoRelDir, workspace, project, autoMergeDisabled} - result := pegomock.GetGenericMockFrom(mock).Invoke("BuildApplyComment", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) - var ret0 string - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) + _params := []pegomock.Param{repoRelDir, workspace, project, autoMergeDisabled, mergeMethod} + _result := pegomock.GetGenericMockFrom(mock).Invoke("BuildApplyComment", _params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var _ret0 string + if len(_result) != 0 { + if _result[0] != nil { + _ret0 = _result[0].(string) } } - return ret0 + return _ret0 } func (mock *MockCommentBuilder) BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockCommentBuilder().") } - params := []pegomock.Param{repoRelDir, workspace, project} - result := pegomock.GetGenericMockFrom(mock).Invoke("BuildApprovePoliciesComment", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) - var ret0 string - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) + _params := []pegomock.Param{repoRelDir, workspace, project} + _result := pegomock.GetGenericMockFrom(mock).Invoke("BuildApprovePoliciesComment", _params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var _ret0 string + if len(_result) != 0 { + if _result[0] != nil { + _ret0 = _result[0].(string) } } - return ret0 + return _ret0 } func (mock *MockCommentBuilder) BuildPlanComment(repoRelDir string, workspace string, project string, commentArgs []string) string { if mock == nil { panic("mock must not be nil. Use myMock := NewMockCommentBuilder().") } - params := []pegomock.Param{repoRelDir, workspace, project, commentArgs} - result := pegomock.GetGenericMockFrom(mock).Invoke("BuildPlanComment", params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) - var ret0 string - if len(result) != 0 { - if result[0] != nil { - ret0 = result[0].(string) + _params := []pegomock.Param{repoRelDir, workspace, project, commentArgs} + _result := pegomock.GetGenericMockFrom(mock).Invoke("BuildPlanComment", _params, []reflect.Type{reflect.TypeOf((*string)(nil)).Elem()}) + var _ret0 string + if len(_result) != 0 { + if _result[0] != nil { + _ret0 = _result[0].(string) } } - return ret0 + return _ret0 } func (mock *MockCommentBuilder) VerifyWasCalledOnce() *VerifierMockCommentBuilder { @@ -106,9 +106,9 @@ type VerifierMockCommentBuilder struct { timeout time.Duration } -func (verifier *VerifierMockCommentBuilder) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool) *MockCommentBuilder_BuildApplyComment_OngoingVerification { - params := []pegomock.Param{repoRelDir, workspace, project, autoMergeDisabled} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildApplyComment", params, verifier.timeout) +func (verifier *VerifierMockCommentBuilder) BuildApplyComment(repoRelDir string, workspace string, project string, autoMergeDisabled bool, mergeMethod string) *MockCommentBuilder_BuildApplyComment_OngoingVerification { + _params := []pegomock.Param{repoRelDir, workspace, project, autoMergeDisabled, mergeMethod} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildApplyComment", _params, verifier.timeout) return &MockCommentBuilder_BuildApplyComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -117,37 +117,51 @@ type MockCommentBuilder_BuildApplyComment_OngoingVerification struct { methodInvocations []pegomock.MethodInvocation } -func (c *MockCommentBuilder_BuildApplyComment_OngoingVerification) GetCapturedArguments() (string, string, string, bool) { - repoRelDir, workspace, project, autoMergeDisabled := c.GetAllCapturedArguments() - return repoRelDir[len(repoRelDir)-1], workspace[len(workspace)-1], project[len(project)-1], autoMergeDisabled[len(autoMergeDisabled)-1] +func (c *MockCommentBuilder_BuildApplyComment_OngoingVerification) GetCapturedArguments() (string, string, string, bool, string) { + repoRelDir, workspace, project, autoMergeDisabled, mergeMethod := c.GetAllCapturedArguments() + return repoRelDir[len(repoRelDir)-1], workspace[len(workspace)-1], project[len(project)-1], autoMergeDisabled[len(autoMergeDisabled)-1], mergeMethod[len(mergeMethod)-1] } -func (c *MockCommentBuilder_BuildApplyComment_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []string, _param2 []string, _param3 []bool) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(string) +func (c *MockCommentBuilder_BuildApplyComment_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []string, _param2 []string, _param3 []bool, _param4 []string) { + _params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(_params) > 0 { + if len(_params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range _params[0] { + _param0[u] = param.(string) + } } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) + if len(_params) > 1 { + _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) + if len(_params) > 2 { + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range _params[2] { + _param2[u] = param.(string) + } } - _param3 = make([]bool, len(c.methodInvocations)) - for u, param := range params[3] { - _param3[u] = param.(bool) + if len(_params) > 3 { + _param3 = make([]bool, len(c.methodInvocations)) + for u, param := range _params[3] { + _param3[u] = param.(bool) + } + } + if len(_params) > 4 { + _param4 = make([]string, len(c.methodInvocations)) + for u, param := range _params[4] { + _param4[u] = param.(string) + } } } return } func (verifier *VerifierMockCommentBuilder) BuildApprovePoliciesComment(repoRelDir string, workspace string, project string) *MockCommentBuilder_BuildApprovePoliciesComment_OngoingVerification { - params := []pegomock.Param{repoRelDir, workspace, project} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildApprovePoliciesComment", params, verifier.timeout) + _params := []pegomock.Param{repoRelDir, workspace, project} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildApprovePoliciesComment", _params, verifier.timeout) return &MockCommentBuilder_BuildApprovePoliciesComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -162,27 +176,33 @@ func (c *MockCommentBuilder_BuildApprovePoliciesComment_OngoingVerification) Get } func (c *MockCommentBuilder_BuildApprovePoliciesComment_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []string, _param2 []string) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(string) + _params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(_params) > 0 { + if len(_params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range _params[0] { + _param0[u] = param.(string) + } } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) + if len(_params) > 1 { + _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) + if len(_params) > 2 { + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range _params[2] { + _param2[u] = param.(string) + } } } return } func (verifier *VerifierMockCommentBuilder) BuildPlanComment(repoRelDir string, workspace string, project string, commentArgs []string) *MockCommentBuilder_BuildPlanComment_OngoingVerification { - params := []pegomock.Param{repoRelDir, workspace, project, commentArgs} - methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildPlanComment", params, verifier.timeout) + _params := []pegomock.Param{repoRelDir, workspace, project, commentArgs} + methodInvocations := pegomock.GetGenericMockFrom(verifier.mock).Verify(verifier.inOrderContext, verifier.invocationCountMatcher, "BuildPlanComment", _params, verifier.timeout) return &MockCommentBuilder_BuildPlanComment_OngoingVerification{mock: verifier.mock, methodInvocations: methodInvocations} } @@ -197,23 +217,31 @@ func (c *MockCommentBuilder_BuildPlanComment_OngoingVerification) GetCapturedArg } func (c *MockCommentBuilder_BuildPlanComment_OngoingVerification) GetAllCapturedArguments() (_param0 []string, _param1 []string, _param2 []string, _param3 [][]string) { - params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) - if len(params) > 0 { - _param0 = make([]string, len(c.methodInvocations)) - for u, param := range params[0] { - _param0[u] = param.(string) + _params := pegomock.GetGenericMockFrom(c.mock).GetInvocationParams(c.methodInvocations) + if len(_params) > 0 { + if len(_params) > 0 { + _param0 = make([]string, len(c.methodInvocations)) + for u, param := range _params[0] { + _param0[u] = param.(string) + } } - _param1 = make([]string, len(c.methodInvocations)) - for u, param := range params[1] { - _param1[u] = param.(string) + if len(_params) > 1 { + _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) + if len(_params) > 2 { + _param2 = make([]string, len(c.methodInvocations)) + for u, param := range _params[2] { + _param2[u] = param.(string) + } } - _param3 = make([][]string, len(c.methodInvocations)) - for u, param := range params[3] { - _param3[u] = param.([]string) + if len(_params) > 3 { + _param3 = make([][]string, len(c.methodInvocations)) + for u, param := range _params[3] { + _param3[u] = param.([]string) + } } } return diff --git a/server/events/models/models.go b/server/events/models/models.go index fe5c0ee09d..f7bd4790db 100644 --- a/server/events/models/models.go +++ b/server/events/models/models.go @@ -185,6 +185,9 @@ type PullRequestOptions struct { // When DeleteSourceBranchOnMerge flag is set to true VCS deletes the source branch after the PR is merged // Applied by GitLab & AzureDevops DeleteSourceBranchOnMerge bool + // MergeMethod specifies the merge method for the VCS + // Implemented only for Github + MergeMethod string } type PullRequestState int diff --git a/server/events/project_command_context_builder.go b/server/events/project_command_context_builder.go index 05d61fda6d..19c1c8ff34 100644 --- a/server/events/project_command_context_builder.go +++ b/server/events/project_command_context_builder.go @@ -130,7 +130,7 @@ func (cb *DefaultProjectCommandContextBuilder) BuildProjectContext( projectCmdContext := newProjectCommandContext( ctx, cmdName, - cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled), + cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled, prjCfg.AutoMergeMethod), cb.CommentBuilder.BuildApprovePoliciesComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name), cb.CommentBuilder.BuildPlanComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, commentFlags), prjCfg, @@ -203,7 +203,7 @@ func (cb *PolicyCheckProjectCommandContextBuilder) BuildProjectContext( projectCmds = append(projectCmds, newProjectCommandContext( ctx, command.PolicyCheck, - cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled), + cb.CommentBuilder.BuildApplyComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, prjCfg.AutoMergeDisabled, prjCfg.AutoMergeMethod), cb.CommentBuilder.BuildApprovePoliciesComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name), cb.CommentBuilder.BuildPlanComment(prjCfg.RepoRelDir, prjCfg.Workspace, prjCfg.Name, commentFlags), prjCfg, diff --git a/server/events/project_command_context_builder_test.go b/server/events/project_command_context_builder_test.go index c3d75e950c..84ce0ff630 100644 --- a/server/events/project_command_context_builder_test.go +++ b/server/events/project_command_context_builder_test.go @@ -51,7 +51,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { t.Run("with project name defined", func(t *testing.T) { When(mockCommentBuilder.BuildPlanComment(projRepoRelDir, projWorkspace, projName, []string{})).ThenReturn(expectedPlanCmt) - When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, projName, false)).ThenReturn(expectedApplyCmt) + When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, projName, false, "")).ThenReturn(expectedApplyCmt) pullStatus.Projects = []models.ProjectStatus{ { @@ -68,7 +68,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { t.Run("with no project name defined", func(t *testing.T) { projCfg.Name = "" When(mockCommentBuilder.BuildPlanComment(projRepoRelDir, projWorkspace, "", []string{})).ThenReturn(expectedPlanCmt) - When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false)).ThenReturn(expectedApplyCmt) + When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false, "")).ThenReturn(expectedApplyCmt) pullStatus.Projects = []models.ProjectStatus{ { Status: models.ErroredPlanStatus, @@ -88,7 +88,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { t.Run("when ParallelApply is set to true", func(t *testing.T) { projCfg.Name = "Apply Comment" When(mockCommentBuilder.BuildPlanComment(projRepoRelDir, projWorkspace, "", []string{})).ThenReturn(expectedPlanCmt) - When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false)).ThenReturn(expectedApplyCmt) + When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false, "")).ThenReturn(expectedApplyCmt) pullStatus.Projects = []models.ProjectStatus{ { Status: models.ErroredPlanStatus, @@ -109,7 +109,7 @@ func TestProjectCommandContextBuilder_PullStatus(t *testing.T) { t.Run("when AbortOnExcecutionOrderFail is set to true", func(t *testing.T) { projCfg.Name = "Apply Comment" When(mockCommentBuilder.BuildPlanComment(projRepoRelDir, projWorkspace, "", []string{})).ThenReturn(expectedPlanCmt) - When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false)).ThenReturn(expectedApplyCmt) + When(mockCommentBuilder.BuildApplyComment(projRepoRelDir, projWorkspace, "", false, "")).ThenReturn(expectedApplyCmt) pullStatus.Projects = []models.ProjectStatus{ { Status: models.ErroredPlanStatus, diff --git a/server/events/vcs/github_client.go b/server/events/vcs/github_client.go index ddc0ff1e1b..826cff8b09 100644 --- a/server/events/vcs/github_client.go +++ b/server/events/vcs/github_client.go @@ -17,7 +17,10 @@ import ( "context" "encoding/base64" "fmt" + "maps" "net/http" + "slices" + "sort" "strconv" "strings" "time" @@ -885,7 +888,7 @@ func (g *GithubClient) UpdateStatus(logger logging.SimpleLogging, repo models.Re } // MergePull merges the pull request. -func (g *GithubClient) MergePull(logger logging.SimpleLogging, pull models.PullRequest, _ models.PullRequestOptions) error { +func (g *GithubClient) MergePull(logger logging.SimpleLogging, pull models.PullRequest, pullOptions models.PullRequestOptions) error { logger.Debug("Merging GitHub pull request %d", pull.Num) // Users can set their repo to disallow certain types of merging. // We detect which types aren't allowed and use the type that is. @@ -896,17 +899,42 @@ func (g *GithubClient) MergePull(logger logging.SimpleLogging, pull models.PullR if err != nil { return errors.Wrap(err, "fetching repo info") } + const ( defaultMergeMethod = "merge" rebaseMergeMethod = "rebase" squashMergeMethod = "squash" ) - method := defaultMergeMethod - if !repo.GetAllowMergeCommit() { - if repo.GetAllowRebaseMerge() { - method = rebaseMergeMethod - } else if repo.GetAllowSquashMerge() { - method = squashMergeMethod + + mergeMethodsAllow := map[string]func() bool{ + defaultMergeMethod: repo.GetAllowMergeCommit, + rebaseMergeMethod: repo.GetAllowRebaseMerge, + squashMergeMethod: repo.GetAllowSquashMerge, + } + + mergeMethodsName := slices.Collect(maps.Keys(mergeMethodsAllow)) + sort.Strings(mergeMethodsName) + + var method string + if pullOptions.MergeMethod != "" { + method = pullOptions.MergeMethod + + isMethodAllowed, isMethodExist := mergeMethodsAllow[method] + if !isMethodExist { + return fmt.Errorf("Merge method '%s' is unknown. Specify one of the valid values: '%s'", method, strings.Join(mergeMethodsName, ", ")) + } + + if !isMethodAllowed() { + return fmt.Errorf("Merge method '%s' is not allowed by the repository Pull Request settings", method) + } + } else { + method = defaultMergeMethod + if !repo.GetAllowMergeCommit() { + if repo.GetAllowRebaseMerge() { + method = rebaseMergeMethod + } else if repo.GetAllowSquashMerge() { + method = squashMergeMethod + } } } diff --git a/server/events/vcs/github_client_test.go b/server/events/vcs/github_client_test.go index ef97c64245..632fe6bca6 100644 --- a/server/events/vcs/github_client_test.go +++ b/server/events/vcs/github_client_test.go @@ -977,10 +977,12 @@ func TestGithubClient_MergePullHandlesError(t *testing.T) { func TestGithubClient_MergePullCorrectMethod(t *testing.T) { logger := logging.NewNoopLogger(t) cases := map[string]struct { - allowMerge bool - allowRebase bool - allowSquash bool - expMethod string + allowMerge bool + allowRebase bool + allowSquash bool + mergeMethodOption string + expMethod string + expErr string }{ "all true": { allowMerge: true, @@ -1012,6 +1014,59 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { allowSquash: false, expMethod: "rebase", }, + "all true: merge with merge: overrided by command": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + mergeMethodOption: "merge", + expMethod: "merge", + }, + "all true: merge with rebase: overrided by command": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + mergeMethodOption: "rebase", + expMethod: "rebase", + }, + "all true: merge with squash: overrided by command": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + mergeMethodOption: "squash", + expMethod: "squash", + }, + "merge with merge: overridden by command: merge not allowed": { + allowMerge: false, + allowRebase: true, + allowSquash: true, + mergeMethodOption: "merge", + expMethod: "", + expErr: "Merge method 'merge' is not allowed by the repository Pull Request settings", + }, + "merge with rebase: overridden by command: rebase not allowed": { + allowMerge: true, + allowRebase: false, + allowSquash: true, + mergeMethodOption: "rebase", + expMethod: "", + expErr: "Merge method 'rebase' is not allowed by the repository Pull Request settings", + }, + "merge with squash: overridden by command: squash not allowed": { + allowMerge: true, + allowRebase: true, + allowSquash: false, + mergeMethodOption: "squash", + expMethod: "", + expErr: "Merge method 'squash' is not allowed by the repository Pull Request settings", + }, + "merge with unknown: overridden by command: unknown doesn't exist": { + allowMerge: true, + allowRebase: true, + allowSquash: true, + mergeMethodOption: "unknown", + expMethod: "", + expErr: "Merge method 'unknown' is unknown. Specify one of the valid values: 'merge, rebase, squash'", + }, } for name, c := range cases { @@ -1086,9 +1141,14 @@ func TestGithubClient_MergePullCorrectMethod(t *testing.T) { Num: 1, }, models.PullRequestOptions{ DeleteSourceBranchOnMerge: false, + MergeMethod: c.mergeMethodOption, }) - Ok(t, err) + if c.expErr == "" { + Ok(t, err) + } else { + ErrContains(t, c.expErr, err) + } }) } }