Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: cargo-denyのadvisoriesだけcronでの実行にする #893

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

qryxip
Copy link
Member

@qryxip qryxip commented Dec 11, 2024

内容

cargo-denyワークフローをlicensesauditに分離し、後者を毎日のcron実行とする。

関連 Issue

その他

RustSecへの登録によって「何もしていないのにCIが落ちた」ケースが最近増えてきたため、ENGINEのようにcronだけで動かす方が妥当かなと思った次第です。
(もしかしたらENGINEと同様、Discordへの送信にした方がよいかもしれない)

@qryxip qryxip requested a review from Hiroshiba December 11, 2024 19:30
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ちょっと目的を整理しておけると!
目的次第ですが、どっちもcron実行・mainブランチで実行にしても良いのかも?

たしかcargo-denyをmainブランチでのテストにしてた理由は、ファイルを更新するのは大変なので、コミッターでなくメンテナが更新すれば良いようにしたかったからだった記憶。
mainブランチのテストはたまに落ちて良いという設計っぽみ。

今回のPRはadvisoriesのみmainブランチから外してcronにし、ほかはmainブランチで動かす感じですよね。
提案としては、まあどっちもcron・mainブランチで動かすようにすればよいのかな、みたいな。
(だとしたらファイルを分けなくて済むので)

追記:
あいや違うか、advisoriesだけはauditで最悪スルーしてリリースできるけど、ほかは無視するとリリースできないみたいな・・・?
だとしたらなぜファイルを分けてるかのメモコメントというか、各ファイルが何を担当しているのかわかるようにするのが良さそう!

(audit(監査)だとなぜadvisoriesだけしているのかわからない)

@qryxip
Copy link
Member Author

qryxip commented Dec 12, 2024

どちらかというと、licenses (advisories以外のcargo-deny)はライブラリを追加したときに反応するので、こちらにはcron実行するメリットは無いと思った次第です。なので役割的には分離した方がよいかなと。

一方audit (advisoriesのみのcargo-deny)はライブラリの追加に関係無く反応するのが主になると思ってます。
(ライブラリを新規追加したときにそれがヤンクされていたり、脆弱性が報告されていたりのケースもまあ有り得なくはないけど、レアケースと考えてよさそう)

advisoriesをリリースで無視してよいかは、内容によりそう。極端な話「作者のアカウントが乗っ取られました。この最新バージョンはマルウェアです」は駄目だし…

@Hiroshiba
Copy link
Member

Hiroshiba commented Dec 15, 2024

なぁるほどです。

ちなみに「advisories以外のcargo-deny」は結構難しいあのdeny.tomlの編集が不要・・・だったりはしないでしょうか。
というのもworkflowファイルがだいぶ多くなっているので、可能なら小分けにしたくないなぁと・・・。
でも無理なら仕方なさそう!

ジャストアイデアでやるべきかはわかりませんが、「advisories以外のcargo-deny」をtest.ymlに移しちゃって、mainブランチ以外では動かないようにするのもあり・・・かも・・・?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants