Skip to content

Commit 66ddbf2

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. Signed-off-by: Leonard Sheng Sheng Lee <305414+sheeeng@users.noreply.github.com>
1 parent 7bcb796 commit 66ddbf2

4 files changed

+249
-16
lines changed

‎github/resource_github_actions_organization_permissions.go‎

Lines changed: 79 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,10 @@ func resourceGithubActionsEnabledRepositoriesObject(d *schema.ResourceData) ([]i
137137
return enabled, nil
138138
}
139139

140-
func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.ResourceData, meta any) error {
140+
func resourceGithubActionsOrganizationPermissionsCreate(d *schema.ResourceData, meta any) error {
141141
client := meta.(*Owner).v3client
142142
orgName := meta.(*Owner).name
143143
ctx := context.Background()
144-
if !d.IsNewResource() {
145-
ctx = context.WithValue(ctx, ctxId, d.Id())
146-
}
147144

148145
err := checkOrganization(meta)
149146
if err != nil {
@@ -158,8 +155,20 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
158155
EnabledRepositories: &enabledRepositories,
159156
}
160157

161-
if v, ok := d.GetOk("sha_pinning_required"); ok {
162-
actionsPermissions.SHAPinningRequired = new(v.(bool))
158+
// Use `GetOkExists` for `sha_pinning_required` to detect explicit
159+
// `false` values.
160+
//
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,68 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
201210
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
202211
}
203212

213+
func resourceGithubActionsOrganizationPermissionsUpdate(d *schema.ResourceData, meta any) error {
214+
client := meta.(*Owner).v3client
215+
orgName := meta.(*Owner).name
216+
ctx := context.WithValue(context.Background(), ctxId, d.Id())
217+
218+
err := checkOrganization(meta)
219+
if err != nil {
220+
return err
221+
}
222+
223+
allowedActions := d.Get("allowed_actions").(string)
224+
enabledRepositories := d.Get("enabled_repositories").(string)
225+
226+
actionsPermissions := github.ActionsPermissions{
227+
AllowedActions: &allowedActions,
228+
EnabledRepositories: &enabledRepositories,
229+
}
230+
231+
// Use `HasChange` + `Get` for `sha_pinning_required` to send the
232+
// value only when it changes.
233+
if d.HasChange("sha_pinning_required") {
234+
actionsPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
235+
}
236+
237+
_, _, err = client.Actions.UpdateActionsPermissions(ctx,
238+
orgName,
239+
actionsPermissions)
240+
if err != nil {
241+
return err
242+
}
243+
244+
if allowedActions == "selected" {
245+
actionsAllowedData := resourceGithubActionsOrganizationAllowedObject(d)
246+
if actionsAllowedData != nil {
247+
log.Printf("[DEBUG] Allowed actions config is set")
248+
_, _, err = client.Actions.UpdateActionsAllowed(ctx,
249+
orgName,
250+
*actionsAllowedData)
251+
if err != nil {
252+
return err
253+
}
254+
} else {
255+
log.Printf("[DEBUG] Allowed actions config not set, skipping")
256+
}
257+
}
258+
259+
if enabledRepositories == "selected" {
260+
enabledReposData, err := resourceGithubActionsEnabledRepositoriesObject(d)
261+
if err != nil {
262+
return err
263+
}
264+
_, err = client.Actions.SetEnabledReposInOrg(ctx,
265+
orgName,
266+
enabledReposData)
267+
if err != nil {
268+
return err
269+
}
270+
}
271+
272+
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
273+
}
274+
204275
func resourceGithubActionsOrganizationPermissionsRead(d *schema.ResourceData, meta any) error {
205276
client := meta.(*Owner).v3client
206277
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: 71 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,12 @@ func resourceGithubActionsRepositoryAllowedObject(d *schema.ResourceData) *githu
108108
return allowed
109109
}
110110

111-
func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.ResourceData, meta any) error {
111+
func resourceGithubActionsRepositoryPermissionsCreate(d *schema.ResourceData, meta any) error {
112112
client := meta.(*Owner).v3client
113113

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

121118
allowedActions := d.Get("allowed_actions").(string)
122119
enabled := d.Get("enabled").(bool)
@@ -131,8 +128,20 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
131128
repoActionPermissions.AllowedActions = &allowedActions
132129
}
133130

134-
if v, ok := d.GetOk("sha_pinning_required"); ok {
135-
repoActionPermissions.SHAPinningRequired = new(v.(bool))
131+
// Use `GetOkExists` for `sha_pinning_required` to detect explicit
132+
// `false` values.
133+
//
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,60 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
164173
return resourceGithubActionsRepositoryPermissionsRead(d, meta)
165174
}
166175

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

‎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)