Skip to content

Commit 0dee61f

Browse files
sheeengsheeeng
committed
fix: handle sha_pinning_required=false
Replace d.GetOk() with d.HasChange() || d.IsNewResource() to properly handle boolean false values. The GetOk() method returns ok=false for zero-value booleans, causing sha_pinning_required=false to be silently ignored and never sent to the GitHub API. This fix ensures both true and false values are correctly applied, eliminating perpetual drift when disabling SHA pinning enforcement. Affects github_actions_organization_permissions and github_actions_repository_permissions resources. Fix #3223.
1 parent db26f16 commit 0dee61f

4 files changed

+164
-4
lines changed

‎github/resource_github_actions_organization_permissions.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
158158
EnabledRepositories: &enabledRepositories,
159159
}
160160

161-
if v, ok := d.GetOk("sha_pinning_required"); ok {
162-
actionsPermissions.SHAPinningRequired = github.Ptr(v.(bool))
161+
if d.HasChange("sha_pinning_required") || d.IsNewResource() {
162+
actionsPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
163163
}
164164

165165
_, _, err = client.Actions.UpdateActionsPermissions(ctx,

‎github/resource_github_actions_organization_permissions_test.go‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,76 @@ func TestAccGithubActionsOrganizationPermissions(t *testing.T) {
104104
})
105105
})
106106

107+
t.Run("test setting sha_pinning_required to true", func(t *testing.T) {
108+
enabledRepositories := "all"
109+
110+
config := fmt.Sprintf(`
111+
resource "github_actions_organization_permissions" "test" {
112+
allowed_actions = "all"
113+
enabled_repositories = "%s"
114+
sha_pinning_required = true
115+
}
116+
`, enabledRepositories)
117+
118+
resource.Test(t, resource.TestCase{
119+
PreCheck: func() { skipUnlessHasOrgs(t) },
120+
ProviderFactories: providerFactories,
121+
Steps: []resource.TestStep{
122+
{
123+
Config: config,
124+
Check: resource.ComposeTestCheckFunc(
125+
resource.TestCheckResourceAttr(
126+
"github_actions_organization_permissions.test", "sha_pinning_required", "true",
127+
),
128+
),
129+
},
130+
},
131+
})
132+
})
133+
134+
t.Run("test setting sha_pinning_required to false", func(t *testing.T) {
135+
enabledRepositories := "all"
136+
137+
configTrue := fmt.Sprintf(`
138+
resource "github_actions_organization_permissions" "test" {
139+
allowed_actions = "all"
140+
enabled_repositories = "%s"
141+
sha_pinning_required = true
142+
}
143+
`, enabledRepositories)
144+
145+
configFalse := fmt.Sprintf(`
146+
resource "github_actions_organization_permissions" "test" {
147+
allowed_actions = "all"
148+
enabled_repositories = "%s"
149+
sha_pinning_required = false
150+
}
151+
`, enabledRepositories)
152+
153+
resource.Test(t, resource.TestCase{
154+
PreCheck: func() { skipUnlessHasOrgs(t) },
155+
ProviderFactories: providerFactories,
156+
Steps: []resource.TestStep{
157+
{
158+
Config: configTrue,
159+
Check: resource.ComposeTestCheckFunc(
160+
resource.TestCheckResourceAttr(
161+
"github_actions_organization_permissions.test", "sha_pinning_required", "true",
162+
),
163+
),
164+
},
165+
{
166+
Config: configFalse,
167+
Check: resource.ComposeTestCheckFunc(
168+
resource.TestCheckResourceAttr(
169+
"github_actions_organization_permissions.test", "sha_pinning_required", "false",
170+
),
171+
),
172+
},
173+
},
174+
})
175+
})
176+
107177
t.Run("test setting of organization allowed actions", func(t *testing.T) {
108178
allowedActions := "selected"
109179
enabledRepositories := "all"

‎github/resource_github_actions_repository_permissions.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
131131
repoActionPermissions.AllowedActions = &allowedActions
132132
}
133133

134-
if v, ok := d.GetOk("sha_pinning_required"); ok {
135-
repoActionPermissions.SHAPinningRequired = github.Ptr(v.(bool))
134+
if d.HasChange("sha_pinning_required") || d.IsNewResource() {
135+
repoActionPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
136136
}
137137

138138
_, _, err := client.Repositories.UpdateActionsPermissions(ctx,

‎github/resource_github_actions_repository_permissions_test.go‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,96 @@ func TestAccGithubActionsRepositoryPermissions(t *testing.T) {
9898
})
9999
})
100100

101+
t.Run("test setting sha_pinning_required to true", func(t *testing.T) {
102+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
103+
repoName := fmt.Sprintf("%srepo-act-perms-%s", testResourcePrefix, randomID)
104+
105+
config := fmt.Sprintf(`
106+
resource "github_repository" "test" {
107+
name = "%[1]s"
108+
description = "Terraform acceptance tests %[1]s"
109+
topics = ["terraform", "testing"]
110+
}
111+
112+
resource "github_actions_repository_permissions" "test" {
113+
allowed_actions = "all"
114+
repository = github_repository.test.name
115+
sha_pinning_required = true
116+
}
117+
`, repoName)
118+
119+
resource.Test(t, resource.TestCase{
120+
PreCheck: func() { skipUnauthenticated(t) },
121+
ProviderFactories: providerFactories,
122+
Steps: []resource.TestStep{
123+
{
124+
Config: config,
125+
Check: resource.ComposeTestCheckFunc(
126+
resource.TestCheckResourceAttr(
127+
"github_actions_repository_permissions.test", "sha_pinning_required", "true",
128+
),
129+
),
130+
},
131+
},
132+
})
133+
})
134+
135+
t.Run("test setting sha_pinning_required to false", func(t *testing.T) {
136+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
137+
repoName := fmt.Sprintf("%srepo-act-perms-%s", testResourcePrefix, randomID)
138+
139+
configTrue := fmt.Sprintf(`
140+
resource "github_repository" "test" {
141+
name = "%[1]s"
142+
description = "Terraform acceptance tests %[1]s"
143+
topics = ["terraform", "testing"]
144+
}
145+
146+
resource "github_actions_repository_permissions" "test" {
147+
allowed_actions = "all"
148+
repository = github_repository.test.name
149+
sha_pinning_required = true
150+
}
151+
`, repoName)
152+
153+
configFalse := fmt.Sprintf(`
154+
resource "github_repository" "test" {
155+
name = "%[1]s"
156+
description = "Terraform acceptance tests %[1]s"
157+
topics = ["terraform", "testing"]
158+
}
159+
160+
resource "github_actions_repository_permissions" "test" {
161+
allowed_actions = "all"
162+
repository = github_repository.test.name
163+
sha_pinning_required = false
164+
}
165+
`, repoName)
166+
167+
resource.Test(t, resource.TestCase{
168+
PreCheck: func() { skipUnauthenticated(t) },
169+
ProviderFactories: providerFactories,
170+
Steps: []resource.TestStep{
171+
{
172+
Config: configTrue,
173+
Check: resource.ComposeTestCheckFunc(
174+
resource.TestCheckResourceAttr(
175+
"github_actions_repository_permissions.test", "sha_pinning_required", "true",
176+
),
177+
),
178+
},
179+
{
180+
Config: configFalse,
181+
Check: resource.ComposeTestCheckFunc(
182+
resource.TestCheckResourceAttr(
183+
"github_actions_repository_permissions.test", "sha_pinning_required", "false",
184+
),
185+
),
186+
},
187+
},
188+
})
189+
})
190+
101191
t.Run("test setting of repository allowed actions", func(t *testing.T) {
102192
allowedActions := "selected"
103193
githubOwnedAllowed := true

0 commit comments

Comments
 (0)