くりにっき

フルスタックキュアエンジニアです

GitHub Actions上でPRを作る時はsecrets.GITHUB_TOKENは使わない方がよさそう

事象

GitHub Actions上で使える secrets.GITHUB_TOKEN だと別のジョブを起動できないというのが一番の理由。

https://docs.github.com/en/actions/reference/events-that-trigger-workflows#triggering-new-workflows-using-a-personal-access-token

具体的にどういうケースで困るのかというと、GitHub Actions上でPRを作った場合にそのPRに対するビルドが行われません。

普通だったらコミットIDの左側に✔が出るはずなんだけど出ない図

f:id:sue445:20200828222616p:plain

pushやPRなどのビルドをGitHub Actions以外(例:CircleCIなど)で行っている場合には問題ないです。

解決策

いくつか案はあるのですが現状だと GitHub App Token を使うのがいいと思ってます。

App作成の手間はあるもののインストールするリポジトリを指定できるので権限の分離ができるのが一番の理由です。 (パーソナルアクセストークンだと業務で所属してるorg含めて自分が見れる全リポジトリが対象になるので漏洩時のリスクが怖い)

f:id:sue445:20200828223657p:plain

実際に動かした図

sue445-botGitHub App)でPRを作ったのでGitHub Actionsでビルドが実行されるようになってます

f:id:sue445:20200831080805p:plain

上記AppだとパーミッションはActionsとPull requestsにRead & writeをつけたけどActionsの方は不要だったかもしれないです。

2020/09/12追記:Pull requestsのRead & writeだけで良かった

f:id:sue445:20200831082740p:plain

go-mod-tidy-prでも同様の問題を抱えていたのでGitHub App Token推奨にしました。

https://github.com/sue445/go-mod-tidy-pr#note-warning

謝辞

という情報を プリッカソン のSlackで知って便利

f:id:sue445:20200829212144p:plain

2020/9/1 22:50 追記

GutHub Appを作った場合は今度はAppの秘密鍵の管理を考える必要がありますが、Organizationであれば Organization secrets が使えるのでこの手の秘密鍵の管理に便利そうです。(ってことを ruby-jp Slack で話していて思い出した)

github.blog

gcp-kmsenvを作った

github.com

モチベーション

condo3app.yaml 内に Cloud KMS で暗号化した秘匿値を埋め込みやすくするために作ったモジュールをライブラリ化しました。

とはいえGoogle App Engine専用というわけではなく、GCP + GoであればCloud Functionsとかでも使えるはずです。

使い方

READMEからかいつまんで説明すると下記

  1. 秘匿値を gcloud kms encrypt で暗号化してBASE64に変換
  2. KMS_ACCESS_TOKEN のように KMS_ を先頭に付けた環境変数として登録
    • KMSで暗号化済なのでこの変数はgitにコミットしても問題ない
  3. GetFromEnvOrKms で呼び出す
    • この時接頭辞の KMS_ は不要

FAQ

Q. Secret Manager使えばいいのでは?

本番環境ではKMSで暗号化された値を使いたいけどローカルでは暗号化していないプレーンな値を環境変数で渡したいことがよくあるので、暗号化していないプレーンな環境変数とKMSで暗号化した環境変数を透過的に扱えるようにしています。( os.Getenv と似たようなインターフェースにしたのも同じ理由)

質問をする技術

以前社内に書いたポエムなんだけど年に1回くらい引用したくなるので公開した

tl;dr;

テンプレ

【質問内容】

【やりたいこと or 今困ってること or 質問の意図】

質問をする時はゴールを提示する【MUST】

  • なにかやりたい
  • けど実現できない、うまくいかない
  • それで質問する

って感じに、質問をする動機としてまず やりたいことありき のはずなので、それを提示すべきです

理由1

質問される側(以下回答者)は質問内容がふわっとしていると色々なケースを想定して回答を組み立てます *1

例「Aの場合は~だけど、Bの場合は~」

こういう場合回答者は質問者のやりたいことを想像しながらいろんな場合分けをする必要があるので、頭を使うので疲れるし、回答を組み立てるのに時間もかかります。

酷い場合だと5つくらい場合分けして回答したことがあります。

こういう時、僕は自分の中で 超能力で回答 という言葉を使ってます。(自分以外で使ってる人は見たことないので同意は得られないことは覚悟してます)

超能力を発揮するのは疲れます。

僕も色々質問を受ける立場ですがゴールが明示されてた方が圧倒的に回答しやすいです。

理由2

初心者によくありがちなのですが、やりたいことに対して質問内容が的外れということが往々にしてあります。

  • 質問者「これってどうやるんですか?」
  • (回答者は頑張って回答を組み立てる)
  • 回答者「こうやるんですよ」
  • 質問者「さっきのだとうまくいきませんでした」

はっきり言ってこういうのは時間の無駄なので先にやりたいことを明示してもらえれば

  • 質問者「こういうことをやりたいんですがここでちょっと詰まってます。これってどうやるんですか?」
  • 回答者「んー。それ、意味ない(質問内容が実現されてもゴールにたどりつけない)っぽいw」

って感じで時間のロスを防げます

コンテキストを詳しく共有する【SHOULD】

*2

質問する側は質問内容に関して長時間思考してるのでコンテキストを理解しています。

しかし回答者はそうとは限らないので可能な限り詳しく書くべきです。

実行したコマンドと実行結果を全て書くくらいが丁度いいと思ってます。

質問内容や人によってはヒアリングしながらコンテキストを少しずつ理解するというのもありそうですが、僕は圧倒的に最初にコンテキストを詳しく書くしそういう風に質問された方が嬉しいです。

チャットとかだと横で見た人が回答しやすいってのもあります。

あと質問内容をまとめる段階で頭の中で問題が整理されるので、質問するために文章を書いてたら答えが分かるという副産物もあります。

期待してた結果(expect)と実際の結果(actual)を書く【IMO】

*3

これはどっかのOSSのissueテンプレートで見たことあるやつ。

質問内容によっては「こういうことをやっててこういう結果になってほしいんだが全然違う結果になるんだが理由がよく分からん」ってことがあると思います。

そういう場合には期待してた結果と実際の結果を書くと質問文も組み立てやすいし、回答者も回答しやすいです

2020/7/22 12:30追記

ブコメレス

id:obatakanae

“こういう場合回答者は質問者のやりたいことを想像しながらいろんな場合分けをする必要がある” それならそこで聞き返せばいいんでないの?そういうことができない状況を想定しているのだろうか?

すぐ後に

質問内容や人によってはヒアリングしながらコンテキストを少しずつ理解するというのもありそうですが、僕は圧倒的に最初にコンテキストを詳しく書くしそういう風に質問された方が嬉しいです。

って書いているように好みの問題だと思います。

チャットだと非同期でのやりとりになってすぐに返信が返ってくる(返せる)とは限らないという理由で、僕は最初にまとまって書かれている方が嬉しいです。

id:jmako

このブログ主は自分視点で考えてるが、相手視点で考えると全く違った発想になる。この本がためになるはず。"「良い質問」をする技術" 是非読んでみて。

Kindle版ポチりました!

2020/7/22 19:00追記

ブコメレス

id:sippo_des

ググれカス、一人で生きていけるようになれ、とか、そんなこと聞くな、とか言われ続けた結果なのかな。 人から与えられた経験とか。交渉とか。

「質問する時はちゃんと準備することでお互いに円滑にコミュニケーションが進むよ」っていうのがこのエントリの主題です。そんなことは一言も言っていないしそんな経験もしたこともないです。

根拠もなく勝手な想像をするのはやめてください。

*1:あくまで自分はこうしています。主語がでかすぎたかもしれないけど前職で聞いたら何人か同じことやってたっぽい

*2:MUST(絶対こうしろ)よりはニュアンス弱めて「なるべくこうすべき」くらいのニュアンス

*3:In My Opinionの略でPullRequestでよく使うスラング。「自分ならこうするけどどうよ?(強制はしない)」くらいのニュアンス

rubocop-itamae v0.1.3とrubocop_auto_corrector v0.4.1をリリースした

rubocop-itamae v0.1.3

https://github.com/sue445/rubocop-itamae/blob/master/CHANGELOG.md#v013

rubocop 0.87.0で v1 Upgrade Notes が出ていたので自分のcopでもv1形式のシンタックスに追従。

rubocopのカスタムcopを作ってる人はv1 Upgrade Notesを読んで対応しといた方がよさそう。

rubocop_auto_corrector v0.4.1

https://github.com/sue445/rubocop_auto_corrector/blob/master/CHANGELOG.md#v041

同じくrubocop 0.87.0関係の対応。

v1 Upgrade Notesだとカスタムcopでautocorrectを実装する時に autocorrect メソッドを実装するのではなく AutoCorrector moduleを extend するようになるのですが *1、そういうcop*2がいる場合にWeekly buildがコケてたので対応。

rubocop_auto_corrector v0.4.0をリリースした

rubocop v0.87.0で --auto-correct の挙動が変わったのと、 --auto-correct-all が追加されたので追従。( rubocop_auto_corrector --all で各cop単位で --auto-correct-all する)

https://github.com/sue445/rubocop_auto_corrector/blob/master/CHANGELOG.md#v040

詳しいこと

koic.hatenablog.com

Railsでdb:migrateの時だけdatabase.ymlを自動で差し替える

前置き

GitLabでPostgreSQLを使う時にPgBouncer経由でDBに接続してる(database.ymlにpgbouncerの接続先を書いている)のですが、 その状態で rake db:migrate すると下記のようなエラーが出ます。

ActiveRecord::ConcurrentMigrationError: 

Failed to release advisory lock


  from active_record/migration.rb:1385:in `with_advisory_lock'
  from active_record/migration.rb:1229:in `migrate'
  from active_record/migration.rb:1061:in `up'
  from active_record/migration.rb:1036:in `migrate'
  from active_record/tasks/database_tasks.rb:238:in `migrate'
  from active_record/railties/databases.rake:86:in `block (3 levels) in <top (required)>'
  from active_record/railties/databases.rake:84:in `each'
  from active_record/railties/databases.rake:84:in `block (2 levels) in <top (required)>'
  from rake/task.rb:273:in `block in execute'
  from rake/task.rb:273:in `each'
  from rake/task.rb:273:in `execute'
  from rake/task.rb:214:in `block in invoke_with_call_chain'
  from monitor.rb:235:in `mon_synchronize'
  from rake/task.rb:194:in `invoke_with_call_chain'
  from rake/task.rb:183:in `invoke'
  from rake/application.rb:160:in `invoke_task'
  from rake/application.rb:116:in `block (2 levels) in top_level'
  from rake/application.rb:116:in `each'
  from rake/application.rb:116:in `block in top_level'
  from rake/application.rb:125:in `run_with_threads'
  from rake/application.rb:110:in `top_level'
  from rake/application.rb:83:in `block in run'
  from rake/application.rb:186:in `standard_exception_handling'
  from rake/application.rb:80:in `run'
  from bundle/ruby/2.6.0/gems/rake-12.3.3/exe/rake:27:in `<top (required)>'
  from bundle/ruby/2.6.0/bin/rake:23:in `load'
  from bundle/ruby/2.6.0/bin/rake:23:in `<main>'

https://docs.gitlab.com/omnibus/update/README.html

If you’re using PgBouncer:

You’ll need to bypass PgBouncer and connect directly to the database master before running migrations.

とあるように db:migrate する時だけPgBouncerを使わず直接PostgreSQLにつなぐのが大正解です。

しかし

  • GitLabのバージョンアップの時だけdatabase.ymlを書き換えるのは面倒
    • 忘れがちなので自動化すべき
    • 運用でカバーするのは時代遅れなので仕組み化すべき
  • いつもCIからのデプロイでGitLabのバージョンアップを行っているのだが、database.ymlの接続先をPostgreSQLにしてデプロイ&自動でdb:migrate(1回目のデプロイ)と接続先をPgBouncerに戻す作業(2回目のデプロイ)で2回デプロイするのが面倒
    • 忘れがちなので(ry
    • 仮に2回デプロイ行うとしても一時的にPostgreSQLのコネクションが少なくなるのでオンラインでバージョンアップを行うとコネクションが足りずにエラーが多発する可能性がある
      • workerの台数によるけど常時100コネクション近く張り付いてる状態なのでメンテいれずにpgbouncer外すのは厳しいと判断
    • SwarmでBlue/GreenデプロイでGitLabをノーメンテでバージョンアップするのに慣れすぎて*1 、バージョンアップ程度でメンテ入れるのが嫌

という理由で、今のデプロイフローをなるべく変えずにどうにかdb:migrateの時だけ自動で接続先を切り替える方法がないか考えました。

モンキーパッチの縛りプレイの内容

モンキーパッチを作るにあたって下記のような縛りプレイの条件がありました

  • RailsやGitLabのバージョンアップでなるべく壊れないようにする
  • Dockerイメージで配布されているGitLabを使っているので *2既存のソースは全て変更不能
    • Dockerイメージの時点でモンキーパッチなんて不可能だと思われがちですが、GitLabのアプリケーション本体はRailsなので config/initializers/lib/tasks にファイルをmountすればRailsが自動で読み込むのでモンキーパッチを適用することができます *3
  • 既存のソースが変更できないのでGitLabに入っていないgemを使うのは難しい
    • bundle installされた状態でDockerイメージとして配布されてるので、そこに新しいgemを追加するのは茨の道
  • 利用しているDockerイメージは https://github.com/sameersbn/docker-gitlab でだが、起動時の処理はDockerイメージ内のスクリプト *4 にあるため、そこに特定の処理を差し込むのはほぼ不可能

モンキーパッチ

このようなモンキーパッチを

version: '3.7'
services:
  gitlab:
    volumes:
      - "./monkey_patch/db_migrate_monkey_patch.rake:/home/git/gitlab/lib/tasks/db_migrate_monkey_patch.rake:ro"

のような感じでDockerコンテナ内にmountして使っています。

注意点

  • かなり黒魔術なので自己責任でお願いします
  • database.ymlの中身は適宜書き換えてください
  • オンラインで実行しても問題ないはずですが、 rake db:migrate 実行中にpumaやunicornをrestartすると意図しない設定が読み込まれるので注意してください

モンキーパッチのポイント

rake taskの実体は Rake::Taskインスタンスで、rake task実行時には Rake::Task#invoke が呼ばれます。

そのため Rake::Task["db:migrate"].invoke という特異メソッドに対してモンキーパッチ用のmoduleを prependすることで rake db:migrate 実行時のみに特定の処理を差し込んでいます。

ボツ案1

Rake::Task["db:migrate"].enhance(["db:migrate:before"]) do
  Rake::Task["db:migrate:after"].invoke
end

みたいな感じで db:migrate:before でdatabase.ymlを書き換えて db:migrate:after で元に戻すという方法もあったのですが、これだと db:migrate でエラーになった時に db:migrate:after が実行されずにdatabase.ymlが元に戻せなくなる不具合があるのでボツになりました。

そのため、上の方で書いているように Rake::Task["db:migrate"].invoke に対してモンキーパッチをあてるしかないと思ってます。

ボツ案2

begin
  Rake::Task["db:migrate"].enhance(["db:migrate:before"])
ensure
  # ロールバック処理
end

これだとrake taskの定義時に ensure が評価されてrake taskの実行時には ensure は評価されないので意図通りに動かないです。