UDN: Ensure pod2service isolation in local gw mode. #4705
Conversation
dceara
added
the
feature/user-defined-network-segmentation
github-actions
bot
added
feature/egress-ip
dceara
force-pushed
the
udn-pod2service-isolation
branch
5 times, most recently
from
47ad5b4
to
44dcb94
Compare
dceara
force-pushed
the
udn-pod2service-isolation
branch
7 times, most recently
from
de54993
to
2b9a6a1
Compare
github-actions
bot
added
the
kind/documentation
dceara
force-pushed
the
udn-pod2service-isolation
branch
3 times, most recently
from
4d4305d
to
a3fc574
Compare
dceara
force-pushed
the
udn-pod2service-isolation
branch
2 times, most recently
from
44ff253
to
72ba440
Compare
dceara
changed the title
dceara
force-pushed
the
udn-pod2service-isolation
branch
2 times, most recently
from
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]>
dceara
force-pushed
the
udn-pod2service-isolation
branch
from
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.
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.
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.
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?