From a25fb03c00a14d11b4ddc5282b2e23056f9c34e6 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 22 Sep 2023 11:37:26 -0400 Subject: [PATCH] Add assertIPTablesChainEquals, to streamline a few tests Rather than checking the entire iptables dump, only check a single chain. --- pkg/proxy/iptables/proxier_test.go | 121 +++++++++++++---------------- 1 file changed, 54 insertions(+), 67 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 7e3d1519818..9c088da13ff 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -1312,6 +1312,32 @@ func assertIPTablesRulesEqual(t *testing.T, line int, checkConsistency bool, exp } } +// assertIPTablesChainEqual asserts that the indicated chain in the indicated table in +// result contains exactly the rules in expected (in that order). +func assertIPTablesChainEqual(t *testing.T, line int, table utiliptables.Table, chain utiliptables.Chain, expected, result string) { + expected = strings.TrimLeft(expected, " \t\n") + + dump, err := iptablestest.ParseIPTablesDump(strings.TrimLeft(result, " \t\n")) + if err != nil { + t.Fatalf("%s", err) + } + + result = "" + if ch, _ := dump.GetChain(table, chain); ch != nil { + for _, rule := range ch.Rules { + result += rule.Raw + "\n" + } + } + + lineStr := "" + if line != 0 { + lineStr = fmt.Sprintf(" (from line %d)", line) + } + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("rules do not match%s:\ndiff:\n%s\nfull result:\n```\n%s```", lineStr, diff, result) + } +} + // addressMatches helps test whether an iptables rule such as "! -s 192.168.0.0/16" matches // ipStr. address.Value is either an IP address ("1.2.3.4") or a CIDR string // ("1.2.3.0/24"). @@ -2544,86 +2570,47 @@ func TestHealthCheckNodePort(t *testing.T) { } func TestDropInvalidRule(t *testing.T) { - for _, testcase := range []bool{false, true} { - t.Run(fmt.Sprintf("tcpLiberal %t", testcase), func(t *testing.T) { + for _, tcpLiberal := range []bool{false, true} { + t.Run(fmt.Sprintf("tcpLiberal %t", tcpLiberal), func(t *testing.T) { ipt := iptablestest.NewFake() fp := NewFakeProxier(ipt) - fp.conntrackTCPLiberal = testcase + fp.conntrackTCPLiberal = tcpLiberal fp.syncProxyRules() - expected := dedent.Dedent(` - *filter - :KUBE-NODEPORTS - [0:0] - :KUBE-SERVICES - [0:0] - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FIREWALL - [0:0] - :KUBE-FORWARD - [0:0] - :KUBE-PROXY-FIREWALL - [0:0] - -A KUBE-FIREWALL -m comment --comment "block incoming localnet connections" -d 127.0.0.0/8 ! -s 127.0.0.0/8 -m conntrack ! --ctstate RELATED,ESTABLISHED,DNAT -j DROP`) - if !testcase { - expected += "\n-A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP" + var expected string + if !tcpLiberal { + expected = "-A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP" } - expected += dedent.Dedent(` - -A KUBE-FORWARD -m comment --comment "kubernetes forwarding rules" -m mark --mark 0x4000/0x4000 -j ACCEPT - -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT - COMMIT - *nat - :KUBE-NODEPORTS - [0:0] - :KUBE-SERVICES - [0:0] - :KUBE-MARK-MASQ - [0:0] - :KUBE-POSTROUTING - [0:0] - -A KUBE-SERVICES -m comment --comment "kubernetes service nodeports; NOTE: this must be the last rule in this chain" -m addrtype --dst-type LOCAL -j KUBE-NODEPORTS - -A KUBE-MARK-MASQ -j MARK --or-mark 0x4000 - -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 - COMMIT - `) + -A KUBE-FORWARD -m comment --comment "kubernetes forwarding rules" -m mark --mark 0x4000/0x4000 -j ACCEPT + -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT + `) - assertIPTablesRulesEqual(t, getLine(), true, expected, fp.iptablesData.String()) + assertIPTablesChainEqual(t, getLine(), utiliptables.TableFilter, kubeForwardChain, expected, fp.iptablesData.String()) }) } } func TestMasqueradeRule(t *testing.T) { - for _, testcase := range []bool{false, true} { - ipt := iptablestest.NewFake().SetHasRandomFully(testcase) - fp := NewFakeProxier(ipt) - fp.syncProxyRules() + for _, randomFully := range []bool{false, true} { + t.Run(fmt.Sprintf("randomFully %t", randomFully), func(t *testing.T) { + ipt := iptablestest.NewFake().SetHasRandomFully(randomFully) + fp := NewFakeProxier(ipt) + fp.syncProxyRules() - expectedFmt := dedent.Dedent(` - *filter - :KUBE-NODEPORTS - [0:0] - :KUBE-SERVICES - [0:0] - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FIREWALL - [0:0] - :KUBE-FORWARD - [0:0] - :KUBE-PROXY-FIREWALL - [0:0] - -A KUBE-FIREWALL -m comment --comment "block incoming localnet connections" -d 127.0.0.0/8 ! -s 127.0.0.0/8 -m conntrack ! --ctstate RELATED,ESTABLISHED,DNAT -j DROP - -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP - -A KUBE-FORWARD -m comment --comment "kubernetes forwarding rules" -m mark --mark 0x4000/0x4000 -j ACCEPT - -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT - COMMIT - *nat - :KUBE-NODEPORTS - [0:0] - :KUBE-SERVICES - [0:0] - :KUBE-MARK-MASQ - [0:0] - :KUBE-POSTROUTING - [0:0] - -A KUBE-SERVICES -m comment --comment "kubernetes service nodeports; NOTE: this must be the last rule in this chain" -m addrtype --dst-type LOCAL -j KUBE-NODEPORTS - -A KUBE-MARK-MASQ -j MARK --or-mark 0x4000 - -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%s - COMMIT - `) - var expected string - if testcase { - expected = fmt.Sprintf(expectedFmt, " --random-fully") - } else { - expected = fmt.Sprintf(expectedFmt, "") - } - assertIPTablesRulesEqual(t, getLine(), true, expected, fp.iptablesData.String()) + expectedFmt := dedent.Dedent(` + -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%s + `) + var expected string + if randomFully { + expected = fmt.Sprintf(expectedFmt, " --random-fully") + } else { + expected = fmt.Sprintf(expectedFmt, "") + } + assertIPTablesChainEqual(t, getLine(), utiliptables.TableNAT, kubePostroutingChain, expected, fp.iptablesData.String()) + }) } }