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

sidecar iptables - istio-specific rules should only be inserted into custom chains #50532

Closed
bleggett opened this issue Apr 18, 2024 · 1 comment · Fixed by #50915
Closed
Labels
area/dual-stack area/networking good first issue Indicates a good first issue for new contributors

Comments

@bleggett
Copy link
Contributor

bleggett commented Apr 18, 2024

Describe the feature request

In ambient, we create custom ISTIO_XXX prefixed chains for our custom iptables rules, and only put our custom rules in these prefixed chains.

This makes cleanup and upgrade simpler (main table/chain has one jump to our custom chain, we can remove the drop and the prefixed chains safely), and is the only pattern we should follow for Istio-specific iptables rules, no matter the context (sidecar or node).

In sidecar, we are still slamming a bunch of rules into the main table/chain, e.g: many of the sidecar DNS rules are just in OUTPUT and should be in a custom chain named ISTIO_OUTPUT or similar:

iptables -t nat -A OUTPUT -p udp --dport 53 -m owner --uid-owner 3 -j RETURN

Sidecar rule creation should be tweaked to create and insert all rules into custom chains, like ambient does.

We have considered adding a WARN nag to rules cleanup that will flag any non-jump rules in main tables: #50328 (comment) - a PR to fix this issue should eliminate all outstanding WARNs.

Describe alternatives you've considered

Affected product area (please put an X in all that apply)

[ ] Ambient
[ ] Docs
[x] Dual Stack
[ ] Installation
[x] Networking
[ ] Performance and Scalability
[ ] Extensions and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Affected features (please put an X in all that apply)

[ ] Multi Cluster
[ ] Virtual Machine
[ ] Multi Control Plane

Additional context

@bleggett
Copy link
Contributor Author

I am adding a good-first-issue because this is pretty simple cleanup if you're not afraid to get a little cozy with iptables

@bleggett bleggett added the good first issue Indicates a good first issue for new contributors label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dual-stack area/networking good first issue Indicates a good first issue for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants