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: fix and standardize checkouts in privileged workflows #27193

Merged
merged 3 commits into from Aug 2, 2023

Conversation

nbusseneau
Copy link
Member

Please see per commit. This PR does not touch upon non-privileged workflows (those that run on pull_request triggers), we might want to address them in a separate PR just to clean them up but there are no security concerns or specific issues to address on these so it's not urgent.

@nbusseneau nbusseneau added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. labels Aug 1, 2023
In 7a9447d we reworked a few workflows
to now be triggered by Ariane, however we missed some changes to make
sure that they checkout actions and code from the correct contexts.

In particular:

- Gateway API / Ingress / Integration tests were checking out
  environment variables from the default branch instead of the
  appropriate context ref (all in all not a big deal and still safe, but
  could be annoying to troubleshoot later down the road).
- Runtime tests were checking out environment variables from the PR
  branch instead of the appropriate context ref (this was a potential
  security issue), and then incorrectly pulling the default branch for
  executing tests instead of the appropriate PR branch context (so we
  were not testing what we expected).

Fixes: 7a9447d

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
No idea why but the steps were not aligned properly here.

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
Workflows running on PRs and based on `pull_request_target` and
`workflow_dispatch` are executed in a privileged context (e.g. access to
repository secrets), hence we take extra care not to execute anything
coming from the PR directly in the context of the workflow steps, but
instead always in a sandboxed or controlled environment (e.g. a managed
Kubernetes cluster or LVH VMs).

This commit standardizes and adds some context around which checkouts
are trusted and which are not, and where to be start being careful with
what the workflow steps are doing.

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
@nbusseneau
Copy link
Member Author

/test

@nbusseneau nbusseneau added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Aug 1, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.1 Aug 1, 2023
@nbusseneau nbusseneau marked this pull request as ready for review August 1, 2023 18:18
@nbusseneau nbusseneau requested review from a team as code owners August 1, 2023 18:18
@nbusseneau
Copy link
Member Author

With the fix in, the runtime tests correctly pull the SHA from the PR, and thus the tests executed properly come from the PR test directory:
image

@pchaigno pchaigno merged commit 96f3fd7 into main Aug 2, 2023
202 checks passed
@pchaigno pchaigno deleted the pr/fix-checkout-workflows branch August 2, 2023 10:50
@sayboras sayboras mentioned this pull request Aug 3, 2023
11 tasks
@sayboras sayboras added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 3, 2023
@nbusseneau nbusseneau added needs-backport/1.12 needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Aug 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.1 Aug 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.12.13 Aug 3, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.6 Aug 3, 2023
@joamaki joamaki removed the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Aug 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.1 Aug 9, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.13 in 1.13.6 Aug 9, 2023
@joamaki joamaki mentioned this pull request Aug 9, 2023
7 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.12 in 1.12.13 Aug 9, 2023
@nebril nebril removed this from Backport pending to v1.13 in 1.13.6 Aug 10, 2023
@nebril nebril added this to Backport pending to v1.13 in 1.13.7 Aug 10, 2023
@joamaki joamaki moved this from Backport pending to v1.12 to Needs backport from main in 1.12.13 Aug 10, 2023
@asauber asauber added this to Backport pending to v1.12 in 1.12.14 Aug 13, 2023
@asauber asauber removed this from Needs backport from main in 1.12.13 Aug 13, 2023
@YutaroHayakawa YutaroHayakawa added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Aug 17, 2023
@joestringer joestringer added needs-backport/1.12 backport/author The backport will be carried out by the author of the PR. and removed backport-pending/1.12 labels Aug 25, 2023
@joestringer
Copy link
Member

cc @nbusseneau there were conflicts for 1.12, so the PR got stuck in pending state. I set backport/author.

@joestringer joestringer moved this from Backport pending to v1.12 to Needs backport from main in 1.12.14 Aug 25, 2023
@joestringer joestringer moved this from Backport pending to v1.13 to Backport done to v1.13 in 1.13.7 Aug 25, 2023
@michi-covalent michi-covalent added this to Needs backport from main in 1.12.15 Sep 9, 2023
@michi-covalent michi-covalent removed this from Needs backport from main in 1.12.14 Sep 9, 2023
@nbusseneau nbusseneau added backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed needs-backport/1.12 labels Sep 27, 2023
@jrajahalme jrajahalme moved this from Needs backport from main to Backport done to v1.12 in 1.12.15 Oct 17, 2023
@jrajahalme jrajahalme removed this from Backport done to v1.12 in 1.12.15 Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow backport/author The backport will be carried out by the author of the PR. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.13.7
Backport done to v1.13
1.14.1
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

8 participants