[BUG] Ensure autolink reference doesn't error when repo is renamed by deiga · Pull Request #3271 · integrations/terraform-provider-github

deiga · 2026-03-12T20:08:18Z

Addresses #3470 for github_repository_autolink_reference

Before the change?

Renaming a repository caused an existing github_repository_autolink_reference to be re-created and throw an error

After the change?

Renaming a repository does not cause an error in github_repository_autolink_reference

Pull request checklist

Schema migrations have been created if needed (example)
Tests for the changes have been added (for bug fixes / features)
Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes
No

github-actions · 2026-03-12T20:08:30Z

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

stevehipwell

Do we need a migration to add the repository ID and then rely on it or we just supporting it as a best effort?

Copilot

Pull request overview

These provider review instructions are being used.

This PR fixes #3470 for github_repository_autolink_reference so that renaming a repository no longer forces the autolink reference to be destroyed and recreated (which previously errored). It adopts the provider's rename-safe convention: drop ForceNew from repository, add a computed repository_id, and wire up CustomizeDiff: diffRepository so replacement is only forced when the underlying repository ID actually changes. The CRUD handlers are also modernized to the *Context variants, and a state upgrader is added to backfill repository_id.

Changes:

Convert CRUD to context-aware handlers, add a no-op Update, extract the importer, and switch logging to tflog.
Add repository_id (computed), remove ForceNew from repository, and add CustomizeDiff: diffRepository; add a state upgrader + migration to populate repository_id.
Update acceptance tests to statecheck/plancheck (incl. a rename scenario), add a migration unit test, and refresh docs/example.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
github/resource_github_repository_autolink_reference.go	Rename-safe schema (`repository_id`, no `ForceNew`, `diffRepository`), context CRUD, no-op `Update`; state upgrader registered with a version mismatch (blocking).
github/resource_github_repository_autolink_reference_migration.go	New V0 schema + state upgrade func that fetches and stores `repository_id`.
github/resource_github_repository_autolink_reference_migration_test.go	Unit test invoking the state upgrade function directly against a mocked API.
github/resource_github_repository_autolink_reference_test.go	Migrates checks to `statecheck`, adds a repository-rename in-place test.
templates/resources/repository_autolink_reference.md.tmpl	Docs template: updated `repository` description, adds `repository_id`.
docs/resources/repository_autolink_reference.md	Generated docs mirror the template/schema updates and `visibility` example.
examples/resources/repository_autolink_reference/example_1.tf	Example switched from deprecated `private` to `visibility`.

Blocking finding: the added StateUpgrader uses Version: 0 while the resource already ships at SchemaVersion: 1, so it will never execute for existing state (which is already stamped v1). The version must be bumped to 2 with the upgrader registered at Version: 1 to actually backfill repository_id.

stevehipwell

Just a quick review.

stevehipwell · 2026-07-10T10:17:35Z

 | `github_release` | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ |
 | `github_repository` | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ |
-| `github_repository_autolink_reference` | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ | ❓ |
+| `github_repository_autolink_reference` | ⚠️ | ✅ | ✅ | ✅ | ✅ | ❓ | ❓ |


Suggested change

| `github_repository_autolink_reference` | ⚠️ | ✅ | ✅ | ✅ | ✅ | ❓ | ❓ |

| `github_repository_autolink_reference` | ⚠️ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ |

stevehipwell · 2026-07-10T10:20:17Z

-func resourceGithubRepositoryAutolinkReferenceDelete(d *schema.ResourceData, meta any) error {
-	client := meta.(*Owner).v3client
+func resourceGithubRepositoryAutolinkReferenceUpdate(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	tflog.Warn(ctx, "Update function of autolink reference. This should not be called. But it's necessary when 'repository' doesn't have `ForceNew`", map[string]any{


Could we make this a single line?

stevehipwell · 2026-07-10T10:21:01Z

+	parts := strings.Split(d.Id(), "/")
+	if len(parts) != 2 {
+		return nil, fmt.Errorf("invalid ID specified: supplied ID must be written as <repository>/<autolink_reference_id>")
+	}


I think the ID should be migrated as part of the schema migration so it can use the ID functions.

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

stevehipwell · 2026-07-15T09:11:34Z

+		return diag.FromErr(err)
+	}
+
+	return resourceGithubRepositoryAutolinkReferenceRead(ctx, d, m)


We should be able to remove this call and just set the etag.

stevehipwell · 2026-07-15T09:12:38Z

-func resourceGithubRepositoryAutolinkReferenceRead(d *schema.ResourceData, meta any) error {
-	client := meta.(*Owner).v3client
+func resourceGithubRepositoryAutolinkReferenceRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics {
+	ctx = tflog.SetField(ctx, "id", d.Id())


Let's keep with the current pattern where we only set attributes when we need them. And as a rule of thumb id shouldn't be set unless there is an unexpected error, we want the field named in a friendly way (e.g. autolink_id).

stevehipwell · 2026-07-15T09:18:28Z

 	autolinkRefID, err := strconv.ParseInt(d.Id(), 10, 64)
 	if err != nil {
-		return unconvertibleIdErr(d.Id(), err)
+		return diag.FromErr(unconvertibleIdErr(d.Id(), err))
 	}


Why not add a new idStringToInt64OK function with the signature func(string) (bool, diag.Diagnostics) (similar to the checkOrganizationOK I'm adding in #3537)?

stevehipwell · 2026-07-15T09:19:15Z

 	if !d.IsNewResource() {
 		ctx = context.WithValue(ctx, ctxEtag, d.Get("etag").(string))
 	}


This can be removed as create shouldn't be calling through.

stevehipwell · 2026-07-15T09:19:32Z

 			if ghErr.Response.StatusCode == http.StatusNotFound {
-				log.Printf("[INFO] Removing autolink reference for repository %s/%s from state because it no longer exists in GitHub",
-					owner, repoName)
+				tflog.Info(ctx, "Autolink reference not found, removing from state.", map[string]any{


Please make this a single line.

stevehipwell · 2026-07-15T09:25:08Z

+		return nil, err
+	}
+
+	return []*schema.ResourceData{d}, nil


You need to set all of the fields as they're force new.

stevehipwell · 2026-07-15T09:26:17Z

+		return nil, fmt.Errorf("state upgrade v1: repository is not a string or not set")
+	}
+
+	repo, _, err := client.Repositories.Get(ctx, owner, repoName)


You can use the migrateRepositoryWithID function to avoid repeating the code.

stevehipwell · 2026-07-15T09:27:48Z

+	"github.com/google/go-github/v89/github"
+)
+
+func buildMockResponsesForRepositoryAutolinkReferenceMigrationV1toV2(mockOwner, mockRepo string, wantRepoID int) []*mockResponse {


You shouldn't need this, please copy the pattern used for Test_resourceGithubCustomPropertyStateUpgradeV0.

stevehipwell · 2026-07-15T09:29:04Z

 		randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)
 		repoName := fmt.Sprintf("%srepo-autolink-%s", testResourcePrefix, randomID)
 		config := fmt.Sprintf(`
 			resource "github_repository" "test" {


Please switch to using the test helpers to create the repo instead of using TF.

stevehipwell · 2026-07-15T09:29:58Z

 				{
 					Config: config,
-					Check:  check,
+					ConfigStateChecks: []statecheck.StateCheck{


I'm pretty sure you don't need the checks given that none are computed (and we should ignore etag).

deiga added this to the v6.12.0 Release milestone Mar 15, 2026

deiga requested a review from stevehipwell March 15, 2026 16:51

deiga modified the milestones: v6.12.0 Release, v6.13.0 Release Apr 14, 2026

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from ad1fdb7 to 00ea1c8 Compare April 18, 2026 18:34

deiga added the Type: Bug Something isn't working as documented label Apr 18, 2026

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from 00ea1c8 to 458f672 Compare May 11, 2026 02:13

deiga marked this pull request as draft May 14, 2026 21:09

stevehipwell reviewed May 15, 2026

View reviewed changes

Comment thread github/resource_github_repository_autolink_reference.go

deiga removed this from the v6.13.0 milestone Jun 3, 2026

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from 458f672 to 852d646 Compare June 6, 2026 05:29

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from 852d646 to 50a9aa7 Compare July 9, 2026 18:29

deiga marked this pull request as ready for review July 9, 2026 18:29

deiga requested a review from Copilot July 9, 2026 18:30

Copilot started reviewing on behalf of deiga July 9, 2026 18:30 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Comment thread github/resource_github_repository_autolink_reference.go Outdated

deiga requested a review from stevehipwell July 9, 2026 18:42

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from cf7ee3b to 4b95c1a Compare July 10, 2026 09:01

stevehipwell reviewed Jul 10, 2026

View reviewed changes

deiga added 10 commits July 14, 2026 22:54

Update to use Context-aware functions

c812520

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Enable repository rename

e40cad1

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Refactor to use ConfigStateChecks instead of Check in tests

54e875a

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Update docs

7ae7789

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Move import function to named function

8a6360b

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Add schema migrations

dc39df0

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Fix changed reference

02afd33

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Ensure migrations actually run for users

d0543de

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Update resources file

d0b4b47

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Fix migration file

aaa15da

Signed-off-by: Timo Sand <timo.sand@f-secure.com>

deiga force-pushed the ensure-autolinks-are-aware-of-repo-rename branch from 4b95c1a to aaa15da Compare July 14, 2026 19:55

stevehipwell requested changes Jul 15, 2026

View reviewed changes

	\| `github_repository_autolink_reference` \| ⚠️ \| ✅ \| ✅ \| ✅ \| ✅ \| ❓ \| ❓ \|
	\| `github_repository_autolink_reference` \| ⚠️ \| ✅ \| ✅ \| ✅ \| ✅ \| ❌ \| ❌ \|

Uh oh!

Conversation

deiga commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

github-actions Bot commented Mar 12, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

deiga commented Mar 12, 2026 •

edited

Loading