fix: Refactor repository custom property by stevehipwell · Pull Request #3476 · integrations/terraform-provider-github

stevehipwell · 2026-06-04T16:41:00Z

Resolves #3358
Resolves #3377
Resolves #2639
Closes #3387
Closes #3378
Closes #2640

Before the change?

github_repository_custom_property had a number of bugs and was using the outdated pattern

After the change?

github_repository_custom_property has been updated to the latest pattern
- Auto generated docs
- Working import
- Value can be changed without forcing a recreate
- Empty values blocked by validation
- Test updated to use cleaner pattern

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-06-04T16:41:16Z

👋 Hi, and thank you for this contribution!

This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can.

You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions.

_{🤖 This is an automated message.}

Copilot

Pull request overview

These provider review instructions are being used. This PR refactors github_repository_custom_property to match newer provider patterns (context-aware CRUD, schema/state migration, tfplugindocs-generated docs/examples) while addressing prior bugs around import and in-place value updates.

Changes:

Refactors github_repository_custom_property to SDKv2 *Context CRUD, adds UpdateContext, diffRepository, and a v0→v1 state upgrader for repository_id.
Updates generated docs/templates and adds example + import snippets for the resource.
Expands acceptance coverage for additional value types/import and adds input validation for empty strings.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
templates/resources/repository_custom_property.md.tmpl	Switches resource doc template to tfplugindocs-driven schema/examples/import sections.
github/resource_github_repository_custom_property.go	Refactors resource to new CRUD/import/migration patterns; adds update support and validation.
github/resource_github_repository_custom_property_test.go	Updates/extends acceptance tests (new URL test, import test, validation test).
github/resource_github_repository_custom_property_migration.go	Adds v0 schema + state upgrader to populate `repository_id`.
github/resource_github_repository_custom_property_migration_test.go	Adds a placeholder (currently commented) migration test scaffold.
examples/resources/github_repository_custom_property/resource_1.tf	Updates example snippet used by docs.
examples/resources/github_repository_custom_property/import.sh	Adds `terraform import` example for docs.
examples/resources/github_repository_custom_property/import-by-string-id.tf	Adds `import { id = ... }` example for docs.
docs/resources/repository_custom_property.md	Updates generated resource docs to new template/schema/import format.
ARCHITECTURE.md	Updates architecture guidance snippet to reflect the newer resource structure patterns.

Comments suppressed due to low confidence (1)

examples/resources/github_repository_custom_property/resource_1.tf:8

The example names the resource github_repository_custom_property.string, but the generated import examples use github_repository_custom_property.example. Keeping these consistent avoids confusion when users copy/paste the example + import snippet from the docs.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

stevehipwell · 2026-06-05T12:46:07Z

@deiga I've fixed your review comments.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

robert-crandall

Really nice refactor!

robert-crandall · 2026-06-05T15:43:09Z

+	repo, _, err := client.Repositories.Get(ctx, owner, repoName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve repository %s: %w", repoName, err)
+	}


How would this handle a 404 if the repository is deleted? Would this require manually removing the repo from terraform state?

Yeah, that's how it currently works in the provider. I don't really have the time to look into changing this now, but it might be that we could relax this if we can clear the ID in the migration; but this would require actual testing so a lot of effort.

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>

stevehipwell · 2026-06-05T18:32:46Z

@deiga this one is just waiting on you.

deiga

Nice!

deiga · 2026-06-05T18:26:33Z

+		{
+			testName:   "succeeds_if_repo_found",
+			statusCode: 200,
+			body: &github.Repository{
+				ID: new(int64(123456)),
+			},
+			rawState: map[string]any{
+				"id":             "my-org:my-repo:my-property",
+				"repository":     "my-repo",
+				"property_name":  "my-property",
+				"property_value": "my-value",
+			},
+			want: map[string]any{
+				"id":             "my-org:my-repo:my-property",
+				"repository":     "my-repo",
+				"repository_id":  123456,
+				"property_name":  "my-property",
+				"property_value": "my-value",
+			},
+		},


We should ensure that we test migrating all possible value combos currently 😬

Suggested change

{

testName: "succeeds_if_repo_found",

statusCode: 200,

body: &github.Repository{

ID: new(int64(123456)),

},

rawState: map[string]any{

"id": "my-org:my-repo:my-property",

"repository": "my-repo",

"property_name": "my-property",

"property_value": "my-value",

},

want: map[string]any{

"id": "my-org:my-repo:my-property",

"repository": "my-repo",

"repository_id": 123456,

"property_name": "my-property",

"property_value": "my-value",

},

},

{

testName: "succeeds_with_array_if_repo_found",

statusCode: 200,

body: &github.Repository{

ID: new(int64(123456)),

},

rawState: map[string]any{

"id": "my-org:my-repo:my-property",

"repository": "my-repo",

"property_name": "my-property",

"property_value": ["my-value"],

},

want: map[string]any{

"id": "my-org:my-repo:my-property",

"repository": "my-repo",

"repository_id": 123456,

"property_name": "my-property",

"property_value": ["my-value"],

},

},

I'm not sure why we would do this given we don't touch anything other than the fields in the test?

deiga · 2026-06-05T18:29:54Z

+	t.Parallel()
+
+	t.Run("single_select", func(t *testing.T) {
+		t.Parallel()


question: I think the recommended approach is using resource.ParallelTest instead of t.Parallel for Acc test. Do you know if there is any actual difference?
Oh, apparently it just calls t.Parallel() inside of it 🤷

deiga · 2026-06-05T18:40:18Z

+  repository     = "%s"
+  property_name  = "%s"
+  property_type  = "%s"
+  property_value = %%s


this is clever

deiga · 2026-06-05T18:40:34Z

+					PlanOnly:    true,
+				},
+				{
+					Config: fmt.Sprintf(config, fmt.Sprintf(`["%s"]`, allowed[0])),


issue: this is getting hard to parse 😬

I think it's easier to parse what's going on now that it was before, and there is no way to remove required complexity.

stevehipwell added this to the v6.13.0 milestone Jun 4, 2026

stevehipwell requested a review from deiga June 4, 2026 16:41

stevehipwell self-assigned this Jun 4, 2026

stevehipwell added the Type: Maintenance Any dependency, housekeeping, and clean up Issue or PR label Jun 4, 2026

stevehipwell force-pushed the refactor-repo-custom-prop branch from db61ca4 to c327081 Compare June 4, 2026 16:44

stevehipwell requested a review from Copilot June 5, 2026 09:03

Copilot started reviewing on behalf of stevehipwell June 5, 2026 09:04 View session

stevehipwell mentioned this pull request Jun 5, 2026

fix: improve github_organization_custom_properties resource #3234

Open