1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

@nakasebow(N M)

既存コードだらけの組込みCプロジェクトでQACをGitHub Actionsに載せてみる

1
Posted at

QACをGitHub Actionsのself-hosted runnerで動かしてみる

組込みCのリポジトリで、QACをGitHub Actionsから動かして、Pull Request（以下PR）に結果を出す仕組みを作った。

QAC自体の使い方というよりは、既存コードが多いプロジェクトでどうCIに載せるか、という話になる。

この記事中の製品名、リポジトリ名、変数名、パス、ネットワーク情報、件数などは一般化している。構成と考え方は実運用のものだが、そのままコピペして動くものではないのであしからず。

前提

今回のQACは、GitHubが用意しているRunnerでは動かせなかった。

  • 商用ライセンスが必要
  • Windowsにしかないツールや設定に依存している
  • cmd.exe 前提のmakefileが残っている
  • 既存コードにQACの指摘がそれなりにある
    • ちなみにいうとコンパイラも商用なのでRunnerじゃ厳しい

という感じで、普通にUbuntuのRunnerで動かすのは難しい。

なので、軽い検査はGitHub-hosted runnerでやって、QACとターゲット向けのビルドだけはWindowsのself-hosted runnerで動かすことにした。

最初から全部self-hosted runnerに載せるより、必要なものだけ分けたほうが、管理する範囲も小さくなると思う。

まず考えたこと

QACをCIに入れるときに、特に避けたかったのは以下。

困ること 今回の対応
既存指摘でPRが埋まる 変更部分だけを解析・表示する
外部PRのコードを社内Runnerで実行する forkからのPRでは実行しない
Runnerやライセンスがないのに成功扱いになる 実行しない理由を必ず表示する
QACの実行失敗が「指摘0件」に見える 失敗と指摘ゼロを別扱いにする
再実行で前の結果が消える 実行履歴を残し、空結果で上書きしない

QACを起動するだけならそこまで難しくないのだが、CIとして運用するなら、このあたりを考えないとすぐに使われなくなる気がする。

変更したところだけ見たい

今回のコードベースには、すでにQACの指摘が残っている。ここでリポジトリ全体を解析して全部PRに出すと、数が多すぎて本当に見たい指摘が埋もれる。

なので、二段階で絞ることにした。

1. 変更したC/Hファイルだけを解析対象にする

まずPR差分から変更されたC/Hファイルを取る。変更されたモジュールだけに絞ってQACを実行することで、解析時間を減らせる。

ヘッダを変更したときは、同じ単位のCファイルも対象に含める。ヘッダの型定義やマクロの変更は、使っている側のCファイルで引っかかることがあるため。

- name: 解析対象を決める
  id: changed
  uses: ./.github/actions/changed-source-files
  with:
    base-ref: ${{ github.base_ref }}

2. PRの追加・変更行にある指摘だけを表示する

ただし、ファイル単位でQACを掛けると、今回触っていない既存行の指摘も出る。そこで、PRに表示する段階でもう一度絞る。

reviewdog \
  -reporter=github-pr-check \
  -filter-mode=added \
  -fail-level=none \
  < qac-findings.txt

-filter-mode=addedで、追加・変更された行にある指摘だけをPR上に出せる。

1段目は解析時間を減らすため、2段目はレビューのノイズを減らすためのもの。似ているが役割が違うので、どちらも必要になる。

self-hosted runnerを誰でも動かせる状態にしない

self-hosted runnerは、ライセンスや社内環境にアクセスできるPCで動く。外部の人が作ったPRをそのままチェックアウトして動かすのは、さすがによろしくない。

そのため、QACを実行してよいかどうかは、GitHub-hosted runner側で先に判定する。

preflight:
  runs-on: ubuntu-latest
  outputs:
    run: ${{ steps.decide.outputs.run }}

  steps:
    - id: decide
      env:
        ENABLED: ${{ vars.CI_STATIC_ANALYSIS_ENABLED }}
        HEAD_REPOSITORY: ${{ github.event.pull_request.head.repo.full_name }}
        BASE_REPOSITORY: ${{ github.repository }}
      run: |
        if [ "$ENABLED" != "true" ]; then
          echo "run=false" >> "$GITHUB_OUTPUT"
          echo "::notice::静的解析は未有効化です"
        elif [ "$HEAD_REPOSITORY" != "$BASE_REPOSITORY" ]; then
          echo "run=false" >> "$GITHUB_OUTPUT"
          echo "::notice::forkからのPRでは専用Runnerを実行しません"
        else
          echo "run=true" >> "$GITHUB_OUTPUT"
        fi

QACのジョブ側は、この結果がtrueのときだけ実行する。

qac:
  needs: preflight
  if: needs.preflight.outputs.run == 'true'
  runs-on: [self-hosted, windows, static-analysis]

ここではpull_request_targetを使わないようにしている。便利そうに見えるが、PRのコードを実行する用途では権限や信頼境界が分かりにくくなる。

forkのPRでQACが走らないのは少し不便かもしれないが、安全側に倒した。必要なら信頼できるブランチに取り込んでから実行すればよい、という考え方。

ライセンス競合と後始末

商用ツールは、同時実行するとライセンスがぶつかることがある。QACを何本も同時に起動して、全部ライセンス待ちになったら悲しい。

なので、QAC用のworkflowには専用のconcurrency groupを設定している。

concurrency:
  group: static-analysis-runner
  cancel-in-progress: false

軽いチェックなら、次のpushが来たときに古い実行をキャンセルしてよい。一方でQACのように時間が掛かるものは、途中で止めると生成物やプロセスが中途半端に残ることがある。

ジョブ開始時には、前回の異常終了で残ったQACプロセスと出力ディレクトリも掃除するようにした。ただし消すのは専用の生成物だけ。ワークスペース全体を消すのは、ちょっと怖い。

文字コードも地味に困る

組込み開発の既存環境だと、ソースやツールの出力がUTF-8ではないことがある。GitHubのChecksやPRコメント、JSON、ブラウザはUTF-8が前提なので、QACの結果を取り込む段階でUTF-8へ変換することにした。

今回の出力形式はツール設定から分かっていたので、既知の文字コードとして変換している。もしUTF-8と別の文字コードが混ざる可能性があるなら、先にUTF-8として厳密に読めるかを確認して、失敗したときだけ別の文字コードを試すほうが安全。

バイト列を見て「Shift_JISっぽい」と決めつけると、UTF-8の日本語を壊すことがある。文字コードは推測ではなく、ツール設定などの根拠を見て決めたほうがよさそう。

「スキップ」と「指摘ゼロ」は別

CIで一番避けたいのは、実行していないのに緑色で「問題なし」に見える状態。

今回の結果は、少なくとも次のように分ける。

状態 意味
skipped 未有効化、対象外、または安全上の理由で実行しなかった
failed ツール、ライセンス、結果の生成などで失敗した
completed / 0 findings 解析は完走し、表示対象の指摘がなかった
completed / findings 解析は完走し、表示対象の指摘があった

「対象ファイルがない」と「QACの結果ファイルが一つもできなかった」は、最終的な指摘件数だけを見るとどちらも0件になり得る。

ただ、前者は問題ないが、後者は明らかに問題。ここを同じ扱いにすると、解析が壊れていても気付けない。なので、スキップ理由や失敗理由はnoticeやコメントに出すようにしている。

PRコメントと詳細画面を分ける

PRには、レビュー中に見ればよい情報だけを出す。

  • 解析対象のファイル数
  • 実際に解析できた件数
  • 指摘件数
  • 実行状態
  • 詳細画面へのリンク

pushのたびにコメントを増やすと読みにくいので、同じ種類のコメントは更新する形にした。以前に指摘があって直った場合は、消す代わりに「解消済み」にして残す運用もできる。監査証跡が必要な場合は、このほうが都合がよいと思う。

細かい指摘は、読み取り専用のダッシュボードで確認できるようにした。実行履歴、重要度、ファイル、ルール番号、メッセージで絞り込める。

JSONの更新は一時ファイルを書いてから置き換える形にしている。さらに、QAC失敗や空結果で、直前の有効な結果を上書きしないようにもした。

「最新の実行」は失敗しているが、「最後に正常に取れた結果」は残しておきたいことがあるため。

最初からPRを止めない

静的解析を導入したばかりのときは、どれくらい指摘が出るか、誤検知が多くないか、Runnerがどのくらい待たされるかも分からない。

この状態でいきなりPRをブロックすると、CI自体が邪魔者になりがち。

なので最初はcontinue-on-error相当で観測することにした。

  1. まずはPRを止めずに動かす
  2. 指摘量、誤検知、失敗理由、実行時間を確認する
  3. 対応方針が決まったルールだけを選ぶ
  4. 別の変更でブロッキングにして、Required checkへ追加する

MISRAのように重要度が複数あるルールセットでは、分類がない状態で全部をエラー扱いにしないほうがよい。重大なものだけ通知を出して、それ以外はCheck Runで見られるようにする、くらいから始めるのが無難だと思う。

CI自体の確認もする

GitHub Actionsの設定は、壊れてもコンパイルエラーのようには落ちないことがある。条件式を間違えてずっとスキップされたり、Runnerラベルを間違えて永遠にキュー待ちになったりする。

そのため、workflowを変更したときにローカルで確認するスクリプトを置いた。

  • Actionの静的解析
  • run:ブロックを取り出してShellCheck
  • run:内に外部入力を直接埋め込んでいないか
  • サードパーティActionを不変SHAで参照しているか
  • 実際に検査したブロック数が0件ではないか

最後の「検査した数が0件ではないか」は、意外と大事。抽出処理が壊れて何も検査していなくても、「指摘なし」で成功する実装は作れてしまう。

まとめ

QACをGitHub Actionsのself-hosted runnerで動かすとき、難しいのはQACを呼び出す部分だけではなかった。

  • 既存資産を責めず、今回の変更だけを見る
  • self-hosted runnerを外部PRから守る
  • ライセンスと生成物を共有資源として扱う
  • 文字コードを境界で揃える
  • 未実行、失敗、指摘ゼロを分ける
  • いきなりブロックせず、様子を見てからゲート化する

静的解析を「全件を出す装置」ではなく、レビューで人が見るべきことを増やさないための仕組みにできれば、既存資産が多いプロジェクトでも入れやすくなると思う。

参考

1
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
1
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?