[BUG]: fix github repository file id escaping by deiga · Pull Request #3415 · integrations/terraform-provider-github

deiga · 2026-05-10T18:52:07Z

Resolves #3335

Before the change?

Terraform would throw an error if filePath in github_repository_resource contained a colon
- This was introduced by me in [MAINT] Fixup github_repository_file #3175 when I changed the ID generation code

After the change?

Terraform successfully operates on resources when filePath in github_repository_resource contains a colon

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-05-10T18:52:16Z

👋 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

Have we tested this doesn't break existing IDs? It looks OK, but worth checking. Also should we add this to the next minor release milestone as we can always cherry pick it into a patch if required?

deiga · 2026-05-24T12:36:25Z

@stevehipwell Thanks for asking, I ran v6.11.0 locally and then tried to run this code: same failure, since it was already the v0 migration that was failing!

deiga · 2026-05-24T12:42:54Z

I think we should release this as a patch, considering that the changes in v6.11.1 are blocking upgrading for anyone with colons in repository_file resources

stevehipwell · 2026-05-26T16:03:36Z

@deiga do you want to rebase this and then we can get it merged in. Once it's been merged we can decide if we want to cherry pick it onto a patch branch.

stevehipwell · 2026-05-26T16:27:14Z

+  direct {
+    exclude = ["registry.terraform.io/hashicorp/github"]
+  }
 }


Could we fix the EOF?

Actually should this file even be in this PR?

No, it's not directly related. It's a Scout Rule thing, since I used the example to verify the changes and noticed that the config file didn't work as we had it

stevehipwell · 2026-05-26T16:30:42Z


 	rawState["repository_id"] = int(repo.GetID())

+	branch, ok := rawState["branch"].(string)


Is there a reason this has been changed from a single line?

forcetypeassert linter bit it :)

I'm not sure I follow why, the code is identical as there isn't a new check for the map lookup (which could be nested single lines anyway).

In this commit I needed to touch that line, which caused it to be checked by make lintcheck-new

And since we didn't use forcetypeassert when that migration was originally created: it had a lot to complain about

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

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

- We want to ensure that users never accidentally get `hashicorp/github` - `~/` failed to expand on macOS Signed-off-by: Timo Sand <timo.sand@f-secure.com>

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

stevehipwell · 2026-05-29T09:31:21Z

+	branch, ok := rawState["branch"].(string)
 	// If branch is missing or empty, fetch the default branch from the repository
-	if branch, ok := rawState["branch"].(string); !ok || branch == "" {
-		rawState["branch"] = repo.GetDefaultBranch()
+	if !ok || branch == "" {
+		branch = repo.GetDefaultBranch()
+		rawState["branch"] = branch
 	}

-	newResourceID, err := buildID(rawState["repository"].(string), rawState["file"].(string), rawState["branch"].(string))
+	filePath, ok := rawState["file"].(string)
+	if !ok {
+		return nil, fmt.Errorf("state upgrade v0: file path is not a string")
+	}


Suggested change

branch, ok := rawState["branch"].(string)

// If branch is missing or empty, fetch the default branch from the repository

if branch, ok := rawState["branch"].(string); !ok || branch == "" {

rawState["branch"] = repo.GetDefaultBranch()

if !ok || branch == "" {

branch = repo.GetDefaultBranch()

rawState["branch"] = branch

}

newResourceID, err := buildID(rawState["repository"].(string), rawState["file"].(string), rawState["branch"].(string))

filePath, ok := rawState["file"].(string)

if !ok {

return nil, fmt.Errorf("state upgrade v0: file path is not a string")

}

branchSet := false

if v, ok := rawState["branch"]; ok {

if _, ok := v.(string); ok || s != "" {

branchSet = true

}

}

if !branchSet {

rawState["branch"] = repo.GetDefaultBranch()

}

pathSet := false

if v, ok := rawState["file"]; ok {

if _, ok := v.(string); ok {

pathSet = true

}

}

if !pathSet {

return nil, fmt.Errorf("state upgrade v0: file path is not a string")

}

This would be my prefered pattern here, otherwise we're still masking a conversion.

github-actions Bot added the Type: Bug Something isn't working as documented label May 10, 2026

deiga marked this pull request as ready for review May 10, 2026 18:54

deiga changed the title ~~fix github repository file id escaping~~ [BUG]: fix github repository file id escaping May 10, 2026

deiga requested a review from stevehipwell May 10, 2026 18:55

deiga added the vNextPatch These issues and PRs should be included in the next patch release label May 10, 2026

stevehipwell reviewed May 15, 2026

View reviewed changes

deiga removed the vNextPatch These issues and PRs should be included in the next patch release label May 24, 2026

deiga force-pushed the fix-github_repository_file-id-escaping branch from 65ea3b4 to 2598ebe Compare May 24, 2026 12:34

deiga requested a review from stevehipwell May 24, 2026 12:35

deiga added the vNextPatch These issues and PRs should be included in the next patch release label May 24, 2026

stevehipwell requested changes May 26, 2026

View reviewed changes

deiga added 6 commits May 26, 2026 20:38

Add failing regression test for files with colon in name

2c6ebca

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

Ensure that all file paths are escaped for resource ID

898e65b

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

Ensure that github_repository_file migration doesn't fail with unes…

2456356

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

Fix example/dev.tfrc

ba962ee

- We want to ensure that users never accidentally get `hashicorp/github` - `~/` failed to expand on macOS Signed-off-by: Timo Sand <timo.sand@f-secure.com>

Fix deprecated attributes in repository_file example

764b78c

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

Make the linter happy

8f0bfe8

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

deiga force-pushed the fix-github_repository_file-id-escaping branch from 7cb06fa to 8f0bfe8 Compare May 26, 2026 17:42

Fix missing newline

aa309fd

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

deiga requested a review from stevehipwell May 26, 2026 17:43

stevehipwell requested changes May 29, 2026

View reviewed changes


		rawState["repository_id"] = int(repo.GetID())

		branch, ok := rawState["branch"].(string)

Conversation

deiga commented May 10, 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 May 10, 2026

Uh oh!

stevehipwell left a comment

Choose a reason for hiding this comment

Uh oh!

deiga commented May 24, 2026

Uh oh!

deiga commented May 24, 2026

Uh oh!

stevehipwell commented May 26, 2026

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!

2 participants

deiga commented May 10, 2026 •

edited

Loading