-
Notifications
You must be signed in to change notification settings - Fork 1
Use TCP redirection in the proxy #34
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @mattbates, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances the proxy's network interception capabilities by introducing new nftables rules. These rules are specifically designed to redirect localhost-destined TCP traffic to the proxy, ensuring that local applications can be properly routed through the proxy. The changes also involve refining the nftables script structure for better maintainability and updating associated container image versions.
Highlights
- Implementation of TCP Redirection for Localhost Traffic: The core change involves adapting the
nftablesrules to redirect all TCP traffic destined for localhost (127.0.0.1/8 and ::1/128) to the proxy. This enhances the proxy's ability to intercept and manage local network traffic. - Refinement of
nftablesRules and Structure: Thenftablesscript has been refined, including renaming the table and chain, and adding explicit rules to ensure Envoy's own traffic and traffic already directed to the Envoy port are not re-redirected. This improves the robustness and correctness of the network interception. - Introduction of Configurable
nftablesParameters: A new Go struct,NftablesParams, was introduced to pass configuration parameters like Envoy UID, Envoy Port, and DNS Proxy Port to thenftablesscript template. This makes the script more dynamic and configurable. - Container Image Version Updates: The pull request includes minor version updates for the
DefaultDebugUIImageandInitHelperImageconstants, moving them fromv0.2.3tov0.2.4.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adapts the nftables rules to redirect localhost-destined TCP traffic to the proxy, in addition to the existing DNS traffic redirection. The changes are well-implemented, using Go templates to make the nftables script more configurable and readable. I've identified one critical issue in the updated script that would cause it to fail during execution. My review includes a comment with details on how to fix it.
| DebugUIContainerName = "spiffe-enable-ui" | ||
| DebugUIPort = 8000 | ||
| DefaultDebugUIImage = "ghcr.io/cofide/spiffe-enable-ui:v0.2.3" | ||
| DefaultDebugUIImage = "ghcr.io/cofide/spiffe-enable-ui:v0.3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in preparation for the next release
| # Redirect loopback TCP traffic (using tcp dport range to match all TCP) | ||
| ip daddr 127.0.0.1/8 tcp dport 1-65535 counter redirect to :{{.EnvoyPort}} comment "Loopback IPv4 to Envoy" | ||
| ip6 daddr ::1/128 tcp dport 1-65535 counter redirect to :{{.EnvoyPort}} comment "Loopback IPv6 to Envoy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you checked that applications are still able to use the loopback interface for other purposes?
Adapted the
nftablesrules to redirect localhost-destined TCP traffic to the proxy and added parametrisation to thenftablesscript template.