Kengo's blog

Technical articles about original projects, JVM, Static Analysis and TypeScript.

SonarQube解析をGitHub Actionsのpull_request_targetイベントで回す

GitHub SonarQube

SQ解析が現時点ではあまりpull_request_targetイベントをサポートしておらず、ちょいちょい手間が必要だったのでまとめます。 完全なYAMLのサンプルは↓にあります。

github.com

checkout する場合はrefを指定する

pull_request_target イベントでcheckoutを実行すると標準ではデフォルトブランチをチェックアウトしてしまいます。 よって fetch-depth: 0 に加えて ref の指定も必要です。以下のようにHEADをチェックアウトするか：

      - uses: actions/checkout@v2
        with:
          fetch-depth: 0
          ref: ${{ github.event.pull_request.head.ref || github.ref }}

以下のようにマージコミットをチェックアウトすることで解決できます*1。

      - name: Decide the ref to check out
        uses: haya14busa/action-cond@v1
        id: condval
        with:
          cond: ${{ github.event_name == 'pull_request_target' }}
          if_true: refs/pull/${{ github.event.pull_request.number }}/merge
          if_false: ${{ github.ref }}
      - uses: actions/checkout@v2
        with:
          fetch-depth: 0
          ref: ${{ steps.condval.outputs.value }}

2021年11月17日更新：前者の方法だとforkからのPRでは動作しないので、後者がおすすめです。

PR解析用のオプションを加える

おそらくScanner側の問題だと思うのですが、Developer Editionの機能のためかコードは見つけられていません。現時点では sonar.pullrequest.key と sonar.pullrequest.branch の双方を手動で設定する必要があります*2。

        run: |
          mvn org.jacoco:jacoco-maven-plugin:prepare-agent verify sonar:sonar -B -e -V \
            ${PR_NUMBER:+ -Dsonar.pullrequest.key=$PR_NUMBER -Dsonar.pullrequest.branch=${{ github.event.pull_request.head.ref }} }
        env:
          PR_NUMBER: ${{ github.event.pull_request.number }}

2021年8月30日更新：

上記実装では脆弱性が残ることをazu さんにご指摘いただきました。ありがとうございました！ ブランチ名はPR作成者が決定可能なので、一度環境変数で受けてから使用するべきとのことです。

        run: |
          mvn org.jacoco:jacoco-maven-plugin:prepare-agent verify sonar:sonar -B -e -V \
            ${PR_NUMBER:+ -Dsonar.pullrequest.key=$PR_NUMBER -Dsonar.pullrequest.branch=${PR_BRANCH} }
        env:
          PR_NUMBER: ${{ github.event.pull_request.number }}
          PR_BRANCH: ${{ github.event.pull_request.head.ref }}

Sonar Scannerの実行にはJava 11が必要

pull_request_targetと直接関係はありませんが、最近更新が入ったようなので念のため。

Sonar Scannerは実行にJava 11以上が必要なので、Java 8のコードを書いている場合でもJDK 11以上でビルドする必要があります。MavenもGradle*3も --release オプションをサポートしているので、それを使うと良いでしょう。Java 8に無いAPIを見つけてコンパイルエラーにしてくれるので、 --target よりも安心です。

*1:https://github.com/actions/checkout/issues/518

*2:https://community.sonarsource.com/t/github-comments-stopped-working-in-github-action-after-switch-to-pull-request-target/42556

*3:https://docs.gradle.org/7.2/dsl/org.gradle.api.tasks.compile.CompileOptions.html#org.gradle.api.tasks.compile.CompileOptions:release

勝手を知らないプロジェクトにPull Requestを送るときに気にしていること

GitHub

初心者向けのPull Request（PR）作成方法はopensource.guideをはじめとして数多く見るのですが、もうちょっと突っ込んだというか中級的な内容の記事を読みたかったので自分で書きます。題材として主に直近で書いた大きめの機能追加用PRを使っています：

github.com

修正や新機能を入れる利点を明確に伝える

何かを提案する場合、その背景には必ず動機となる利点があるはずです。これはとても単純に伝えられることもあれば、テストやグラフを作成しないと伝わりにくいものもあります。

今回の変更では、GitHub Actions workflowの定義ファイル簡素化とパフォーマンス改善が利点でした。定義ファイルが簡素化されることはドキュメントで簡単に伝えられますが、パフォーマンスは測定しなければわかりません。よってパフォーマンス検証用のリポジトリを作成し測定を行うことで、ビルド時間が約28%高速化されることを示しました。

github.com

このリポジトリにあるActionsを実行すると、グラフを描くために必要なCSVが出力されます。それをGoogle Sheetsに突っ込んで以下のようなグラフを作成し、PRページに添付しています。結果として、本当に高速化に繋がるのか？といった疑問や質問は来ず、純粋に実装手法についての議論から入ることができました。

f:id:eller:20210826152424p:plain
パフォーマンスがどの程度具合を説明するためのグラフ

こうして根拠となる事実を自動的に出せるようにしておくと、レビューを受けて実装が変わったときに再確認を行えるという利点もあります。今回の例で言うとキャッシュの解決とJDKのインストールとを並行することでの高速化を意図していたのですが、セキュリティ上の兼ね合いから順次実行するようコードを変更せざるを得ませんでした。このような大きな変更があっても、既に自動化されたパフォーマンス比較手段があったため、パフォーマンス改善が維持できていることを示せました。

考慮すべきことも伝える

変更には利点だけではなく、互換性や運用など考慮しなければならないこともあります。これらをどこまで認識していて、どのような検証を行ったのかを事前に共有することで、多角的な議論の下地とすることができます。

今回の変更では upload-chunk-size が設定できなくなりユーザの自由度が失われること、IvyやGrapeといったより多くのパッケージマネージャをサポートすべきかもしれないことを俎上に載せ、その上でそれらが現時点で問題にならない理由を述べています。

新機能をメンテナンスし続けることのコストを最小化する

新機能追加のためにPRを送る場合、その機能のメンテナンスを行うのが自分ではなく相手だという点を意識する必要があります。言い換えれば、メンテナンスの工数が高いと判断された場合、PRをマージしてもらえない可能性があるということです。

基本的には単体テストと統合テストの双方を実装し提供することで対応します。単体テストがあることで実装自体のテスト可能性を担保し、統合テストがあることでリグレッションに気づける状態を作ります。またユーザに対する説明が必要な機能なら、READMEのような書類もあわせて準備します。

また自己流のやり方を持ち込むのではなく、既存のコードベースに溶け込むコードを書くことも重要です。今回は既にある定数クラスを再利用したり、統合テストのための仕組みに乗ったりという工夫をしました。特に統合テストのやり方はリポジトリによって様々なので、既存のビルド設定やコードに目を通す必要があります。

早めに返信するが、相手には求めない

質問や指摘があった場合、24時間以内には必ず何らかのリアクションを返すようにします。どうせタイムゾーンの問題があるので”即レス”は難しいのですが、できるだけ早めに返すことで議論の沈静化を防ぎます。

逆に、向こうから連絡がなくても気長に待ったほうが良いことも多いです。今回は企業の中の人が業務としてレビューしてくれましたが、それでも2週間ほど返信がなかった期間がありました。自分が相手の立場でもそういう事はあるだろうと気長に待つのが得策です。

変更の理由をオーバー気味に説明する

PRを提案する際、依存先の更新やリファクタリングのような、一見不要に見える変更を入れることがあります。例えばこちらのPRでは、ネットワークエラー発生時でもHTTPリクエストのリトライを行うために typed-rest-client のバージョンを上げています。何も説明しないと、依存を上げるのは他のPRでやればいいのでは？という疑問が湧くかもしれません。

github.com

こうした変更を入れる際は、コミットコメントかPR内コメントでその意図を伝えるようにしています。やはりコードでWHYを伝えるのは難しいので、少しオーバーかなと思うくらいの勢いで伝えたほうが良さそうです。

その他、細かいこと

  • README.md はもちろん、 CONTRIBUTING.md や CODE_OF_CONDUCT.md といったドキュメントに目を通しましょう。
  • Pull Requestテンプレートがある場合は利用しましょう。
  • わからないことはわからないと言い、素直に尋ねる方が良いです。相手もこちらを助けやすくなります。
  • 新しいライブラリの導入など相手に判断を求めるケースでも、自分の考えを述べた上で素直に尋ねるようにしましょう。

以上です。どなたかの参考になれば幸いです。 Happy hacking!

GitPodでJavaプロジェクトを開発する

Maven Gradle java

GitHub Codespacesがなかなか個人向けに来ないので、changelog.comで宣伝していたGitPodを試しています。 どうも公式のJava向けの説明が古いようで、既にDeprecatedになっているtheiaを前提としているため、調べたことをメモしておきます。

最新のJavaを使う

普通に gitpod/workspace-full イメージ内でJavaを起動すると、Zuluの11が使われていることがわかります：

$ java --version
openjdk 11.0.12 2021-07-20 LTS
OpenJDK Runtime Environment Zulu11.50+19-CA (build 11.0.12+7-LTS)
OpenJDK 64-Bit Server VM Zulu11.50+19-CA (build 11.0.12+7-LTS, mixed mode)

SDKMANやhomebrewが入っているので好きなバージョンを入れてもいいですが、ワークスペースを立ち上げる度に実行するのは面倒なので、 azul/zulu-openjdk のようなイメージを使ってしまうのが楽でいいと思います。

# .gitpod.yml
image: azul/zulu-openjdk:16

Mavenの依存をダウンロードしておく

Mavenの場合、dependency:go-offline でプラグインや依存をすべてダウンロードできます。 これをイメージ作成時に実行しておくのが良さそうです。

# .gitpod.yml
image: azul/zulu-openjdk:16

tasks:
  - init: ./mvnw -B dependency:go-offline

Gradleの依存をダウンロードしておく

Gradleには標準的な手法がないので、単にビルドを回しておきます。

# .gitpod.yml
image: azul/zulu-openjdk:16

tasks:
  - init: ./gradlew build

Extensionを導入する

Java向けExtensionを3つ入れて様子を見ています。

# .gitpod.yml
vscode:
  extensions:
    - redhat.java
    - vscjava.vscode-java-dependency
    - vscjava.vscode-java-debug

ポートを開けておく

Spring Framework標準の8080ポートを開けておく場合は ports の設定 で足ります。 が、URLの取得にgpコマンドが必要なので azul/zulu-openjdk ではなく gitpod/workspace-full をベースとしたイメージを用意する必要があります。URLの決定ロジックがいまのところ非常に単純なのでなくてもなんとかなりそうではありますが、一応。

# .gitpod.yml
image:
  file: .gitpod.Dockerfile
ports:
  - port: 8080

# .gitpod.Dockerfile
FROM gitpod/workspace-full
RUN bash -c ". /home/gitpod/.sdkman/bin/sdkman-init.sh && sdk install java 16.0.2-zulu"

バッジを付ける

バッジはDiscourseに落ちています。 README.md とかに貼っておくと、Contributorの敷居が下がって良いんじゃないでしょうか。

community.gitpod.io

The 2021 State of DevOps Reportが出た

management

ということでスキマ時間に読み進めていたので、感想と面白いと感じた点をまとめときます。網羅性が高く公平なまとめが必要な方は、3〜4ページに掲載のExective Summaryをおすすめします。

テーマは「中間層からの脱出」

6ページ目を見てもらえれば一目瞭然、今回のテーマは生産性が高くできず中間層で留まっているチームに対する処方箋の発見にあるようです。

近年のレポートは「今更それは無いでしょ」って感じのLow levelが5%強、「リアルチートじゃん」って感じのHighが10%強で、ほとんどのチームがMiddleに属していました。ごく少数の上澄みが高いパフォーマンスを出していて、その鍵となるKPIもベストプラクティスもわかっているのに、ほとんどの中間層は現状から脱出できず指を咥えて見ていた……と言ったら言いすぎでしょうか。この8割を占めるMiddle levelをさらに細分化し、中間層を脱するための働き方を見つけようということです。

f:id:eller:20210726175415p:plain
DevOps evolutionary levels (State of DevOps Report 2021 p.6 より引用)

DevOpsのやり方がだいぶ浸透してきて「何を計測し何をやるか」から「どうやるか」に関心がシフトしてきたとも言えます。同じ6ページにはDevOpsについて語ることよりも働き方について語るチームが多いことが述べられています：

In fact, many of the teams that are “doing DevOps” well don’t even talk about DevOps anymore—it’s simply how they work.

面白いと思ったところ

  • トップダウンによりボトムアップ型の改革を可能にする （p.11）
    • 経営の協力は当然必要というか「DevOpsによる生産性の向上＝経営の関心事」という大前提がこのレポートには感じられる
    • 生産性が低い組織はリスクを避けるためにリスクを増大する手段を選択する傾向にある（p.31）が、これも部下を信じられない・Continuous Deliveryの適用をためらう経営なのではと思った
    • executive summaryによれば何でも話して疑問を解消し論点を明確にできる組織が高い生産性を持つわけで、マネジメントやリーダーの日々のコミュニケーションの賜物なのだろうと思った
  • DevOpsチームの存在は組織の進化を助けず、その曖昧な責務が組織に混乱をもたらす（p.13）
    • 開発とオペレーションの間に独立したDevOpsチームを配置するのはアンチパターンとして知られる（p.13）
  • 高度に進化した組織にとって文化は障害にはならず、これこそが高度に進化した理由である（p.14）
  • Team Topologiesという概念の導入（p.17）
    • Fast flow（p.16）実現にとっての障害を取り除くことに注力するものである（p.29）
    • 役割が多く、小規模のチームに当てはめるのは難しそうだと感じた
    • これを実現するための教育はどうやるのだろう？各チームの役割について専門家を探すのも難しそうな気がする
  • internal platformsについて
    • 2020でも触れられたが更に掘り下げて説明している（p.36）
    • 社内向けシステムを構築する際に利用できる認証・認可・デプロイ・保守・監視に使えるPlatformという理解をした
    • データレイクにおけるデータカタログとかも入ってくるんだろうなという理解をした
    • 高度に進化した組織はこうした基盤をinternal customerのビジネスを理解してエンジニアに構築提供している（p.30）
    • こうしたplatform teamがあっても生産性が向上するとは限らないが、DevOps transformationを加速できる（p.30）
    • 標準やルールではなくサービスを提供するということ、すなわち人をチームを信じることが肝要なのだと感じた
  • DevOpsの進化とinternal platformsの利用状況には関連がありそう（p.18）
    • 知識やベストプラクティスを他のチームと活発に共有することが生産性と関連しており、（p.19） これこそがMid levelを脱出するためのとっかかりになる（p.29）
    • だからこそ「でも保守（conservatism）も必要だよね」というCTOコメントが面白い（p.31）
    • 他チームがデプロイの課題を解決したことについて話す事例（p.28）があり、チームの壁を超えて課題解決や挑戦について共有する仕組みや文化が必要なのだと感じた、成果発表会のような季節性の仕組みでは動作しなさそう
    • Chat、金曜のメール、月次学習セッションといった共有手法の一例が共有されている（p.29）
  • 文化を嘆くのをやめて手を動かせ（p.28）
    • これ個人的にはとても気に入った、文化は個人の持ち物ではなくmutabilityを確信できないので
    • 表層の変えやすく失われやすい変化ではなく、5 Whyで組織文化の根っこを変えよう （p.32）
    • 日々正しい方向に組織の振る舞いをnudge（誘導？）していこう（p.32）

過去ログ

2019と2020の感想はTwitterを漁れば出てきます。

Gradleのbaseプラグインに書くべきでないconventionとは何か

Gradle

ひとつ前の記事では「Gradleの設定やコードをどこに書くべきか」のうち、答えが明確な「build.gradleファイルとbuildSrcディレクトリの使い分け」について書きました。この記事ではまだ自分の中でもよくわかっていない「Gradleでbaseプラグインに書くべきでないconventionとは何か」について書きます。

baseプラグインとは

ここで言うbaseプラグインは、plugins { id "base" } で使えるThe base pluginのことではありません。Gradleプラグインの設計手法のひとつとして推奨されている「設定より規約」の文脈で登場するものです。「設定より規約」を実現しつつ、（大部分のケースをカバーするであろう）規約が適用できないケースでもプラグインの機能を使えるようにするため、プラグインの規約（conventions）と基本的な機能（capabilities）とを別のプラグインに定義する設計手法です。

例えば java プラグインと java-base プラグインの場合、 java プラグインが規約を提供し、 java-base プラグインが基本的な機能を提供します。 java プラグインはjava-base プラグインに依存することで、java-base プラグインが提供するtaskやextensionを利用します。

なお公式にはbaseプラグイン「ではない」方のプラグインには固有名詞がないのですが、ここでは記載を簡略化するために「規約プラグイン」と呼ぶことにします。

GradleはConventionという用語を多義的に使っている

ではbaseプラグインに書くべきconventionとは何か。この話題に触れるにあたり、GradleがConventionという用語をいつどのように使っているか確認する必要があります。私の知る限り、大きく分けて以下の3つが存在します：

  1. ”設定より規約”の文脈でのconventionのこと。一般的な用法。
  2. プロパティを指定しなかった場合に暗黙的に採用される値。default valueのこと。
  3. プラグイン設定用のconvention
    • 例えばJavaPluginConventionなど。
    • 現時点では非推奨なため、Extensionを利用する必要がある。

こうなるとよくわからないのが「baseプラグインに書くべきでないconventionはどれ？」です。

1のconventionはプラグインが提供する規約そのものでありbaseプラグインに書くべきでないのは明らかですが、JavaBasePluginやCargoBasePluginは2と3のconventionを含んでいます。

規約プラグインであるJavaPluginやCargoPluginはconventionを持っていることを期待され、実際1や2のconventionを含んでいます。

非推奨となった3のconventionは脇に置くとしても、2の置き場所やそれを決定する判断基準は明瞭に持ちたいところです。

メソッド名に惑わされないのが吉か？

私も2021年7月現在では明瞭な判断基準は持っていないのですが、ポイントはProperty#convention(Provider)というメソッド名に惑わされないことだと思われます。言い換えれば、2のconventionはどこにでも出てくるものだと割り切り、1のconventionを規約プラグインに限定して書くことのみに注力します。そして機能を書く上で必要なプロパティがある場合、baseプラグインでも気にせずにデフォルト値をProperty#convention(Provider)メソッドで設定してしまいましょう。

公式フォーラムに問い合わせてみた

自分的にはわかった気になれましたが、なんかしっくりこないので公式フォーラムに問い合わせてみました。

discuss.gradle.org

返信もらえたらこちらのブログ記事も更新します。

2021年9月29日更新: 公式Slackの方で「JavaBasePluginが異色なだけ、普通は1と2と3は全く同じものでbaseプラグインには出てこないべき」というコメントを貰いました。確かに一番シンプルなまとめ方かもしれません。

GradleのbuildSrcとどう付き合うべきか

Gradle

Gradleで複数サブプロジェクトをもつプロジェクトを作成する - kdnakt blog を見て、buildSrcディレクトリ周りで混乱した記憶が蘇ってきました。

ということで「Gradleの設定やコードをどこに書くべきか」のうち、答えが明確なbuild.gradleファイルとbuildSrcディレクトリの使い分けについて現時点での見解をまとめておきます。

宣言的か命令的か

Gradleのビルドスクリプトを分割統治する手法として、昔はscript pluginと呼ばれる手法が使われていましたが、Gradle 7.1.1時点ではbuildSrcプロジェクトを使ってimperative logicを抽象化することが推奨されています。

imperative logicというのはつまり”命令口調”なビルドスクリプトのことですね。ビルドスクリプト自身が極力宣言的（declarative）になるように、命令的なスクリプトをビルドスクリプトとは別のところに書いておこうということです。ビルドスクリプトを宣言的に保つことは、Gradleベストプラクティス堂々の一位に位置づけられており、その重要性が伺えます。

そしてimperative logicを隠しビルドスクリプトを宣言的に保つことは、プラグインの説明に書かれているように、プラグインの設計意図そのままです：

What plugins do (omit) Encapsulates imperative logic and allows build scripts to be as declarative as possible

自分が最近書いた例でいうと、チェックサムをGitHub Releasesに掲載するためのタスク定義はTHE・命令的でした。この定義、あるいは抽象化した定義をプラグインとして書いておいてビルドスクリプトに適用することで、ビルドスクリプトそのものを宣言的に保てるということです。

命令的なコードを置くためのbuildSrcディレクトリ

通常プラグインはGradle Plugin Portalにpublishして使いますが、プロジェクトごとにpublishしていたらキリがありません。そこでプロジェクト固有のローカルな"命令的"スクリプトを置くための場所として buildSrc プロジェクトが存在します。プロジェクトルートに buildSrc ディレクトリを置くと、中のコードをコンパイルしたものをビルドスクリプトのCLASSPATHに置いてくれるのです。

以上のように、全体像がわかってしまえばbuild.gradleファイルとbuildSrcディレクトリどちらを使うべきかは見えてきます。 宣言的なコードはbuild.gradleファイルに、命令的なコードはbuildSrcディレクトリに置きましょう。

buildSrcプロジェクトの書き方はmike-neckさんのブログに詳しいのでオススメです。プラグインはテスト手法も確立しているので、ビルド性能だけでなくビルドスクリプト管理の生産性も上がるといいですね。

mike-neck.hatenadiary.com

GitHub Actionsのworkflow_runイベントでテストを回すときの要点

GitHub

Dependabotの作ったPRがSecretsにアクセスできないためにことごとく失敗していたのを修正しました。

github.com

SecretsにアクセスできないのはKeeping your GitHub Actions and workflows secure: Preventing pwn requestsで説明されているようにセキュリティ向上のためです。workflow_run イベントでCheckを回すとワークフロー定義は常にデフォルトブランチのものが使われるため、PRでワークフローファイルが悪意を持って変更されてもマージしなければ悪影響を受けません。ので今後、基本的にはSecretsを必要とするワークフローはworkflow_runイベントで回すことになります。

上記securitylab.github.comの記事で色々説明されていますが、わりと限定的なユースケースについて述べているので、ここでは自分のケースで必要だった知見についてまとめます。

actions/checkout ではrefプロパティを指定する

ソースコードをチェックアウトする場合、どのrefをチェックアウトするのかを明示的に示す必要があります。何も指定しない状態ではデフォルトブランチをチェックアウトしてしまうため、PRで変更する内容をテストできません。

on:
  workflow_run:
    workflows:
      - build
    types:
      - completed
jobs:
  integration-test:
    runs-on: ubuntu-latest
    if: github.event.workflow_run.conclusion == 'success'
    steps:
      - uses: actions/checkout@v2
        with:
          ref: ${{ github.event.workflow_run.head_sha }}

Pull RequestのChecksを登録する際もshaを指定する

この「何もしなければデフォルトブランチの最新refが対象になる」のが曲者です。例えばChecksに結果を反映する際も、明示的にshaを指定する必要があります。

      - uses: LouisBrunner/checks-action@v1.1.2
        if: always()
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          name: integration-test
          sha: ${{ github.event.workflow_run.head_sha }}
          conclusion: ${{ job.status }}
          details_url : https://github.com/${{github.repository}}/actions/runs/${{github.run_id}}

Secretsなしでもテストできるようにする

すべてのテストをworkflow_runの方に寄せても良いですが、Secretsなしでも実行できる単体テストや静的解析はpull_requestで実行してしまったほうが良いかもしれません。 その場合は、./gradlew build や npm run all がSecretsなしでも正常ステータスコードで完了する必要があります。

JUnitならassumptionsでTokenがあるか確認して無ければテスト実行をスキップすればいいでしょう。Jestではうまい方法が見つからなかったので、Tokenの有無でtestとtest.skipを呼び分けることにしました。

const token = process.env.GITHUB_TOKEN || ''
const integrationTest = token ? test : test.skip

integrationTest('fork() does nothing if the forked repo already exists', async () => {
  ...
})