-
Notifications
You must be signed in to change notification settings - Fork 338
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
UDN: Ensure pod2service isolation in local gw mode. #4705
base: master
Are you sure you want to change the base?
Conversation
47ad5b4
to
44dcb94
Compare
de54993
to
2b9a6a1
Compare
4d4305d
to
a3fc574
Compare
44ff253
to
72ba440
Compare
0331a07
to
779b8ce
Compare
…ces. In local gateway mode all UDN traffic that needs to exit the node was redirected to the host (via the mp-X port). There however it was injected into the br-ex/breth0 bridge where it got sent into the default network patch port (because the destination IP is that of a service CIDR). That traffic eventually reached the default network endpoint and the connection was successfully set up. The mode above is not the correct mode of operation for UDN. UDN pods should not be allowed to access any default network services (except select ones like kapi). To achieve that we change the br-ex/breth0 flows and add new per-UDN flows that detect traffic originated by a given network and only direct it back to that network's patch port. This commit updates the e2e test to cover this scenario. This change also indirectly fixes the fact that node port default services should not be accessible from UDN when the target IP is the local node IP (that traffic is now sent back to the UDN GR where no LB is configured for the default service). New flow specific unit tests are also added to make sure these flows are not accidentally removed in the future. Signed-off-by: Dumitru Ceara <[email protected]>
779b8ce
to
d63887e
Compare
"actions=ct(commit,zone=%d,table=2)", | ||
defaultOpenFlowCookie, npw.ofm.defaultBridge.ofPortHost, ipPrefix, ipPrefix, masqueradeSubnet, ipPrefix, service.Spec.ClusterIP, config.Default.HostMasqConntrackZone)}) | ||
// table 2, user-defined network host -> OVN towards default cluster network services | ||
defaultNetConfig := npw.ofm.defaultBridge.netConfig[types.DefaultNetworkName] |
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.
out of curiosity: What was wrong with the previous way of accessing the ofport: npw.ofm.defaultBridge.ofPortHost ? If this is duplicate data stored in the netconfig should we remove the ofPortHost field from the bridgeConfiguration?
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.
Uhm nothing? But this flow is changing: instead of matching of inport=ofPortHost in table 0 it now is matching on ip_src=masq_subnet with action=set_mac; output to ofPortUDNPatch in table 2.
The job of the flow I'm changing is now done by the new one I'm adding in table 0 (for the whole svc CIDR, not only for select services like kapi) here:
https://github.com/ovn-org/ovn-kubernetes/pull/4705/files#diff-d3aa58d9b58a0a09264f072df46ab01d0501eb508c4656411ae2dc1ac68fb3c4R1329
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.
Maybe this helps explain it better. On top of the change @kyrtapz added to allow access to the kapi, I do:
a. In table 0: I add a priority 550 flow matching on ip_src=masqSubnet that skips SNAT (covers all UDNs).
b. In table 2: I add a flow per UDN to send traffic to the patch port-x if src-IP=UDN-masq-IP-X, priority 200
c. I moved the flow from table 0 that used to skip SNAT for packets going to kapi to table 2 (it's now covered by "a"), priority 300 and change it to match on ip_src=masqSubnet && ip_dst == kapi and change it to send to the default network patch port (higher priority than "b").
What this PR does and why is it needed
Ensures isolation between UDN pods and services from the default network.
Which issue(s) this PR fixes
Fixes #4687
Special notes for reviewers
How to verify it
E2E tests covering this use case have been updated.
Unit tests added to check the correct openflows are added to br-ex.
Details to documentation updates
N/A
Description for the changelog
Does this PR introduce a user-facing change?