fix(vpcnatgw): cannot DNAT same EIP and same external port on two different protocols#6201
Conversation
…ferent protocols You should be able to DNAT 1.1.1.1:22 TCP and 1.1.1.1:22 UDP on the same EIP, but the check doesn't check if it is two different protocols. On Windows, RDP asks to open both the UDP and TCP protocols on the same port. It cannot be done behind an EIP attached to a VPC NAT gateway because of this bug. Signed-off-by: SkalaNetworks <contact@skala.network>
dosubot
bot
added
the
size:S
Summary of ChangesHello @SkalaNetworks, 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 addresses a critical bug in the VPC NAT gateway's DNAT rule management, where the system incorrectly prevented the creation of rules that shared an EIP and external port if the protocols were different. By extending the DNAT duplication check to include the network protocol, this change ensures that services requiring both TCP and UDP on the same port can now be properly configured, enhancing the flexibility and functionality of the NAT gateway. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
|
@oilbeater do you think we can get this backported to 1.15? |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request effectively addresses the bug where DNAT rules were not correctly distinguishing between different protocols (TCP/UDP) on the same EIP and external port. The changes correctly modify the isDnatDuplicated function to include the protocol in its duplication check, and update all call sites and error messages accordingly. The implementation is straightforward and directly resolves the reported issue, ensuring that services requiring both TCP and UDP on the same port can now be configured correctly. The code changes are clear, concise, and maintain the existing code style.
Pull Request Test Coverage Report for Build 21259886797Details
💛 - Coveralls |
…ferent protocols (#6201) You should be able to DNAT 1.1.1.1:22 TCP and 1.1.1.1:22 UDP on the same EIP, but the check doesn't check if it is two different protocols. On Windows, RDP asks to open both the UDP and TCP protocols on the same port. It cannot be done behind an EIP attached to a VPC NAT gateway because of this bug. Signed-off-by: SkalaNetworks <contact@skala.network> (cherry picked from commit bc813ee)
…ferent protocols (#6201) You should be able to DNAT 1.1.1.1:22 TCP and 1.1.1.1:22 UDP on the same EIP, but the check doesn't check if it is two different protocols. On Windows, RDP asks to open both the UDP and TCP protocols on the same port. It cannot be done behind an EIP attached to a VPC NAT gateway because of this bug. Signed-off-by: SkalaNetworks <contact@skala.network>
…ferent protocols (kubeovn#6201) You should be able to DNAT 1.1.1.1:22 TCP and 1.1.1.1:22 UDP on the same EIP, but the check doesn't check if it is two different protocols. On Windows, RDP asks to open both the UDP and TCP protocols on the same port. It cannot be done behind an EIP attached to a VPC NAT gateway because of this bug. Signed-off-by: SkalaNetworks <contact@skala.network> (cherry picked from commit bc813ee)
3 participants
You should be able to DNAT 1.1.1.1:22 TCP and 1.1.1.1:22 UDP on the same EIP, but the check doesn't verify if it is two different protocols.
On Windows, RDP asks to open both the UDP and TCP protocols on the same port. It cannot be done behind an EIP attached to a VPC NAT gateway because of this bug.