Skip to content

Commit 59e2bd5

Browse files
steveteuberbzarboni1nickfloyd
authored
fix: commit signoff is enforced by the organization (#2763)
* fix: 5.43 upgrade signoff * fix: 5.43 upgrade signoff * fix: go formatting * fix: go formatting * fix: check organization settings to avoid signoff error * fix: remove web_commit_signoff_required when archive on destroy --------- Co-authored-by: Ben Zarboni <ben.zarboni@focisolutions.com> Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
1 parent d0ea13a commit 59e2bd5

File tree

2 files changed

+126
-1
lines changed

2 files changed

+126
-1
lines changed

github/resource_github_repository.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,20 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta interface{}) er
779779
owner := meta.(*Owner).name
780780
ctx := context.WithValue(context.Background(), ctxId, d.Id())
781781

782+
// When the organization has "Require sign off on web-based commits" enabled,
783+
// the API doesn't allow you to send `web_commit_signoff_required` in order to
784+
// update the repository with this field or it will throw a 422 error.
785+
// As a workaround, we check if the organization requires it, and if so,
786+
// we remove the field from the request.
787+
if d.HasChange("web_commit_signoff_required") && meta.(*Owner).IsOrganization {
788+
organization, _, err := client.Organizations.Get(ctx, owner)
789+
if err == nil {
790+
if organization != nil && organization.GetWebCommitSignoffRequired() {
791+
repoReq.WebCommitSignoffRequired = nil
792+
}
793+
}
794+
}
795+
782796
repo, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq)
783797
if err != nil {
784798
return err
@@ -880,6 +894,8 @@ func resourceGithubRepositoryDelete(d *schema.ResourceData, meta interface{}) er
880894
return err
881895
}
882896
repoReq := resourceGithubRepositoryObject(d)
897+
// Always remove `web_commit_signoff_required` when archiving, to avoid 422 error
898+
repoReq.WebCommitSignoffRequired = nil
883899
log.Printf("[DEBUG] Archiving repository on delete: %s/%s", owner, repoName)
884900
_, _, err := client.Repositories.Edit(ctx, owner, repoName, repoReq)
885901
return err

github/resource_github_repository_test.go

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func TestAccGithubRepositories(t *testing.T) {
9393

9494
})
9595

96-
t.Run("updates a repositories name without error", func(t *testing.T) {
96+
t.Run("updates a repository's name without error", func(t *testing.T) {
9797

9898
oldName := fmt.Sprintf(`tf-acc-test-rename-%[1]s`, randomID)
9999
newName := fmt.Sprintf(`%[1]s-renamed`, oldName)
@@ -1567,6 +1567,115 @@ func TestAccGithubRepositoryVisibility(t *testing.T) {
15671567

15681568
}
15691569

1570+
func TestAccGithubRepositoryWebCommitSignoffRequired(t *testing.T) {
1571+
1572+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
1573+
1574+
t.Run("changes the web_commit_signoff_required attribute for a repository", func(t *testing.T) {
1575+
1576+
config := fmt.Sprintf(`
1577+
resource "github_repository" "test" {
1578+
name = "tf-acc-%s"
1579+
auto_init = true
1580+
web_commit_signoff_required = true
1581+
}
1582+
`, randomID)
1583+
1584+
check := resource.ComposeTestCheckFunc(
1585+
resource.TestCheckResourceAttr(
1586+
"github_repository.test", "web_commit_signoff_required",
1587+
"true",
1588+
),
1589+
)
1590+
1591+
testCase := func(t *testing.T, mode string) {
1592+
resource.Test(t, resource.TestCase{
1593+
PreCheck: func() { skipUnlessMode(t, mode) },
1594+
Providers: testAccProviders,
1595+
Steps: []resource.TestStep{
1596+
{
1597+
Config: config,
1598+
Check: check,
1599+
},
1600+
},
1601+
})
1602+
}
1603+
1604+
t.Run("with an anonymous account", func(t *testing.T) {
1605+
t.Skip("anonymous account not supported for this operation")
1606+
})
1607+
1608+
t.Run("with an individual account", func(t *testing.T) {
1609+
testCase(t, individual)
1610+
})
1611+
1612+
t.Run("with an organization account", func(t *testing.T) {
1613+
testCase(t, organization)
1614+
})
1615+
1616+
})
1617+
1618+
// Test that setting any other setting than web_commit_signoff_required
1619+
// being set, doesn't set the value of web_commit_signoff_required to true
1620+
// or false in the GitHub API call.
1621+
t.Run("changes a non web_commit_signoff_required attribute for a repository", func(t *testing.T) {
1622+
1623+
config := fmt.Sprintf(`
1624+
resource "github_repository" "test" {
1625+
name = "tf-acc-%s"
1626+
auto_init = true
1627+
allow_merge_commit = true
1628+
web_commit_signoff_required = true
1629+
}
1630+
`, randomID)
1631+
1632+
checks := map[string]resource.TestCheckFunc{
1633+
"before": resource.ComposeTestCheckFunc(
1634+
resource.TestCheckResourceAttr(
1635+
"github_repository.test", "web_commit_signoff_required",
1636+
"true",
1637+
),
1638+
),
1639+
"after": resource.ComposeTestCheckFunc(
1640+
resource.TestCheckResourceAttr(
1641+
"github_repository.test", "web_commit_signoff_required",
1642+
"true",
1643+
),
1644+
),
1645+
}
1646+
1647+
testCase := func(t *testing.T, mode string) {
1648+
resource.Test(t, resource.TestCase{
1649+
PreCheck: func() { skipUnlessMode(t, mode) },
1650+
Providers: testAccProviders,
1651+
Steps: []resource.TestStep{
1652+
{
1653+
Config: config,
1654+
Check: checks["before"],
1655+
},
1656+
{
1657+
Config: config,
1658+
Check: checks["after"],
1659+
},
1660+
},
1661+
})
1662+
}
1663+
1664+
t.Run("with an anonymous account", func(t *testing.T) {
1665+
t.Skip("anonymous account not supported for this operation")
1666+
})
1667+
1668+
t.Run("with an individual account", func(t *testing.T) {
1669+
testCase(t, individual)
1670+
})
1671+
1672+
t.Run("with an organization account", func(t *testing.T) {
1673+
testCase(t, organization)
1674+
})
1675+
1676+
})
1677+
}
1678+
15701679
func TestGithubRepositoryTopicPassesValidation(t *testing.T) {
15711680
resource := resourceGithubRepository()
15721681
schema := resource.Schema["topics"].Elem.(*schema.Schema)

0 commit comments

Comments
 (0)