[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

- Register 4 resources in provider.go: - github_enterprise_cost_center - github_enterprise_cost_center_users - github_enterprise_cost_center_organizations - github_enterprise_cost_center_repositories - Register 2 data sources in provider.go: - github_enterprise_cost_center - github_enterprise_cost_centers - Add navigation links in website/github.erb T_EDITOR=true git rebase --continue t status

Add example Terraform configuration demonstrating how to: - Create a cost center - Assign users, organizations, and repositories - Use data sources to query cost centers

Update import paths from go-github/v81 to go-github/v82 to match the current version in upstream/main.

Co-authored-by: Timo Sand <timo.sand@iki.fi>

Remove expandStringSet from util.go and replace usages with direct expandStringList(set.List()) calls. The function was unnecessary since schema.Set from d.Get() is never nil. Resolves PR comments #1-2.

Move the archived/deleted state check from Update to Read function. If the cost center is archived (deleted), it will be removed from Terraform state during Read rather than blocking updates. Resolves PR comment #3.

Split resourceGithubEnterpriseCostCenterUsersCreateOrUpdate into separate Create and Update functions. Create only adds users, Update handles the full diff. Both return nil instead of calling Read. Resolves PR comments #4-5.

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 integrations#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

I'm assuming that you've used a consistent pattern so I've reviewed up to the first assignment resource.

stevehipwell · 2026-02-23T17:14:58Z

github/resource_github_enterprise_cost_center.go

+}
+
+func resourceGithubEnterpriseCostCenterImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) {
+	enterpriseSlug, costCenterID, err := parseID2(d.Id())


You need the name here, either in the ID or you'll need to lookup as it must be set or it'll cause a recreate churn. You should have a test to catch this?

Fixed — Import now calls GetCostCenter to populate name from the API before returning. See commit 757c071.

stevehipwell · 2026-02-23T17:15:57Z

github/resource_github_enterprise_cost_center_organizations.go

+	id, err := buildID(enterpriseSlug, costCenterID)
+	if err != nil {
+		return diag.FromErr(err)
+	}
+	d.SetId(id)


The ID here is inconsistent with the cost center resource, they should match. Also the ID shouldn't be set until the API call has been made successfully.

Fixed — Sub-resources now use the plain costCenterID as ID (consistent with the main resource), and SetId is called after the API call succeeds. See commit c27c343.

stevehipwell · 2026-02-23T17:27:14Z

github/resource_github_enterprise_cost_center_organizations.go

+	desiredOrgsSet := d.Get("organization_logins").(*schema.Set)
+	toAdd := expandStringList(desiredOrgsSet.List())
+
+	if len(toAdd) > 0 {


This shouldn't pass validation, so is unnecessary. That said I'd expect this resource to error if there are already organizations assigned. It would also cause churn due to how the update function is implemented.

Agreed — Create now calls GetCostCenter first and returns an error if the cost center already has resources of the managed type assigned, asking the user to import or remove them manually. See commit bd51909.

I also took the opportunity to refactor Update to use a single-map diff pattern (9cc1b71) and rewrite Delete to fetch current assignments from the API instead of relying on state (34031c6).

stevehipwell · 2026-02-23T17:39:37Z

github/resource_github_enterprise_cost_center_organizations.go

+	currentOrgs := make(map[string]bool)
+	for _, ccResource := range cc.Resources {
+		if ccResource != nil && ccResource.Type == CostCenterResourceTypeOrg {
+			currentOrgs[ccResource.Name] = true
+		}
+	}
+
+	desiredOrgsSet := d.Get("organization_logins").(*schema.Set)
+	desiredOrgs := make(map[string]bool)
+	for _, org := range desiredOrgsSet.List() {
+		desiredOrgs[org.(string)] = true
+	}
+
+	var toAdd, toRemove []string
+	for org := range desiredOrgs {
+		if !currentOrgs[org] {
+			toAdd = append(toAdd, org)
+		}
+	}
+	for org := range currentOrgs {
+		if !desiredOrgs[org] {
+			toRemove = append(toRemove, org)
+		}
+	}


Suggested change

currentOrgs := make(map[string]bool)

for _, ccResource := range cc.Resources {

if ccResource != nil && ccResource.Type == CostCenterResourceTypeOrg {

currentOrgs[ccResource.Name] = true

}

}

desiredOrgsSet := d.Get("organization_logins").(*schema.Set)

desiredOrgs := make(map[string]bool)

for _, org := range desiredOrgsSet.List() {

desiredOrgs[org.(string)] = true

}

var toAdd, toRemove []string

for org := range desiredOrgs {

if !currentOrgs[org] {

toAdd = append(toAdd, org)

}

}

for org := range currentOrgs {

if !desiredOrgs[org] {

toRemove = append(toRemove, org)

}

}

toAdd := make([]string)

toRemove := make([]string)

current := make(map[string]bool)

for _, ccResource := range cc.Resources {

if ccResource != nil && ccResource.Type == CostCenterResourceTypeOrg {

current[ccResource.Name] = false

}

}

desiredOrgsSet := d.Get("organization_logins").(*schema.Set)

for _, o:= range desiredOrgsSet.List() {

org := o.(string)

if _, ok := current[org]; ok {

current[org] = true

} else {

toAdd = append(toAdd, org)

}

}

for org, keep := range current {

if !keep {

toRemove = append(toRemove, org)

}

}

I think a pattern like this is easier to read.

Adopted — refactored to this exact pattern in all 3 sub-resources. See commit 9cc1b71.

stevehipwell · 2026-02-23T17:40:40Z

github/resource_github_enterprise_cost_center_organizations.go

+	costCenterID := d.Get("cost_center_id").(string)
+
+	organizationsSet := d.Get("organization_logins").(*schema.Set)
+	organizations := expandStringList(organizationsSet.List())


I think this logic is incorrect. As this is an authoritative resource, all linked organizations should be removed.

Addressed in 34031c6 — Delete now fetches all linked organizations from the API (not from state) before removing them. Same pattern applied to users and repositories sub-resources.

@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.

vmvarela · 2026-02-24T10:45:30Z

Hey @deiga @stevehipwell — quick naming question before we finalize.

Would you think it's appropriate to add billing to the cost center resource names? This would better reflect their placement under the GitHub Billing API and help avoid ambiguity as the enterprise namespace grows.

The rename would be:

Resources:

github_enterprise_cost_center → github_enterprise_billing_cost_center
github_enterprise_cost_center_users → github_enterprise_billing_cost_center_users
github_enterprise_cost_center_organizations → github_enterprise_billing_cost_center_organizations
github_enterprise_cost_center_repositories → github_enterprise_billing_cost_center_repositories

Data sources:

github_enterprise_cost_center → github_enterprise_billing_cost_center
github_enterprise_cost_centers → github_enterprise_billing_cost_centers

The change is straightforward — just a rename across files, provider registration, and docs. Happy to keep the current names if you prefer; just wanted to get your take before tagging a release.

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

github-actions bot and others added 20 commits February 23, 2026 18:10

docs(cost-centers): add usage example

fd7ee76

Add example Terraform configuration demonstrating how to: - Create a cost center - Assign users, organizations, and repositories - Use data sources to query cost centers

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

b381d96

Update import paths from go-github/v81 to go-github/v82 to match the current version in upstream/main.

fix: removed linter config, it shouldn't be needed

841d1e9

Co-authored-by: Timo Sand <timo.sand@iki.fi>

fix(cost-centers): remove unnecessary expandStringSet function

78d73ae

Remove expandStringSet from util.go and replace usages with direct expandStringList(set.List()) calls. The function was unnecessary since schema.Set from d.Get() is never nil. Resolves PR comments #1-2.

fix(cost-centers): check deleted state in Read instead of Update

4396ca4

Move the archived/deleted state check from Update to Read function. If the cost center is archived (deleted), it will be removed from Terraform state during Read rather than blocking updates. Resolves PR comment #3.

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

32fe45d

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

fda96bf

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…

c38514d

…sary comments

fix: address review - rename maxResourcesPerRequest to maxCostCenterR…

25f43d0

…esourcesPerRequest

fix: address review - simplify import test with ImportStateIdPrefix

c0d7219

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

c0ce0cf

…unkStringSlice

fix: address review - migrate cost center tests to ConfigStateChecks

371f37c

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

35e0092

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

8395f25

…ters data source

vmvarela force-pushed the cost-centers branch from 5cf15cb to 8395f25 Compare February 23, 2026 17:10

stevehipwell requested changes Feb 23, 2026

View reviewed changes

vmvarela added 6 commits February 24, 2026 08:51

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

9cc1b71

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

e7db627

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.

vmvarela requested a review from stevehipwell February 24, 2026 08:14

Comments

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!

stevehipwell 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

Uh oh!

vmvarela commented Dec 15, 2025 •

edited

Loading