Skip to content

Commit bb27f7a

Browse files
sheeengsheeeng
committed
fix: detect explicit sha_pinning_required=false
Split CreateOrUpdate into separate Create and Update functions for both `github_actions_organization_permissions` and `github_actions_repository_permissions` resources. In Create, use `GetOkExists` instead of `GetOk` so that an explicit `sha_pinning_required = false` is sent to the API rather than silently dropped. In Update, use `HasChange` + `Get` which avoids the zero-value problem entirely. Fix #3223.
1 parent 7bcb796 commit bb27f7a

4 files changed

Lines changed: 251 additions & 16 deletions

‎github/resource_github_actions_organization_permissions.go‎

Lines changed: 80 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import (
1212

1313
func resourceGithubActionsOrganizationPermissions() *schema.Resource {
1414
return &schema.Resource{
15-
Create: resourceGithubActionsOrganizationPermissionsCreateOrUpdate,
15+
Create: resourceGithubActionsOrganizationPermissionsCreate,
1616
Read: resourceGithubActionsOrganizationPermissionsRead,
17-
Update: resourceGithubActionsOrganizationPermissionsCreateOrUpdate,
17+
Update: resourceGithubActionsOrganizationPermissionsUpdate,
1818
Delete: resourceGithubActionsOrganizationPermissionsDelete,
1919
Importer: &schema.ResourceImporter{
2020
StateContext: schema.ImportStatePassthroughContext,
@@ -137,13 +137,13 @@ func resourceGithubActionsEnabledRepositoriesObject(d *schema.ResourceData) ([]i
137137
return enabled, nil
138138
}
139139

140-
func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.ResourceData, meta any) error {
140+
// resourceGithubActionsOrganizationPermissionsCreate handles resource creation.
141+
// Uses GetOkExists for sha_pinning_required to detect explicit false values.
142+
// Ref: https://github.com/integrations/terraform-provider-github/pull/3224#pullrequestreview-3868068333
143+
func resourceGithubActionsOrganizationPermissionsCreate(d *schema.ResourceData, meta any) error {
141144
client := meta.(*Owner).v3client
142145
orgName := meta.(*Owner).name
143146
ctx := context.Background()
144-
if !d.IsNewResource() {
145-
ctx = context.WithValue(ctx, ctxId, d.Id())
146-
}
147147

148148
err := checkOrganization(meta)
149149
if err != nil {
@@ -158,8 +158,17 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
158158
EnabledRepositories: &enabledRepositories,
159159
}
160160

161-
if v, ok := d.GetOk("sha_pinning_required"); ok {
162-
actionsPermissions.SHAPinningRequired = new(v.(bool))
161+
// The `sha_pinning_required` is an Optional and Computed boolean.
162+
//
163+
// When a user writes `sha_pinning_required = false`,
164+
// `GetOk` returns `(false, false)`. The second `false` argument
165+
// means "not set", which is indistinguishable from the user omitting
166+
// the field entirely. So the explicit false was silently ignored.
167+
//
168+
// `GetOkExists` returns `(false, true)`.
169+
// The `true` value correctly indicates the user did want to set it.
170+
if v, ok := d.GetOkExists("sha_pinning_required"); ok { //nolint:staticcheck
171+
actionsPermissions.SHAPinningRequired = github.Ptr(v.(bool))
163172
}
164173

165174
_, _, err = client.Actions.UpdateActionsPermissions(ctx,
@@ -201,6 +210,69 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
201210
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
202211
}
203212

213+
// resourceGithubActionsOrganizationPermissionsUpdate handles resource updates.
214+
// Uses HasChange + Get for sha_pinning_required to send the value only when it changes.
215+
// Ref: https://github.com/integrations/terraform-provider-github/pull/3224#pullrequestreview-3868068333
216+
func resourceGithubActionsOrganizationPermissionsUpdate(d *schema.ResourceData, meta any) error {
217+
client := meta.(*Owner).v3client
218+
orgName := meta.(*Owner).name
219+
ctx := context.WithValue(context.Background(), ctxId, d.Id())
220+
221+
err := checkOrganization(meta)
222+
if err != nil {
223+
return err
224+
}
225+
226+
allowedActions := d.Get("allowed_actions").(string)
227+
enabledRepositories := d.Get("enabled_repositories").(string)
228+
229+
actionsPermissions := github.ActionsPermissions{
230+
AllowedActions: &allowedActions,
231+
EnabledRepositories: &enabledRepositories,
232+
}
233+
234+
if d.HasChange("sha_pinning_required") {
235+
actionsPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
236+
}
237+
238+
_, _, err = client.Actions.UpdateActionsPermissions(ctx,
239+
orgName,
240+
actionsPermissions)
241+
if err != nil {
242+
return err
243+
}
244+
245+
if allowedActions == "selected" {
246+
actionsAllowedData := resourceGithubActionsOrganizationAllowedObject(d)
247+
if actionsAllowedData != nil {
248+
log.Printf("[DEBUG] Allowed actions config is set")
249+
_, _, err = client.Actions.UpdateActionsAllowed(ctx,
250+
orgName,
251+
*actionsAllowedData)
252+
if err != nil {
253+
return err
254+
}
255+
} else {
256+
log.Printf("[DEBUG] Allowed actions config not set, skipping")
257+
}
258+
}
259+
260+
if enabledRepositories == "selected" {
261+
enabledReposData, err := resourceGithubActionsEnabledRepositoriesObject(d)
262+
if err != nil {
263+
return err
264+
}
265+
_, err = client.Actions.SetEnabledReposInOrg(ctx,
266+
orgName,
267+
enabledReposData)
268+
if err != nil {
269+
return err
270+
}
271+
}
272+
273+
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
274+
}
275+
204276
func resourceGithubActionsOrganizationPermissionsRead(d *schema.ResourceData, meta any) error {
205277
client := meta.(*Owner).v3client
206278
ctx := context.Background()

‎github/resource_github_actions_organization_permissions_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,49 @@ func TestAccGithubActionsOrganizationPermissions(t *testing.T) {
183183
})
184184
})
185185

186+
t.Run("test setting sha_pinning_required to true then updating to false", func(t *testing.T) {
187+
enabledRepositories := "all"
188+
189+
configTrue := fmt.Sprintf(`
190+
resource "github_actions_organization_permissions" "test" {
191+
allowed_actions = "all"
192+
enabled_repositories = "%s"
193+
sha_pinning_required = true
194+
}
195+
`, enabledRepositories)
196+
197+
configFalse := fmt.Sprintf(`
198+
resource "github_actions_organization_permissions" "test" {
199+
allowed_actions = "all"
200+
enabled_repositories = "%s"
201+
sha_pinning_required = false
202+
}
203+
`, enabledRepositories)
204+
205+
resource.Test(t, resource.TestCase{
206+
PreCheck: func() { skipUnlessHasOrgs(t) },
207+
ProviderFactories: providerFactories,
208+
Steps: []resource.TestStep{
209+
{
210+
Config: configTrue,
211+
Check: resource.ComposeTestCheckFunc(
212+
resource.TestCheckResourceAttr(
213+
"github_actions_organization_permissions.test", "sha_pinning_required", "true",
214+
),
215+
),
216+
},
217+
{
218+
Config: configFalse,
219+
Check: resource.ComposeTestCheckFunc(
220+
resource.TestCheckResourceAttr(
221+
"github_actions_organization_permissions.test", "sha_pinning_required", "false",
222+
),
223+
),
224+
},
225+
},
226+
})
227+
})
228+
186229
t.Run("test setting of organization enabled repositories", func(t *testing.T) {
187230
allowedActions := "all"
188231
enabledRepositories := "selected"

‎github/resource_github_actions_repository_permissions.go‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import (
1111

1212
func resourceGithubActionsRepositoryPermissions() *schema.Resource {
1313
return &schema.Resource{
14-
Create: resourceGithubActionsRepositoryPermissionsCreateOrUpdate,
14+
Create: resourceGithubActionsRepositoryPermissionsCreate,
1515
Read: resourceGithubActionsRepositoryPermissionsRead,
16-
Update: resourceGithubActionsRepositoryPermissionsCreateOrUpdate,
16+
Update: resourceGithubActionsRepositoryPermissionsUpdate,
1717
Delete: resourceGithubActionsRepositoryPermissionsDelete,
1818
Importer: &schema.ResourceImporter{
1919
StateContext: schema.ImportStatePassthroughContext,
@@ -108,15 +108,15 @@ func resourceGithubActionsRepositoryAllowedObject(d *schema.ResourceData) *githu
108108
return allowed
109109
}
110110

111-
func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.ResourceData, meta any) error {
111+
// resourceGithubActionsRepositoryPermissionsCreate handles resource creation.
112+
// Uses GetOkExists for sha_pinning_required to detect explicit false values.
113+
// Ref: https://github.com/integrations/terraform-provider-github/pull/3224#pullrequestreview-3868068333
114+
func resourceGithubActionsRepositoryPermissionsCreate(d *schema.ResourceData, meta any) error {
112115
client := meta.(*Owner).v3client
113116

114117
owner := meta.(*Owner).name
115118
repoName := d.Get("repository").(string)
116119
ctx := context.Background()
117-
if !d.IsNewResource() {
118-
ctx = context.WithValue(ctx, ctxId, d.Id())
119-
}
120120

121121
allowedActions := d.Get("allowed_actions").(string)
122122
enabled := d.Get("enabled").(bool)
@@ -131,8 +131,17 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
131131
repoActionPermissions.AllowedActions = &allowedActions
132132
}
133133

134-
if v, ok := d.GetOk("sha_pinning_required"); ok {
135-
repoActionPermissions.SHAPinningRequired = new(v.(bool))
134+
// The `sha_pinning_required` is an Optional and Computed boolean.
135+
//
136+
// When a user writes `sha_pinning_required = false`,
137+
// `GetOk` returns `(false, false)`. The second `false` argument
138+
// means "not set", which is indistinguishable from the user omitting
139+
// the field entirely. So the explicit false was silently ignored.
140+
//
141+
// `GetOkExists` returns `(false, true)`.
142+
// The `true` value correctly indicates the user did want to set it.
143+
if v, ok := d.GetOkExists("sha_pinning_required"); ok { //nolint:staticcheck
144+
repoActionPermissions.SHAPinningRequired = github.Ptr(v.(bool))
136145
}
137146

138147
_, _, err := client.Repositories.UpdateActionsPermissions(ctx,
@@ -164,6 +173,61 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
164173
return resourceGithubActionsRepositoryPermissionsRead(d, meta)
165174
}
166175

176+
// resourceGithubActionsRepositoryPermissionsUpdate handles resource updates.
177+
// Uses HasChange + Get for sha_pinning_required to send the value only when it changes.
178+
// Ref: https://github.com/integrations/terraform-provider-github/pull/3224#pullrequestreview-3868068333
179+
func resourceGithubActionsRepositoryPermissionsUpdate(d *schema.ResourceData, meta any) error {
180+
client := meta.(*Owner).v3client
181+
182+
owner := meta.(*Owner).name
183+
repoName := d.Get("repository").(string)
184+
ctx := context.WithValue(context.Background(), ctxId, d.Id())
185+
186+
allowedActions := d.Get("allowed_actions").(string)
187+
enabled := d.Get("enabled").(bool)
188+
log.Printf("[DEBUG] Actions enabled: %t", enabled)
189+
190+
repoActionPermissions := github.ActionsPermissionsRepository{
191+
Enabled: &enabled,
192+
}
193+
194+
// Only specify `allowed_actions` if actions are enabled
195+
if enabled {
196+
repoActionPermissions.AllowedActions = &allowedActions
197+
}
198+
199+
if d.HasChange("sha_pinning_required") {
200+
repoActionPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
201+
}
202+
203+
_, _, err := client.Repositories.UpdateActionsPermissions(ctx,
204+
owner,
205+
repoName,
206+
repoActionPermissions,
207+
)
208+
if err != nil {
209+
return err
210+
}
211+
212+
if allowedActions == "selected" {
213+
actionsAllowedData := resourceGithubActionsRepositoryAllowedObject(d)
214+
if actionsAllowedData != nil {
215+
log.Printf("[DEBUG] Allowed actions config is set")
216+
_, _, err = client.Repositories.EditActionsAllowed(ctx,
217+
owner,
218+
repoName,
219+
*actionsAllowedData)
220+
if err != nil {
221+
return err
222+
}
223+
} else {
224+
log.Printf("[DEBUG] Allowed actions config not set, skipping")
225+
}
226+
}
227+
228+
return resourceGithubActionsRepositoryPermissionsRead(d, meta)
229+
}
230+
167231
func resourceGithubActionsRepositoryPermissionsRead(d *schema.ResourceData, meta any) error {
168232
client := meta.(*Owner).v3client
169233

‎github/resource_github_actions_repository_permissions_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,62 @@ func TestAccGithubActionsRepositoryPermissions(t *testing.T) {
185185
})
186186
})
187187

188+
t.Run("test setting sha_pinning_required to true then updating to false", func(t *testing.T) {
189+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
190+
repoName := fmt.Sprintf("%srepo-act-perms-%s", testResourcePrefix, randomID)
191+
192+
configTrue := fmt.Sprintf(`
193+
resource "github_repository" "test" {
194+
name = "%[1]s"
195+
description = "Terraform acceptance tests %[1]s"
196+
topics = ["terraform", "testing"]
197+
}
198+
199+
resource "github_actions_repository_permissions" "test" {
200+
allowed_actions = "all"
201+
repository = github_repository.test.name
202+
sha_pinning_required = true
203+
}
204+
`, repoName)
205+
206+
configFalse := fmt.Sprintf(`
207+
resource "github_repository" "test" {
208+
name = "%[1]s"
209+
description = "Terraform acceptance tests %[1]s"
210+
topics = ["terraform", "testing"]
211+
}
212+
213+
resource "github_actions_repository_permissions" "test" {
214+
allowed_actions = "all"
215+
repository = github_repository.test.name
216+
sha_pinning_required = false
217+
}
218+
`, repoName)
219+
220+
resource.Test(t, resource.TestCase{
221+
PreCheck: func() { skipUnauthenticated(t) },
222+
ProviderFactories: providerFactories,
223+
Steps: []resource.TestStep{
224+
{
225+
Config: configTrue,
226+
Check: resource.ComposeTestCheckFunc(
227+
resource.TestCheckResourceAttr(
228+
"github_actions_repository_permissions.test", "sha_pinning_required", "true",
229+
),
230+
),
231+
},
232+
{
233+
Config: configFalse,
234+
Check: resource.ComposeTestCheckFunc(
235+
resource.TestCheckResourceAttr(
236+
"github_actions_repository_permissions.test", "sha_pinning_required", "false",
237+
),
238+
),
239+
},
240+
},
241+
})
242+
})
243+
188244
t.Run("test disabling actions on a repository", func(t *testing.T) {
189245
actionsEnabled := false
190246
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)

0 commit comments

Comments
 (0)