proxy/iptables: Don't create unused chains, and enable the unit test for that

This commit is contained in:
Dan Winship 2022-01-07 14:17:11 -05:00
parent ef4324eaf5
commit 37ada4b04f
2 changed files with 24 additions and 10 deletions

View File

@ -1101,7 +1101,7 @@ func (proxier *Proxier) syncProxyRules() {
}
svcXlbChain := svcInfo.serviceLBChainName
if svcInfo.NodeLocalExternal() {
if hasEndpoints && svcInfo.NodeLocalExternal() {
// Only for services request OnlyLocal traffic
// create the per-service LB chain, retaining counters if possible.
if lbChain, ok := existingNATChains[svcXlbChain]; ok {

View File

@ -721,6 +721,10 @@ func checkIPTablesRuleJumps(ruleData string) error {
// Find all of the lines like ":KUBE-SERVICES", indicating chains that
// iptables-restore would create when loading the data.
createdChains := sets.NewString(findAllMatches(lines, `^:([^ ]*)`)...)
// Find all of the lines like "-X KUBE-SERVICES ..." indicating chains
// that we are deleting because they are no longer used, and remove
// those chains from createdChains.
createdChains = createdChains.Delete(findAllMatches(lines, `-X ([^ ]*)`)...)
// Find all of the lines like "-A KUBE-SERVICES ..." indicating chains
// that we are adding at least one rule to.
@ -754,12 +758,11 @@ func checkIPTablesRuleJumps(ruleData string) error {
// Find cases where we have ":BAR" but no "-A FOO ... -j BAR", meaning
// that we are creating an empty chain but not using it for anything.
// FIXME: This currently fails
// extraChains := createdChains.Difference(jumpedChains)
// extraChains.Delete(string(kubeServicesChain), string(kubeExternalServicesChain), string(kubeNodePortsChain), string(kubePostroutingChain), string(kubeForwardChain), string(KubeMarkMasqChain))
// if len(extraChains) > 0 {
// return fmt.Errorf("some chains in %s are created but not used: %v", tableName, extraChains.List())
// }
extraChains := createdChains.Difference(jumpedChains)
extraChains.Delete(string(kubeServicesChain), string(kubeExternalServicesChain), string(kubeNodePortsChain), string(kubePostroutingChain), string(kubeForwardChain), string(KubeMarkMasqChain))
if len(extraChains) > 0 {
return fmt.Errorf("some chains in %s are created but not used: %v", tableName, extraChains.List())
}
}
return nil
@ -826,6 +829,20 @@ COMMIT
`,
error: "some chains in nat are used but were not created: [KUBE-SVC-XPGD46QRK7WJZT7O]",
},
{
name: "can't create chain and then not use it",
input: `
*filter
COMMIT
*nat
:KUBE-MARK-MASQ - [0:0]
:KUBE-SERVICES - [0:0]
:KUBE-SVC-XPGD46QRK7WJZT7O - [0:0]
-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" ...
COMMIT
`,
error: "some chains in nat are created but not used: [KUBE-SVC-XPGD46QRK7WJZT7O]",
},
} {
t.Run(tc.name, func(t *testing.T) {
err := checkIPTablesRuleJumps(tc.input)
@ -1899,7 +1916,6 @@ COMMIT
:KUBE-NODEPORTS - [0:0]
:KUBE-POSTROUTING - [0:0]
:KUBE-MARK-MASQ - [0:0]
:KUBE-XLB-XPGD46QRK7WJZT7O - [0:0]
-A KUBE-POSTROUTING -m mark ! --mark 0x4000/0x4000 -j RETURN
-A KUBE-POSTROUTING -j MARK --xor-mark 0x4000
-A KUBE-POSTROUTING -m comment --comment "kubernetes service traffic requiring SNAT" -j MASQUERADE
@ -2289,7 +2305,6 @@ COMMIT
:KUBE-NODEPORTS - [0:0]
:KUBE-POSTROUTING - [0:0]
:KUBE-MARK-MASQ - [0:0]
:KUBE-XLB-XPGD46QRK7WJZT7O - [0:0]
-A KUBE-POSTROUTING -m mark ! --mark 0x4000/0x4000 -j RETURN
-A KUBE-POSTROUTING -j MARK --xor-mark 0x4000
-A KUBE-POSTROUTING -m comment --comment "kubernetes service traffic requiring SNAT" -j MASQUERADE
@ -5117,7 +5132,6 @@ COMMIT
:KUBE-NODEPORTS - [0:0]
:KUBE-POSTROUTING - [0:0]
:KUBE-MARK-MASQ - [0:0]
:KUBE-XLB-AQI2S6QIMU7PVVRP - [0:0]
-A KUBE-POSTROUTING -m mark ! --mark 0x4000/0x4000 -j RETURN
-A KUBE-POSTROUTING -j MARK --xor-mark 0x4000
-A KUBE-POSTROUTING -m comment --comment "kubernetes service traffic requiring SNAT" -j MASQUERADE