diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 0000000000..66ae2fb8cb --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,778 @@ +# Architecture Guide + +This document serves as a guide for contributors implementing new features and resources in the Terraform Provider for GitHub. + +- [Architecture Guide](#architecture-guide) + - [Module Map](#module-map) + - [Core Principles](#core-principles) + - [1. One Resource = One API Entity](#1-one-resource--one-api-entity) + - [2. Minimize API Calls](#2-minimize-api-calls) + - [3. API-Only Operations](#3-api-only-operations) + - [Resource Design](#resource-design) + - [File Organization](#file-organization) + - [Resource Structure](#resource-structure) + - [Schema Field Guidelines](#schema-field-guidelines) + - [ID Patterns](#id-patterns) + - [Implementation Patterns](#implementation-patterns) + - [Authentication](#authentication) + - [CRUD Function Signatures](#crud-function-signatures) + - [Accessing the API Client](#accessing-the-api-client) + - [Error Handling](#error-handling) + - [Import](#import) + - [State Migrations](#state-migrations) + - [Logging](#logging) + - [Testing](#testing) + - [Test Structure](#test-structure) + - [Test Modes](#test-modes) + - [Running Tests](#running-tests) + - [Debugging Tests](#debugging-tests) + - [Gotchas \& Known Issues](#gotchas--known-issues) + - [API Preview Headers](#api-preview-headers) + - [Deprecated Resources](#deprecated-resources) + - [Deprecated Data Sources](#deprecated-data-sources) + - [Known Limitations](#known-limitations) + - [Workarounds in Code](#workarounds-in-code) + - [Pending go-github Updates](#pending-go-github-updates) + - [Appendix](#appendix) + - [Common Utilities](#common-utilities) + - [Naming Conventions](#naming-conventions) + - [Decision Log](#decision-log) + - [January 2026](#january-2026) + - [Use StateUpgraders for State Migrations](#use-stateupgraders-for-state-migrations) + - [Explicit Authentication Configuration](#explicit-authentication-configuration) + - [Transport Layer Rework](#transport-layer-rework) + - [Migrate to `terraform-plugin-testing`](#migrate-to-terraform-plugin-testing) + - [No Local Git CLI Support](#no-local-git-cli-support) + - [2025](#2025) + - [Replace `log` Package with `tflog`](#replace-log-package-with-tflog) + - [Finalize SDK v2 Migration](#finalize-sdk-v2-migration) + +--- + +## Module Map + +```text +terraform-provider-github/ +├── github/ +│ ├── provider.go # Entry point, registers all resources/data sources +│ ├── config.go # Auth setup, HTTP client, rate limiting, transport +│ │ +│ ├── resource_github_*.go # Resource implementations +│ ├── resource_*_migration.go # Resource state migration functions (StateUpgraders) +│ ├── data_source_github_*.go # Data source implementations +│ │ +│ │ +│ ├── util.go # Core utilities (ID parsing, validation) +│ ├── util_*.go # Domain utilities (rules, labels, etc.) +│ │ +│ └── transport.go # HTTP transports: ETag caching, rate limiting, retries +│ +├── ARCHITECTURE.md # This file - implementation guide +├── MAINTAINERS.md # Maintainers and contributors +└── CONTRIBUTING.md # How to contribute +``` + +--- + +## Core Principles + +### 1. One Resource = One API Entity + +Each Terraform resource should map to a single GitHub API entity. Avoid creating resources that combine multiple API concerns. + +**Do:** + +```go +// Manages a single repository +func resourceGithubRepository() *schema.Resource { ... } + +// Manages repository topics separately +func resourceGithubRepositoryTopics() *schema.Resource { ... } +``` + +**Don't:** + +```go +// Don't combine unrelated API entities into one resource +func resourceGithubRepositoryWithTopicsAndLabels() *schema.Resource { ... } +``` + +### 2. Minimize API Calls + +Each resource should use the minimum number of API calls necessary. Consolidate operations where possible and avoid redundant reads. + +**Do:** + +```go +func resourceExampleRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Single API call to get all needed data + resource, _, err := client.Resources.Get(ctx, owner, name) + if err != nil { + return diag.FromErr(err) + } + // Set all attributes from single response + d.Set("field1", resource.Field1) + d.Set("field2", resource.Field2) + return nil +} +``` + +**Don't:** + +```go +func resourceExampleRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Don't make separate calls for each field + field1, _ := client.Resources.GetField1(ctx, owner, name) + field2, _ := client.Resources.GetField2(ctx, owner, name) + // ... +} +``` + +### 3. API-Only Operations + +We do not support using local git CLI to operate on repositories. All operations must go through the GitHub API. + +**Do:** + +```go +// Use go-github client for all git operations +_, _, err := client.Git.CreateRef(ctx, owner, repo, ref) +``` + +**Don't:** + +```go +// Never shell out to git CLI +exec.Command("git", "push", "origin", "main") +``` + +--- + +## Resource Design + +### File Organization + +Resources follow a consistent file naming and organization pattern: + +```text +github/ +├── resource_github_.go # Main resource implementation +├── resource_github__test.go # Acceptance tests +├── resource_github__migration.go # State migration functions (if needed) +├── data_source_github_.go # Data source implementation +├── data_source_github__test.go # Data source tests +└── util_.go # Domain-specific utilities +``` + +### Resource Structure + +Use context-aware CRUD functions with the `*Context` suffix: + +```go +func resourceGithubExample() *schema.Resource { + return &schema.Resource{ + CreateContext: resourceGithubExampleCreate, + ReadContext: resourceGithubExampleRead, + UpdateContext: resourceGithubExampleUpdate, + DeleteContext: resourceGithubExampleDelete, + Importer: &schema.ResourceImporter{ + StateContext: resourceGithubExampleImport, + }, + + // Include SchemaVersion and StateUpgraders if state migrations exist + SchemaVersion: 1, + StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceGithubExampleResourceV0().CoreConfigSchema().ImpliedType(), + Upgrade: resourceGithubExampleInstanceStateUpgradeV0, + Version: 0, + }, + }, + + Schema: map[string]*schema.Schema{ + // Schema definition + }, + } +} +``` + +### Schema Field Guidelines + +Full reference: [Schema Behaviors](https://developer.hashicorp.com/terraform/plugin/sdkv2/schemas/schema-behaviors) + +**Primitive options:** + +- `Type` — field type (`TypeString`, `TypeBool`, `TypeInt`, `TypeFloat`, `TypeList`, `TypeSet`, `TypeMap`) +- `Description` — human-readable description (always include) +- `Elem` — element type for `TypeList`, `TypeSet`, and `TypeMap` fields (e.g., `&schema.Schema{Type: schema.TypeString}` or a nested `&schema.Resource{}`) +- `Default` — static default value when field is not set in config +- `DefaultFunc` — dynamic default (e.g., read from env var via `schema.EnvDefaultFunc`) + +**Behavior flags:** + +- `Required` — must be provided in config (mutually exclusive with `Optional`, `Computed`) +- `Optional` — may be omitted from config +- `Computed` — set by the provider (API-derived); combine with `Optional` for optional fields with server defaults +- `ForceNew` — changing this field destroys and recreates the resource +- `Sensitive` — value is masked in plan/state output (secrets, tokens) + +**Validation:** + +- `ValidateDiagFunc` — validate field value with diagnostics (preferred) + +**Constraints:** + +- `MaxItems` / `MinItems` — cardinality bounds for `TypeList` and `TypeSet` +- `ConflictsWith` — list of field paths that cannot be set together with this field +- `ExactlyOneOf` — exactly one of these fields must be set +- `AtLeastOneOf` — at least one of these fields must be set +- `RequiredWith` — these fields must all be set if this field is set + +**Advanced:** + +- `StateFunc` — transform value before storing in state (e.g., normalize to lowercase) +- `DiffSuppressFunc` — suppress plan diffs when old and new values are semantically equal +- `DiffSuppressOnRefresh` — also apply `DiffSuppressFunc` during refresh +- `Set` — custom hash function for `TypeSet` elements +- `Deprecated` — marks field as deprecated with a message shown to users + +### ID Patterns + +For single-part IDs (most common): + +```go +d.SetId(resource.GetName()) +``` + +For composite IDs, use `buildID` to create and `parseID2`/`parseID3`/`parseID4` to parse: + +```go +// Two-part ID +id, err := buildID(owner, name) +d.SetId(id) +// Parse: +owner, name, err := parseID2(id) + +// Three-part ID +id, err := buildID(owner, repo, name) +d.SetId(id) +// Parse: +owner, repo, name, err := parseID3(id) + +// Four-part ID +owner, repo, env, name, err := parseID4(id) + +// IDs with special characters (colons, etc.) +id, err := buildID(escapeIDPart(part1), part2) +``` + +> **Note:** The legacy functions `buildTwoPartID`, `parseTwoPartID`, `buildThreePartID`, and `parseThreePartID` are deprecated. Use `buildID` and `parseID2`/`parseID3`/`parseID4` instead. + +--- + +## Implementation Patterns + +### Authentication + +The provider resolves credentials using the following fallback chain (first match wins): + +1. **Token** — `token` attribute or `GITHUB_TOKEN` env var +2. **GitHub App** — `app_auth` block with `id`, `installation_id`, and `pem_file` +3. **GitHub CLI** — Falls back to `gh auth token` if neither token nor app_auth is set +4. **Anonymous** — Read-only access when no credentials are available + +All authentication configuration is handled in `config.go`. See the [Explicit Authentication Configuration](#explicit-authentication-configuration) decision for design rationale. + +### CRUD Function Signatures + +```go +func resourceGithubExampleCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + client := meta.(*Owner).v3client + owner := meta.(*Owner).name + + // Implementation + return nil // Never call Read at end of Create, set any Computed fields in Create +} + +func resourceGithubExampleRead(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Implementation + return nil +} + +func resourceGithubExampleUpdate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Implementation + return nil // Never call Read at end of Update, set any Computed fields in Update +} + +func resourceGithubExampleDelete(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + // Implementation + return nil // Never call Read after Delete +} +``` + +### Accessing the API Client + +```go +func resourceExampleRead(ctx context.Context, d *schema.ResourceData, m any) diag.Diagnostics { + meta := m.(*Owner) + // REST API client (go-github) + client := meta.v3client + + // GraphQL client (for queries not available in REST) + v4client := meta.v4client + + // Owner context + owner := meta.name + isOrg := meta.IsOrganization + orgID := meta.id + + // ... +} +``` + +### Error Handling + +Handle 404s gracefully by removing from state: + +```go +resource, _, err := client.Resources.Get(ctx, owner, name) +if err != nil { + var ghErr *github.ErrorResponse + if errors.As(err, &ghErr) { + if ghErr.Response.StatusCode == http.StatusNotFound { + tflog.Info(ctx, "Removing resource from state because it no longer exists", map[string]any{"name": name}) + d.SetId("") + return nil + } + } + return diag.FromErr(err) +} +``` + +Or use the helper function: + +```go +if err := deleteResourceOn404AndSwallow304OtherwiseReturnError(err, d, "resource %s", name); err != nil { + return diag.FromErr(err) +} +``` + +### Import + +Import is registered via the `Importer` field with a `StateContext` function. After import runs, Terraform **automatically calls `Read`** — so the import function's only job is to set enough state for `Read` to succeed. Do not duplicate `Read` logic in the import function. + +For resources with a single-part ID, the default passthrough importer is often sufficient: + +```go +Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, +}, +``` + +For resources with composite IDs, the import function must parse the user-provided ID and populate any schema attributes that `Read` depends on: + +```go +func resourceGithubExampleImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { + owner, name, err := parseID2(d.Id()) + if err != nil { + return nil, err + } + + // Set attributes that Read needs to make API calls + d.Set("owner", owner) + // Re-build a normalized ID if needed + id, err := buildID(owner, name) + if err != nil { + return nil, err + } + d.SetId(id) + + return []*schema.ResourceData{d}, nil +} +``` + +**Key principle:** Import sets the minimum state required for `Read` to fetch the full resource. `Read` then populates all remaining attributes. + +### State Migrations + +When adding new fields or changing schema, use `StateUpgraders` (not the deprecated `MigrateState`): + +**Migration file (`resource_github_example_migration.go`):** + +```go +// resourceGithubExampleResourceV0 returns the schema for version 0 +func resourceGithubExampleResourceV0() *schema.Resource { + return &schema.Resource{ + Schema: map[string]*schema.Schema{ + // Previous schema version + }, + } +} + +// resourceGithubExampleInstanceStateUpgradeV0 migrates from version 0 to 1 +func resourceGithubExampleInstanceStateUpgradeV0(ctx context.Context, rawState map[string]any, meta any) (map[string]any, error) { + log.Printf("[DEBUG] State before migration: %#v", rawState) + + // Add new field with default value + if _, ok := rawState["new_field"]; !ok { + rawState["new_field"] = "default_value" + } + + log.Printf("[DEBUG] State after migration: %#v", rawState) + return rawState, nil +} +``` + +**Register in resource:** + +```go +SchemaVersion: 1, +StateUpgraders: []schema.StateUpgrader{ + { + Type: resourceGithubExampleResourceV0().CoreConfigSchema().ImpliedType(), + Upgrade: resourceGithubExampleInstanceStateUpgradeV0, + Version: 0, + }, +}, +``` + +### Logging + +Use `tflog` for structured logging (replacing `log` package): + +```go +import "github.com/hashicorp/terraform-plugin-log/tflog" + +func resourceExampleCreate(ctx context.Context, d *schema.ResourceData, meta any) diag.Diagnostics { + tflog.Debug(ctx, "Creating resource", map[string]any{ + "name": name, + "owner": owner, + }) + // ... +} +``` + +**Note:** Migration from `log` to `tflog` is in progress. New code should use `tflog`. + +--- + +## Testing + +### Test Structure + +Use `ConfigStateChecks` for post-apply state assertions and `ConfigPlanChecks` for plan-level assertions (e.g., verifying `ForceNew` triggers). These replace the legacy `Check:` + `resource.ComposeTestCheckFunc` pattern. + +```go +import ( + "github.com/hashicorp/terraform-plugin-testing/compare" + "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/plancheck" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" +) + +func TestAccGithubExample(t *testing.T) { + + t.Run("creates resource without error", func(t *testing.T) { + randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum) + testResourceName := fmt.Sprintf("%s%s", testResourcePrefix, randomID) + config := fmt.Sprintf(` + resource "github_example" "test" { + name = "%s" + } + `, testResourceName) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + // Verify computed values are populated + statecheck.ExpectKnownValue("github_example.test", tfjsonpath.New("etag"), knownvalue.NotNull()), + statecheck.ExpectKnownValue("github_example.test", tfjsonpath.New("node_id"), knownvalue.NotNull()), + // Compare computed values across resources + statecheck.CompareValuePairs("github_example.test", tfjsonpath.New("repo_id"), "github_repository.test", tfjsonpath.New("repo_id"), compare.ValuesSame()), + }, + }, + }, + }) + }) + + t.Run("forces new when field changes", func(t *testing.T) { + // ... config steps ... + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + {Config: configBefore}, + { + Config: configAfter, + ConfigPlanChecks: resource.ConfigPlanChecks{ + PreApply: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("github_example.test", plancheck.ResourceActionDestroyBeforeCreate), + }, + }, + }, + }, + }) + }) +} +``` + +> **Legacy pattern:** Existing tests may still use `Check: resource.ComposeTestCheckFunc(...)`. New tests should use `ConfigStateChecks` and `ConfigPlanChecks` instead. See `data_source_github_ip_ranges_test.go` for a real-world example. + +### Test Modes + +Use these skip functions to run tests in appropriate contexts: + +- `skipUnauthenticated(t)` — skips if anonymous mode +- `skipUnlessHasOrgs(t)` — requires organization, team, or enterprise mode +- `skipUnlessHasPaidOrgs(t)` — requires team or enterprise mode (paid orgs) +- `skipUnlessEnterprise(t)` — requires enterprise mode +- `skipUnlessHasAppInstallations(t)` — requires GitHub App installations +- `skipUnlessEMUEnterprise(t)` — requires EMU enterprise +- `skipIfEMUEnterprise(t)` — skips if EMU enterprise +- `skipUnlessMode(t, testModes...)` — generic mode check + +| Mode | Environment Variables Required | +| -------------- | -------------------------------------- | +| `anonymous` | None (read-only, GitHub.com only) | +| `individual` | `GITHUB_TOKEN` + `GITHUB_OWNER` | +| `organization` | `GITHUB_TOKEN` + `GITHUB_ORGANIZATION` | +| `enterprise` | Enterprise deployment configuration | + +### Running Tests + +```bash +# Run specific acceptance test +make testacc T=TestAccGithubExample + +# Run unit tests (non-acceptance) +make test + +# With coverage +make testacc T=TestAccGithubExample COV=true + +# Clean up leaked test resources (repos, teams prefixed with tf-acc-test-) +make sweep +``` + +### Debugging Tests + +```bash +# With debug logging +TF_LOG=DEBUG make testacc T=TestAccGithubExample +``` + +--- + +## Gotchas & Known Issues + +This section documents provider-specific quirks and known limitations discovered through development. + +### API Preview Headers + +- Stone Crop preview header is still required for some GraphQL features (`config.go:64-65`) +- The `previewHeaderInjectorTransport` handles automatic header injection for these cases + +### Deprecated Resources + +The following resources are deprecated and will be removed in future versions: + +| Deprecated Resource | Replacement | +| -------------------------------------------- | ------------------------------------------------- | +| `github_organization_security_manager` | `github_organization_role_team` | +| `github_organization_custom_role` | `github_organization_repository_role` | +| `github_organization_role_team_assignment` | `github_organization_role_team` | +| `github_repository_deployment_branch_policy` | `github_repository_environment_deployment_policy` | +| `github_organization_project` | None (Classic Projects API removed) | +| `github_project_card` | None (Classic Projects API removed) | +| `github_project_column` | None (Classic Projects API removed) | +| `github_repository_project` | None (Classic Projects API removed) | + +### Deprecated Data Sources + +| Deprecated Data Source | Replacement | +| ---------------------------------------------- | --------------------------------------------------- | +| `github_organization_custom_role` | `github_organization_repository_role` | +| `github_organization_security_managers` | `github_organization_role_teams` | +| `github_repository_deployment_branch_policies` | `github_repository_environment_deployment_policies` | + +### Known Limitations + +- **Branch Protection `contexts`**: Deprecated, use the `checks` array instead +- **Runner Groups**: Selected repository IDs are not exposed via API (`resource_github_actions_runner_group.go:179`) +- **Organization Settings**: Test requires manual cleanup (`resource_github_organization_settings_test.go:11`) +- **Repository Search**: Tests may hit rate limits (`data_source_github_repositories_test.go:12`) + +### Workarounds in Code + +- **EMU with SSO**: Odd behavior with user tokens when using Enterprise Managed Users (`resource_github_enterprise_organization.go:122`) + +### Pending go-github Updates + +The following features are blocked waiting for upstream changes in [google/go-github#3364](https://github.com/google/go-github/issues/3364) (adds `assignment`, `parent_team_id`, `parent_team_slug` fields): + +- `data_source_github_organization_role_users.go:41` +- `data_source_github_organization_role_teams.go:51` + +--- + +## Appendix + +### Common Utilities + +**ID Parsing & Building** (`util.go`): + +| Function | Purpose | +| -------------------- | ---------------------------------------------- | +| `buildID(parts...)` | Create composite ID (e.g., `"a:b"`, `"a:b:c"`) | +| `parseID2(id)` | Parse two-part composite ID | +| `parseID3(id)` | Parse three-part composite ID | +| `parseID4(id)` | Parse four-part composite ID | +| `escapeIDPart(part)` | Escape colons in ID parts | +| `buildChecksumID(v)` | Create MD5-based checksum ID from string slice | + +**Error Handling & Validation** (`util.go`): + +| Function | Purpose | +| ----------------------------------------------------------- | ---------------------------------------------- | +| `wrapErrors([]error)` | Convert errors to diagnostics | +| `checkOrganization(meta)` | Verify org context | +| `deleteResourceOn404AndSwallow304OtherwiseReturnError(...)` | Handle 404/304 responses | +| `validateValueFunc(values)` | Create enum validator from allowed values | +| `validateSecretNameFunc` | Validate GitHub secret naming rules | +| `caseInsensitive()` | `DiffSuppressFunc` for case-insensitive fields | +| `isArchivedRepositoryError(err)` | Detect archived repo 403 errors | + +**Data Conversion** (`util.go`): + +| Function | Purpose | +| ----------------------------- | --------------------- | +| `expandStringList([]any)` | Convert to `[]string` | +| `flattenStringList([]string)` | Convert to `[]any` | + +**Team & Repository Resolution** (`util_team.go`, `util_v4_repository.go`): + +| Function | Purpose | +| ---------------------------------- | ------------------------------------ | +| `getTeamID(ctx, meta, idOrSlug)` | Resolve team ID from ID or slug | +| `getTeamSlug(ctx, meta, idOrSlug)` | Resolve team slug from ID or slug | +| `getRepositoryID(name, meta)` | Resolve repository node ID from name | + +**Permission Mapping** (`util_permissions.go`): + +| Function | Purpose | +| --------------------------- | ---------------------------------------------------------- | +| `getPermission(permission)` | Normalize permission names (`read`↔`pull`, `write`↔`push`) | + +**CustomizeDiffFuncs** (`util_diff.go`): + +| Function | Purpose | +| ------------------------------ | -------------------------------------------------- | +| `diffRepository` | Force replacement on repo ID change | +| `diffSecret` | Detect remote secret drift via timestamps | +| `diffSecretVariableVisibility` | Validate `selected_repository_ids` vs `visibility` | +| `diffTeam` | Force new resource on team ID change | + +### Naming Conventions + +| Component | Pattern | Example | +| -------------------- | ------------------------------------------------ | ------------------------------------------------ | +| Resource function | `resourceGithub` | `resourceGithubRepository` | +| Data source function | `dataSourceGithub` | `dataSourceGithubRepository` | +| CRUD functions | `resourceGithub` | `resourceGithubRepositoryCreate` | +| Migration function | `resourceGithubInstanceStateUpgradeV` | `resourceGithubRepositoryInstanceStateUpgradeV0` | +| Schema function | `resourceGithubResourceV` | `resourceGithubRepositoryResourceV0` | +| Test function | `TestAccGithub` | `TestAccGithubRepository` | +| Utility file | `util_.go` | `util_rules.go` | + +--- + +## Decision Log + +### January 2026 + +#### Use StateUpgraders for State Migrations + +**Decision:** Use `StateUpgraders` instead of the deprecated `MigrateState` function. + +**Rationale:** `StateUpgraders` provides a cleaner, more maintainable approach to state migrations that works better with the SDK v2 architecture. + +**Implementation:** + +- Create `resource_github__migration.go` with versioned schema and upgrade functions +- Register in resource with `SchemaVersion` and `StateUpgraders` +- See [ARCHITECTURE.md](ARCHITECTURE.md#state-migrations) for implementation pattern + +#### Explicit Authentication Configuration + +**Decision:** Make all authentication concerns of the provider entirely explicit. Users must explicitly configure their authentication method. + +**Rationale:** Implicit auth detection can lead to confusion and security issues. Explicit configuration makes the provider's behavior predictable and auditable. + +**Reference:** + +#### Transport Layer Rework + +**Decision:** Rework the transport layer to utilize: + +- [`github-conditional-http-transport`](https://github.com/bored-engineer/github-conditional-http-transport) for conditional requests +- [`go-github-ratelimit`](https://github.com/gofri/go-github-ratelimit) for rate limiting + +**Rationale:** These libraries provide better handling of GitHub API rate limits and conditional requests than our current custom implementation. + +**Reference:** + +#### Migrate to `terraform-plugin-testing` + +**Decision:** Migrate from the SDK testing package (`github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource`) to `terraform-plugin-testing` (`github.com/hashicorp/terraform-plugin-testing`). Use `ConfigStateChecks` and `ConfigPlanChecks` as the preferred assertion patterns, replacing `Check:` + `resource.ComposeTestCheckFunc`. + +**Rationale:** `terraform-plugin-testing` is the standalone testing framework that decouples test utilities from the SDK. `ConfigStateChecks` and `ConfigPlanChecks` provide type-safe, composable assertions with better error messages. + +**Reference:** + +#### No Local Git CLI Support + +**Decision:** Do not support using local git CLI to operate on repositories; use purely API operations. + +**Rationale:** API-only operations ensure consistency, security, and avoid environment dependencies. The provider should not assume git is installed or configured on the user's machine. + +### 2025 + +#### Replace `log` Package with `tflog` + +**Decision:** Replace all usage of the standard `log` package with `tflog` from terraform-plugin-log. + +**Rationale:** `tflog` provides structured logging that integrates better with Terraform's logging infrastructure and supports log filtering, structured fields, and proper log levels. + +**Migration pattern:** + +```go +// Before +log.Printf("[DEBUG] Creating resource: %s", name) + +// After +tflog.Debug(ctx, "Creating resource", map[string]any{"name": name}) +``` + +#### Finalize SDK v2 Migration + +**Decision:** Complete migration to Terraform Plugin SDK v2. + +**Rationale:** SDK v2 provides better diagnostics, context-aware functions, and improved schema validation. + +**Key changes:** + +- Use `*Context` functions (`CreateContext`, `ReadContext`, etc.) +- Use `ValidateDiagFunc` instead of `ValidateFunc` +- Use `diag.Diagnostics` for error returns +- Use `any` instead of `interface{}` + +**Reference:** + +--- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a924c1be9f..9da2021fcf 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,8 +13,10 @@ Before submitting an issue or a pull request, please search the repository for e 1. Fork and clone the repository. 2. Create a new branch: `git switch -c my-branch-name`. 3. Make your change, add tests, and make sure the tests still pass. -4. Push to your fork and submit a pull request. -5. Pat yourself on the back and wait for your pull request to be reviewed and merged. +4. Update or add documentation to reflect your changes. +5. Ensure formatting and linting are passing. +6. Push to your fork and submit a pull request. +7. Pat yourself on the back and wait for your pull request to be reviewed and merged. Here are a few things you can do that will increase the likelihood of your pull request being accepted: @@ -47,21 +49,21 @@ Once you have the repository cloned, there's a couple of additional steps you'll ``` - Build the project with `make build` -- Try an example test run from the default (`main`) branch, like `TF_LOG=DEBUG TF_ACC=1 go test -v ./... -run ^TestAccGithubRepositories`. All those tests should pass. +- Try an example test run from the default (`main`) branch, like `TF_LOG=DEBUG make testacc T=TestAccGithubRepositories`. All those tests should pass. ### Local Development Iteration -1. Write a test describing what you will fix. See [`github_label`](./github/resource_github_issue_label_test.go) for an example format. -2. Run your test and observe it fail. Enabling debug output allows for observing the underlying requests and responses made as well as viewing state (search `STATE:`) generated during the acceptance test run. +1. Write a test describing what you will fix. See [`github_ip_ranges`](./github/data_source_github_ip_ranges_test.go) for an example using the preferred `ConfigStateChecks` pattern, and [ARCHITECTURE.md](ARCHITECTURE.md#test-structure) for full guidance. +2. Run your test and observe it fail. Enabling debug output allows for observing the underlying requests and responses made during the acceptance test run. ```sh -TF_LOG=DEBUG TF_ACC=1 go test -v ./... -run ^TestAccGithubIssueLabel +TF_LOG=DEBUG make testacc T=TestAccGithubIssueLabel ``` 1. Align the resource's implementation to your test case and observe it pass: ```sh -TF_ACC=1 go test -v ./... -run ^TestAccGithubIssueLabel +make testacc T=TestAccGithubIssueLabel ``` Note that some resources still use a previous format that is incompatible with automated test runs, which depend on using the `skipUnlessMode` helper. When encountering these resources, tests should be rewritten to the latest format. @@ -70,7 +72,7 @@ Also note that there is no build / `terraform init` / `terraform plan` sequence ### Debugging the terraform provider -Println debugging can easily be used to obtain information about how code changes perform. If the `TF_LOG=DEBUG` level is set, calls to `log.Printf("[DEBUG] your message here")` will be printed in the program's output. +Println debugging can easily be used to obtain information about how code changes perform. If the `TF_LOG=DEBUG` level is set, debug messages will be printed. Use `tflog.Debug(ctx, "your message here", map[string]any{...})` for new code. Some existing code still uses `log.Printf("[DEBUG] ...")` — see [ARCHITECTURE.md](ARCHITECTURE.md#logging) for the migration pattern. If a full debugger is desired, VSCode may be used. In order to do so, @@ -88,7 +90,7 @@ If a full debugger is desired, VSCode may be used. In order to do so, Setting a `processId` of 0 allows a dropdown to select the process of the provider. -1. Add a sleep call (e.g. `time.Sleep(10 * time.Second)`) in the [`func providerConfigure(p *schema.Provider) schema.ConfigureFunc`](https://github.com/integrations/terraform-provider-github/blob/cec7e175c50bb091feecdc96ba117067c35ee351/github/provider.go#L274C1-L274C64) before the immediate `return` call. This will allow time to connect the debugger while the provider is initializing, before any critical logic happens. +1. Add a sleep call (e.g. `time.Sleep(10 * time.Second)`) in `providerConfigure` (in `github/provider.go`) before the immediate `return` call. This will allow time to connect the debugger while the provider is initializing, before any critical logic happens. 2. Build the terraform provider with debug flags enabled and copy it to the appropriate bin folder with a command like `go build -gcflags="all=-N -l" -o ~/go/bin/`. @@ -100,11 +102,11 @@ Setting a `processId` of 0 allows a dropdown to select the process of the provid ## Manual Testing -Manual testing should be performed on each PR opened in order to validate the provider's correct behavior and discover any regressions. Our automated testing is in an unhealthy spot at this point unfortunately, so extra care is required with manual testing. See [issue #1414](https://github.com/integrations/terraform-provider-github/issues/1414) for more details. +> **Note:** Automated test coverage is incomplete ([#1414](https://github.com/integrations/terraform-provider-github/issues/1414)). Manual testing on each PR is essential until this is resolved. ### Using a local version of the provider -Build the provider and specify the output directory: +Build the provider with debug flags for attaching a debugger: ```sh go build -gcflags="all=-N -l" -o ~/go/bin/ @@ -140,7 +142,29 @@ The following provider development overrides are set in the CLI configuration: - integrations/github in /Users/jcudit/go/bin ``` -### Environment variable reference +See the [Environment Variable Reference](#environment-variable-reference) below for the full list of configuration options. + +There are also a small number of unit tests in the provider. Due to the nature of the provider, such tests are currently only recommended for exercising functionality completely internal to the provider. These may be executed by running `make test`. + +### Cleaning Up Test Resources + +Acceptance tests create real GitHub resources prefixed with `tf-acc-test-`. If tests fail or are interrupted, these resources may be left behind. Run the sweeper to clean them up: + +```sh +make sweep +``` + +This removes leaked test repositories and teams matching the `tf-acc-test-` prefix. + +### GitHub Organization + +If you do not have an organization already that you are comfortable running tests against, you will need to [create one](https://help.github.com/en/articles/creating-a-new-organization-from-scratch). The free "Team for Open Source" org type is fine for these tests. The name of the organization must then be exported in your environment as `GITHUB_OWNER`. + +Make sure that your organization has a `terraform-template-module` repository ([terraformtesting/terraform-template-module](https://github.com/terraformtesting/terraform-template-module) is an example you can clone) and that its "Template repository" item in Settings is checked. + +If you are interested in using and/or testing GitHub's [Team synchronization](https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/synchronizing-teams-between-your-identity-provider-and-github) feature, please contact a maintainer as special arrangements can be made for your convenience. + +## Environment Variable Reference Commonly required environment variables are listed below: @@ -179,6 +203,7 @@ export GH_TEST_EXTERNAL_USER2= # Configure values for the enterprise under test export GH_TEST_ENTERPRISE_EMU_GROUP_ID= +export GITHUB_ENTERPRISE_SLUG= # Configure test options export GH_TEST_ADVANCED_SECURITY= @@ -187,16 +212,6 @@ export GH_TEST_ADVANCED_SECURITY= export GH_TEST_ENTERPRISE_IS_EMU= ``` -There are also a small amount of unit tests in the provider. Due to the nature of the provider, such tests are currently only recommended for exercising functionality completely internal to the provider. These may be executed by running `make test`. - -### GitHub Organization - -If you do not have an organization already that you are comfortable running tests against, you will need to [create one](https://help.github.com/en/articles/creating-a-new-organization-from-scratch). The free "Team for Open Source" org type is fine for these tests. The name of the organization must then be exported in your environment as `GITHUB_OWNER`. - -Make sure that your organization has a `terraform-template-module` repository ([terraformtesting/terraform-template-module](https://github.com/terraformtesting/terraform-template-module) is an example you can clone) and that its "Template repository" item in Settings is checked. - -If you are interested in using and/or testing GitHub's [Team synchronization](https://help.github.com/en/github/setting-up-and-managing-organizations-and-teams/synchronizing-teams-between-your-identity-provider-and-github) feature, please contact a maintainer as special arrangements can be made for your convenience. - ### Example _.vscode/settings.json_ file To run acceptance tests the `TF_ACC` environment variable must be set. Below is an example `settings.json` file for VSCode that sets this variable and the other necessary environment variables when running tests from the editor. diff --git a/MAINTAINERS.md b/MAINTAINERS.md new file mode 100644 index 0000000000..cfeb5d618e --- /dev/null +++ b/MAINTAINERS.md @@ -0,0 +1,23 @@ +# Maintainers + +This document lists the current maintainers of the Terraform Provider for GitHub. + +## Current Maintainers + +| Name | GitHub | +| ------------------ | ---------------------------------------------------------- | +| Diógenes Fernandes | [@diofeher](https://github.com/diofeher) | +| Nick Floyd | [@nickfloyd](https://github.com/nickfloyd) | +| Steve Hipwell | [@stevehipwell](https://github.com/stevehipwell) | +| Viacheslav Kudinov | [@ViacheslavKudinov](https://github.com/ViacheslavKudinov) | +| Timo Sand | [@deiga](https://github.com/deiga) | + +## Emeritus Maintainers (in alphabetical order) + +| Name | GitHub | +| --------------- | -------------------------------------------- | +| Keegan Campbell | [@kfcampbell](https://github.com/kfcampbell) | + +## Contributors + +See the full list of contributors on [GitHub](https://github.com/integrations/terraform-provider-github/graphs/contributors). diff --git a/README.md b/README.md index 6e9aff2259..81b592de38 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,26 @@ # Terraform Provider GitHub -========================= -This project is used to manipulate GitHub resources (repositories, teams, files, etc.) using Terraform. Its Terraform Registry page can be found [here](https://registry.terraform.io/providers/integrations/github/). +This provider manages GitHub resources — repositories, teams, branch protections, actions secrets/variables, organization settings, rulesets, deploy keys, webhooks, and more — using Terraform. It supports both GitHub.com and GitHub Enterprise Server via the REST and GraphQL APIs. + +See the [GitHub Provider page on the Terraform Registry](https://registry.terraform.io/providers/integrations/github/) for installation and documentation. + +## Quick Start + +```hcl +provider "github" { + owner = "my-org" +} + +resource "github_repository" "example" { + name = "example-repo" + description = "Managed by Terraform" + visibility = "private" +} +``` ## Requirements @@ -14,11 +29,11 @@ This project is used to manipulate GitHub resources (repositories, teams, files, ## Usage -Detailed documentation for the GitHub provider can be found [here](https://registry.terraform.io/providers/integrations/github). +Comprehensive documentation for the GitHub Terraform provider is available on the [Terraform Registry – GitHub Provider page](https://registry.terraform.io/providers/integrations/github). ## Contributing -Detailed documentation for contributing to the GitHub provider can be found [here](CONTRIBUTING.md). +For instructions on how to contribute to the GitHub Terraform provider, see the [Contributing Guide](CONTRIBUTING.md). ## Roadmap diff --git a/github/util.go b/github/util.go index e16e568e04..99688c42fc 100644 --- a/github/util.go +++ b/github/util.go @@ -140,6 +140,8 @@ func validateValueFunc(values []string) schema.SchemaValidateDiagFunc { } // return the pieces of id `left:right` as left, right. +// +// Deprecated: use parseID2 instead. func parseTwoPartID(id, left, right string) (string, string, error) { parts := strings.SplitN(id, ":", 2) if len(parts) != 2 { @@ -150,11 +152,15 @@ func parseTwoPartID(id, left, right string) (string, string, error) { } // format the strings into an id `a:b`. +// +// Deprecated: use buildID instead. func buildTwoPartID(a, b string) string { return fmt.Sprintf("%s:%s", a, b) } // return the pieces of id `left:center:right` as left, center, right. +// +// Deprecated: Use parseID3 instead. func parseThreePartID(id, left, center, right string) (string, string, string, error) { parts := strings.SplitN(id, ":", 3) if len(parts) != 3 { @@ -165,6 +171,8 @@ func parseThreePartID(id, left, center, right string) (string, string, string, e } // format the strings into an id `a:b:c`. +// +// Deprecated: use buildID instead. func buildThreePartID(a, b, c string) string { return fmt.Sprintf("%s:%s:%s", a, b, c) }