Skip to content

Commit b07ed84

Browse files
deigadeiga
committed
fixup! Address review comments
Signed-off-by: Timo Sand <[email protected]>
1 parent 08a8c4c commit b07ed84

5 files changed

Lines changed: 75 additions & 69 deletions

‎github/repository_utils.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ import (
99
"strings"
1010

1111
"github.com/google/go-github/v82/github"
12+
"github.com/hashicorp/terraform-plugin-log/tflog"
1213
)
1314

1415
// checkRepositoryBranchExists tests if a branch exists in a repository.
15-
func checkRepositoryBranchExists(client *github.Client, owner, repo, branch string) error {
16-
ctx := context.WithValue(context.Background(), ctxId, buildTwoPartID(repo, branch))
16+
func checkRepositoryBranchExists(ctx context.Context, client *github.Client, owner, repo, branch string) error {
17+
tflog.Debug(ctx, "Checking if branch exists", map[string]any{
18+
"branch": branch,
19+
"owner": owner,
20+
"repo": repo,
21+
})
1722
_, _, err := client.Repositories.GetBranch(ctx, owner, repo, branch, 2)
1823
if err != nil {
1924
var ghErr *github.ErrorResponse

‎github/resource_github_repository_file.go‎

Lines changed: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,17 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
177177
checkOpt := github.RepositoryContentGetOptions{}
178178

179179
repoInfo, _, err := client.Repositories.Get(ctx, owner, repo)
180+
if err != nil {
181+
return diag.FromErr(err)
182+
}
183+
var branch string
180184

181-
if branch, ok := d.GetOk("branch"); ok {
185+
if branchFieldVal, ok := d.GetOk("branch"); ok {
186+
branch = branchFieldVal.(string)
182187
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
183-
"branch": branch.(string),
188+
"branch": branch,
184189
})
185-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
190+
if err := checkRepositoryBranchExists(ctx, client, owner, repo, branch); err != nil {
186191
if d.Get("autocreate_branch").(bool) {
187192
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
188193
return diag.FromErr(err)
@@ -191,14 +196,9 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
191196
return diag.FromErr(err)
192197
}
193198
}
194-
checkOpt.Ref = branch.(string)
199+
checkOpt.Ref = branch
195200
} else {
196-
if err != nil {
197-
return diag.FromErr(err)
198-
}
199-
if err := d.Set("branch", repoInfo.GetDefaultBranch()); err != nil {
200-
return diag.FromErr(err)
201-
}
201+
branch = repoInfo.GetDefaultBranch()
202202
}
203203

204204
if err := d.Set("repository_id", int(repoInfo.GetID())); err != nil {
@@ -243,12 +243,17 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
243243
return diag.FromErr(err)
244244
}
245245

246-
branch := d.Get("branch").(string)
246+
if err := d.Set("branch", branch); err != nil {
247+
return diag.FromErr(err)
248+
}
247249

248250
newResourceID, err := buildID(repo, file, branch)
249251
if err != nil {
250252
return diag.FromErr(err)
251253
}
254+
tflog.Debug(ctx, "Setting ID", map[string]any{
255+
"id": newResourceID,
256+
})
252257
d.SetId(newResourceID)
253258
if err = d.Set("commit_sha", create.GetSHA()); err != nil {
254259
return diag.FromErr(err)
@@ -261,34 +266,34 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
261266
client := meta.(*Owner).v3client
262267
owner := meta.(*Owner).name
263268

264-
repo, file, _, err := parseID3(d.Id())
265-
if err != nil {
266-
return diag.FromErr(err)
269+
repoName, ok := d.Get("repository").(string)
270+
if !ok {
271+
return diag.FromErr(fmt.Errorf("repository not found or is not a string"))
272+
}
273+
file, ok := d.Get("file").(string)
274+
if !ok {
275+
return diag.FromErr(fmt.Errorf("file not found or is not a string"))
267276
}
268277

269-
ctx = tflog.SetField(ctx, "repository", repo)
278+
ctx = tflog.SetField(ctx, "repository", repoName)
270279
ctx = tflog.SetField(ctx, "file", file)
271280
ctx = tflog.SetField(ctx, "owner", owner)
272281

273282
opts := &github.RepositoryContentGetOptions{}
274283

275-
if branch, ok := d.GetOk("branch"); ok {
276-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
277-
"branch": branch.(string),
278-
})
279-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
280-
if d.Get("autocreate_branch").(bool) {
281-
branch = d.Get("autocreate_branch_source_branch").(string)
282-
} else {
283-
tflog.Info(ctx, "Removing repository path from state because the branch no longer exists in GitHub")
284-
d.SetId("")
285-
return nil
286-
}
284+
branch := d.Get("branch").(string)
285+
if err := checkRepositoryBranchExists(ctx, client, owner, repoName, branch); err != nil {
286+
if d.Get("autocreate_branch").(bool) {
287+
branch = d.Get("autocreate_branch_source_branch").(string)
288+
} else {
289+
tflog.Info(ctx, "Removing repository path from state because the branch no longer exists in GitHub")
290+
d.SetId("")
291+
return nil
287292
}
288-
opts.Ref = branch.(string)
289293
}
294+
opts.Ref = branch
290295

291-
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repo, file, opts)
296+
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repoName, file, opts)
292297
if err != nil {
293298
var errorResponse *github.ErrorResponse
294299
if errors.As(err, &errorResponse) && errorResponse.Response.StatusCode == http.StatusTooManyRequests {
@@ -309,12 +314,7 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
309314
if err = d.Set("content", content); err != nil {
310315
return diag.FromErr(err)
311316
}
312-
if err = d.Set("repository", repo); err != nil {
313-
return diag.FromErr(err)
314-
}
315-
if err = d.Set("file", file); err != nil {
316-
return diag.FromErr(err)
317-
}
317+
318318
if err = d.Set("sha", fc.GetSHA()); err != nil {
319319
return diag.FromErr(err)
320320
}
@@ -339,10 +339,10 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
339339
tflog.Debug(ctx, "Using known commit SHA", map[string]any{
340340
"commit_sha": sha.(string),
341341
})
342-
commit, _, err = client.Repositories.GetCommit(ctx, owner, repo, sha.(string), nil)
342+
commit, _, err = client.Repositories.GetCommit(ctx, owner, repoName, sha.(string), nil)
343343
} else {
344344
tflog.Debug(ctx, "Commit SHA unknown for file, looking for commit")
345-
commit, err = getFileCommit(ctx, client, owner, repo, file, ref)
345+
commit, err = getFileCommit(ctx, client, owner, repoName, file, ref)
346346
}
347347
if err != nil {
348348
return diag.FromErr(err)
@@ -387,18 +387,15 @@ func resourceGithubRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD
387387
ctx = tflog.SetField(ctx, "file", file)
388388
ctx = tflog.SetField(ctx, "owner", owner)
389389

390-
if branch, ok := d.GetOk("branch"); ok {
391-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
392-
"branch": branch.(string),
393-
})
394-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
395-
if d.Get("autocreate_branch").(bool) {
396-
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
397-
return diag.FromErr(err)
398-
}
399-
} else {
390+
branch := d.Get("branch").(string)
391+
392+
if err := checkRepositoryBranchExists(ctx, client, owner, repo, branch); err != nil {
393+
if d.Get("autocreate_branch").(bool) {
394+
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
400395
return diag.FromErr(err)
401396
}
397+
} else {
398+
return diag.FromErr(err)
402399
}
403400
}
404401

@@ -433,22 +430,18 @@ func resourceGithubRepositoryFileDelete(ctx context.Context, d *schema.ResourceD
433430
opts.Message = github.Ptr(fmt.Sprintf("Delete %s", file))
434431
}
435432

436-
if b, ok := d.GetOk("branch"); ok {
437-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
438-
"branch": b.(string),
439-
})
440-
if err := checkRepositoryBranchExists(client, owner, repo, b.(string)); err != nil {
441-
if d.Get("autocreate_branch").(bool) {
442-
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
443-
return diag.FromErr(err)
444-
}
445-
} else {
433+
branch := d.Get("branch").(string)
434+
435+
if err := checkRepositoryBranchExists(ctx, client, owner, repo, branch); err != nil {
436+
if d.Get("autocreate_branch").(bool) {
437+
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
446438
return diag.FromErr(err)
447439
}
440+
} else {
441+
return diag.FromErr(err)
448442
}
449-
branch := b.(string)
450-
opts.Branch = github.Ptr(branch)
451443
}
444+
opts.Branch = github.Ptr(branch)
452445

453446
_, _, err := client.Repositories.DeleteFile(ctx, owner, repo, file, opts)
454447
return diag.FromErr(handleArchivedRepoDelete(err, "repository file", file, owner, repo))
@@ -476,16 +469,16 @@ func resourceGithubRepositoryFileImport(ctx context.Context, d *schema.ResourceD
476469
opts := &github.RepositoryContentGetOptions{}
477470

478471
repoInfo, _, err := client.Repositories.Get(ctx, owner, repo)
472+
if err != nil {
473+
return nil, err
474+
}
479475
if branch != "" {
480476
opts.Ref = branch
481477
if err := d.Set("branch", branch); err != nil {
482478
return nil, err
483479
}
484480
} else {
485481

486-
if err != nil {
487-
return nil, err
488-
}
489482
defaultBranch := repoInfo.GetDefaultBranch()
490483
branch = defaultBranch
491484
if err := d.Set("branch", branch); err != nil {
@@ -504,10 +497,20 @@ func resourceGithubRepositoryFileImport(ctx context.Context, d *schema.ResourceD
504497
return nil, fmt.Errorf("filePath %s is not a file in repository %s/%s or repository is not readable", filePath, owner, repo)
505498
}
506499

500+
if err := d.Set("repository", repo); err != nil {
501+
return nil, err
502+
}
503+
if err := d.Set("file", filePath); err != nil {
504+
return nil, err
505+
}
506+
507507
newResourceID, err := buildID(repo, filePath, branch)
508508
if err != nil {
509509
return nil, err
510510
}
511+
tflog.Debug(ctx, "Setting ID", map[string]any{
512+
"id": newResourceID,
513+
})
511514
d.SetId(newResourceID)
512515
if err = d.Set("overwrite_on_create", false); err != nil {
513516
return nil, err

‎github/resource_github_repository_file_migration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
// TODO: Enable this test once we have a way to mock the GitHub API
1+
package github
22

3-
// package github
3+
// TODO: Enable this test once we have a way to mock the GitHub API
44

55
// import (
66
// "context"

‎github/resource_github_repository_file_test.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,6 @@ func TestAccGithubRepositoryFile(t *testing.T) {
418418
ResourceName: "github_repository_file.test",
419419
ImportState: true,
420420
ImportStateVerify: true,
421-
ImportStateId: fmt.Sprintf("%s:%s:", repoName, "test"),
422421
ImportStateVerifyIgnore: []string{"commit_author", "commit_email"}, // For some reason `d` doesn't contain the commit author and email when importing.
423422
},
424423
},
@@ -468,7 +467,6 @@ func TestAccGithubRepositoryFile(t *testing.T) {
468467
ResourceName: "github_repository_file.test",
469468
ImportState: true,
470469
ImportStateVerify: true,
471-
ImportStateId: fmt.Sprintf("%s:%s:main", repoName, "test"),
472470
ImportStateVerifyIgnore: []string{"commit_author", "commit_email"}, // For some reason `d` doesn't contain the commit author and email when importing.
473471
},
474472
},

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ go 1.24.4
44

55
require (
66
github.com/go-jose/go-jose/v3 v3.0.4
7-
github.com/google/go-cmp v0.7.0
87
github.com/google/go-github/v82 v82.0.0
98
github.com/google/uuid v1.6.0
109
github.com/hashicorp/go-cty v1.5.0
@@ -22,6 +21,7 @@ require (
2221
github.com/cloudflare/circl v1.6.1 // indirect
2322
github.com/fatih/color v1.18.0 // indirect
2423
github.com/golang/protobuf v1.5.4 // indirect
24+
github.com/google/go-cmp v0.7.0 // indirect
2525
github.com/google/go-querystring v1.2.0 // indirect
2626
github.com/hashicorp/errwrap v1.0.0 // indirect
2727
github.com/hashicorp/go-checkpoint v0.5.0 // indirect

0 commit comments

Comments
 (0)