diff --git a/github/resource_github_repository.go b/github/resource_github_repository.go index fe7db447fc..2c57b54182 100644 --- a/github/resource_github_repository.go +++ b/github/resource_github_repository.go @@ -287,8 +287,8 @@ func resourceGithubRepository() *schema.Resource { "web_commit_signoff_required": { Type: schema.TypeBool, Optional: true, - Default: false, - Description: "Require contributors to sign off on web-based commits. Defaults to 'false'.", + Computed: true, + Description: "Require contributors to sign off on web-based commits.", }, "auto_init": { Type: schema.TypeBool, @@ -586,29 +586,28 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository { visibility := calculateVisibility(d) repository := &github.Repository{ - Name: github.Ptr(d.Get("name").(string)), - Description: github.Ptr(d.Get("description").(string)), - Homepage: github.Ptr(d.Get("homepage_url").(string)), - Visibility: github.Ptr(visibility), - HasDownloads: github.Ptr(d.Get("has_downloads").(bool)), - HasIssues: github.Ptr(d.Get("has_issues").(bool)), - HasDiscussions: github.Ptr(d.Get("has_discussions").(bool)), - HasProjects: github.Ptr(d.Get("has_projects").(bool)), - HasWiki: github.Ptr(d.Get("has_wiki").(bool)), - IsTemplate: github.Ptr(d.Get("is_template").(bool)), - AllowMergeCommit: github.Ptr(d.Get("allow_merge_commit").(bool)), - AllowSquashMerge: github.Ptr(d.Get("allow_squash_merge").(bool)), - AllowRebaseMerge: github.Ptr(d.Get("allow_rebase_merge").(bool)), - AllowAutoMerge: github.Ptr(d.Get("allow_auto_merge").(bool)), - DeleteBranchOnMerge: github.Ptr(d.Get("delete_branch_on_merge").(bool)), - WebCommitSignoffRequired: github.Ptr(d.Get("web_commit_signoff_required").(bool)), - AutoInit: github.Ptr(d.Get("auto_init").(bool)), - LicenseTemplate: github.Ptr(d.Get("license_template").(string)), - GitignoreTemplate: github.Ptr(d.Get("gitignore_template").(string)), - Archived: github.Ptr(d.Get("archived").(bool)), - Topics: expandStringList(d.Get("topics").(*schema.Set).List()), - AllowUpdateBranch: github.Ptr(d.Get("allow_update_branch").(bool)), - SecurityAndAnalysis: calculateSecurityAndAnalysis(d), + Name: github.Ptr(d.Get("name").(string)), + Description: github.Ptr(d.Get("description").(string)), + Homepage: github.Ptr(d.Get("homepage_url").(string)), + Visibility: github.Ptr(visibility), + HasDownloads: github.Ptr(d.Get("has_downloads").(bool)), + HasIssues: github.Ptr(d.Get("has_issues").(bool)), + HasDiscussions: github.Ptr(d.Get("has_discussions").(bool)), + HasProjects: github.Ptr(d.Get("has_projects").(bool)), + HasWiki: github.Ptr(d.Get("has_wiki").(bool)), + IsTemplate: github.Ptr(d.Get("is_template").(bool)), + AllowMergeCommit: github.Ptr(d.Get("allow_merge_commit").(bool)), + AllowSquashMerge: github.Ptr(d.Get("allow_squash_merge").(bool)), + AllowRebaseMerge: github.Ptr(d.Get("allow_rebase_merge").(bool)), + AllowAutoMerge: github.Ptr(d.Get("allow_auto_merge").(bool)), + DeleteBranchOnMerge: github.Ptr(d.Get("delete_branch_on_merge").(bool)), + AutoInit: github.Ptr(d.Get("auto_init").(bool)), + LicenseTemplate: github.Ptr(d.Get("license_template").(string)), + GitignoreTemplate: github.Ptr(d.Get("gitignore_template").(string)), + Archived: github.Ptr(d.Get("archived").(bool)), + Topics: expandStringList(d.Get("topics").(*schema.Set).List()), + AllowUpdateBranch: github.Ptr(d.Get("allow_update_branch").(bool)), + SecurityAndAnalysis: calculateSecurityAndAnalysis(d), } // only configure merge commit if we are in commit merge strategy @@ -638,6 +637,15 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository { } } + // only configure web commit signoff if explicitly set in the configuration + if d.IsNewResource() || d.HasChange("web_commit_signoff_required") { + if webCommitSignoffRequired, ok := d.GetOkExists("web_commit_signoff_required"); ok { //nolint:staticcheck,SA1019 // We sometimes need to use GetOkExists for booleans + if val, ok := webCommitSignoffRequired.(bool); ok { + repository.WebCommitSignoffRequired = github.Ptr(val) + } + } + } + return repository } @@ -947,20 +955,6 @@ func resourceGithubRepositoryUpdate(ctx context.Context, d *schema.ResourceData, owner := meta.(*Owner).name ctx = context.WithValue(ctx, ctxId, d.Id()) - // When the organization has "Require sign off on web-based commits" enabled, - // the API doesn't allow you to send `web_commit_signoff_required` in order to - // update the repository with this field or it will throw a 422 error. - // As a workaround, we check if the organization requires it, and if so, - // we remove the field from the request. - if d.HasChange("web_commit_signoff_required") && meta.(*Owner).IsOrganization { - organization, _, err := client.Organizations.Get(ctx, owner) - if err == nil { - if organization != nil && organization.GetWebCommitSignoffRequired() { - repoReq.WebCommitSignoffRequired = nil - } - } - } - repo, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) if err != nil { return diag.FromErr(err) @@ -1043,8 +1037,6 @@ func resourceGithubRepositoryDelete(ctx context.Context, d *schema.ResourceData, return diag.FromErr(err) } repoReq := resourceGithubRepositoryObject(d) - // Always remove `web_commit_signoff_required` when archiving, to avoid 422 error - repoReq.WebCommitSignoffRequired = nil log.Printf("[DEBUG] Archiving repository on delete: %s/%s", owner, repoName) _, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq) return diag.FromErr(err) diff --git a/github/resource_github_repository_test.go b/github/resource_github_repository_test.go index d0c363ea2d..47d300ddb8 100644 --- a/github/resource_github_repository_test.go +++ b/github/resource_github_repository_test.go @@ -1260,6 +1260,111 @@ resource "github_repository" "test" { }) }) + t.Run("check_web_commit_signoff_required_enabled", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%scommit-signoff-%s", testResourcePrefix, randomID) + config := ` + resource "github_repository" "test" { + name = "%s" + auto_init = true + web_commit_signoff_required = %s + } + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName, "true"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "true"), + ), + }, + }, + }) + }) + + t.Run("check_web_commit_signoff_required_disabled", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%scommit-signoff-%s", testResourcePrefix, randomID) + config := ` + resource "github_repository" "test" { + name = "%s" + auto_init = true + web_commit_signoff_required = %s + } + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName, "false"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "false"), + ), + }, + }, + }) + }) + + t.Run("check_web_commit_signoff_required_not_set", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%scommit-signoff-%s", testResourcePrefix, randomID) + config := ` + resource "github_repository" "test" { + name = "%s" + auto_init = true + } + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "false"), + ), + }, + }, + }) + }) + + t.Run("check_web_commit_signoff_required_organization_enabled_but_not_set", func(t *testing.T) { + t.Skip("This test should be run manually after confirming that the test organization has 'Require contributors to sign off on web-based commits' enabled under Organizations -> Settings -> Repository -> Repository defaults.") + + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testRepoName := fmt.Sprintf("%scommit-signoff-%s", testResourcePrefix, randomID) + + config := ` + resource "github_repository" "test" { + name = "%s" + description = "%s" + visibility = "private" + } + ` + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: fmt.Sprintf(config, testRepoName, "foo"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("github_repository.test", "web_commit_signoff_required", "true"), + ), + }, + { + Config: fmt.Sprintf(config, testRepoName, "bar"), + }, + }, + }) + }) + t.Run("check_allow_forking_not_set", func(t *testing.T) { t.Skip("This test should be run manually after confirming that the test organization has been correctly configured to disable setting forking at the repo level.") diff --git a/website/docs/r/repository.html.markdown b/website/docs/r/repository.html.markdown index 864c2596a2..157fd01caa 100644 --- a/website/docs/r/repository.html.markdown +++ b/website/docs/r/repository.html.markdown @@ -112,7 +112,7 @@ The following arguments are supported: * `delete_branch_on_merge` - (Optional) Automatically delete head branch after a pull request is merged. Defaults to `false`. -* `web_commit_signoff_required` - (Optional) Require contributors to sign off on web-based commits. See more [here](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-the-commit-signoff-policy-for-your-repository). Defaults to `false`. +* `web_commit_signoff_required` - (Optional) Require contributors to sign off on web-based commits. See more [here](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/managing-the-commit-signoff-policy-for-your-repository). * `has_downloads` - (**DEPRECATED**) (Optional) Set to `true` to enable the (deprecated) downloads features on the repository. This attribute is no longer in use, but it hasn't been removed yet. It will be removed in a future version. See [this discussion](https://github.com/orgs/community/discussions/102145#discussioncomment-8351756).