diff --git a/terraform/30-locals.tf b/terraform/30-locals.tf index d9b8070..c252411 100644 --- a/terraform/30-locals.tf +++ b/terraform/30-locals.tf @@ -35,16 +35,18 @@ locals { # outside this set indicates a typo or schema drift and is rejected by # terraform_data.framework_validation. # - # NOTE: `allow_forking` is intentionally absent. The setting is org-only - # in the GitHub API and this framework does not currently manage it via a - # provider-backed resource (github_repository_setting / org-level config). - # Accepting the key would be a silent no-op, so it is rejected at the - # unknown-key stage with a clear error pointing at the offending repo. + # NOTE: `allow_forking` is opt-in. The GitHub API rejects PATCH calls + # that include the field on personal-account private repositories, so + # this framework treats the field as null-by-default. A repo YAML may + # explicitly opt in by setting `allow_forking: true|false` — the + # provider includes the field in PATCH only when non-null, so opt-in + # repos manage the setting normally and opt-out (default) repos never + # trip the API rejection. allowed_repo_keys = toset([ "description", "homepage_url", "topics", "fork", "source_owner", "source_repo", "visibility", "has_discussions", "has_issues", "has_projects", "has_wiki", "is_template", - "allow_auto_merge", "allow_merge_commit", "allow_rebase_merge", + "allow_auto_merge", "allow_forking", "allow_merge_commit", "allow_rebase_merge", "allow_squash_merge", "allow_update_branch", "delete_branch_on_merge", "merge_commit_message", "merge_commit_title", "squash_merge_commit_message", "squash_merge_commit_title", @@ -433,6 +435,29 @@ locals { local.repo_setting_defaults.allow_auto_merge ) + # Default policy (ownership-aware): + # 1. YAML explicitly sets true/false → honor it. + # 2. YAML omits the key + visibility == "private" + # + github_is_organization == false → null. + # (Personal-account private: the API rejects any PATCH + # containing allow_forks regardless of value, so the + # provider must omit the field entirely.) + # 3. Every other case → false (secure-by-default; forks disabled + # unless YAML opts in via `allow_forking: true`). + # Covers public, internal, AND org-owned private — + # GitHub's API accepts the field for all three, so the + # framework enforces the policy. + allow_forking = ( + try(repository.allow_forking, null) != null + ? repository.allow_forking + : ( + coalesce(try(repository.visibility, null), "private") == "private" + && !var.github_is_organization + ? null + : false + ) + ) + allow_merge_commit = coalesce( try(repository.allow_merge_commit, null), local.repo_setting_defaults.allow_merge_commit diff --git a/terraform/41-resources-github.tf b/terraform/41-resources-github.tf index c76eccc..190161b 100644 --- a/terraform/41-resources-github.tf +++ b/terraform/41-resources-github.tf @@ -49,6 +49,7 @@ resource "github_repository" "repo" { is_template = each.value.is_template # Merge Behavior + allow_forking = each.value.allow_forking allow_merge_commit = each.value.allow_merge_commit allow_squash_merge = each.value.allow_squash_merge allow_rebase_merge = each.value.allow_rebase_merge @@ -153,11 +154,13 @@ resource "github_repository" "repo" { # check repo-scoped invariants so their errors point at the specific # github_repository.repo[] address. lifecycle { - # `allow_forking` is an org-owned private/internal repository setting in - # GitHub's API. The provider may observe it during refresh, but this - # framework does not expose it in repo YAML and must not PATCH it for - # personal-account repositories. - ignore_changes = [auto_init, license_template, allow_forking] + # `auto_init` and `license_template` are CREATE-time only; ignoring + # them prevents spurious diff after the initial create. `allow_forking` + # is now explicitly managed in YAML (default null for private repos so + # the provider omits it from PATCH; default false for public/internal; + # YAML may override either). It is NOT ignored anymore — drift in + # YAML-managed values should surface as a real diff. + ignore_changes = [auto_init, license_template] precondition { condition = contains(["public", "private", "internal"], each.value.visibility) diff --git a/terraform/tests/COVERAGE.md b/terraform/tests/COVERAGE.md index ff65618..9b5ed01 100644 --- a/terraform/tests/COVERAGE.md +++ b/terraform/tests/COVERAGE.md @@ -36,7 +36,7 @@ terraform test |---|---|---|---| | G01 | Duplicate repo keys across `public/` and `private/` | ✅ | `validation.tftest.hcl::rejects_duplicate_repo_keys` | | G02 | Unknown top-level YAML key | ✅ | `validation.tftest.hcl::rejects_unknown_top_level_key` | -| G03 | `allow_forking` rejected as unknown key | ✅ | `validation.tftest.hcl::rejects_allow_forking` | +| G03 | `allow_forking` accepted and applied with visibility-aware default (private→null, else false; YAML may override) | ⚠️ | needs apply-time test against synthetic provider | | G04 | Unknown nested: `actions.*` (top) | ✅ | `validation.tftest.hcl::rejects_unknown_nested_key` + multi-typo | | G05 | Unknown nested: `actions.allowed_actions_config.*` | ✅ | `validation.tftest.hcl::rejects_multiple_nested_typos_in_one_repo` | | G06 | Unknown nested: `pages.*` | ✅ | `validation.tftest.hcl::rejects_multiple_nested_typos_in_one_repo` | diff --git a/terraform/tests/fixtures/bad-allow-forking/private/.gitkeep b/terraform/tests/fixtures/bad-allow-forking/private/.gitkeep deleted file mode 100644 index e69de29..0000000 diff --git a/terraform/tests/fixtures/bad-allow-forking/public/bad.yml b/terraform/tests/fixtures/bad-allow-forking/public/bad.yml deleted file mode 100644 index c1e6fe4..0000000 --- a/terraform/tests/fixtures/bad-allow-forking/public/bad.yml +++ /dev/null @@ -1,8 +0,0 @@ -bad-allow-forking-repo: - description: "allow_forking fixture" - visibility: private - # allow_forking is intentionally absent from local.allowed_repo_keys - # because the framework does not currently manage the setting. Accepting - # it would be a silent no-op, so it is rejected at the unknown-top-level - # key validation stage. - allow_forking: true diff --git a/terraform/tests/validation.tftest.hcl b/terraform/tests/validation.tftest.hcl index 7564f05..ec846ae 100644 --- a/terraform/tests/validation.tftest.hcl +++ b/terraform/tests/validation.tftest.hcl @@ -73,18 +73,6 @@ run "rejects_unknown_nested_key" { ] } -run "rejects_allow_forking" { - command = plan - - variables { - repo_yaml_path = "tests/fixtures/bad-allow-forking" - } - - expect_failures = [ - terraform_data.framework_validation, - ] -} - run "rejects_duplicate_repo_keys" { command = plan