Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 33 additions & 41 deletions github/resource_github_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
105 changes: 105 additions & 0 deletions github/resource_github_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Comment thread
steveteuber marked this conversation as resolved.
),
},
{
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.")

Expand Down
2 changes: 1 addition & 1 deletion website/docs/r/repository.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down