diff --git a/config/dev-infra-protection.json b/config/dev-infra-protection.json index e6c0ce6..05323e0 100644 --- a/config/dev-infra-protection.json +++ b/config/dev-infra-protection.json @@ -2,16 +2,24 @@ "$schema-note": "開発インフラリポジトリのブランチ保護とマージ設定の desired state。方針は https://github.com/smkwlab/latex-ecosystem/blob/main/docs/DEPENDENCY-MANAGEMENT.md を参照。学生リポジトリは対象外で、student-repo-management の setup-branch-protection.sh が別に管理する。", "branch": "main", "invariants": { - "allow_auto_merge": "false 固定。GitHub の auto-merge はブランチ保護の要件が満たされた時点でマージするため、required status checks が空のリポジトリでは CI 完了前に入る。マージは Renovate 自前の automerge に任せる", + "allow_auto_merge": "false 固定。GitHub の auto-merge はブランチ保護の要件が満たされた時点でマージするため、required に入っていない check を待たない。マージは Renovate 自前の automerge に任せる", "enforce_admins": "false 固定。CI が壊れた緊急時に管理者が明示的に突破できるようにする。Renovate App はブランチ保護をバイパスしないので bot 側は必ずゲートされる", - "strict": "false 固定。up-to-date 要求を付けると、1 本マージするたび全 PR が rebase と再ビルドになる", - "contexts": "その PR で必ず check run が生成されるジョブだけを列挙する。job レベルの if: で skip されたジョブは conclusion=skipped の check run が出るので指定してよい。workflow レベルの paths: / branches: フィルタで発火しない workflow は check run 自体が出ず永久 pending になるので指定してはいけない" + "strict": "latex 系は false。up-to-date 要求を付けると、1 本マージするたび全 PR が rebase と再ビルドになる。elixir 系は true のまま置いている。マージが月 0〜4 件で再ビルドの費用が出ておらず、個別には緑でも組み合わせると壊れる PR の検出を優先している", + "contexts": "その PR で必ず check run が生成されるジョブだけを列挙する。job レベルの if: で skip されたジョブは conclusion=skipped の check run が出るので指定してよい。workflow レベルの paths: / branches: フィルタで発火しない workflow は check run 自体が出ず永久 pending になるので指定してはいけない", + "require_pull_request": "true なら「Require a pull request before merging」を有効にする。これが無いと main へ直接 push できる。有効な 9 リポジトリは設定内容が完全に同じなので、形は review_settings に一度だけ書き、リポジトリごとには有無だけを宣言する。latex-environment と tenbin_cache は現在無効で、揃えるかどうかは未決。現状を写して監査対象に載せてある", + "review_settings": "ここに書いたフィールドだけを管理する。require_last_push_approval や bypass_pull_request_allowances のような書いていないフィールドは、audit では突き合わせず、apply では PUT の全項目置換によって API の既定値に戻る。現在 11 リポジトリすべてが既定値なので両者は一致している。既定から外した値を残したいなら、ここに書き足すこと" + }, + "review_settings": { + "required_approving_review_count": 0, + "dismiss_stale_reviews": true, + "require_code_owner_reviews": false }, "repositories": [ { "name": "texlive-ja-textlint", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": true, "required_status_checks": { "strict": false, "contexts": ["changes", "build-alpine", "build-debian", "build-debian-arm64"] @@ -21,6 +29,7 @@ "name": "latex-environment", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": false, "required_status_checks": { "strict": false, "contexts": ["build-and-release-pdf"] @@ -30,6 +39,7 @@ "name": "latex-release-action", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": true, "required_status_checks": { "strict": false, "contexts": ["yaml-lint", "test-build"] @@ -39,6 +49,7 @@ "name": "ai-academic-paper-reviewer", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": true, "required_status_checks": { "strict": false, "contexts": ["test"] @@ -48,6 +59,7 @@ "name": "student-repo-management", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": true, "required_status_checks": { "strict": false, "contexts": ["Validate YAML files"] @@ -57,10 +69,61 @@ "name": ".github", "allow_auto_merge": false, "enforce_admins": false, + "require_pull_request": true, "required_status_checks": { "strict": false, "contexts": ["actionlint"] } + }, + { + "name": "tenbin_dns", + "allow_auto_merge": false, + "enforce_admins": false, + "require_pull_request": true, + "required_status_checks": { + "strict": true, + "contexts": ["ci / Code Quality", "ci / All checks"] + } + }, + { + "name": "tdig", + "allow_auto_merge": false, + "enforce_admins": false, + "require_pull_request": true, + "required_status_checks": { + "strict": true, + "contexts": ["ci / Code Quality", "ci / All checks"] + } + }, + { + "name": "tenbin_ex", + "allow_auto_merge": false, + "enforce_admins": false, + "require_pull_request": true, + "required_status_checks": { + "strict": true, + "contexts": ["ci / Code Quality", "ci / All checks"] + } + }, + { + "name": "tenbin_cache", + "allow_auto_merge": false, + "enforce_admins": false, + "require_pull_request": false, + "required_status_checks": { + "strict": true, + "contexts": ["ci / Code Quality", "ci / All checks"] + } + }, + { + "name": "elixir_dnstap", + "allow_auto_merge": false, + "enforce_admins": false, + "require_pull_request": true, + "required_status_checks": { + "strict": true, + "contexts": ["ci / Code Quality", "ci / All checks"] + } } ] } diff --git a/scripts/README.md b/scripts/README.md index d73ae82..4799863 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -111,10 +111,14 @@ caller を追加したら対象リポジトリで動作を確認してくださ [依存管理基盤(Renovate 一本化)](https://github.com/smkwlab/latex-ecosystem/blob/main/docs/DEPENDENCY-MANAGEMENT.md) にあります。 -対象は `texlive-ja-textlint` / `latex-environment` / `latex-release-action` / -`ai-academic-paper-reviewer` / `student-repo-management` / `.github` の 6 つ。学生 -リポジトリは対象外で、`student-repo-management` の `setup-branch-protection.sh` が -別に管理します。 +対象は 11 リポジトリです。latex 系が `texlive-ja-textlint` / `latex-environment` / +`latex-release-action` / `ai-academic-paper-reviewer` / `student-repo-management` / +`.github` の 6 つ、elixir 系が `tenbin_dns` / `tdig` / `tenbin_ex` / `tenbin_cache` / +`elixir_dnstap` の 5 つ。学生リポジトリは対象外で、`student-repo-management` の +`setup-branch-protection.sh` が別に管理します。 + +値は系統ごとに違うところがあります(elixir 系は `strict: true`)。理由は desired +state の `invariants` に書いてあります。 ```bash # 乖離があれば非ゼロ終了(読み取りのみ) @@ -150,7 +154,11 @@ secret が未設定のときは skip せず失敗します。何も見ていな 一部フィールドが黙って落ちる事例が出ています(smkwlab/student-repo-management#577)。 管理者の PAT で手動実行してください。audit は読み取りのみなので App token でも動きます - ブランチ保護の PUT は**全項目置換**です。desired state が宣言していない項目は消えます。 - 宣言を増やすときは apply スクリプトの送信ペイロードも合わせて広げること + 宣言を増やすときは apply スクリプトの送信ペイロードも合わせて広げること。 + 実際に踏んだ例として、`required_pull_request_reviews` を宣言せず `null` で送っていた + 時期があり、`--apply` すると「Require a pull request before merging」が外れて + main へ直接 push できる状態になっていました(現在は `require_pull_request` として + 宣言しています) - `contexts` には**その PR で必ず check run が生成されるジョブだけ**を並べます。 workflow レベルの `paths:` / `branches:` フィルタで発火しない workflow を required に すると、非該当 PR が永久 pending になります(job レベルの `if:` による skip は diff --git a/scripts/apply-repo-protection.sh b/scripts/apply-repo-protection.sh index 81a1ca7..ad0e0ba 100755 --- a/scripts/apply-repo-protection.sh +++ b/scripts/apply-repo-protection.sh @@ -56,6 +56,16 @@ log() { branch=$(jq -r '.branch' "$CONFIG_PATH") count=$(jq '.repositories | length' "$CONFIG_PATH") +# require_pull_request が true のリポジトリに送る中身。有効な側は設定が全て同じ +# なので宣言では 1 箇所にまとめてある。 +# +# 欠けていたら止める。null のまま進むと required_pull_request_reviews に null を +# 送ることになり、「Require a pull request before merging」を外す動作に戻る +review_settings=$(jq -c '.review_settings' "$CONFIG_PATH") +if [ -z "$review_settings" ] || [ "$review_settings" = "null" ]; then + log "review_settings が宣言に無い。require_pull_request の適用先が決まらないので中止する" + exit 1 +fi log "desired state: $CONFIG_PATH" log "対象: ${count} リポジトリ (org: ${ORG}, branch: ${branch})" @@ -86,9 +96,10 @@ while IFS= read -r spec; do am=$(printf '%s' "$spec" | jq -r '.allow_auto_merge') admins=$(printf '%s' "$spec" | jq -r '.enforce_admins') checks=$(printf '%s' "$spec" | jq -c '.required_status_checks') + pr_required=$(printf '%s' "$spec" | jq -r '.require_pull_request') if [ "$APPLY" != "true" ]; then - log " would apply: ${name} — allow_auto_merge=${am} enforce_admins=${admins} checks=${checks}" + log " would apply: ${name} — allow_auto_merge=${am} enforce_admins=${admins} require_pull_request=${pr_required} checks=${checks}" applied=$((applied + 1)) continue fi @@ -102,10 +113,14 @@ while IFS= read -r spec; do log " ERROR: ${name} — allow_auto_merge を設定できなかった: ${err}" fi - body=$(printf '%s' "$spec" | jq '{ + # required_pull_request_reviews は宣言から組み立てる。PUT は全項目置換なので、 + # ここを無条件に null にすると「Require a pull request before merging」が外れ、 + # main へ直接 push できる状態になる。宣言している 11 リポジトリのうち 9 つが + # この設定を持っており、null 固定は保護を落とす操作だった + body=$(printf '%s' "$spec" | jq --argjson reviews "$review_settings" '{ required_status_checks: .required_status_checks, enforce_admins: .enforce_admins, - required_pull_request_reviews: null, + required_pull_request_reviews: (if .require_pull_request then $reviews else null end), restrictions: null }') if ! err=$(printf '%s' "$body" | gh api -X PUT \ diff --git a/scripts/audit-repo-protection.sh b/scripts/audit-repo-protection.sh index 2bc8fff..08332e3 100755 --- a/scripts/audit-repo-protection.sh +++ b/scripts/audit-repo-protection.sh @@ -55,6 +55,14 @@ log() { branch=$(jq -r '.branch' "$CONFIG_PATH") count=$(jq '.repositories | length' "$CONFIG_PATH") +# require_pull_request が true のリポジトリに期待する中身。 +# 欠けていたら止める。null のまま進むと全リポジトリが drift として報告され、 +# 実際にはずれていないのに「ずれている」と読める報告が毎週出ることになる +review_settings=$(jq -cS '.review_settings' "$CONFIG_PATH") +if [ -z "$review_settings" ] || [ "$review_settings" = "null" ]; then + log "review_settings が宣言に無い。期待値が決まらないので中止する" + exit 1 +fi log "desired state: $CONFIG_PATH" log "対象: ${count} リポジトリ (org: ${ORG}, branch: ${branch})" @@ -130,6 +138,23 @@ while IFS= read -r spec; do report "${name}: enforce_admins want=${want_admins} got=${got_admins}" fi + # 「Require a pull request before merging」の有無。これが外れると main へ + # 直接 push できるようになるが、PR も check run も出ないので気付く手段が無い + want_pr=$(printf '%s' "$spec" | jq -r '.require_pull_request') + got_pr=$(printf '%s' "$prot" | jq -r '.required_pull_request_reviews != null') + if [ "$want_pr" != "$got_pr" ]; then + report "${name}: require_pull_request want=${want_pr} got=${got_pr}" + elif [ "$want_pr" = "true" ]; then + got_reviews=$(printf '%s' "$prot" | jq -cS '.required_pull_request_reviews | { + required_approving_review_count, + dismiss_stale_reviews, + require_code_owner_reviews + }') + if [ "$review_settings" != "$got_reviews" ]; then + report "${name}: review settings want=${review_settings} got=${got_reviews}" + fi + fi + want_strict=$(printf '%s' "$spec" | jq -r '.required_status_checks.strict') # `// "なし"` は使えない。jq の // は false も空として扱うため、 # strict=false が「なし」に化ける