この記事はコインチェック株式会社の アドベントカレンダー8日目(シリーズ1)の記事です。
TL;DR
- GitHub では CODEOWNERS ファイルを利用して、コードオーナーを設定できる
- CODEOWNERS ファイル自体の変更に細かい制限を設定 する方法は 2024/12 時点では存在しない
- ブランチ保護ルールと CI で CODEOWNERS ファイルのフォーマットを制限した上で、分割されたファイルを利用することで 、 「コードオーナーの変更にコードオーナー自身のレビューを必須にする」 という制限をかけることができる
- CODEOWNERS ファイルを編集した後、 codeowners_owners/ ディレクトリ下に自動生成ファイルを作る方式での具体的な実装(コード / 設定内容)を紹介する
- 紹介した実装をそのまま利用することも可能 だが、 プロジェクト毎に最適化した方法を構築することを推奨 する
- これによって、細かな統制と、開発のアジリティの両立 が実現できた
はじめに
こんにちは!
アプリケーション基盤部アプリケーション基盤グループ の 日下です。
コインチェックでは、 アドベントカレンダー4日目(シリーズ1)の Kaigi on Rails 2024に参加&登壇しました での登壇内容で紹介されているように、巨大なモノリシック なコードとなっていて、 packwerk を利用した機能のパッケージ化による分割を進めています。
モノリシック との戦いにおいては、packwerk の活用と並行して、 GitHub のコードオーナー機能も活用 しています。コードオーナー機能の活用に関しては、 「13,000 以上のソースファイル全てにコードのオーナーを割り振りを行った方法」など、今後紹介していきたい話もあるのですが、今回は ゲシュタルト崩壊気味のタイトルとともに、Tips を紹介したいと思います。
コードオーナーと統制
コードオーナーの設定
コードにオーナーを設定して管理する事は、コードベースと開発組織の拡大に伴い、必要性が増してきます。
オーナーが不明瞭なコードは、機能追加時や不具合発生時などの調査が、有識者を探して場当たり的な対応になりがちです。問題が根深い場合、根本的な解決は不可能です。
コードにオーナーを設定し、適切なレビュアーをアサインすることで、ナレッジが適切な場所に貯まり、対応の質が大幅に向上 するでしょう。
GitHub では、コードオーナー を設定できる機能があります。
GitHub のコードオーナー機能は、コードオーナーファイル (以下、CODEOWNERS ファイル) を利用して、ファイル単位で、オーナーを設定できます。
オーナーを設定しておくと、 GitHub 上でファイルを開いた時にそのファイルのオーナーを確認できることに加え、コードの変更提案(プルリクエスト作成)時に、 自動でレビュアーとしてアサインされます。
コードオーナーのレビューを必須にする
GitHub のブランチ保護は、リリースの統制において有益で、弊社でも活用しています。
GitHub の ブランチ保護の設定では、 コードオーナーのレビューを必須に設定 することができます。
レビューを必須にすることは、手続きを増やし、アジリティを下げる可能性があります。
そのため、対応の質を向上させるという側面に比べて、リリースを統制する意味合いが強くなります。
つまり、コードオーナーのレビューを必須にすることは、より細かいリリースの統制を実施することを意味します。
CODEOWNERS に対するオーナー設定
統制の観点では、コードのオーナーの定義の変更、つまり、 CODEOWNERS ファイルの変更時のレビューの設定 を考える必要があります。
つまり、コードオーナーに基づいたレビューを必須にしたとしても、 CODEOWNERS ファイルを自由に書き換え可能である場合、CODEOWNERS ファイルを変更することで、レビューの必須を解除することが可能 となります。
(もちろん、他の ブランチ保護ルールによって、不正なCODEOWNERS ファイルの変更は、運用で検知可能でしょう。ただし、他のブランチ保護ルールの承認者が、CODEOWNERS ファイルの変更内容を理解し、承認する権限を持つことになります)
そのため、GitHub のドキュメントにも、以下のような記載があります。
承認されていない変更からリポジトリを完全に保護するには、CODEOWNERS ファイル自体の所有者を定義する必要もあります。
CODEOWNERS ファイルに対して、コードオーナーを設定することは、 全てのコードオーナーの変更に対して責任を持つ承認者 が必要となり、これは、 開発のアジリティを更に下げる ことに繋がります。
CODEOWNERS ファイルに対して、コードオーナーを設定することで十分なケースもあるでしょう。例えばトップのディレクトリ単位でコードオーナーを設定し、その変更が頻繁に行われないケースなどです。
一方で、細かくコードオーナーを設定し、かつ、コードオーナーの変更が頻繁に発生する場合、CODEOWNERS ファイルに対してコードオーナーを設定して、開発のアジリティが下がるのを許容できないケースもあるでしょう。このケースについての対策を考えてみます。
コードオーナーの変更に、コードオーナー自身のレビューを必須にする
コードオーナーの変更により、アジリティが下がらないように、コードオーナーの変更に対するオーナーを新たに設定することは、再帰的になってしまい根本解決ではありません。
必要なことは、コードオーナー自身が 知らないうちにコードオーナーから外されないこと、つまり、 「コードオーナーの変更に、そのコードオーナー自身のレビューが必須である」 というアプローチが有効と考えました。
コードオーナーの変更に、コードオーナー自身のレビューを必須にするような機能は、2024/12 現在ではありません。
本記事では コードオーナー機能だけではなく、 CI (GitHub Actions)も利用して 、これを実現する方法を考えます。
コードオーナーの変更に、コードオーナー自身のレビューを必須にする方法
方式設計
方式
実現するために、以下を基本方針として仕組みを構築します。
- GitHub の 機能を利用するため、CODEOWNERS をファイルを使用する。
(独自の設定ファイル と WF などを作成せずに、GitHub の機能を活用する) - CODEOWNERS ファイル自身には、コードオーナーを設定しない。
(全てのコードオーナーの変更に対する、承認者が必要になるため) - CODEOWNERS ファイルを分割した、コードオーナー毎のファイル を codeowners_owners/ ディレクトリの下に作成する。
(コードオーナーは、ファイル単位での設定となるため)- GitHub がサポートしている CODEOWNERS ファイルをメンテ対象として、コードオーナー毎のファイルを作成する Script を用意する。
- コードオーナー毎のファイル には、CODEOWNERS ファイルでオーナーを設定する
- コードオーナー毎のファイル の内容と、CODEOWNERS ファイルの一致を担保する
この方針で、仕組みを実現するにはいくつかの注意点があります。
CODEOWNERS の文法と制限
CODEOWNERS ファイルの構文は .gitignore ファイルで使われている文法が使えます。
そのため、 *
を利用したワイルドカード構文が利用できますが、ワイルドカード構文を利用すると、どのルールが有効なのかの判定が非常に難しくなるので、この利用を制限します。
(ワイルドカード構文を利用せずとも、フォルダ名を指定することでフォルダ単位でのコードオーナーの設定が可能です)
また、 CODEOWNERS ファイルでは、同一ルールが複数指定されていた場合、詳細度に関わらず 下側の定義が優先されるため、これも制限します。
folder_1/a.txt @Alice
folder_1 @Bob # この定義で、 folder_1/a.txt についても オーナーが Bob になる
処理内容
上記から、 CODEOWNERS ファイルから コードオーナー毎のファイル を出力する Script の処理は、以下のようになるでしょう。
- CODEOWNERS ファイルの解析
- 各行を読み込み、
ファイルパス @owner1 @owner2 @owner3... # コメント
という形式と想定して、パースする - ファイルパスについて、正規化(
./a/b/../c
などをa/c
に変換) と以下のチェックを行う- ワイルドカード(
*
) が使用されていないこと - 同一パスが存在しないこと
- 子 => 親の順で定義がされていないこと
- ワイルドカード(
- コードオーナー毎のファイル定義が、CODEOWNERS ファイルに含まれていることの確認
- コードオーナー毎のファイルの出力
- codeowners_owners フォルダを作成し、その下に 「オーナー名.txt」でファイルを出力する
- team の場合は「company_name/team_name」のようになるので、その場合は company_name のフォルダを作成する
- ファイルは再作成(remove => create)とする。
- ファイルの内容は、以下の2種類を想定します
- 自身がオーナーの定義について、コメントを除いて
ファイルパス @owner1 @owner2 @owner3
のように記載- 定義にオーナーのリストを含めることで、自身がオーナーのファイルのオーナーのメンバ変更に対してもチェックが可能となる
- 自身がオーナーの定義の 子定義について、それが自身のオーナーではない場合 !ファイルパス という形式で記載
- ディレクトリで指定されている場合、その子定義でオーバーライドしてオーナーを変更することを抑止するため
- 自身がオーナーの定義について、コメントを除いて
- codeowners_owners フォルダを作成し、その下に 「オーナー名.txt」でファイルを出力する
- 各行を読み込み、
実装
スクリプト作成
ruby のスクリプトとして、 bin/generate_codeowners_owners.rb
ファイルを以下の内容で用意します。
(本記事のコードは制約なしの利用を歓迎しますが、使用した結果に関する一切の責任を負いません。)
#! /usr/bin/ruby
require "fileutils"
module GenerateCodeownersOwners
module_function
def main
# CODEOWNERS ファイルの解析
parsed_data, parse_errors = parse_file
# コードオーナー毎のファイル定義が、CODEOWNERS ファイルに含まれていることの確認
check_errors = check_exist_self(parsed_data)
unless parse_errors.empty? && check_errors.empty?
puts "CODEOWNERS ファイルの内容が不正です。以下のエラーの内容を修正してください。"
(parse_errors + check_errors).each { |e| puts "[ERROR] #{e}" }
exit(1)
end
# コードオーナー毎のファイルの出力
recreate_codeowners_owners(parsed_data)
puts "codeowners_owners/ 配下のファイルの再生成が正常に終了しました"
end
def parse_file
parsed_data = {
owner_hash: {},
path_hash: {},
}
errors = []
File.open(File.expand_path("../CODEOWNERS", __dir__)) do |f|
f.each_line.with_index(1) do |line, index|
next if line.match?(/^\s*($|#)/) # 空行,コメント行は無視
valid_lin_splite = line.gsub(/\s*#.*$/, "").split(/\s+/) # 末尾のコメントを削除して分割
path = File.expand_path(valid_lin_splite[0], "/")[1..] # パスの正規化
if path.include?("*")
errors.push "CODEOWNERS ファイルのパスに、ワイルドカード(*) が含まれています。 (#{index}行目)"
next
end
if (same_path = parsed_data[:path_hash][path])
errors.push "CODEOWNERS ファイルのパスに、重複があります。 (#{same_path[:index]}行目, #{index}行目)"
next
end
if (child_path = parsed_data[:path_hash].find { |_, v| v[:path].start_with?("#{path}/") }&.dig(1))
errors.push "CODEOWNERS ファイルのパスに、親ディレクトリによる上書きの定義があります。 (#{child_path[:index]}行目, #{index}行目)"
next
end
owners = valid_lin_splite[1..]
unless owners.all? { |o| o.start_with?("@") }
errors.push "CODEOWNERS ファイルのオーナーに、'@' で始まらないオーナーが指定されています。 (#{index}行目)"
next
end
owners.sort!.map! { |o| o[1..] } # 先頭の "@" を除去
value = {
path: path,
index: index,
owners: owners,
children: [],
}
split_path = path.split("/")
(split_path.size - 1).downto(1) do |i| # 親ディレクトリを探索
parent_path = split_path[0...i].join("/")
if parsed_data[:path_hash][parent_path]
parsed_data[:path_hash][parent_path][:children].push value
break
end
end
owners.each do |o|
parsed_data[:owner_hash][o] ||= []
parsed_data[:owner_hash][o].push value
end
parsed_data[:path_hash][path] = value
end
end
parsed_data[:owner_hash].each_value { |vs| vs.sort_by! { |v| v[:path] } }
return parsed_data, errors
end
def check_exist_self(parsed_data)
errors = []
parsed_data[:owner_hash].each do |owner, values|
unless values.any? { |v| v[:path] == codeowners_owner_path(owner) && v[:owners].size == 1 }
errors.push "CODEOWNERS ファイルに '#{codeowners_owner_path(owner)} @#{owner}' の設定が必要です。"
end
end
return errors
end
def recreate_codeowners_owners(parsed_data)
codeowners_owners_dir = File.expand_path("../codeowners_owners", __dir__)
if Dir.exist?(codeowners_owners_dir)
FileUtils.rm_rf(codeowners_owners_dir)
end
parsed_data[:owner_hash].each do |owner, values|
expand_codeowners_owner_path = File.expand_path("../#{codeowners_owner_path(owner)}", __dir__)
# team の場合 @org/team-name のように、ディレクトリ構造となるためディレクトリを作成する
FileUtils.mkdir_p(File.dirname(expand_codeowners_owner_path))
File.open(expand_codeowners_owner_path, "w") do |f|
values.each do |v|
# codeowners_owner/ 下は CODEOWNERS 側でのオーナーの設定を含めてチェックしているため、不要
next if v[:path] == codeowners_owner_path(owner)
owners_str = v[:owners].map { |o| "@#{o}" }.join(" ")
f.puts "#{v[:path]} #{owners_str}"
end
ignore_values = values.flat_map { |v| v[:children].filter { |c| !c[:owners].include?(owner) } }
ignore_values.each do |v|
f.puts "!#{v[:path]}"
end
end
end
end
def codeowners_owner_path(owner)
"codeowners_owners/#{owner}.txt"
end
end
GenerateCodeownersOwners.main if __FILE__ == $0
GitHub Actions の WF 作成
上記で作成した bin/generate_codeowners_owners.rb
を利用した、ワークフローを作成します。
( bin/generate_codeowners_owners.rb
は、ワークフローのみでなく、コミット前に開発者が実行するためにも使います)
ワークフローで確認する内容は、以下の2点です
- スクリプトで発生したエラーの確認
- 自動生成ファイルの再作成後の差分の確認
以下の .yml ファイルを、 .github/workflows/codeowners_owners_check.yml
に配置することで、 ワークフローが登録されます。
name: Codeowners owners check
on:
pull_request:
types: [opened, synchronize, reopened]
jobs:
codeowners_owners_check:
name: codeowners_owners_check
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v4
- name: exec generate_codeowners_owners.rb
run: ruby bin/generate_codeowners_owners.rb
- name: check file diff
run: |
echo "以下の差分が発生している場合は、 `ruby bin/generate_codeowners_owners.rb` を実行して差分を解消してください。"
git --no-pager diff && test -z "$(git status -s)"
ブランチ保護ルールの設定
Require review from Code Owners
ブランチの保護ルールの、「Require a pull request before merging」 を有効にし、その詳細項目中の「Require review from Code Owners」を有効にします。
これにより、コードオーナーが設定されているファイルについて、コードオーナーのレビューが必須となります
Require status checks to pass
ブランチの保護ルールの、「Require status checks to pass」 を有効にし、「 + Add checks 」 から 「codeowners_owners_check」を選択して、作成した GitHub Actions のチェックを有効にします。
これにより、 CODEOWNERS ファイルのチェックと、オーナー毎のコードオーナーが設定された codeowners/ 配下のファイルとの整合性が担保されます。
CODEOWNERS への設定
最後に、作成した bin/generate_codeowners_owners.rb
と .github/workflows/codeowners_owners_check.yml
に コードオーナーを設定します。
これらのファイルを改竄すると、今回作成した仕組みが働かなくなることと、これらのファイルは頻繁に更新される必要がないため、 「ブランチ保護ルールの設定が可能」と同等程度の権限と考えて、コードオーナーを設定すると良いでしょう。
以下に、(統制として最低限必要な) CODEOWNERS ファイルのサンプルを記載します
bin/generate_codeowners_owners.rb @org/maintain-team
.github/workflows/codeowners_owners_check.yml @org/maintain-team
codeowners_owners/org/maintain-team.txt @org/maintain-team
上記を記載して bin/generate_codeowners_owners.rb
を動かすことで、 codeowners_owners/org/maintain-team.txt
に以下の内容のファイルが作成されますので、この生成されたファイルと合わせて、更新してください。
.github/workflows/codeowners_owners_check.yml @org/maintain-team
bin/generate_codeowners_owners.rb @org/maintain-team
動作確認
正常系
以下の通りに オーナーが既に設定されているとします。
bin/generate_codeowners_owners.rb @org/maintain-team
.github/workflows/codeowners_owners_check.yml @org/maintain-team
codeowners_owners/org/maintain-team.txt @org/maintain-team
data/owner_a @owner_a
data/owner_a_or_b @owner_a @owner_b
data/owner_a_or_b/important.txt @owner_a
codeowners_owners/owner_a.txt @owner_a
codeowners_owners/owner_b.txt @owner_b
data/owner_a @owner_a
data/owner_a_or_b @owner_a @owner_b
data/owner_a_or_b/important.txt @owner_a
data/owner_a_or_b @owner_a @owner_b
!data/owner_a_or_b/important.txt
ここで、 CODEOWNERS
ファイルに記載されている、 important.txt
のオーナーを変更します。
data/owner_a_or_b @owner_a @owner_b
-data/owner_a_or_b/important.txt @owner_a
+data/owner_a_or_b/important.txt @owner_b
./bin/generate_codeowners_owners.rb を実行して、差分を確認します
data/owner_a_or_b @owner_a @owner_b
-data/owner_a_or_b/important.txt @owner_a
+!data/owner_a_or_b/important.txt
data/owner_a_or_b @owner_a @owner_b
-!data/owner_a_or_b/important.txt
+data/owner_a_or_b/important.txt @owner_b
codeowners_owners/owner_a.txt と codeowners_owners/owner_b.txt に 差分が発生しています。
この変更に対する PR を作成します。
( ※ 以下、画像は Reviewers の部分など一部マスクしたものとなります。 )
PR 作成画面において、コードオーナーのレビューが必要なことが示されます。
(作成後)
設定した「Require review from Code Owners」により、Code owner の レビューが必要なため、 approve がつくまで merge が抑制されます
コマンド実行結果は割愛しますが、以下のようなケースで適切なオーナーへのレビューが必須になることが確認できます
- A が owner の設定を追加した場合
- A のレビューが必須となる
- owner を A → B に変更した場合
- A と B のレビューが必須となる
- A と B が owner のファイルのオーナーに、 C を追加した場合
- A と B と C のレビューが必須となる
- A が owner の設定を削除した場合
- A のレビューが必須となる
- A が owner のフォルダの配下の、B が owner のファイルのオーナーを C に変更した場合
- B と C のレビューが必須になる
- A のレビューは不要
ただし、上記には 1点、条件があります。
GitHub のドキュメントには以下の記載があります。
コードオーナーがレビューのリクエストを受け取るためには、CODEOWNERS ファイルがプルリクエストの base ブランチになければなりません。
これは、PR でコードオーナーを追加するときに、既に追加されているコードオーナーに対してのみ、レビューのリクエストが発生する、と読み替えられます。
つまり、既にオーナーとなっているファイルが存在する ( codeowners_owners/owner_a.txt が存在する)場合には、上記の条件の通り動くのですが、まだ、オーナーとなっているファイルが存在していないオーナーに対しての、最初のPR においては、追加時のレビューは必須とはなりません。
コードオーナーの変更 / 削除時は、制限を変更するため、統制上の観点からレビュー必須が大事になりますが、追加時は制限を強化するだけという理由と、運用などで回避も可能なので、この記事としては、これについての対応は対象外とします。
異常系
CODEOWNERS ファイルの定義がおかしい以下のファイルを作成します
bin/generate_codeowners_owners.rb @org/maintain-team
.github/workflows/codeowners_owners_check.yml @org/maintain-team
codeowners_owners/org/maintain-team.txt @org/maintain-team
data/owner_a @owner_a
data/owner_a_or_b/owner_a.txt @owner_a
data/owner_a_or_b @owner_a @owner_b
data/owner_b owner_b # コメント
data/owner_c @owner_c
data/team_a/* @org/team_a
data/team_a/owner_b.txt @owner_b
data/team_a/owner_b.txt @owner_a @owner_b
data/common/owner_c @owner_c
codeowners_owners/owner_a.txt @owner_a
codeowners_owners/owner_b.txt @owner_b
codeowners_owners/owner_c.txt @owner_c
codeowners_owners/org/team_a.txt @org/team_a
CI の結果が不正となり、 merge が抑制されます
Details をクリックすることで、CODEOWNERS のエラー内容が確認できます
また、別のケースとしてCODEOWNERS ファイルだけを更新して、 bin/generate_codeowners_owners.rb を動かさなケースを考えます。
差分が発生するため、 CI の結果が不正となり、 merge が抑制されます
Details をクリックすることで、 bin/generate_codeowners_owners.rb の実行が必要であることに気づくことができます。
おわりに
この記事で紹介した「コードオーナーの変更に、コードオーナー自身のレビューを必須にする方法」は、コインチェックの業務で、そのまま使っているわけではありません。
というのも、コインチェックでは以下のような前提があります。
- CODEOWNERS ファイル自体をツールで生成しているため、この記事で書いたフォーマットチェックが不要
- コードオーナーには、原則として Team を設定するが、通知などの用途で合わせて Team 内の個人を設定することも可能としていて、コードオーナーの変更レビューは Team のみに設定したい
つまり、「仕組み自体入れているが、 実装は一部異なる 」といえます。
また、この「コードオーナーの変更に、コードオーナー自身のレビューを必須にする」という制約は、数あるブランチ保護ルールの一つ であり、統制のためのフィルターの一部でしかありません。
そのため、ソースファイル全てにコードのオーナーを割り振りを行ったのですが、CODEOWNERS ファイルに設定を入れて、レビュー必須の対象とするものは一部のファイルに限っている、という事情もあります。
(一律ではなく、GitHub の CODEOWNERS ファイル側に レビュー必須の有無を設定できるように、GitHub 側の機能が拡充されることを、密かに望んでいます)
この記事は、実際に今年取り入れた仕組みの紹介する、という点はあるものの、「コードオーナーに対するコードオーナー」という再帰的ともいえるテーマが楽しいかな?という動機もありました。
結果として、コードにオーナーを設定するコードオーナーの仕組み」と「不正なコードが入らないための、ブランチ保護ルールの仕組み」を組み合わせるという、やや発展的で、モノリス かつ (金融系など) 厳しい統制が必要な場合の、ニッチな領域の話になりました。
読み物として楽しんでいただき、何かの参考になれば幸いです。
コインチェックのアドベントカレンダーは、まだまだ続きますのでお楽しみに!