Skip to content

Commit f179762

Browse files
oda251claude
andcommitted
fix: address review feedback for code_scanning_default_setup resource
- Replace ForceNew with diffRepository + repository_id for rename support - Add archived repo check in Create/Update - Handle 404 in Read (remove from state) and Delete (return nil) - Use tflog instead of log.Printf - Simplify Import to only set repository (Read is called automatically) - Add nil check in waitForCodeScanningState to prevent panic - Migrate tests to ConfigStateChecks, consolidate redundant cases Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
1 parent f26ff06 commit f179762

2 files changed

Lines changed: 138 additions & 126 deletions

File tree

github/resource_github_repository_code_scanning_default_setup.go

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import (
44
"context"
55
"errors"
66
"fmt"
7-
"log"
7+
"net/http"
88
"time"
99

1010
"github.com/google/go-github/v84/github"
11+
"github.com/hashicorp/terraform-plugin-log/tflog"
1112
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
1213
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
1314
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -30,13 +31,19 @@ func resourceGithubRepositoryCodeScanningDefaultSetup() *schema.Resource {
3031
Delete: schema.DefaultTimeout(5 * time.Minute),
3132
},
3233

34+
CustomizeDiff: diffRepository,
35+
3336
Schema: map[string]*schema.Schema{
3437
"repository": {
3538
Type: schema.TypeString,
3639
Required: true,
37-
ForceNew: true,
3840
Description: "The GitHub repository name.",
3941
},
42+
"repository_id": {
43+
Type: schema.TypeInt,
44+
Computed: true,
45+
Description: "The ID of the GitHub repository.",
46+
},
4047
"state": {
4148
Type: schema.TypeString,
4249
Required: true,
@@ -73,6 +80,14 @@ func resourceGithubRepositoryCodeScanningDefaultSetupCreateOrUpdate(ctx context.
7380
repoName := d.Get("repository").(string)
7481
state := d.Get("state").(string)
7582

83+
repo, _, err := client.Repositories.Get(ctx, owner, repoName)
84+
if err != nil {
85+
return diag.Errorf("error reading repository %s/%s: %s", owner, repoName, err)
86+
}
87+
if repo.GetArchived() {
88+
return diag.Errorf("repository %s/%s is archived", owner, repoName)
89+
}
90+
7691
options := &github.UpdateDefaultSetupConfigurationOptions{
7792
State: state,
7893
}
@@ -86,7 +101,7 @@ func resourceGithubRepositoryCodeScanningDefaultSetupCreateOrUpdate(ctx context.
86101
options.Languages = expandStringList(v.(*schema.Set).List())
87102
}
88103

89-
_, _, err := client.CodeScanning.UpdateDefaultSetupConfiguration(ctx, owner, repoName, options)
104+
_, _, err = client.CodeScanning.UpdateDefaultSetupConfiguration(ctx, owner, repoName, options)
90105
if err != nil {
91106
// 202 Accepted is expected — go-github surfaces it as AcceptedError
92107
var acceptedErr *github.AcceptedError
@@ -117,8 +132,21 @@ func resourceGithubRepositoryCodeScanningDefaultSetupRead(ctx context.Context, d
117132
owner := meta.(*Owner).name
118133
repoName := d.Get("repository").(string)
119134

120-
if repoName == "" {
121-
repoName = d.Id()
135+
repo, _, err := client.Repositories.Get(ctx, owner, repoName)
136+
if err != nil {
137+
var ghErr *github.ErrorResponse
138+
if errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusNotFound {
139+
tflog.Info(ctx, "Repository not found, removing from state", map[string]any{
140+
"owner": owner,
141+
"repository": repoName,
142+
})
143+
d.SetId("")
144+
return nil
145+
}
146+
return diag.Errorf("error reading repository %s/%s: %s", owner, repoName, err)
147+
}
148+
if err := d.Set("repository_id", int(repo.GetID())); err != nil {
149+
return diag.Errorf("error setting repository_id: %s", err)
122150
}
123151

124152
config, _, err := client.CodeScanning.GetDefaultSetupConfiguration(ctx, owner, repoName)
@@ -141,16 +169,26 @@ func resourceGithubRepositoryCodeScanningDefaultSetupDelete(ctx context.Context,
141169
_, _, err := client.CodeScanning.UpdateDefaultSetupConfiguration(ctx, owner, repoName, options)
142170
if err != nil {
143171
var acceptedErr *github.AcceptedError
144-
if !errors.As(err, &acceptedErr) {
172+
var ghErr *github.ErrorResponse
173+
switch {
174+
case errors.As(err, &acceptedErr):
175+
// 202 Accepted is expected
176+
case errors.As(err, &ghErr) && ghErr.Response.StatusCode == http.StatusNotFound:
177+
// repository already gone
178+
return nil
179+
default:
145180
return diag.Errorf("error disabling code scanning default setup for %s/%s: %s", owner, repoName, err)
146181
}
147182
}
148183

149-
log.Printf("[INFO] Code scanning default setup disabled for %s/%s", owner, repoName)
184+
tflog.Info(ctx, "Code scanning default setup disabled", map[string]any{
185+
"owner": owner,
186+
"repository": repoName,
187+
})
150188
return nil
151189
}
152190

153-
func resourceGithubRepositoryCodeScanningDefaultSetupImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
191+
func resourceGithubRepositoryCodeScanningDefaultSetupImport(_ context.Context, d *schema.ResourceData, _ any) ([]*schema.ResourceData, error) {
154192
repoName := d.Id()
155193
if repoName == "" {
156194
return nil, fmt.Errorf("repository name must not be empty")
@@ -160,11 +198,6 @@ func resourceGithubRepositoryCodeScanningDefaultSetupImport(ctx context.Context,
160198
return nil, err
161199
}
162200

163-
diags := resourceGithubRepositoryCodeScanningDefaultSetupRead(ctx, d, meta)
164-
if diags.HasError() {
165-
return nil, fmt.Errorf("error importing code scanning default setup for %s: %s", repoName, diags[0].Summary)
166-
}
167-
168201
return []*schema.ResourceData{d}, nil
169202
}
170203

@@ -205,6 +238,9 @@ func waitForCodeScanningState(ctx context.Context, client *github.Client, owner,
205238
if err != nil {
206239
return nil, err
207240
}
241+
if result == nil {
242+
return nil, fmt.Errorf("code scanning default setup returned nil result for %s/%s", owner, repo)
243+
}
208244

209245
return result.(*github.DefaultSetupConfiguration), nil
210246
}

0 commit comments

Comments
 (0)