Pod in nat-outgoing should not be SNATed when it accesses local cluster #8961
Conversation
marvin-tigera
added
release-note-required
|
/sem-approve |
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
919b3e0 to
5df5556
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
670e42d to
009fd68
Compare
|
I think this is achievable already today by creating a disabled IP pool containing your host IP range which tells Calico these IPs aren't external to the cluster. This is obviously not ideal for clusters where the host IPs might not fall cleanly into a small set of IP pools. I do think a setting like this makes a bit more sense on FelixConfiguration than on IP pools - setting this for a single IP pool (at least based on this implementation) is going to enforce the setting on every IP pool with NATOutgoing set, so the config model is a little bit misleading. @fasaxc WDYT? |
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
009fd68 to
e782906
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
e782906 to
f4603d9
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
f4603d9 to
cde432b
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
cde432b to
bae1508
Compare
|
This is likely going to work for eBPF mode as well, however, it would be better to propagate the setting to somewhere here |
| // When set to true and ip pool setting `natOutgoing` is true, packets sent from Calico networked containers in this pool | ||
| // to cluster host subnet will not be excluded from being masqueraded. [Default: false] | ||
| DisableHostSubnetNATExclusion bool `json:"disableHostSubnetNATExclusion,omitempty"` | ||
|
|
There was a problem hiding this comment.
Too many negatives for me to make much sense out of this. Could you make it a bit easier to read? Perhaps rename it to just HostSubnetSNATExclusion. And expand in the PR description on the two behaviours?
There was a problem hiding this comment.
I have made the adjustments. PTAL!
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
bae1508 to
c4e462b
Compare
In the related issue, I mentioned that Calico for Windows also needs such a setting. I think we can continue to improve it through additional PRs. |
|
Could we make this an enum; say: then we can improve over time with new, better options. For example, we could match on the all-hosts IP set to offer |
felix/rules/nat.go
Outdated
| @@ -58,6 +58,11 @@ func (r *DefaultRuleRenderer) makeNATOutgoingRuleIPTables(ipVersion uint8, proto | |||
| SourceIPSet(masqIPsSetName). | |||
| NotDestIPSet(allIPsSetName) | |||
|
|
|||
| if r.Config.HostSubnetNATExclusion { | |||
There was a problem hiding this comment.
You are using the all-host IP set so the config param name is misleading since it's not using the subnet at all.
There was a problem hiding this comment.
You are right, IPPoolsAndHostIPs can convey the meaning of all-host IPs.
So, should I start changing this config to an enum?
@tomastigera WDYT?
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
c4e462b to
1d8e1d3
Compare
|
Hi, I updated the code and description. PTAL, thanks! @fasaxc |
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
1d8e1d3 to
53e2f39
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
ce93465 to
a5158e3
Compare
wayne-cheng
force-pushed
the
fix-iptables-nat-host
branch
from
a5158e3 to
d705776
Compare
Description
When
natOutgoingis enabled in a IPPool, packets sent from Calico networked containers in this IP pool to destinations will be SNATed(msaqueraded).However, we would prefer that the traffic accessing local cluster hosts should not be msaqueraded.
Provide a enum setting
natOutgoingExclusions(default:IPPoolsOnly) in felix config.Configure which type of destinations is excluded from being masqueraded.
To maintain compatibility, this setting is not unconditionally enabled. We can consider switching its default value to
IPPoolsAndHostIPsin the future version.Related issues/PRs
fixes linux part of #8960
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.