[FEAT] Add support for Enterprise Cost Centers by vmvarela · Pull Request #3000 · integrations/terraform-provider-github

vmvarela · 2025-12-15T13:20:53Z

Resolves #2739

Before the change?

No support for enterprise cost centers resources or data

After the change?

Two new resources and data sources (github_enterprise_cost_center and github_enterprise_cost_centers)

NOTE: This API (billing) has no support for APP or fine-grained tokens.

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 · 2025-12-15T13:21:03Z

👋 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! 🚀

deiga · 2025-12-19T11:00:55Z

We're trying to move towards using Context-aware provider functions, could you update all schemas to use those? So ReadContext etc

Copilot

Pull request overview

This PR adds comprehensive support for GitHub Enterprise Cost Centers, enabling Terraform management of cost center resources and their assignments. The implementation addresses issue #2739 by leveraging the newly public GitHub Enterprise billing API.

Key changes:

Introduces github_enterprise_cost_center resource for creating, updating, and archiving cost centers with authoritative management of user, organization, and repository assignments
Adds github_enterprise_cost_center and github_enterprise_cost_centers data sources for querying cost center information
Implements robust resource assignment synchronization with batching (50 resources per request) and retry logic for transient failures

Reviewed changes

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

Show a summary per file

File	Description
website/github.erb	Adds navigation links for the new data sources and resource
website/docs/r/enterprise_cost_center.html.markdown	Documents the cost center resource with examples, arguments, attributes, and import instructions
website/docs/d/enterprise_cost_center.html.markdown	Documents the single cost center data source for retrieving by ID
website/docs/d/enterprise_cost_centers.html.markdown	Documents the data source for listing cost centers with optional state filtering
github/util_cost_centers.go	Provides utility functions for API interactions including CRUD operations and resource assignment management
github/resource_github_enterprise_cost_center.go	Implements the cost center resource with full lifecycle management and authoritative assignment synchronization
github/provider.go	Registers the new resource and data sources with the provider
github/resource_github_enterprise_cost_center_test.go	Provides acceptance tests covering create, update, assignment changes, and import scenarios
github/data_source_github_enterprise_cost_centers_test.go	Tests the list data source with state filtering
github/data_source_github_enterprise_cost_centers.go	Implements the data source for listing cost centers
github/data_source_github_enterprise_cost_center_test.go	Tests the single cost center data source retrieval
github/data_source_github_enterprise_cost_center.go	Implements the data source for retrieving a specific cost center by ID
examples/cost_centers/main.tf	Provides a comprehensive example demonstrating resource and data source usage

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

website/docs/r/enterprise_cost_center.html.markdown

website/docs/d/enterprise_cost_centers.html.markdown

website/docs/d/enterprise_cost_center.html.markdown

github/resource_github_enterprise_cost_center.go

joraff · 2026-01-06T16:47:10Z

I built and tested your branch @vmvarela and it works great! I was able to read, import, and create cost centers and resources using your examples and a few of my own. Thank you for the work.

deiga

Half-way through. Please consider if my comments could apply to other places in your changes as well, I wasn't able to comment on every duplicate thing

examples/cost_centers/main.tf

github/data_source_github_enterprise_cost_center.go

github/data_source_github_enterprise_cost_center_test.go

github/resource_github_enterprise_cost_center.go

vmvarela

Thank you for the thorough review @deiga! I've addressed all the feedback. Here's a summary of the changes:

Implemented:

✅ Added top-level Description attribute to all resources and data sources
✅ Removed ctx = context.WithValue(ctx, ctxId, ...) pattern from all files
✅ Replaced log.Printf with tflog.Info and added tflog.Warn for 404 removals
✅ Used diag.Errorf instead of diag.FromErr(fmt.Errorf(...))
✅ Create and Update no longer call Read - computed fields are set directly
✅ Extracted anonymous retry functions to named functions (retryCostCenterAddResources, retryCostCenterRemoveResources)
✅ Changed import ID separator from / to : and using parseTwoPartID
✅ Replaced stringSliceToAnySlice with existing flattenStringList
✅ Used http.StatusNotFound and other constants instead of magic numbers
✅ Moved is404 to util.go
✅ Updated tests to use providerFactories and skipUnlessEnterprise(t)
✅ Added owner = var.enterprise_slug to example provider config
✅ Used buildTwoPartID in data source
✅ Inlined check variables in tests

Regarding questions:

The resources attribute provides the raw API view while users, organizations, repositories are filtered convenience attributes. I kept both for flexibility and debugging purposes.
diffStringSlices differs from setChanges - it compares string slices directly while setChanges works with Terraform state changes. They serve different purposes.
In Create, we only fetch from API after assignments are configured to populate computed fields, since we're not calling Read anymore.

All tests pass and the code compiles without errors.

deiga

Please pay attention to what I'm asking form you and make sure to make similar changes in other places where they might apply.

Consider these also for your other PRs

github/data_source_github_enterprise_cost_center_test.go

github/data_source_github_enterprise_cost_centers_test.go

github/resource_github_enterprise_cost_center_test.go

github/resource_github_enterprise_cost_center.go

github/data_source_github_enterprise_cost_center.go

github/data_source_github_enterprise_cost_centers.go

github/resource_github_enterprise_cost_center.go

Split resourceGithubEnterpriseCostCenterOrganizationsCreateOrUpdate into separate Create and Update functions. Create only adds organizations, Update handles the full diff. Both return nil instead of calling Read. Resolves PR comments #6-7.

Split resourceGithubEnterpriseCostCenterRepositoriesCreateOrUpdate into separate Create and Update functions. Create only adds repositories, Update handles the full diff. Both return nil instead of calling Read. Resolves PR comments #8-9.

…tions The API returns type strings as 'User', 'Org', and 'Repo' but the tests were checking for lowercase 'user', 'organization', and 'repository'. This fix ensures CheckDestroy properly detects remaining assignments.

@deiga

Add CostCenterResourceType constants (User, Org, Repo) to avoid magic strings throughout the codebase. This prevents typos and makes the code more maintainable. Addresses review feedback from @deiga.

Replace terraform-plugin-sdk/v2 test imports with terraform-plugin-testing to fix flag redefinition conflict ('sweep' flag registered twice). This aligns cost center tests with the rest of the codebase.

Keep chunkStringSlice(items, maxSize) generic in util.go to avoid coupling with cost-center-specific constants. Add nolint:unparam with explicit rationale because current call sites pass the same value, while preserving future reuse for other resources.

…sary comments

…esourcesPerRequest

…unkStringSlice

…ters data source

@stevehipwell

The import function now calls GetCostCenter to populate the 'name' field from the API response. Without this, the Required 'name' field would be empty after import, causing recreate churn on the next plan. Addresses review comment from @stevehipwell.

@stevehipwell

Sub-resources (_users, _organizations, _repositories) now use a simple cost_center_id as their Terraform ID, consistent with the main github_enterprise_cost_center resource. Also moves d.SetId() to after the API call succeeds, preventing corrupt state if the add-resources call fails. Tests updated to use ImportStateIdPrefix and read attributes from state instead of parsing the now-simple ID. Addresses review comments from @stevehipwell.

Before adding resources to a cost center, check via API whether the cost center already has resources of the managed type assigned. If so, return an error asking the user to import or remove them manually. This prevents silently clobbering pre-existing assignments.

Replace the two-map diff (currentX + desiredX) with a single map where false=remove and true=keep. Desired items not in the map are added. This is shorter and avoids constructing a second map.

Instead of reading resource names from Terraform state (which may be stale or incomplete), Delete now calls GetCostCenter to fetch the current list of resources of the managed type from the API and removes them all. Also handles 404 gracefully (cost center already gone).

The cost center sub-resources (organizations, repositories, users) were registered in provider.go and had docs, but were accidentally omitted from the website sidebar navigation.

… after rebase

Closes #51

deiga · 2026-03-23T16:35:04Z

@vmvarela thanks for keeping these up-to-date! We haven't had enough capacity to get these fully reviewed and merged yet, but we do still intend to get there! ☺️

deiga

We're close! :)

deiga · 2026-03-23T19:02:57Z

github/resource_github_enterprise_cost_center.go

+	if cc == nil || cc.ID == "" {
+		return diag.Errorf("failed to create cost center: missing id in response (unexpected API response; please retry or contact support)")
+	}


This is unnecessary defensive programming. There are no real cases where this could be true

Suggested change

if cc == nil || cc.ID == "" {

return diag.Errorf("failed to create cost center: missing id in response (unexpected API response; please retry or contact support)")

}

deiga · 2026-03-23T19:04:03Z

github/resource_github_enterprise_cost_center.go

+		if errIs404(err) {
+			tflog.Warn(ctx, "Cost center not found, removing from state", map[string]any{
+				"enterprise_slug": enterpriseSlug,
+				"cost_center_id":  costCenterID,
+			})
+			d.SetId("")
+			return nil
+		}


This could be replaced with a call to deleteResourceOn404AndSwallow304OtherwiseReturnError

deiga · 2026-03-23T19:08:13Z

github/resource_github_enterprise_cost_center_organizations.go

+		if errIs404(err) {
+			tflog.Warn(ctx, "Cost center not found, removing from state", map[string]any{
+				"enterprise_slug": enterpriseSlug,
+				"cost_center_id":  costCenterID,
+			})
+			d.SetId("")
+			return nil
+		}


suggestion: could be replaced by deleteResourceOn404AndSwallow304OtherwiseReturnError

The modernize/newexpr rule incorrectly suggests replacing github.Ptr(x) with new(x), but new() takes a type not an expression — the auto-fix generates invalid Go code. Exclude this specific rule pattern.

…n404AndSwallow304OtherwiseReturnError helper

deiga · 2026-03-24T23:38:26Z

.golangci.yml

+      - linters:
+          - modernize
+        text: "newexpr: call of Ptr"


issue: This seems wrong. We've already applied modernize all over the codebase and it works

It did not work for me, but I will delete this.

vmvarela force-pushed the cost-centers branch 2 times, most recently from 13dcd40 to 1ab2c0f Compare December 22, 2025 06:32

Copilot AI review requested due to automatic review settings December 26, 2025 09:00

Copilot started reviewing on behalf of vmvarela December 26, 2025 09:01 View session

Copilot AI reviewed Dec 26, 2025

View reviewed changes

vmvarela force-pushed the cost-centers branch 3 times, most recently from a3e128c to 0bdad0f Compare January 8, 2026 16:35

vmvarela changed the title ~~feat: Add support for Enterprise Cost Centers~~ [FEAT] Add support for Enterprise Cost Centers Jan 8, 2026

vmvarela force-pushed the cost-centers branch 2 times, most recently from 3314826 to 1d13386 Compare January 10, 2026 11:11

deiga suggested changes Jan 15, 2026

View reviewed changes

vmvarela commented Jan 16, 2026

View reviewed changes

vmvarela force-pushed the cost-centers branch 5 times, most recently from b66a9c0 to 2763ff4 Compare January 20, 2026 18:58

vmvarela requested a review from deiga January 20, 2026 19:15

deiga suggested changes Jan 20, 2026

View reviewed changes

vmvarela force-pushed the cost-centers branch 3 times, most recently from 52a3d8a to 5e58a8c Compare January 21, 2026 09:28

vmvarela requested a review from deiga January 21, 2026 10:08

deiga suggested changes Jan 21, 2026

View reviewed changes

vmvarela force-pushed the cost-centers branch from 5e58a8c to 0143ec1 Compare January 22, 2026 10:10

vmvarela requested a review from deiga January 22, 2026 10:12

vmvarela added 20 commits March 20, 2026 13:46

refactor(cost-centers): use type constants instead of magic strings

4696150

Add CostCenterResourceType constants (User, Org, Repo) to avoid magic strings throughout the codebase. This prevents typos and makes the code more maintainable. Addresses review feedback from @deiga.

fix(cost-centers): use terraform-plugin-testing imports in test files

1855b1c

Replace terraform-plugin-sdk/v2 test imports with terraform-plugin-testing to fix flag redefinition conflict ('sweep' flag registered twice). This aligns cost center tests with the rest of the codebase.

fix: address review - rename single-char variables and remove unneces…

847022d

…sary comments

fix: address review - rename maxResourcesPerRequest to maxCostCenterR…

12a759f

…esourcesPerRequest

fix: address review - simplify import test with ImportStateIdPrefix

f10a9e2

fix: address review - add unit tests for errIs404, errIsRetryable, ch…

538b6e6

…unkStringSlice

fix: address review - migrate cost center tests to ConfigStateChecks

ce07138

fix(cost-centers): update go-github import from v82 to v83

4ad701c

fix(cost-centers): add default 'all' value to state field in cost cen…

a567202

…ters data source

refactor(cost-center): simplify Update diff with map pattern

98fe0fb

Replace the two-map diff (currentX + desiredX) with a single map where false=remove and true=keep. Desired items not in the map are added. This is shorter and avoids constructing a second map.

fix(cost-center): add missing sub-resources to website sidebar

7c39d61

The cost center sub-resources (organizations, repositories, users) were registered in provider.go and had docs, but were accidentally omitted from the website sidebar navigation.

fix: update go-github import to v84 and remove duplicate team helpers…

dd6d4bb

… after rebase

vmvarela force-pushed the cost-centers branch from bce2995 to dd6d4bb Compare March 20, 2026 12:46

vmvarela mentioned this pull request Mar 20, 2026

feat(cost-centers): Enterprise Cost Centers resource (CRUD) vmvarela/terraform-provider-github#45

Merged

fix(cost-centers): use third-person verb in data source descriptions

4ae98a1

Closes #51

deiga suggested changes Mar 23, 2026

View reviewed changes

vmvarela added 2 commits March 23, 2026 20:22

chore(lint): exclude modernize newexpr rule for Ptr calls

f9c34a9

The modernize/newexpr rule incorrectly suggests replacing github.Ptr(x) with new(x), but new() takes a type not an expression — the auto-fix generates invalid Go code. Exclude this specific rule pattern.

fix(cost-centers): remove defensive nil check and use deleteResourceO…

cf198b9

…n404AndSwallow304OtherwiseReturnError helper

vmvarela requested a review from deiga March 23, 2026 20:24

deiga suggested changes Mar 24, 2026

View reviewed changes

fix: undo .golangci.yml changes

ca39dac

	if cc == nil \|\| cc.ID == "" {
	return diag.Errorf("failed to create cost center: missing id in response (unexpected API response; please retry or contact support)")
	}

Conversation

vmvarela commented Dec 15, 2025 • 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 Dec 15, 2025

Uh oh!

deiga commented Dec 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joraff commented Jan 6, 2026

Uh oh!

deiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vmvarela left a comment

Choose a reason for hiding this comment

Uh oh!

deiga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deiga commented Mar 23, 2026

Uh oh!

deiga left a comment

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

vmvarela commented Dec 15, 2025 •

edited

Loading