From 16b6e5efb3f18e1b85c0f54e9f37096d0650ac8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20Delaune?= Date: Sat, 12 Aug 2023 00:18:37 +0200 Subject: [PATCH] Do not change squash_merge/merge_commit if it is not allowed in conf (#1834) Co-authored-by: Keegan Campbell --- github/resource_github_repository.go | 66 ++++++++++------ github/resource_github_repository_test.go | 96 +++++++++++++++++++++++ 2 files changed, 136 insertions(+), 26 deletions(-) diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index 6d030a4761..37d44e9432 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -465,32 +465,28 @@ func calculateSecurityAndAnalysis(d *schema.ResourceData) *github.SecurityAndAna func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository { return &github.Repository{ - Name: github.String(d.Get("name").(string)), - Description: github.String(d.Get("description").(string)), - Homepage: github.String(d.Get("homepage_url").(string)), - Visibility: github.String(calculateVisibility(d)), - HasDownloads: github.Bool(d.Get("has_downloads").(bool)), - HasIssues: github.Bool(d.Get("has_issues").(bool)), - HasDiscussions: github.Bool(d.Get("has_discussions").(bool)), - HasProjects: github.Bool(d.Get("has_projects").(bool)), - HasWiki: github.Bool(d.Get("has_wiki").(bool)), - IsTemplate: github.Bool(d.Get("is_template").(bool)), - AllowMergeCommit: github.Bool(d.Get("allow_merge_commit").(bool)), - AllowSquashMerge: github.Bool(d.Get("allow_squash_merge").(bool)), - AllowRebaseMerge: github.Bool(d.Get("allow_rebase_merge").(bool)), - AllowAutoMerge: github.Bool(d.Get("allow_auto_merge").(bool)), - SquashMergeCommitTitle: github.String(d.Get("squash_merge_commit_title").(string)), - SquashMergeCommitMessage: github.String(d.Get("squash_merge_commit_message").(string)), - MergeCommitTitle: github.String(d.Get("merge_commit_title").(string)), - MergeCommitMessage: github.String(d.Get("merge_commit_message").(string)), - DeleteBranchOnMerge: github.Bool(d.Get("delete_branch_on_merge").(bool)), - AutoInit: github.Bool(d.Get("auto_init").(bool)), - LicenseTemplate: github.String(d.Get("license_template").(string)), - GitignoreTemplate: github.String(d.Get("gitignore_template").(string)), - Archived: github.Bool(d.Get("archived").(bool)), - Topics: expandStringList(d.Get("topics").(*schema.Set).List()), - AllowUpdateBranch: github.Bool(d.Get("allow_update_branch").(bool)), - SecurityAndAnalysis: calculateSecurityAndAnalysis(d), + Name: github.String(d.Get("name").(string)), + Description: github.String(d.Get("description").(string)), + Homepage: github.String(d.Get("homepage_url").(string)), + Visibility: github.String(calculateVisibility(d)), + HasDownloads: github.Bool(d.Get("has_downloads").(bool)), + HasIssues: github.Bool(d.Get("has_issues").(bool)), + HasDiscussions: github.Bool(d.Get("has_discussions").(bool)), + HasProjects: github.Bool(d.Get("has_projects").(bool)), + HasWiki: github.Bool(d.Get("has_wiki").(bool)), + IsTemplate: github.Bool(d.Get("is_template").(bool)), + AllowMergeCommit: github.Bool(d.Get("allow_merge_commit").(bool)), + AllowSquashMerge: github.Bool(d.Get("allow_squash_merge").(bool)), + AllowRebaseMerge: github.Bool(d.Get("allow_rebase_merge").(bool)), + AllowAutoMerge: github.Bool(d.Get("allow_auto_merge").(bool)), + DeleteBranchOnMerge: github.Bool(d.Get("delete_branch_on_merge").(bool)), + AutoInit: github.Bool(d.Get("auto_init").(bool)), + LicenseTemplate: github.String(d.Get("license_template").(string)), + GitignoreTemplate: github.String(d.Get("gitignore_template").(string)), + Archived: github.Bool(d.Get("archived").(bool)), + Topics: expandStringList(d.Get("topics").(*schema.Set).List()), + AllowUpdateBranch: github.Bool(d.Get("allow_update_branch").(bool)), + SecurityAndAnalysis: calculateSecurityAndAnalysis(d), } } @@ -523,6 +519,24 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta interface{}) er } } + // only configure merge commit if we are in commit merge strategy + allowMergeCommit, ok := d.Get("allow_merge_commit").(bool) + if ok { + if allowMergeCommit { + repoReq.MergeCommitTitle = github.String(d.Get("merge_commit_title").(string)) + repoReq.MergeCommitMessage = github.String(d.Get("merge_commit_message").(string)) + } + } + + // only configure squash commit if we are in squash merge strategy + allowSquashMerge, ok := d.Get("allow_squash_merge").(bool) + if ok { + if allowSquashMerge { + repoReq.SquashMergeCommitTitle = github.String(d.Get("squash_merge_commit_title").(string)) + repoReq.SquashMergeCommitMessage = github.String(d.Get("squash_merge_commit_message").(string)) + } + } + repoReq.Private = github.Bool(isPrivate) if template, ok := d.GetOk("template"); ok { diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index a1fb4528ab..05a28fb2b3 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -770,6 +770,102 @@ func TestAccGithubRepositories(t *testing.T) { }) }) + + t.Run("modify merge commit strategy without error", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_repository" "test" { + + name = "tf-acc-test-modify-co-str-%[1]s" + allow_merge_commit = true + merge_commit_title = "PR_TITLE" + merge_commit_message = "BLANK" + } + `, randomID) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "github_repository.test", "merge_commit_title", + "PR_TITLE", + ), + resource.TestCheckResourceAttr( + "github_repository.test", "merge_commit_message", + "BLANK", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + + t.Run("modify squash merge strategy without error", func(t *testing.T) { + config := fmt.Sprintf(` + resource "github_repository" "test" { + name = "tf-acc-test-modify-sq-str-%[1]s" + allow_squash_merge = true + squash_merge_commit_title = "PR_TITLE" + squash_merge_commit_message = "BLANK" + } + `, randomID) + + check := resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "github_repository.test", "squash_merge_commit_title", + "PR_TITLE", + ), + resource.TestCheckResourceAttr( + "github_repository.test", "squash_merge_commit_message", + "BLANK", + ), + ) + + testCase := func(t *testing.T, mode string) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnlessMode(t, mode) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: config, + Check: check, + }, + }, + }) + } + + t.Run("with an anonymous account", func(t *testing.T) { + t.Skip("anonymous account not supported for this operation") + }) + + t.Run("with an individual account", func(t *testing.T) { + testCase(t, individual) + }) + + t.Run("with an organization account", func(t *testing.T) { + testCase(t, organization) + }) + }) + } func TestAccGithubRepositoryPages(t *testing.T) {