🦉

OSS (Hasura) へコントリビュートした

2022/09/24に公開

はじめに

はじめて OSS へのコミットに挑戦してみました。なかなか達成感があったのでやったことを記事にして残しておくことにしました。対象は仕事でもプライベートでもお世話になっている Hasura を選びました。業務で遭遇したバグがあったこともきっかけのひとつになりました。
Hasura を使っている・興味のある方、OSS へのコミットに興味のある方の参考になれば幸いです。

ウォーミングアップ

全く自分と同じような境遇の記事をたまたま見つけたので目を通しました。

https://qiita.com/sho-hata/items/9140710ad4e409e20854

いきなりコードの改修に手を出すのはハードルが高いと思ったので、はじめはコントリビュートのお作法を学ぶ目的でドキュメントの typo 修正からやってみました。この段階でコントリビューションガイドの確認などを行い PR の出し方を把握しました。その時のログはスクラップに残しています。

https://zenn.dev/kmtym1998/scraps/a503ad1cd447fc

PR はこちら。この修正は 1 日ほどでマージされました。

https://github.com/hasura/graphql-engine/pull/8897

修正対象の issue

本命の issue はこちらです。

https://github.com/hasura/graphql-engine/issues/8579

Hasura Console で Remote Schemas のパーミッションを保存した後に吐き出されるメタデータが間違っているというバグです。メタデータのファイルに記述されているロールに許可された GraphQL のスキーマから schema { query: Query mutation: Mutation }[1] が無くなってしまい、このメタデータを適用すると Remote Schemas のパーミッションがされない状態となってしまいます。

結構致命的な感じがしますが v2.6.0 以降で発生し放置されていました。Remote Schemas はよく使うのでこれは直したいです。issue のコメントや open な PR を見て、他にも同じバグを修正している人はいなさそうなことを確認してバグの調査と修正に取り掛かります。

バグの原因調査

Hasura の仕様を調査

バグの原因を説明するための前提知識として Hasura の仕様の説明をします。自分がコードを読んで解釈した部分も説明するので公式ドキュメント等に記載のない内容も含まれています。誤った説明があればご指摘ください。

Hasura は大きく分けて 3 つのコンポーネントに分かれています。

コンポーネント 役割
Server (Haskell) GraphQL サーバ。DB スキーマから自動生成した API を提供する
CLI (Go) metadata や migration の適用・削除、開発用の Console の立ち上げなどを行う
Console (JavaScript) Hasura の管理画面。各種設定を編集・確認できる

今回修正をしたのは CLI のバグだったため、CLI についてもう少し補足を入れます。
開発環境では hasura console コマンドを使って Console を立ち上げて作業をしますが、その際に実は 2 つサーバが立ち上がっています。1 つは Hasura Console (9695 番で立ち上がる方)、もう 1 つは Migrate API (9693 番で立ち上がる方) です。

立ち上がった Console で Hasura のメタデータの変更を加える操作をすると、Console から Migrate API にリクエストが飛びます。Migrate API で Hasura が接続している DB にメタデータの変更を保存します。このときの保存先は Server から参照される場所とは別です。PostgreSQL では Hasura が接続している DB に hdb_catalog というスキーマが作られており、その配下に適用されたメタデータを保存するテーブルが存在します。(このあたりの仕様は DBMS によって違いそうです。) メタデータが DB に保存されたらそれがローカルに YAML ファイルとしてエクスポートされます。

以上を踏まえて、Remote Schemas のパーミッションをローカルの Hasura Console で設定するフローを図にするとこんな感じです。

(hasura console コマンドで Console と Migrate API を立ち上げた状態で)

  1. パーミッションの編集・保存
  2. Console から Migrate API にパーミッションに関するデータを送る (ここはロール名やそのロールに対して許可する GraphQL スキーマを文字列にしたものが含まれます)
  3. Migrate API が変更後のメタデータを DB に保存
  4. 変更後のメタデータをローカルのメタデータにエクスポート

という流れになります。

バグの原因

前置きが長かったので、修正対象のバグがどんなバグだったかもう一度確認します。(issue を再掲)

https://github.com/hasura/graphql-engine/issues/8579

パーミッションを保存したときロールに許可するスキーマから schema { query: Query mutation: Mutation } が消えてしまうことが問題でした。
図の ① 〜 ④ の順にデータの流れを追ってゆき、どこで schema { query: Query mutation: Mutation } が欠落するのかを調べました。その結果、問題のありそうな箇所は図の ④ の箇所にあることがわかりました。具体的にコードでいうとこちら。

https://github.com/hasura/graphql-engine/blob/92f601587cda0c22f6f9779270087e0bbd02d88f/cli/internal/metadataobject/remoteschemas/remote_schemas.go#L105-L116

GraphQL スキーマの文字列を読み込んでフォーマットしている処理です。フォーマッタに vektah/gqlparser が使われています。フォーマットメソッド FormatSchema の結果に schema { query: Query mutation: Mutation } が含まれていませんでした。フォーマッタの実装を確認すると query の型名が Query の場合に意図的に省略するような仕様になっているようです。(mutation, subscription の場合も同じ)

https://github.com/vektah/gqlparser/blob/eae2341861b4421718a4007ff1d2e13f85d65e50/formatter/formatter.go#L150-L164

フォーマッタなのにブロックまるごと消えてしまうような挙動をするのはどうなんでしょうか?vektah/gqlparsergraphql-js の仕様に沿った実装がされているとのことなので、graphql-js にそういった仕様が書かれているのでしょうか。graphql-js をざっと眺めてみましたが「schemaquery フィールドの型が Query という名前だったら schema は省略可能」的な仕様は見つけられませんでした。vektah/gqlparser の実装のせいなのか、自分が仕様を見落としているだけなのかわからなかったので、issue を作って聞いてみました。(しかし音沙汰なし...)

https://github.com/vektah/gqlparser/issues/237

修正

FormatSchema の代わりに schema を欠落させないフォーマットメソッドを使えば解決しそうです。gqlparser に別のフォーマットメソッドがあるか探しました。こういうときにちゃんとテストが実装されているとメソッドがどんな結果を返すのかわかりやすくて助かりますね。

フォーマッタのテストをみて挙動を確認した感じだと FormatSchemaDocumentschema { query: Query mutation: Mutation } を省略せずにフォーマットしてくれそうでした。これを metadata のエクスポート時にも使うようにしました。以上を踏まえて作成した PR はこちらです。

https://github.com/hasura/graphql-engine/pull/8921

レビューされるまでに結構時間がかかりました。1 週間くらいなんの音沙汰もなかったので、Hasura の Discord の contrib チャンネルで催促してしまいました。

レビュワーの方がお忙しかったとのことで、PR 作成 ~ レビュー ~ マージまでに 2 週間ほどかかりました。

雑多な感想

テスト大事

hasura でも gqlparser でも、それなりに使われている OSS なだけあってモジュールごとにちゃんとテストが書かれていました。自分が改修をしたことによってほかに影響が出ないかどうかの確認がきちんとできることはもちろん、そのモジュールが何をしているのかを把握するのに大いに役立ちました。テストを書かねばという意欲が湧きました。

CI のステータス見られないの辛い

PR を Open すると circleci のチェックが走るのですが、ステータスがすべて Expected — Waiting for status to be reported となっていて最終的なステータスが見られないんですよね。CI の結果が見られないからにはローカルでテストが通ることを確認しておきたいです。CLI のテストでは dockertest を使ってさまざまなバージョンの Hasura のコンテナを立ててテストをしています。私は普段 M1 Mac で開発しているのですが v2.1 系より古いイメージでは Hasura コンテナが M1 Mac では正しく動かないので必ずテストが失敗します。テストが通ることを確認しないわけにもいかないのでわざわざ Intel Mac でテストが pass することを確認しました。CI のステータスが見られればそこで確認できたのにな〜と強く思いました。OSS では CI の結果が見られないことはよくあることなのでしょうか?

多分取っ掛かりが一番大事

冒頭で紹介した記事にも同じことが書いてありましたが、OSS のコードはその気になれば普通に読めます。地道にデバッグしまくっていれば時間はかかるかもしれませんがバグの原因はいつか分かると思います。コードを読んで修正することは頑張ればできそうなので、OSS コミットへの一番のハードルは手を付けやすいバグや issue を見つけるところなのかなと思いました。今回はたまたま手頃なバグを引けたことで、OSS へのコントリビューションのきっかけになったのでラッキーでした。

追記

2022/10/14 追記:
ここで取り上げた修正が正式にリリースされました 🎉

https://github.com/hasura/graphql-engine/releases/tag/v2.13.0

参考

https://qiita.com/sho-hata/items/9140710ad4e409e20854

https://graphql.org/learn/schema/#the-query-and-mutation-types

https://github.com/vektah/gqlparser

https://github.com/graphql/graphql-js

脚注
  1. schema については GraphQL の仕様を参照。 ↩︎

GitHubで編集を提案
株式会社BuySell Technologies

Discussion