[CI] - workflow maintenance

[RomarQ]

What does it do?

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

[github-actions]

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2280 KB (no changes) ✅

Moonbeam runtime: 2236 KB (no changes) ✅

Moonriver runtime: 2256 KB (no changes) ✅

Compared to latest release (runtime-3200)

Moonbase runtime: 2280 KB (+320 KB compared to latest release) ⚠️

Moonbeam runtime: 2236 KB (+312 KB compared to latest release) ⚠️

Moonriver runtime: 2256 KB (+332 KB compared to latest release) ⚠️

[github-actions]

Coverage Report

@@                        Coverage Diff                        @@
##           master   noandrea-workflow-maintenance      +/-   ##
=================================================================
+ Coverage   79.01%                          79.12%   +0.11%     
- Files         305                             303       -2     
- Lines       88446                           88015     -431     
=================================================================
- Hits        69878                           69637     -241     
- Misses      18568                           18378     -190     

Files Changed	Coverage
/pallets/erc20-xcm-bridge/src/erc20_trap.rs	68.18% (+68.18%)	🔼
/pallets/erc20-xcm-bridge/src/xcm_holding_ext.rs	94.74% (-5.26%)	🔽
/pallets/moonbeam-foreign-assets/src/lib.rs	69.06% (-1.34%)	🔽
/pallets/moonbeam-xcm-benchmarks/src/weights/generic.rs	4.26% (+1.67%)	🔼
/pallets/moonbeam-xcm-benchmarks/src/weights/mod.rs	13.61% (+3.66%)	🔼
/pallets/xcm-transactor/src/lib.rs	89.44% (-0.02%)	🔽
/precompiles/gmp/src/lib.rs	15.87% (+1.65%)	🔼
/precompiles/gmp/src/mock.rs	26.53% (+6.06%)	🔼
/precompiles/xtokens/src/lib.rs	95.70% (-0.44%)	🔽
/precompiles/xtokens/src/mock.rs	77.88% (+24.50%)	🔼
/precompiles/xtokens/src/tests.rs	99.51% (+0.08%)	🔼
/primitives/xcm/src/origin_conversion.rs	61.76% (-28.04%)	🔽
/runtime/common/src/migrations.rs	72.09% (+5.99%)	🔼
/runtime/common/src/weights/pallet_assets.rs	8.94% (+0.06%)	🔼
/runtime/common/src/weights/pallet_parachain_staking.rs	19.59% (+0.23%)	🔼
/runtime/common/src/weights/pallet_xcm.rs	0.00% (-5.23%)	🔽
/runtime/moonbase/src/lib.rs	52.54% (-0.01%)	🔽
/runtime/moonbase/tests/integration_test.rs	99.37% (-0.01%)	🔽
/runtime/moonbase/tests/xcm_mock/parachain.rs	60.85% (-0.22%)	🔽
/runtime/moonbeam/src/lib.rs	46.70% (-0.08%)	🔽
/runtime/moonbeam/src/xcm_config.rs	47.47% (+2.02%)	🔼
/runtime/moonbeam/tests/integration_test.rs	99.37% (-0.01%)	🔽
/runtime/moonbeam/tests/xcm_mock/parachain.rs	60.85% (-0.22%)	🔽
/runtime/moonriver/src/lib.rs	46.92% (-0.08%)	🔽
/runtime/moonriver/tests/integration_test.rs	99.35% (-0.01%)	🔽
/runtime/moonriver/tests/xcm_mock/parachain.rs	61.57% (-0.22%)	🔽
/runtime/moonriver/tests/xcm_tests.rs	99.93% (-0.01%)	🔽

Coverage generated Thu Oct 17 10:10:35 UTC 2024

[vanhauser-thc]

@RomarQ should this PR strengthen the security of the github actions? if so, how? (besides the repository content being set to read-only)

[RomarQ]

@RomarQ should this PR strengthen the security of the github actions? if so, how? (besides the repository content being set to read-only)

@noandrea and @pLabarta are the ones working on the CI/CD improvements. To properly strengthen it, the continuous delivery jobs need to be moved to another repository without self-hosted runners.

[noandrea]

@vanhauser-thc this pr focus on limiting the permissions of the github token used by the runners and limiting the use of self-hosted runners to the minimum.

[vanhauser-thc]

@noandrea

If I understand this correctly:
lazy-loading-tests and zombie_upgrade_test use secrets and allow being triggered from PRs from forked repositories.

twiggy could be used to write messages to a PR - not a huge security impact though.

[RomarQ]

@noandrea

If I understand this correctly: lazy-loading-tests and zombie_upgrade_test use secrets and allow being triggered from PRs from forked repositories.

twiggy could be used to write messages to a PR - not a huge security impact though.

It is nothing related to any test component. It is just a combination of how github workflows can get triggered and with the fact that we use self-hosted runners. You can find more info here: https://www.synacktiv.com/en/publications/github-actions-exploitation-self-hosted-runners

[vanhauser-thc]

@noandrea
If I understand this correctly: lazy-loading-tests and zombie_upgrade_test use secrets and allow being triggered from PRs from forked repositories.
twiggy could be used to write messages to a PR - not a huge security impact though.

It is nothing related to any test component. It is just a combination of how github workflows can get triggered and with the fact that we use self-hosted runners. You can find more info here: https://www.synacktiv.com/en/publications/github-actions-exploitation-self-hosted-runners

I know. I am pointing out though what issues are likely still present after merging this PR which tries to restrict what workflows can do.

[crystalin]

Overall looks good, but I have some questions about the GHA instances and about the versioning of the docker images

[crystalin]

We used to rely on bare-metal for coverage as it required a lot of computation (for compiling rust with coverage data IIRC). Is it intended to move it back default instances ?

[crystalin]

Is it intended ?

[crystalin]

It might make the wholme process a lot slower

[noandrea]

it is intended, a default runner provider better isolation and the job is part of the release process, and therefore is a relatively rare event

[crystalin]

same

[noandrea]

see the reply above

[crystalin]

Using stable mean that the version might change and introduce failure at some point.
I'm not sure if it is better than specifying a version that we manually update every 2 years when we have time to test it

[noandrea]

across the MBF organization, there are different versions of docker images involved in running/testing/... the moonbeam binary, and changing a version in one repository usually causes other processes to break. The change aims to have a consistent version of the image across repositories, and to keep using an up-to-date version (without being bleeding-edge) of the build images. The stable release do not changes very often, around every 2y afaik

[noandrea]

@noandrea

If I understand this correctly: lazy-loading-tests and zombie_upgrade_test use secrets and allow being triggered from PRs from forked repositories.

twiggy could be used to write messages to a PR - not a huge security impact though.

on the repository configuration (admin interface) we have setup that pr from forks need to be approved and also any changes to pr requires a new authorization.