Repository navigation
feat(policy): unify root layout and module contract (three-root parity) - #53
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Alexey Zhokhov <alexey@zhokhov.com>
d1030fa to
b339e05
Compare
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1030fa8df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| condition = toset(keys(var.repository_policies)) == toset(["homebrew-tap", "jackin", "jackin-agent-smith", "jackin-dev", "jackin-github-terraform", "jackin-marketplace", "jackin-role-action", "jackin-sentinel", "jackin-the-architect"]) | ||
| error_message = "repository_policies must contain exactly the 9 managed jackin-project repositories (8 standard plus jackin-marketplace)." |
There was a problem hiding this comment.
When repository_policies contains an extra or misspelled key, this assertion only produces a warning because OpenTofu check failures do not stop planning or applying. The module subsequently iterates over that key and can create or manage an unintended GitHub repository despite the message claiming that exactly nine repositories are allowed; enforce this through variable validation or a resource/module precondition instead.
Useful? React with 👍 / 👎.
| to = module.repository_policy.github_repository_ruleset.protect_main["homebrew-tap"] | ||
| id = "homebrew-tap:15170998" |
There was a problem hiding this comment.
When github_organization is overridden for another owner, fresh-state planning still attempts to import this ruleset using a numeric ID taken from jackin-project; the repository-name import resolves under the configured owner, but its ruleset IDs do not. Consequently the newly configurable provider cannot bootstrap another organization and the import fails, so either keep the owner fixed or make the import IDs owner-specific.
Useful? React with 👍 / 👎.
| output "repository_ids" { | ||
| description = "Map of repository name to repository ID" | ||
| value = module.repository_policy.repository_ids | ||
| } |
There was a problem hiding this comment.
The module adds ruleset_enforcement, but the root passes through only the other four newly standardized outputs. Root consumers therefore cannot inspect branch/tag enforcement, review-thread resolution, or bypass actors, leaving this root inconsistent with the stated shared output contract; add a root ruleset_enforcement output alongside these passthroughs.
Useful? React with 👍 / 👎.
Plan-only unification change per the approved three-root plan (Steps 2-6). No applies; no .github/.velnor changes.
What changed: provider.tf->providers.tf (owner=var.github_organization, nullable sensitive token); branch-protection.tf->repositories.tf (content unchanged); deleted tests/verify-policy.sh + tests/ dir; deleted migrations.tf (drained: moved blocks from merged #44, no local state file present); variables.tf canonical schema (drop disposition, add no_ci_reason=optional(string), visibility + XOR validations); locals.tf check_templates {standard=[Required,DCO], policy=[Policy]} as copy-paste source — variable defaults MUST be literals in Terraform so entries repeat the lists verbatim; module rewritten to contract (single managed_repositories set, managed visibility NOT in ignore_changes, rulesets reference managed_settings names, squash-only PR_TITLE/PR_BODY, allow_update_branch=false, delete_branch_on_merge=true, prevent_destroy, strict=false, zero bypass); outputs managed_repositories/repository_settings/required_check_contexts/repository_ids + ruleset_enforcement (shared name/shape with sibling roots); checks.tf: mandatory-set == full 9 keys (fixes 8-vs-9 skew), self-protection = key present, CI-gate [Required,DCO], XOR match, allow_update_branch/squash/thread-resolution+active-enforcement/zero-bypass, empty no_ci allowlist; imports.tf restored (9 triples = 27 blocks); new outputs.tf + minimal README.md. Kept old jackin-github-terraform inventory key (rename is a later step).
Verification: tofu fmt -check -recursive pass; tofu init -backend=false -lockfile=readonly pass; tofu validate pass; grep RepoSettingsOnly/disposition/.sh in *.tf empty; fresh-state tofu plan: 27 to import, 0 to add/change/destroy.