Skip to content

Commit 5b44ee6

Browse files
sheeengsheeeng
committed
fix: handle sha_pinning_required=false
Split `CreateOrUpdate` into separate `Create` and `Update` functions to properly handle the `sha_pinning_required` boolean field's zero-value problem: - Use `d.GetOkExists()` in `Create` to only send the field when the user explicitly sets it in config, even to `false`. - Use `d.HasChange()` in `Update` to detect when the value changes and `d.Get()` to read the new value. Keep schema as `Optional` + `Computed` (not `Default: false`) so the resource inherits the organization-level setting when not explicitly configured. This follows the same pattern used by `allow_forking` in `resource_github_repository.go`. Fixes #3223
1 parent 1af72d4 commit 5b44ee6

4 files changed

+254
-14
lines changed

‎github/resource_github_actions_organization_permissions.go‎

Lines changed: 65 additions & 7 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,7 +155,7 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
158155
EnabledRepositories: &enabledRepositories,
159156
}
160157

161-
if v, ok := d.GetOk("sha_pinning_required"); ok {
158+
if v, ok := d.GetOkExists("sha_pinning_required"); ok { //nolint:staticcheck,SA1019 // Use `GetOkExists` to detect explicit false for booleans.
162159
actionsPermissions.SHAPinningRequired = github.Ptr(v.(bool))
163160
}
164161

@@ -201,6 +198,67 @@ func resourceGithubActionsOrganizationPermissionsCreateOrUpdate(d *schema.Resour
201198
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
202199
}
203200

201+
func resourceGithubActionsOrganizationPermissionsUpdate(d *schema.ResourceData, meta any) error {
202+
client := meta.(*Owner).v3client
203+
orgName := meta.(*Owner).name
204+
ctx := context.WithValue(context.Background(), ctxId, d.Id())
205+
206+
err := checkOrganization(meta)
207+
if err != nil {
208+
return err
209+
}
210+
211+
allowedActions := d.Get("allowed_actions").(string)
212+
enabledRepositories := d.Get("enabled_repositories").(string)
213+
214+
actionsPermissions := github.ActionsPermissions{
215+
AllowedActions: &allowedActions,
216+
EnabledRepositories: &enabledRepositories,
217+
}
218+
219+
if d.HasChange("sha_pinning_required") {
220+
actionsPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
221+
}
222+
223+
_, _, err = client.Actions.UpdateActionsPermissions(ctx,
224+
orgName,
225+
actionsPermissions)
226+
if err != nil {
227+
return err
228+
}
229+
230+
if allowedActions == "selected" {
231+
actionsAllowedData := resourceGithubActionsOrganizationAllowedObject(d)
232+
if actionsAllowedData != nil {
233+
log.Printf("[DEBUG] Update `actionsAllowedData` configuration.")
234+
_, _, err = client.Actions.UpdateActionsAllowed(ctx,
235+
orgName,
236+
*actionsAllowedData)
237+
if err != nil {
238+
return err
239+
}
240+
} else {
241+
log.Printf("[DEBUG] Skip updating `actionsAllowedData` because no configuration is set.")
242+
}
243+
}
244+
245+
if enabledRepositories == "selected" {
246+
enabledReposData, err := resourceGithubActionsEnabledRepositoriesObject(d)
247+
if err != nil {
248+
return err
249+
}
250+
_, err = client.Actions.SetEnabledReposInOrg(ctx,
251+
orgName,
252+
enabledReposData)
253+
if err != nil {
254+
return err
255+
}
256+
}
257+
258+
d.SetId(orgName)
259+
return resourceGithubActionsOrganizationPermissionsRead(d, meta)
260+
}
261+
204262
func resourceGithubActionsOrganizationPermissionsRead(d *schema.ResourceData, meta any) error {
205263
client := meta.(*Owner).v3client
206264
ctx := context.Background()

‎github/resource_github_actions_organization_permissions_test.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import (
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
88
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
9+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
10+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
11+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
912
)
1013

1114
func TestAccGithubActionsOrganizationPermissions(t *testing.T) {
@@ -104,6 +107,62 @@ func TestAccGithubActionsOrganizationPermissions(t *testing.T) {
104107
})
105108
})
106109

110+
t.Run("test setting sha_pinning_required to true", func(t *testing.T) {
111+
enabledRepositories := "all"
112+
113+
config := fmt.Sprintf(`
114+
resource "github_actions_organization_permissions" "test" {
115+
allowed_actions = "all"
116+
enabled_repositories = "%s"
117+
sha_pinning_required = true
118+
}
119+
`, enabledRepositories)
120+
121+
resource.Test(t, resource.TestCase{
122+
PreCheck: func() { skipUnlessHasOrgs(t) },
123+
ProviderFactories: providerFactories,
124+
Steps: []resource.TestStep{
125+
{
126+
Config: config,
127+
ConfigStateChecks: []statecheck.StateCheck{
128+
statecheck.ExpectKnownValue("github_actions_organization_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(true)),
129+
},
130+
},
131+
},
132+
})
133+
})
134+
135+
t.Run("test setting sha_pinning_required to false", func(t *testing.T) {
136+
enabledRepositories := "all"
137+
138+
configTmpl := `
139+
resource "github_actions_organization_permissions" "test" {
140+
allowed_actions = "all"
141+
enabled_repositories = "%s"
142+
sha_pinning_required = %t
143+
}
144+
`
145+
146+
resource.Test(t, resource.TestCase{
147+
PreCheck: func() { skipUnlessHasOrgs(t) },
148+
ProviderFactories: providerFactories,
149+
Steps: []resource.TestStep{
150+
{
151+
Config: fmt.Sprintf(configTmpl, enabledRepositories, true),
152+
ConfigStateChecks: []statecheck.StateCheck{
153+
statecheck.ExpectKnownValue("github_actions_organization_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(true)),
154+
},
155+
},
156+
{
157+
Config: fmt.Sprintf(configTmpl, enabledRepositories, false),
158+
ConfigStateChecks: []statecheck.StateCheck{
159+
statecheck.ExpectKnownValue("github_actions_organization_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(false)),
160+
},
161+
},
162+
},
163+
})
164+
})
165+
107166
t.Run("test setting of organization allowed actions", func(t *testing.T) {
108167
allowedActions := "selected"
109168
enabledRepositories := "all"

‎github/resource_github_actions_repository_permissions.go‎

Lines changed: 57 additions & 7 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,7 +128,7 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
131128
repoActionPermissions.AllowedActions = &allowedActions
132129
}
133130

134-
if v, ok := d.GetOk("sha_pinning_required"); ok {
131+
if v, ok := d.GetOkExists("sha_pinning_required"); ok { //nolint:staticcheck,SA1019 // Use `GetOkExists` to detect explicit false for booleans.
135132
repoActionPermissions.SHAPinningRequired = github.Ptr(v.(bool))
136133
}
137134

@@ -164,6 +161,59 @@ func resourceGithubActionsRepositoryPermissionsCreateOrUpdate(d *schema.Resource
164161
return resourceGithubActionsRepositoryPermissionsRead(d, meta)
165162
}
166163

164+
func resourceGithubActionsRepositoryPermissionsUpdate(d *schema.ResourceData, meta any) error {
165+
client := meta.(*Owner).v3client
166+
167+
owner := meta.(*Owner).name
168+
repoName := d.Get("repository").(string)
169+
ctx := context.WithValue(context.Background(), ctxId, d.Id())
170+
171+
allowedActions := d.Get("allowed_actions").(string)
172+
enabled := d.Get("enabled").(bool)
173+
log.Printf("[DEBUG] The `enabled` value of `allowedActions` is %t.", enabled)
174+
175+
repoActionPermissions := github.ActionsPermissionsRepository{
176+
Enabled: &enabled,
177+
}
178+
179+
// Specify `allowed_actions` only if actions are enabled.
180+
if enabled {
181+
repoActionPermissions.AllowedActions = &allowedActions
182+
}
183+
184+
if d.HasChange("sha_pinning_required") {
185+
repoActionPermissions.SHAPinningRequired = github.Ptr(d.Get("sha_pinning_required").(bool))
186+
}
187+
188+
_, _, err := client.Repositories.UpdateActionsPermissions(ctx,
189+
owner,
190+
repoName,
191+
repoActionPermissions,
192+
)
193+
if err != nil {
194+
return err
195+
}
196+
197+
if allowedActions == "selected" {
198+
actionsAllowedData := resourceGithubActionsRepositoryAllowedObject(d)
199+
if actionsAllowedData != nil {
200+
log.Printf("[DEBUG] Update `actionsAllowedData` configuration.")
201+
_, _, err = client.Repositories.EditActionsAllowed(ctx,
202+
owner,
203+
repoName,
204+
*actionsAllowedData)
205+
if err != nil {
206+
return err
207+
}
208+
} else {
209+
log.Printf("[DEBUG] Skip updating `actionsAllowedData` because no configuration is set.")
210+
}
211+
}
212+
213+
d.SetId(repoName)
214+
return resourceGithubActionsRepositoryPermissionsRead(d, meta)
215+
}
216+
167217
func resourceGithubActionsRepositoryPermissionsRead(d *schema.ResourceData, meta any) error {
168218
client := meta.(*Owner).v3client
169219

‎github/resource_github_actions_repository_permissions_test.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ import (
66

77
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
88
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
9+
"github.com/hashicorp/terraform-plugin-testing/knownvalue"
10+
"github.com/hashicorp/terraform-plugin-testing/statecheck"
11+
"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
912
)
1013

1114
func TestAccGithubActionsRepositoryPermissions(t *testing.T) {
@@ -98,6 +101,76 @@ func TestAccGithubActionsRepositoryPermissions(t *testing.T) {
98101
})
99102
})
100103

104+
t.Run("test setting sha_pinning_required to true", func(t *testing.T) {
105+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
106+
repoName := fmt.Sprintf("%srepo-act-perms-%s", testResourcePrefix, randomID)
107+
108+
config := fmt.Sprintf(`
109+
resource "github_repository" "test" {
110+
name = "%[1]s"
111+
description = "Set sha_pinning_required to true for github_repository, %[1]s."
112+
topics = ["terraform", "testing"]
113+
}
114+
115+
resource "github_actions_repository_permissions" "test" {
116+
allowed_actions = "all"
117+
repository = github_repository.test.name
118+
sha_pinning_required = true
119+
}
120+
`, repoName)
121+
122+
resource.Test(t, resource.TestCase{
123+
PreCheck: func() { skipUnauthenticated(t) },
124+
ProviderFactories: providerFactories,
125+
Steps: []resource.TestStep{
126+
{
127+
Config: config,
128+
ConfigStateChecks: []statecheck.StateCheck{
129+
statecheck.ExpectKnownValue("github_actions_repository_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(true)),
130+
},
131+
},
132+
},
133+
})
134+
})
135+
136+
t.Run("test setting sha_pinning_required to false", func(t *testing.T) {
137+
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
138+
repoName := fmt.Sprintf("%srepo-act-perms-%s", testResourcePrefix, randomID)
139+
140+
configTmpl := `
141+
resource "github_repository" "test" {
142+
name = "%[1]s"
143+
description = "Set sha_pinning_required to false for github_repository, %[1]s."
144+
topics = ["terraform", "testing"]
145+
}
146+
147+
resource "github_actions_repository_permissions" "test" {
148+
allowed_actions = "all"
149+
repository = github_repository.test.name
150+
sha_pinning_required = %[2]t
151+
}
152+
`
153+
154+
resource.Test(t, resource.TestCase{
155+
PreCheck: func() { skipUnauthenticated(t) },
156+
ProviderFactories: providerFactories,
157+
Steps: []resource.TestStep{
158+
{
159+
Config: fmt.Sprintf(configTmpl, repoName, true),
160+
ConfigStateChecks: []statecheck.StateCheck{
161+
statecheck.ExpectKnownValue("github_actions_repository_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(true)),
162+
},
163+
},
164+
{
165+
Config: fmt.Sprintf(configTmpl, repoName, false),
166+
ConfigStateChecks: []statecheck.StateCheck{
167+
statecheck.ExpectKnownValue("github_actions_repository_permissions.test", tfjsonpath.New("sha_pinning_required"), knownvalue.Bool(false)),
168+
},
169+
},
170+
},
171+
})
172+
})
173+
101174
t.Run("test setting of repository allowed actions", func(t *testing.T) {
102175
allowedActions := "selected"
103176
githubOwnedAllowed := true

0 commit comments

Comments
 (0)