[FEAT] Add support for Enterprise Cost Centers - #3482
Open
timja wants to merge 42 commits into
Open
Conversation
Add utility functions needed for enterprise cost center resources: - errIs404(): check if error is a GitHub 404 Not Found response - errIsRetryable(): check if error is retryable (409, 5xx) - expandStringSet(): convert schema.Set to []string - chunkStringSlice(): split slice into chunks for batching
Add util_enterprise_cost_center.go with helper functions for managing cost center assignments with proper retry logic and batching support.
Manages GitHub Enterprise cost center entities (create, read, update, archive). Includes: - Resource implementation with CRUD operations - Acceptance tests - Documentation
Authoritative management of user assignments to cost centers. Includes: - Resource implementation with batched add/remove operations - Acceptance tests - Documentation
…esource Authoritative management of organization assignments to cost centers. Includes: - Resource implementation with batched add/remove operations - Acceptance tests - Documentation
…source Authoritative management of repository assignments to cost centers. Includes: - Resource implementation with batched add/remove operations - Acceptance tests - Documentation
Add two data sources: - github_enterprise_cost_center: retrieve a cost center by ID - github_enterprise_cost_centers: list cost centers with optional state filter Includes: - Data source implementations - Acceptance tests - Documentation
- 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 integrations#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 integrations#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 integrations#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 integrations#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.
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.
…esourcesPerRequest
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.
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.
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
|
👋 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. |
1 task
Contributor
There was a problem hiding this comment.
Pull request overview
These provider review instructions are being used.
Adds enterprise cost-center management to the provider.
Changes:
- Adds cost-center and authoritative assignment resources.
- Adds single/list data sources, retries, batching, and acceptance tests.
- Adds documentation and an example configuration.
Blocking findings include incompatible go-github v88 imports, unsafe partial batch creation, and missing update-path tests.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
github/util.go |
Adds error and batching helpers. |
github/util_test.go |
Tests new helpers. |
github/util_enterprise_cost_center.go |
Adds assignment retry helpers. |
github/resource_github_enterprise_cost_center.go |
Manages cost centers. |
github/resource_github_enterprise_cost_center_test.go |
Tests cost-center lifecycle. |
github/resource_github_enterprise_cost_center_users.go |
Manages user assignments. |
github/resource_github_enterprise_cost_center_users_test.go |
Tests user assignments. |
github/resource_github_enterprise_cost_center_organizations.go |
Manages organization assignments. |
github/resource_github_enterprise_cost_center_organizations_test.go |
Tests organization assignments. |
github/resource_github_enterprise_cost_center_repositories.go |
Manages repository assignments. |
github/resource_github_enterprise_cost_center_repositories_test.go |
Tests repository assignments. |
github/provider.go |
Registers new resources and data sources. |
github/data_source_github_enterprise_cost_center.go |
Reads one cost center. |
github/data_source_github_enterprise_cost_center_test.go |
Tests single lookup. |
github/data_source_github_enterprise_cost_centers.go |
Lists cost centers. |
github/data_source_github_enterprise_cost_centers_test.go |
Tests filtered listing. |
examples/cost_centers/main.tf |
Demonstrates cost-center workflows. |
docs/resources/enterprise_cost_center.html.markdown |
Documents the cost-center resource. |
docs/resources/enterprise_cost_center_users.html.markdown |
Documents user assignments. |
docs/resources/enterprise_cost_center_organizations.html.markdown |
Documents organization assignments. |
docs/resources/enterprise_cost_center_repositories.html.markdown |
Documents repository assignments. |
docs/data-sources/enterprise_cost_center.html.markdown |
Documents single lookup. |
docs/data-sources/enterprise_cost_centers.html.markdown |
Documents cost-center listing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/google/go-github/v88/github" |
| "context" | ||
| "time" | ||
|
|
||
| "github.com/google/go-github/v88/github" |
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/google/go-github/v88/github" |
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/google/go-github/v88/github" |
| "context" | ||
| "fmt" | ||
|
|
||
| "github.com/google/go-github/v88/github" |
| PreCheck: func() { skipUnlessEnterprise(t) }, | ||
| ProviderFactories: providerFactories, | ||
| CheckDestroy: testAccCheckGithubEnterpriseCostCenterOrganizationsDestroy, | ||
| Steps: []resource.TestStep{ |
| PreCheck: func() { skipUnlessEnterprise(t) }, | ||
| ProviderFactories: providerFactories, | ||
| CheckDestroy: testAccCheckGithubEnterpriseCostCenterRepositoriesDestroy, | ||
| Steps: []resource.TestStep{ |
Comment on lines
+72
to
+78
| for _, batch := range chunkStringSlice(toAdd, maxCostCenterResourcesPerRequest) { | ||
| if diags := retryCostCenterAddResources(ctx, client, enterpriseSlug, costCenterID, github.CostCenterResourceRequest{Users: batch}); diags.HasError() { | ||
| return diags | ||
| } | ||
| } | ||
|
|
||
| d.SetId(costCenterID) |
Comment on lines
+72
to
+78
| for _, batch := range chunkStringSlice(toAdd, maxCostCenterResourcesPerRequest) { | ||
| if diags := retryCostCenterAddResources(ctx, client, enterpriseSlug, costCenterID, github.CostCenterResourceRequest{Repositories: batch}); diags.HasError() { | ||
| return diags | ||
| } | ||
| } | ||
|
|
||
| d.SetId(costCenterID) |
Comment on lines
+72
to
+78
| for _, batch := range chunkStringSlice(toAdd, maxCostCenterResourcesPerRequest) { | ||
| if diags := retryCostCenterAddResources(ctx, client, enterpriseSlug, costCenterID, github.CostCenterResourceRequest{Organizations: batch}); diags.HasError() { | ||
| return diags | ||
| } | ||
| } | ||
|
|
||
| d.SetId(costCenterID) |
Author
|
Note: I'm away for a couple weeks after today and likely don't have time to get to this today, may be mid September before I can get back to this unless you're able to take it over. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resubmission of #3000
Thanks to vmvarela for the initial contribution.
I've resolved conflicts and re-run the tests:
Resolves #2739
Before the change?
After the change?
If you would prefer I'm happy to split out the PR for easier review
NOTE: This API (billing) has no support for APP or fine-grained tokens. 😢
Pull request checklist
Does this introduce a breaking change?
Please see our docs on breaking changes to help!