Skip to content

Commit 256cdd8

Browse files
deigadeiga
authored
[MAINT] Fixup github_repository_file (#3175)
* Improve docs of importing `github_repository_file` Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Add missing test for import Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Ensure we only set `branch` on import if it's not the default branch Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Update `branch` to `Computed` field Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Add state migration for missing branch field Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Add ID migration in the same bunch Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Fix ID parsing Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Fix ID parsing Always use 2 part ID for import. Use buildID for resource ID * fixup! fixup! Fix ID parsing Use 3 part ID for import as well * fixup! fixup! fixup! Add computed `repository_id` field to resource Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! fixup! fixup! fixup! Add migration of the `repository_id` field. This required changing all tests to need a mock of the GH API Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Comment out test in evaluation Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Address review comments Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Address review comments Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Remove usage of autocreate_branch in `Read`, `Update` and `Delete` Signed-off-by: Timo Sand <timo.sand@f-secure.com> * Mark `autocreate_branch` as deprecated Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Address review comments Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Change to use `UpdateFile` in `Update` Signed-off-by: Timo Sand <timo.sand@f-secure.com> * fixup! Update docs with missing fields Signed-off-by: Timo Sand <timo.sand@f-secure.com> --------- Signed-off-by: Timo Sand <timo.sand@f-secure.com>
1 parent 68741e0 commit 256cdd8

7 files changed

+547
-178
lines changed

‎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: 112 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package github
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76
"net/http"
87
"net/url"
@@ -23,15 +22,28 @@ func resourceGithubRepositoryFile() *schema.Resource {
2322
StateContext: resourceGithubRepositoryFileImport,
2423
},
2524

25+
SchemaVersion: 1,
26+
StateUpgraders: []schema.StateUpgrader{
27+
{
28+
Type: resourceGithubRepositoryFileV0().CoreConfigSchema().ImpliedType(),
29+
Upgrade: resourceGithubRepositoryFileStateUpgradeV0,
30+
Version: 0,
31+
},
32+
},
33+
2634
Description: "This resource allows you to create and manage files within a GitHub repository.",
2735

2836
Schema: map[string]*schema.Schema{
2937
"repository": {
3038
Type: schema.TypeString,
3139
Required: true,
32-
ForceNew: true,
3340
Description: "The repository name",
3441
},
42+
"repository_id": {
43+
Type: schema.TypeInt,
44+
Computed: true,
45+
Description: "The repository ID",
46+
},
3547
"file": {
3648
Type: schema.TypeString,
3749
Required: true,
@@ -47,6 +59,7 @@ func resourceGithubRepositoryFile() *schema.Resource {
4759
Type: schema.TypeString,
4860
Optional: true,
4961
ForceNew: true,
62+
Computed: true,
5063
Description: "The branch name, defaults to the repository's default branch",
5164
},
5265
"ref": {
@@ -97,6 +110,7 @@ func resourceGithubRepositoryFile() *schema.Resource {
97110
Description: "Automatically create the branch if it could not be found. Subsequent reads if the branch is deleted will occur from 'autocreate_branch_source_branch'",
98111
Default: false,
99112
DiffSuppressFunc: autoBranchDiffSuppressFunc,
113+
Deprecated: "Use `github_branch` resource instead",
100114
},
101115
"autocreate_branch_source_branch": {
102116
Type: schema.TypeString,
@@ -105,6 +119,7 @@ func resourceGithubRepositoryFile() *schema.Resource {
105119
Description: "The branch name to start from, if 'autocreate_branch' is set. Defaults to 'main'.",
106120
RequiredWith: []string{"autocreate_branch"},
107121
DiffSuppressFunc: autoBranchDiffSuppressFunc,
122+
Deprecated: "Use `github_branch` resource instead",
108123
},
109124
"autocreate_branch_source_sha": {
110125
Type: schema.TypeString,
@@ -113,8 +128,10 @@ func resourceGithubRepositoryFile() *schema.Resource {
113128
Description: "The commit hash to start from, if 'autocreate_branch' is set. Defaults to the tip of 'autocreate_branch_source_branch'. If provided, 'autocreate_branch_source_branch' is ignored.",
114129
RequiredWith: []string{"autocreate_branch"},
115130
DiffSuppressFunc: autoBranchDiffSuppressFunc,
131+
Deprecated: "Use `github_branch` resource instead",
116132
},
117133
},
134+
CustomizeDiff: diffRepository,
118135
}
119136
}
120137

@@ -161,11 +178,18 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
161178

162179
checkOpt := github.RepositoryContentGetOptions{}
163180

164-
if branch, ok := d.GetOk("branch"); ok {
181+
repoInfo, _, err := client.Repositories.Get(ctx, owner, repo)
182+
if err != nil {
183+
return diag.FromErr(err)
184+
}
185+
var branch string
186+
187+
if branchFieldVal, ok := d.GetOk("branch"); ok {
188+
branch = branchFieldVal.(string)
165189
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
166-
"branch": branch.(string),
190+
"branch": branch,
167191
})
168-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
192+
if err := checkRepositoryBranchExists(ctx, client, owner, repo, branch); err != nil {
169193
if d.Get("autocreate_branch").(bool) {
170194
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
171195
return diag.FromErr(err)
@@ -174,9 +198,12 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
174198
return diag.FromErr(err)
175199
}
176200
}
177-
checkOpt.Ref = branch.(string)
201+
} else {
202+
branch = repoInfo.GetDefaultBranch()
178203
}
179204

205+
checkOpt.Ref = branch
206+
180207
opts := resourceGithubRepositoryFileOptions(d)
181208

182209
if opts.Message == nil {
@@ -215,10 +242,25 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD
215242
return diag.FromErr(err)
216243
}
217244

218-
d.SetId(fmt.Sprintf("%s/%s", repo, file))
245+
newResourceID, err := buildID(repo, file, branch)
246+
if err != nil {
247+
return diag.FromErr(err)
248+
}
249+
tflog.Debug(ctx, "Setting ID", map[string]any{
250+
"id": newResourceID,
251+
})
252+
d.SetId(newResourceID)
253+
254+
// Set computed values after the resource is created and in state
219255
if err = d.Set("commit_sha", create.GetSHA()); err != nil {
220256
return diag.FromErr(err)
221257
}
258+
if err := d.Set("branch", branch); err != nil {
259+
return diag.FromErr(err)
260+
}
261+
if err := d.Set("repository_id", int(repoInfo.GetID())); err != nil {
262+
return diag.FromErr(err)
263+
}
222264

223265
return resourceGithubRepositoryFileRead(ctx, d, meta)
224266
}
@@ -227,35 +269,22 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
227269
client := meta.(*Owner).v3client
228270
owner := meta.(*Owner).name
229271

230-
repo, file := splitRepoFilePath(d.Id())
231-
ctx = tflog.SetField(ctx, "repository", repo)
272+
repoName := d.Get("repository").(string)
273+
file := d.Get("file").(string)
274+
branch := d.Get("branch").(string)
275+
276+
ctx = tflog.SetField(ctx, "repository", repoName)
232277
ctx = tflog.SetField(ctx, "file", file)
233278
ctx = tflog.SetField(ctx, "owner", owner)
279+
ctx = tflog.SetField(ctx, "owner", owner)
234280

235281
opts := &github.RepositoryContentGetOptions{}
236282

237-
if branch, ok := d.GetOk("branch"); ok {
238-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
239-
"branch": branch.(string),
240-
})
241-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
242-
if d.Get("autocreate_branch").(bool) {
243-
branch = d.Get("autocreate_branch_source_branch").(string)
244-
} else {
245-
tflog.Info(ctx, "Removing repository path from state because the branch no longer exists in GitHub")
246-
d.SetId("")
247-
return nil
248-
}
249-
}
250-
opts.Ref = branch.(string)
251-
}
283+
opts.Ref = branch
252284

253-
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repo, file, opts)
285+
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repoName, file, opts)
254286
if err != nil {
255-
var errorResponse *github.ErrorResponse
256-
if errors.As(err, &errorResponse) && errorResponse.Response.StatusCode == http.StatusTooManyRequests {
257-
return diag.FromErr(err)
258-
}
287+
return diag.FromErr(deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "repository file %s/%s:%s:%s", owner, repoName, file, branch))
259288
}
260289
if fc == nil {
261290
tflog.Info(ctx, "Removing repository path from state because it no longer exists in GitHub")
@@ -271,12 +300,7 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
271300
if err = d.Set("content", content); err != nil {
272301
return diag.FromErr(err)
273302
}
274-
if err = d.Set("repository", repo); err != nil {
275-
return diag.FromErr(err)
276-
}
277-
if err = d.Set("file", file); err != nil {
278-
return diag.FromErr(err)
279-
}
303+
280304
if err = d.Set("sha", fc.GetSHA()); err != nil {
281305
return diag.FromErr(err)
282306
}
@@ -301,10 +325,10 @@ func resourceGithubRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
301325
tflog.Debug(ctx, "Using known commit SHA", map[string]any{
302326
"commit_sha": sha.(string),
303327
})
304-
commit, _, err = client.Repositories.GetCommit(ctx, owner, repo, sha.(string), nil)
328+
commit, _, err = client.Repositories.GetCommit(ctx, owner, repoName, sha.(string), nil)
305329
} else {
306330
tflog.Debug(ctx, "Commit SHA unknown for file, looking for commit")
307-
commit, err = getFileCommit(ctx, client, owner, repo, file, ref)
331+
commit, err = getFileCommit(ctx, client, owner, repoName, file, ref)
308332
}
309333
if err != nil {
310334
return diag.FromErr(err)
@@ -345,40 +369,36 @@ func resourceGithubRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD
345369

346370
repo := d.Get("repository").(string)
347371
file := d.Get("file").(string)
372+
branch := d.Get("branch").(string)
373+
348374
ctx = tflog.SetField(ctx, "repository", repo)
349375
ctx = tflog.SetField(ctx, "file", file)
350376
ctx = tflog.SetField(ctx, "owner", owner)
351-
352-
if branch, ok := d.GetOk("branch"); ok {
353-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
354-
"branch": branch.(string),
355-
})
356-
if err := checkRepositoryBranchExists(client, owner, repo, branch.(string)); err != nil {
357-
if d.Get("autocreate_branch").(bool) {
358-
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
359-
return diag.FromErr(err)
360-
}
361-
} else {
362-
return diag.FromErr(err)
363-
}
364-
}
365-
}
377+
ctx = tflog.SetField(ctx, "branch", branch)
366378

367379
opts := resourceGithubRepositoryFileOptions(d)
368380

369381
if *opts.Message == fmt.Sprintf("Add %s", file) {
370382
opts.Message = github.Ptr(fmt.Sprintf("Update %s", file))
371383
}
372384

373-
create, _, err := client.Repositories.CreateFile(ctx, owner, repo, file, opts)
385+
update, _, err := client.Repositories.UpdateFile(ctx, owner, repo, file, opts)
374386
if err != nil {
375387
return diag.FromErr(err)
376388
}
377389

378-
if err = d.Set("commit_sha", create.GetSHA()); err != nil {
390+
if err = d.Set("commit_sha", update.GetSHA()); err != nil {
379391
return diag.FromErr(err)
380392
}
381393

394+
if d.HasChanges("repository", "file", "branch") {
395+
newResourceID, err := buildID(repo, file, branch)
396+
if err != nil {
397+
return diag.FromErr(err)
398+
}
399+
d.SetId(newResourceID)
400+
}
401+
382402
return resourceGithubRepositoryFileRead(ctx, d, meta)
383403
}
384404

@@ -395,22 +415,8 @@ func resourceGithubRepositoryFileDelete(ctx context.Context, d *schema.ResourceD
395415
opts.Message = github.Ptr(fmt.Sprintf("Delete %s", file))
396416
}
397417

398-
if b, ok := d.GetOk("branch"); ok {
399-
tflog.Debug(ctx, "Using explicitly set branch", map[string]any{
400-
"branch": b.(string),
401-
})
402-
if err := checkRepositoryBranchExists(client, owner, repo, b.(string)); err != nil {
403-
if d.Get("autocreate_branch").(bool) {
404-
if err := resourceGithubRepositoryFileCreateBranch(ctx, d, meta); err != nil {
405-
return diag.FromErr(err)
406-
}
407-
} else {
408-
return diag.FromErr(err)
409-
}
410-
}
411-
branch := b.(string)
412-
opts.Branch = github.Ptr(branch)
413-
}
418+
branch := d.Get("branch").(string)
419+
opts.Branch = github.Ptr(branch)
414420

415421
_, _, err := client.Repositories.DeleteFile(ctx, owner, repo, file, opts)
416422
return diag.FromErr(handleArchivedRepoDelete(err, "repository file", file, owner, repo))
@@ -427,28 +433,56 @@ func autoBranchDiffSuppressFunc(k, _, _ string, d *schema.ResourceData) bool {
427433
}
428434

429435
func resourceGithubRepositoryFileImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
430-
repoFilePath, branch, err := parseID2(d.Id())
436+
repo, filePath, branch, err := parseID3(d.Id())
431437
if err != nil {
432-
return nil, fmt.Errorf("invalid ID specified. Supplied ID must be written as <repository>/<file path>:<branch>. %w", err)
438+
return nil, fmt.Errorf("invalid ID specified. Supplied ID must be written as <repository>:<file path>: (when branch is default) or <repository>:<file path>:<branch>. %w", err)
433439
}
434440

435441
client := meta.(*Owner).v3client
436442
owner := meta.(*Owner).name
437-
repo, file := splitRepoFilePath(repoFilePath)
438443

439-
opts := &github.RepositoryContentGetOptions{Ref: branch}
440-
if err := d.Set("branch", branch); err != nil {
444+
opts := &github.RepositoryContentGetOptions{}
445+
446+
repoInfo, _, err := client.Repositories.Get(ctx, owner, repo)
447+
if err != nil {
441448
return nil, err
442449
}
443-
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repo, file, opts)
450+
if branch == "" {
451+
branch = repoInfo.GetDefaultBranch()
452+
}
453+
454+
opts.Ref = branch
455+
456+
fc, _, _, err := client.Repositories.GetContents(ctx, owner, repo, filePath, opts)
444457
if err != nil {
445458
return nil, err
446459
}
447460
if fc == nil {
448-
return nil, fmt.Errorf("file %s is not a file in repository %s/%s or repository is not readable", file, owner, repo)
461+
return nil, fmt.Errorf("filePath %s is not a file in repository %s/%s or repository is not readable", filePath, owner, repo)
449462
}
450463

451-
d.SetId(fmt.Sprintf("%s/%s", repo, file))
464+
if err := d.Set("repository", repo); err != nil {
465+
return nil, err
466+
}
467+
if err := d.Set("file", filePath); err != nil {
468+
return nil, err
469+
}
470+
471+
newResourceID, err := buildID(repo, filePath, branch)
472+
if err != nil {
473+
return nil, err
474+
}
475+
tflog.Debug(ctx, "Setting ID", map[string]any{
476+
"id": newResourceID,
477+
})
478+
d.SetId(newResourceID)
479+
480+
if err := d.Set("branch", branch); err != nil {
481+
return nil, err
482+
}
483+
if err := d.Set("repository_id", int(repoInfo.GetID())); err != nil {
484+
return nil, err
485+
}
452486
if err = d.Set("overwrite_on_create", false); err != nil {
453487
return nil, err
454488
}

0 commit comments

Comments
 (0)