Skip to content

Envoy: ignore explicit listener rules on other listeners #39079

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

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Apr 22, 2025

Pass the proxy port of an explicit Listener reference in CNP as a proxy ID and ignore rules with non-zero proxy IDs unless the proxy ID matches the one configured for the Listener handling the connection. This scopes the policy rule containing the explicit Listener reference to only apply on the specific named listener. This way possibly stricter allow rules specified otherwise will not be bypassed due to the laxer allow rule for the specific listener.

CNP rule redirecting to a named Listener is now ignored on other Listeners.

@jrajahalme jrajahalme added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. dont-merge/preview-only Only for preview or testing, don't merge it. release-note/misc This PR makes changes that have no direct user impact. labels Apr 22, 2025
@jrajahalme jrajahalme requested review from a team as code owners April 22, 2025 11:45
@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch 2 times, most recently from ce17f00 to 47616f5 Compare April 22, 2025 12:11
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 47616f5 to 7c70ac2 Compare April 23, 2025 08:24
@jrajahalme
Copy link
Member Author

rebased to fix missing junit file problem.

@jrajahalme
Copy link
Member Author

/test

@mhofstetter mhofstetter added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Apr 29, 2025
@mhofstetter
Copy link
Member

@jrajahalme i saw that you added the label dont-merge/preview-only -> i will mark this PR as draft. (It needs a rebase anyway).

@mhofstetter mhofstetter marked this pull request as draft April 29, 2025 09:02
@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 7c70ac2 to 9366f53 Compare May 12, 2025 18:19
@jrajahalme jrajahalme marked this pull request as ready for review May 12, 2025 18:20
@jrajahalme jrajahalme removed dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. dont-merge/preview-only Only for preview or testing, don't merge it. labels May 12, 2025
@jrajahalme jrajahalme changed the title Envoy proxy listener policy refinement Envoy proxy policy enforcement fixes May 12, 2025
@jrajahalme
Copy link
Member Author

fixed daemon tests

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from a423206 to 91a232e Compare May 13, 2025 07:51
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 91a232e to 2b38c04 Compare May 13, 2025 09:48
Copy link
Contributor

@marseel marseel left a comment

Choose a reason for hiding this comment

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

Thanks! vendor changes lgtm

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 2b38c04 to 412babe Compare May 14, 2025 08:29
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 412babe to 3633186 Compare May 14, 2025 12:38
@jrajahalme jrajahalme changed the title Envoy proxy policy enforcement fixes Envoy proxy policy ignore explicit listener rules on other listeners May 14, 2025
@jrajahalme jrajahalme changed the title Envoy proxy policy ignore explicit listener rules on other listeners Envoy: ignore explicit listener rules on other listeners May 14, 2025
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 3633186 to 757b9ae Compare May 20, 2025 14:06
@jrajahalme
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 26, 2025
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Pass the proxy port of an explicit Listener reference in CNP as a proxy
ID and ignore rules with non-zero proxy IDs unless the proxy ID matches
the one configured for the Listener handling the connection. This scopes
the policy rule containing the explicit Listener reference to only apply
on the specific named listener. This way possibly stricter allow rules
specified otherwise will not be bypassed due to the laxer allow rule for
the specific listener.

Disable HTTP rule short-circuiting also when a rule has a non-zero
proxy-id.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
@jrajahalme jrajahalme force-pushed the envoy-proxy-listener-policy-refinement branch from 757b9ae to b583a70 Compare May 28, 2025 08:58
@jrajahalme
Copy link
Member Author

rebased, API update was already done on main

@jrajahalme jrajahalme enabled auto-merge May 28, 2025 08:59
@jrajahalme jrajahalme requested a review from bowei May 28, 2025 09:02
@jrajahalme
Copy link
Member Author

/test

@jrajahalme jrajahalme added this pull request to the merge queue May 29, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 29, 2025
Merged via the queue into cilium:main with commit 8433946 May 29, 2025
69 checks passed
@jrajahalme jrajahalme deleted the envoy-proxy-listener-policy-refinement branch May 29, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants