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

v1.13 Backports 2024-01-23 #30386

Merged
merged 8 commits into from
Jan 26, 2024
Merged

v1.13 Backports 2024-01-23 #30386

merged 8 commits into from
Jan 26, 2024

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented Jan 23, 2024

PRs skipped due to conflicts:

Once this PR is merged, a GitHub action will update the labels of these PRs:

 30207 28286 30131 30335 30167

@giorio94 giorio94 added kind/backports This PR provides functionality previously merged into master. backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. labels Jan 23, 2024
giorio94 and others added 8 commits January 23, 2024 18:48
[ upstream commit 247e6e0 ]

[ backporter's notes: hit multiple conflicts due to the absence of the
  helm-default action in the v1.14 branch, resolved updating the
  chart-dir path as appropriate. Additionally, skipped the e2e-upgrade
  hunks, as not present. ]

A few GHA workflows got recently modified to hardcode the repository and
branch of the actions hosted locally (e.g., [1]). This was a security
measure, as they are triggered after checking out the untrusted context
(i.e., PR branch), and thus it would be possible for an external PR to
inject malicious code.

Yet, at the same time, this change mostly defeats the smooth development
process enabled by ariane (which automatically uses the workflow and
context from the PR for trusted branches -- i.e., in cilium/cilium),
requiring again to manually modify those references for testing purposes.
Similarly, it also requires manual adaptations when changes are backported
to stable branches, or to allow running them from forks, which are easy
to overlook.

As an alternative solution, let's only check out the helm chart from the
untrusted context in a separate directory, without overriding any of
the trusted files (i.e., from the target branch) retrieved initially.
This way, we are guaranteed that the local github actions are always
trusted (as we are not overriding them, nor we are executing any script
which could modify them), and can be invoked directly, without any additional
constraint. A key aspect for this is that helm charts cannot execute
arbitrary code in the client host.

Another difference, compared to the previous approach, is that now we
also execute the `./contrib/scripts/kind.sh` script from the trusted
context (i.e., target branch) instead of the PR context. However, this
file is effectively part of the workflow definition, and this change
brings consistency with the rest of it. The same also applies for the
Gateway API conformance tests.

[1]: 654d92f ("ci-e2e: Use lvh-kind in secure way")

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 0c080f6 ]

[ backporter's notes: skipped the e2e-upgrade hunks, as not present,
  and resolved a bunch of minor conflicts. ]

As an additional security measure, let's postpone the checkout of the
untrusted context after the setup of the test environment.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 8180cac ]

Added a doc to update installation instructions of cilium via Azure CNI
Powered by Cilium AKS cluster. Added a page to describe about delegated
ipam.

Signed-off-by: Tamilmani <tamanoha@microsoft.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit fb4e560 ]

Signed-off-by: Andrii Iuspin <andrii.iuspin@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit b20038e ]

Clustermesh workflows need to setup two multi-node kind clusters, which
don't fit well in the default GH runners (2 vCPU and 7GiB or RAM).
Although GitHub recently upgraded [1] the default runners for OSS projects
to 4 vCPU and 16GiB of RAM, let's still make it explicit that these
workflow actually need that amount of power to run seamlessly.

[1]: https://github.blog/2024-01-17-github-hosted-runners-double-the-power-for-open-source/

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit f299dc1 ]

[ backporter's notes: hit conflicts due to cilium to cilium-dbg renaming,
  resolved running git cherry-pick -Xrename-threshold=20% $SHA ]

Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 7cdadbc ]

[ backporter's notes: hit conflicts due to cilium to cilium-dbg rename,
  resolved running git cherry-pick -Xrename-threshold=20% $SHA ]

Global variable `countErrors` converted to the function local.

Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
[ upstream commit 000edce ]

[ backporter's notes: hit conflicts due to cilium to cilium-dbg rename,
  resolved running git cherry-pick -Xrename-threshold=20% $SHA ]

Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
@giorio94
Copy link
Member Author

/test-backport-1.13

@giorio94 giorio94 marked this pull request as ready for review January 24, 2024 07:31
@giorio94 giorio94 requested review from a team as code owners January 24, 2024 07:31
@giorio94
Copy link
Member Author

Conformance AKS failure is #22162, which got fixed in newer versions of the CLI (but the version is pinned to v0.14.8 in the v1.13 branch). Rerunning

@giorio94
Copy link
Member Author

@viktor-kurchenko Gentle ping

@viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko Gentle ping

Approved! Thank you.

@giorio94
Copy link
Member Author

CI is green and most of the reviews are in (the remaining PRs didn't incur in any conflict, and I performed an additional sanity check). Marking as ready to merge.

@giorio94 giorio94 added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 25, 2024
@julianwiedmann julianwiedmann merged commit 52e082c into v1.13 Jan 26, 2024
137 checks passed
@julianwiedmann julianwiedmann deleted the pr/v1.13-backport-2024-01-23 branch January 26, 2024 09:42
@maintainer-s-little-helper maintainer-s-little-helper bot removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.13 This PR represents a backport for Cilium 1.13.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

6 participants