containerd / runwasi

Facilitates running Wasm / WASI workloads managed by containerd
Apache License 2.0
1.01k stars 82 forks source link

ci(*): merge build action #548

Closed Mossaka closed 2 months ago

Mossaka commented 3 months ago

this commit merges the build step in the release action and the one in the ci action since they share a large overlap code.

the build step not only builds the binary, but signs it, and upload as action artifacts. this step is now running for every ci run, including PR and release pipeline.

jsturtevant commented 3 months ago

Are there implications for signing the binary every PR?

Mossaka commented 3 months ago

Are there implications for signing the binary every PR?

Since we are signing the artifacts that we publish on every build, I can imagine increased security and potentially degraded performance and increased complexity of managing keys.

Although stated in the release doc for how to verify the binaries, we probably want to make that even clearer by copying the exact verification command in the release doc everytime we release signed artifacts.

I am not sure if anyone would want to verify build artifacts for each PR / commit, but if they want, it's do-able.

marosset commented 2 months ago

If you add # yaml-language-server: $schema=https://json.schemastore.org/github-action.json to the top of all the github action files you'll get schema validation in a lot of editors like vs code

Mossaka commented 2 months ago

Are there implications for signing the binary every PR?

Now that I thought more about the implications, and realized that I missed one of the biggest implication in my previous comment - signing requires feature branches to be pushed to upstream, because they require id-token: write permission which will not grant to public forked repos (the max permission for id-token is read for forked repos, so even though you specified write permission, the pipeline won't grant you the context you want).

I am okay to put a condition on the signing to differentiate build / release pipelines so we can disable signing on build. Wdyt?

Mossaka commented 2 months ago

Could you please this PR again? @cpuguy83 @devigned 🙏

Mossaka commented 2 months ago

Going to merge this now and any follow up can be addressed later