Skip to content

Egress masquerade interfaces: type change reverted #36504

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

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

viktor-kurchenko
Copy link
Contributor

@viktor-kurchenko viktor-kurchenko commented Dec 11, 2024

MasqueradeInterfaces type reverted to string slice to not break existing users.
Also, Viper can do this out of the box.

Restore the original flag semantics for --egress-masquerade-interfaces to the same as v1.17.0-pre.2 or earlier

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Dec 11, 2024
@github-actions github-actions bot added the cilium-cli This PR contains changes related with cilium-cli label Dec 11, 2024
@viktor-kurchenko viktor-kurchenko added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/iptables Impacts how Cilium interacts with iptables. labels Dec 11, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Dec 11, 2024
@viktor-kurchenko viktor-kurchenko added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Dec 11, 2024
@viktor-kurchenko
Copy link
Contributor Author

/test

@viktor-kurchenko viktor-kurchenko marked this pull request as ready for review December 11, 2024 13:28
@viktor-kurchenko viktor-kurchenko requested review from a team as code owners December 11, 2024 13:28
@viktor-kurchenko viktor-kurchenko changed the title masquerade: type change reverted Egress masquerade interfaces: type change reverted Dec 11, 2024
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Could you also update the flag declaration back to StringSlice in daemon/cmd/daemon_main.go?

@joestringer joestringer added release-note/misc This PR makes changes that have no direct user impact. release-blocker/1.17 This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium. release-note/misc This PR makes changes that have no direct user impact. labels Dec 11, 2024
@joestringer
Copy link
Member

joestringer commented Dec 11, 2024

I went back and forth a bit on the release note for this. In general PRs should have only one release note label and since this is fixing a regression in v1.17.0-pre.3 I figure that release-note/bug is probably the most appropriate.

@viktor-kurchenko viktor-kurchenko force-pushed the vk/pr/egress/masquerade/fix branch from 9a2772c to 42b02bf Compare December 11, 2024 19:28
@viktor-kurchenko
Copy link
Contributor Author

Could you also update the flag declaration back to StringSlice in daemon/cmd/daemon_main.go?

Oh, thank you very much! Done.

@viktor-kurchenko
Copy link
Contributor Author

/test

@joestringer joestringer added this pull request to the merge queue Dec 12, 2024
Merged via the queue into main with commit b8da507 Dec 12, 2024
288 checks passed
@joestringer joestringer deleted the vk/pr/egress/masquerade/fix branch December 12, 2024 10:59
@maintainer-s-little-helper maintainer-s-little-helper bot added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Dec 12, 2024
@viktor-kurchenko viktor-kurchenko added backport/1.16 This PR represents a backport for Cilium 1.16.x of a PR that was merged to main. needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Jan 20, 2025
@julianwiedmann julianwiedmann removed the backport/1.16 This PR represents a backport for Cilium 1.16.x of a PR that was merged to main. label Jan 22, 2025
@rastislavs rastislavs mentioned this pull request Jan 22, 2025
19 tasks
@rastislavs rastislavs added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Jan 22, 2025
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Jan 24, 2025
@giorio94
Copy link
Member

Additionally marking for backport to v1.15, as #36103 got marked as well.

@giorio94 giorio94 added the backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. label Jan 27, 2025
@giorio94 giorio94 mentioned this pull request Jan 27, 2025
8 tasks
@joestringer
Copy link
Member

I agree that if we are backporting #36103 then this should also be backported. I don't understand why we are nominating #36103 however because it's outside the regular backport criteria: https://docs.cilium.io/en/stable/contributing/release/backports/#backport-criteria . CC @nbusseneau Users should be upgrading to the latest stable release if they want all the latest bugfixes. We really should be only backporting critical changes or security fixes to the older releases at this stage, as every change comes with a degree of risk that we break something else in the older releases. The less churn we have on older branches, the less burden we take on as an open source community due to the work of backporting and potentially then also subsequently fixing broken backports on older releases. This translates into more time we can spend making the current stable or next feature release better.

@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/iptables Impacts how Cilium interacts with iptables. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. cilium-cli This PR contains changes related with cilium-cli ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.17 This issue will prevent the release of the next version of Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants