pkg/util/iptables/testing: Add IPTables dump-parsing helpers

pkg/proxy/iptables/proxier_test.go

@@ -675,18 +675,22 @@ func TestParseIPTablesData(t *testing.T) {
 	}
 }
 
-func countRules(tableName string, ruleData string) int {
+func countRules(tableName utiliptables.Table, ruleData string) int {
-	tables, err := parseIPTablesData(ruleData)
+	dump, err := iptablestest.ParseIPTablesDump(ruleData)
 	if err != nil {
 		klog.ErrorS(err, "error parsing iptables rules")
 		return -1
 	}
 
 	rules := 0
-	for _, line := range tables[tableName] {
+	table, err := dump.GetTable(tableName)
-		if line[0] == '-' {
+	if err != nil {
-			rules++
+		klog.ErrorS(err, "can't find table", "table", tableName)
-		}
+		return -1
+	}
+
+	for _, c := range table.Chains {
+		rules += len(c.Rules)
 	}
 	return rules
 }
@@ -705,20 +709,6 @@ func findAllMatches(lines []string, pattern string) []string {
 	return allMatches.List()
 }
 
-// moveMatchingLines moves lines that match pattern from input to output
-func moveMatchingLines(pattern string, input, output []string) ([]string, []string) {
-	var newIn []string
-	regex := regexp.MustCompile(pattern)
-	for _, line := range input {
-		if regex.FindString(line) != "" {
-			output = append(output, line)
-		} else {
-			newIn = append(newIn, line)
-		}
-	}
-	return newIn, output
-}
-
 // checkIPTablesRuleJumps checks that every `-j` in the given rules jumps to a chain
 // that we created and added rules to
 func checkIPTablesRuleJumps(ruleData string) error {
@@ -910,76 +900,107 @@ func TestCheckIPTablesRuleJumps(t *testing.T) {
 	}
 }
 
+// orderByCommentServiceName is a helper function that orders two IPTables rules
+// based on the service name in their comment. (If either rule has no comment then the
+// return value is undefined.)
+func orderByCommentServiceName(rule1, rule2 *iptablestest.Rule) bool {
+	if rule1.Comment == nil || rule2.Comment == nil {
+		return false
+	}
+	name1, name2 := rule1.Comment.Value, rule2.Comment.Value
+
+	// The service name is the comment up to the first space or colon
+	i := strings.IndexAny(name1, " :")
+	if i != -1 {
+		name1 = name1[:i]
+	}
+	i = strings.IndexAny(name2, " :")
+	if i != -1 {
+		name2 = name2[:i]
+	}
+
+	return name1 < name2
+}
+
 // sortIPTablesRules sorts `iptables-restore` output so as to not depend on the order that
 // Services get processed in, while preserving the relative ordering of related rules.
 func sortIPTablesRules(ruleData string) (string, error) {
-	tables, err := parseIPTablesData(ruleData)
+	dump, err := iptablestest.ParseIPTablesDump(ruleData)
 	if err != nil {
 		return "", err
 	}
 
-	tableNames := make([]string, 0, len(tables))
+	// Sort tables
-	for tableName := range tables {
+	sort.Slice(dump.Tables, func(i, j int) bool {
-		tableNames = append(tableNames, tableName)
+		return dump.Tables[i].Name < dump.Tables[j].Name
-	}
+	})
-	sort.Strings(tableNames)
 
-	var output []string
+	// Sort chains
-	for _, name := range tableNames {
+	for t := range dump.Tables {
-		lines := tables[name]
+		table := &dump.Tables[t]
-
+		sort.Slice(table.Chains, func(i, j int) bool {
-		// Move "*TABLENAME" line
+			switch {
-		lines, output = moveMatchingLines(`^\*`, lines, output)
+			case table.Chains[i].Name == kubeNodePortsChain:
-
+				// KUBE-NODEPORTS comes before anything
-		// findAllMatches() returns a sorted list of unique matches. So for
+				return true
-		// each of the following, we find all the matches for the regex, then
+			case table.Chains[j].Name == kubeNodePortsChain:
-		// for each unique match (in sorted order), move all of the lines that
+				// anything goes after KUBE-NODEPORTS
-		// contain that match.
+				return false
-
+			case table.Chains[i].Name == kubeServicesChain:
-		// Move and sort ":CHAINNAME" lines (in the same order we will sort
+				// KUBE-SERVICES comes before anything (except KUBE-NODEPORTS)
-		// the chains themselves below).
+				return true
-		lines, output = moveMatchingLines(`:KUBE-NODEPORTS`, lines, output)
+			case table.Chains[j].Name == kubeServicesChain:
-		lines, output = moveMatchingLines(`:KUBE-SERVICES`, lines, output)
+				// anything (except KUBE-NODEPORTS) goes after KUBE-SERVICES
-		for _, chainName := range findAllMatches(lines, `^(:KUBE-[^ ]*) `) {
+				return false
-			lines, output = moveMatchingLines(chainName, lines, output)
+			case strings.HasPrefix(string(table.Chains[i].Name), "KUBE-") && !strings.HasPrefix(string(table.Chains[j].Name), "KUBE-"):
-		}
+				// KUBE-* comes before non-KUBE-*
-		for _, chainName := range findAllMatches(lines, `^(:[^ ]*) `) {
+				return true
-			lines, output = moveMatchingLines(chainName, lines, output)
+			case !strings.HasPrefix(string(table.Chains[i].Name), "KUBE-") && strings.HasPrefix(string(table.Chains[j].Name), "KUBE-"):
-		}
+				// non-KUBE-* goes after KUBE-*
-
+				return false
-		// Move KUBE-NODEPORTS rules for each service, sorted by service name
+			default:
-		for _, nextNodePortService := range findAllMatches(lines, `-A KUBE-NODEPORTS.*--comment "?([^ ]*)`) {
+				// We have two KUBE-* chains or two non-KUBE-* chains; either
-			lines, output = moveMatchingLines(fmt.Sprintf(`^-A KUBE-NODEPORTS.*%s`, nextNodePortService), lines, output)
+				// way they sort alphabetically
-		}
+				return table.Chains[i].Name < table.Chains[j].Name
-
+			}
-		// Move KUBE-SERVICES rules for each service, sorted by service name. The
+		})
-		// relative ordering of actual per-service lines doesn't matter, but keep
-		// the "must be the last rule" rule last because it's confusing otherwise...
-		lines, tmp := moveMatchingLines(`KUBE-SERVICES.*must be the last rule`, lines, nil)
-		for _, nextService := range findAllMatches(lines, `-A KUBE-SERVICES.*--comment "?([^ ]*)`) {
-			lines, output = moveMatchingLines(fmt.Sprintf(`^-A KUBE-SERVICES.*%s`, nextService), lines, output)
-		}
-		_, output = moveMatchingLines(`.`, tmp, output)
-
-		// Move remaining chains, sorted by chain name
-		for _, nextChain := range findAllMatches(lines, `(-A KUBE-[^ ]* )`) {
-			lines, output = moveMatchingLines(nextChain, lines, output)
-		}
-
-		// Some tests have deletions...
-		for _, nextChain := range findAllMatches(lines, `(-X KUBE-.*)`) {
-			lines, output = moveMatchingLines(nextChain, lines, output)
-		}
-
-		// Move the "COMMIT" line and anything else left. (There shouldn't be anything
-		// else, but if there is, it will show up in the diff later.)
-		_, output = moveMatchingLines(".", lines, output)
 	}
 
-	// Input ended with a "\n", so make sure the output does too
+	// Sort KUBE-NODEPORTS chains by service name
-	output = append(output, "")
+	chain, _ := dump.GetChain(utiliptables.TableFilter, kubeNodePortsChain)
+	if chain != nil {
+		sort.SliceStable(chain.Rules, func(i, j int) bool {
+			return orderByCommentServiceName(chain.Rules[i], chain.Rules[j])
+		})
+	}
+	chain, _ = dump.GetChain(utiliptables.TableNAT, kubeNodePortsChain)
+	if chain != nil {
+		sort.SliceStable(chain.Rules, func(i, j int) bool {
+			return orderByCommentServiceName(chain.Rules[i], chain.Rules[j])
+		})
+	}
 
-	return strings.Join(output, "\n"), nil
+	// Sort KUBE-SERVICES chains by service name (but keeping the "must be the last
+	// rule" rule in the "nat" table's KUBE-SERVICES chain last).
+	chain, _ = dump.GetChain(utiliptables.TableFilter, kubeServicesChain)
+	if chain != nil {
+		sort.SliceStable(chain.Rules, func(i, j int) bool {
+			return orderByCommentServiceName(chain.Rules[i], chain.Rules[j])
+		})
+	}
+	chain, _ = dump.GetChain(utiliptables.TableNAT, kubeServicesChain)
+	if chain != nil {
+		sort.SliceStable(chain.Rules, func(i, j int) bool {
+			if chain.Rules[i].Comment != nil && strings.Contains(chain.Rules[i].Comment.Value, "must be the last rule") {
+				return false
+			} else if chain.Rules[j].Comment != nil && strings.Contains(chain.Rules[j].Comment.Value, "must be the last rule") {
+				return true
+			}
+			return orderByCommentServiceName(chain.Rules[i], chain.Rules[j])
+		})
+	}
+
+	return dump.String(), nil
 }
 
 func TestSortIPTablesRules(t *testing.T) {
@@ -1136,22 +1157,6 @@ func TestSortIPTablesRules(t *testing.T) {
 				COMMIT
 				`),
 		},
-		{
-			name: "not enough tables",
-			input: dedent.Dedent(`
-				*filter
-				:KUBE-SERVICES - [0:0]
-				:KUBE-EXTERNAL-SERVICES - [0:0]
-				:KUBE-FORWARD - [0:0]
-				:KUBE-NODEPORTS - [0:0]
-				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
-				-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
-				`),
-			error: "bad ruleData (1 tables)",
-		},
 		{
 			name: "extra tables",
 			input: dedent.Dedent(`
@@ -2213,7 +2218,7 @@ func TestOverallIPTablesRulesWithMultipleServices(t *testing.T) {
 	}
 	nNatRules := int(natRulesMetric)
 
-	expectedNatRules := countRules("nat", fp.iptablesData.String())
+	expectedNatRules := countRules(utiliptables.TableNAT, fp.iptablesData.String())
 
 	if nNatRules != expectedNatRules {
 		t.Fatalf("Wrong number of nat rules: expected %d received %d", expectedNatRules, nNatRules)
@@ -5182,7 +5187,7 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) {
 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err)
 	}
 	nFilterRules := int(nFilterRulesMetric)
-	expectedFilterRules := countRules("filter", iptablesData)
+	expectedFilterRules := countRules(utiliptables.TableFilter, iptablesData)
 
 	if nFilterRules != expectedFilterRules {
 		t.Fatalf("Wrong number of filter rule: expected %d got %d\n%s", expectedFilterRules, nFilterRules, iptablesData)
@@ -5193,7 +5198,7 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) {
 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err)
 	}
 	nNatRules := int(nNatRulesMetric)
-	expectedNatRules := countRules("nat", iptablesData)
+	expectedNatRules := countRules(utiliptables.TableNAT, iptablesData)
 
 	if nNatRules != expectedNatRules {
 		t.Fatalf("Wrong number of nat rules: expected %d got %d\n%s", expectedNatRules, nNatRules, iptablesData)
@@ -5223,7 +5228,7 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) {
 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err)
 	}
 	nFilterRules = int(nFilterRulesMetric)
-	expectedFilterRules = countRules("filter", iptablesData)
+	expectedFilterRules = countRules(utiliptables.TableFilter, iptablesData)
 
 	if nFilterRules != expectedFilterRules {
 		t.Fatalf("Wrong number of filter rule: expected %d got %d\n%s", expectedFilterRules, nFilterRules, iptablesData)
@@ -5234,7 +5239,7 @@ func TestProxierMetricsIptablesTotalRules(t *testing.T) {
 		t.Errorf("failed to get %s value, err: %v", metrics.IptablesRulesTotal.Name, err)
 	}
 	nNatRules = int(nNatRulesMetric)
-	expectedNatRules = countRules("nat", iptablesData)
+	expectedNatRules = countRules(utiliptables.TableNAT, iptablesData)
 
 	if nNatRules != expectedNatRules {
 		t.Fatalf("Wrong number of nat rules: expected %d got %d\n%s", expectedNatRules, nNatRules, iptablesData)

pkg/util/iptables/testing/parse.go

@@ -20,11 +20,180 @@ import (
 	"fmt"
 	"reflect"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/util/iptables"
 )
 
+// IPTablesDump represents a parsed IPTables rules dump (ie, the output of
+// "iptables-save" or input to "iptables-restore")
+type IPTablesDump struct {
+	Tables []Table
+}
+
+// Table represents an IPTables table
+type Table struct {
+	Name   iptables.Table
+	Chains []Chain
+}
+
+// Chain represents an IPTables chain
+type Chain struct {
+	Name    iptables.Chain
+	Packets uint64
+	Bytes   uint64
+	Rules   []*Rule
+
+	// Deleted is set if the input contained a "-X Name" line; this would never
+	// appear in iptables-save output but it could appear in iptables-restore *input*.
+	Deleted bool
+}
+
+var declareTableRegex = regexp.MustCompile(`^\*(.*)$`)
+var declareChainRegex = regexp.MustCompile(`^:([^ ]*) - \[([0-9]*):([0-9]*)\]$`)
+var addRuleRegex = regexp.MustCompile(`^-A ([^ ]*) (.*)$`)
+var deleteChainRegex = regexp.MustCompile(`^-X (.*)$`)
+
+type parseState int
+
+const (
+	parseTableDeclaration parseState = iota
+	parseChainDeclarations
+	parseChains
+)
+
+// ParseIPTablesDump parses an IPTables rules dump. Note: this may ignore some bad data.
+func ParseIPTablesDump(data string) (*IPTablesDump, error) {
+	dump := &IPTablesDump{}
+	state := parseTableDeclaration
+	lines := strings.Split(strings.Trim(data, "\n"), "\n")
+	var t *Table
+
+	for _, line := range lines {
+	retry:
+		line = strings.TrimSpace(line)
+		if line == "" || line[0] == '#' {
+			continue
+		}
+
+		switch state {
+		case parseTableDeclaration:
+			// Parse table declaration line ("*filter").
+			match := declareTableRegex.FindStringSubmatch(line)
+			if match == nil {
+				return nil, fmt.Errorf("could not parse iptables data (table %d starts with %q not a table name)", len(dump.Tables)+1, line)
+			}
+			dump.Tables = append(dump.Tables, Table{Name: iptables.Table(match[1])})
+			t = &dump.Tables[len(dump.Tables)-1]
+			state = parseChainDeclarations
+
+		case parseChainDeclarations:
+			match := declareChainRegex.FindStringSubmatch(line)
+			if match == nil {
+				state = parseChains
+				goto retry
+			}
+
+			chain := iptables.Chain(match[1])
+			packets, _ := strconv.ParseUint(match[2], 10, 64)
+			bytes, _ := strconv.ParseUint(match[3], 10, 64)
+
+			t.Chains = append(t.Chains,
+				Chain{
+					Name:    chain,
+					Packets: packets,
+					Bytes:   bytes,
+				},
+			)
+
+		case parseChains:
+			if match := addRuleRegex.FindStringSubmatch(line); match != nil {
+				chain := iptables.Chain(match[1])
+
+				c, err := dump.GetChain(t.Name, chain)
+				if err != nil {
+					return nil, fmt.Errorf("error parsing rule %q: %v", line, err)
+				}
+				if c.Deleted {
+					return nil, fmt.Errorf("cannot add rules to deleted chain %q", chain)
+				}
+
+				rule, err := ParseRule(line, false)
+				if err != nil {
+					return nil, err
+				}
+				c.Rules = append(c.Rules, rule)
+			} else if match := deleteChainRegex.FindStringSubmatch(line); match != nil {
+				chain := iptables.Chain(match[1])
+
+				c, err := dump.GetChain(t.Name, chain)
+				if err != nil {
+					return nil, fmt.Errorf("error parsing rule %q: %v", line, err)
+				}
+				if len(c.Rules) != 0 {
+					return nil, fmt.Errorf("cannot delete chain %q after adding rules", chain)
+				}
+				c.Deleted = true
+			} else if line == "COMMIT" {
+				state = parseTableDeclaration
+			} else {
+				return nil, fmt.Errorf("error parsing rule %q", line)
+			}
+		}
+	}
+
+	if state != parseTableDeclaration {
+		return nil, fmt.Errorf("could not parse iptables data (no COMMIT line?)")
+	}
+
+	return dump, nil
+}
+
+func (dump *IPTablesDump) String() string {
+	buffer := &strings.Builder{}
+	for _, t := range dump.Tables {
+		fmt.Fprintf(buffer, "*%s\n", t.Name)
+		for _, c := range t.Chains {
+			fmt.Fprintf(buffer, ":%s - [%d:%d]\n", c.Name, c.Packets, c.Bytes)
+		}
+		for _, c := range t.Chains {
+			for _, r := range c.Rules {
+				fmt.Fprintf(buffer, "%s\n", r.Raw)
+			}
+		}
+		for _, c := range t.Chains {
+			if c.Deleted {
+				fmt.Fprintf(buffer, "-X %s\n", c.Name)
+			}
+		}
+		fmt.Fprintf(buffer, "COMMIT\n")
+	}
+	return buffer.String()
+}
+
+func (dump *IPTablesDump) GetTable(table iptables.Table) (*Table, error) {
+	for i := range dump.Tables {
+		if dump.Tables[i].Name == table {
+			return &dump.Tables[i], nil
+		}
+	}
+	return nil, fmt.Errorf("no such table %q", table)
+}
+
+func (dump *IPTablesDump) GetChain(table iptables.Table, chain iptables.Chain) (*Chain, error) {
+	t, err := dump.GetTable(table)
+	if err != nil {
+		return nil, err
+	}
+	for i := range t.Chains {
+		if t.Chains[i].Name == chain {
+			return &t.Chains[i], nil
+		}
+	}
+	return nil, fmt.Errorf("no such chain %q", chain)
+}
+
 // Rule represents a single parsed IPTables rule. (This currently covers all of the rule
 // types that we actually use in pkg/proxy/iptables or pkg/proxy/ipvs.)
 //

pkg/util/iptables/testing/parse_test.go

@@ -17,10 +17,13 @@ limitations under the License.
 package testing
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
 
+	"github.com/lithammer/dedent"
+
 	"k8s.io/kubernetes/pkg/util/iptables"
 	utilpointer "k8s.io/utils/pointer"
 )
@@ -219,3 +222,374 @@ func TestParseRule(t *testing.T) {
 		})
 	}
 }
+
+// Helper for TestParseIPTablesDump. Obviously it should not be used in TestParseRule...
+func mustParseRule(rule string) *Rule {
+	parsed, err := ParseRule(rule, false)
+	if err != nil {
+		panic(fmt.Sprintf("failed to parse test case rule %q: %v", rule, err))
+	}
+	return parsed
+}
+
+func TestParseIPTablesDump(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		input  string
+		output *IPTablesDump
+		error  string
+	}{
+		{
+			name: "basic test",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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-SERVICES - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				:KUBE-POSTROUTING - [0:0]
+				:KUBE-MARK-MASQ - [0:0]
+				:KUBE-SVC-XPGD46QRK7WJZT7O - [0:0]
+				:KUBE-SEP-SXIVWICOYRO3J4NJ - [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
+				-A KUBE-MARK-MASQ -j MARK --or-mark 0x4000
+				-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 ! -s 10.0.0.0/24 -j KUBE-MARK-MASQ
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80
+				-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
+				COMMIT
+				`),
+			output: &IPTablesDump{
+				Tables: []Table{{
+					Name: iptables.TableFilter,
+					Chains: []Chain{{
+						Name: iptables.Chain("KUBE-SERVICES"),
+					}, {
+						Name: iptables.Chain("KUBE-EXTERNAL-SERVICES"),
+					}, {
+						Name: iptables.Chain("KUBE-FORWARD"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP`),
+							mustParseRule(`-A KUBE-FORWARD -m comment --comment "kubernetes forwarding rules" -m mark --mark 0x4000/0x4000 -j ACCEPT`),
+							mustParseRule(`-A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-NODEPORTS"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`),
+						},
+					}},
+				}, {
+					Name: iptables.TableNAT,
+					Chains: []Chain{{
+						Name: iptables.Chain("KUBE-SERVICES"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O`),
+							mustParseRule(`-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`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-NODEPORTS"),
+					}, {
+						Name: iptables.Chain("KUBE-POSTROUTING"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-POSTROUTING -m mark ! --mark 0x4000/0x4000 -j RETURN`),
+							mustParseRule(`-A KUBE-POSTROUTING -j MARK --xor-mark 0x4000`),
+							mustParseRule(`-A KUBE-POSTROUTING -m comment --comment "kubernetes service traffic requiring SNAT" -j MASQUERADE`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-MARK-MASQ"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-MARK-MASQ -j MARK --or-mark 0x4000`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-SVC-XPGD46QRK7WJZT7O"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 ! -s 10.0.0.0/24 -j KUBE-MARK-MASQ`),
+							mustParseRule(`-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-SEP-SXIVWICOYRO3J4NJ"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ`),
+							mustParseRule(`-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80`),
+						},
+					}},
+				}},
+			},
+		},
+		{
+			name: "deletion",
+			input: dedent.Dedent(`
+				*nat
+				:KUBE-SERVICES - [0:0]
+				:KUBE-SVC-XPGD46QRK7WJZT7O - [0:0]
+				:KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0]
+				-X KUBE-SVC-XPGD46QRK7WJZT7O
+				-X KUBE-SEP-SXIVWICOYRO3J4NJ
+				-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
+				COMMIT
+				`),
+			output: &IPTablesDump{
+				Tables: []Table{{
+					Name: iptables.TableNAT,
+					Chains: []Chain{{
+						Name: iptables.Chain("KUBE-SERVICES"),
+						Rules: []*Rule{
+							mustParseRule(`-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`),
+						},
+					}, {
+						Name:    iptables.Chain("KUBE-SVC-XPGD46QRK7WJZT7O"),
+						Deleted: true,
+					}, {
+						Name:    iptables.Chain("KUBE-SEP-SXIVWICOYRO3J4NJ"),
+						Deleted: true,
+					}},
+				}},
+			},
+		},
+		{
+			name: "whitespace and comments",
+			input: dedent.Dedent(`
+				# Generated by iptables-save v1.8.7 on Mon May  9 11:22:21 2022
+				# (not really...)
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				  -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
+				# This rule does a thing
+				-A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
+				COMMIT
+				# Completed on Mon May  9 11:22:21 2022
+				`),
+			output: &IPTablesDump{
+				Tables: []Table{{
+					Name: iptables.TableFilter,
+					Chains: []Chain{{
+						Name: iptables.Chain("KUBE-SERVICES"),
+					}, {
+						Name: iptables.Chain("KUBE-EXTERNAL-SERVICES"),
+					}, {
+						Name: iptables.Chain("KUBE-FORWARD"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP`),
+							mustParseRule(`-A KUBE-FORWARD -m comment --comment "kubernetes forwarding rules" -m mark --mark 0x4000/0x4000 -j ACCEPT`),
+							mustParseRule(`-A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT`),
+						},
+					}, {
+						Name: iptables.Chain("KUBE-NODEPORTS"),
+						Rules: []*Rule{
+							mustParseRule(`-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`),
+						},
+					}},
+				}},
+			},
+		},
+		{
+			name: "no COMMIT line",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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
+				`),
+			error: "no COMMIT line?",
+		},
+		{
+			name: "two tables, no second COMMIT line",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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-SERVICES - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				:KUBE-POSTROUTING - [0:0]
+				:KUBE-MARK-MASQ - [0:0]
+				:KUBE-SVC-XPGD46QRK7WJZT7O - [0:0]
+				:KUBE-SEP-SXIVWICOYRO3J4NJ - [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
+				-A KUBE-MARK-MASQ -j MARK --or-mark 0x4000
+				-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 ! -s 10.0.0.0/24 -j KUBE-MARK-MASQ
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80
+				-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
+				`),
+			error: "no COMMIT line?",
+		},
+		{
+			name: "two tables, no second header line",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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
+				:KUBE-SERVICES - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				:KUBE-POSTROUTING - [0:0]
+				:KUBE-MARK-MASQ - [0:0]
+				:KUBE-SVC-XPGD46QRK7WJZT7O - [0:0]
+				:KUBE-SEP-SXIVWICOYRO3J4NJ - [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
+				-A KUBE-MARK-MASQ -j MARK --or-mark 0x4000
+				-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 10.20.30.41 --dport 80 ! -s 10.0.0.0/24 -j KUBE-MARK-MASQ
+				-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ
+				-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80
+				-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
+				COMMIT
+				`),
+			error: "not a table name",
+		},
+		{
+			name: "trailing junk",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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
+				junk
+				`),
+			error: `table 3 starts with "junk"`,
+		},
+		{
+			name: "add to missing chain",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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
+				`),
+			error: `no such chain "KUBE-FORWARD"`,
+		},
+		{
+			name: "add to deleted chain",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-X KUBE-FORWARD
+				-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
+				`),
+			error: `cannot add rules to deleted chain`,
+		},
+		{
+			name: "deleted non-empty chain",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-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
+				-X KUBE-FORWARD
+				COMMIT
+				`),
+			error: `cannot delete chain "KUBE-FORWARD" after adding rules`,
+		},
+		{
+			name: "junk rule",
+			input: dedent.Dedent(`
+				*filter
+				:KUBE-SERVICES - [0:0]
+				:KUBE-EXTERNAL-SERVICES - [0:0]
+				:KUBE-FORWARD - [0:0]
+				:KUBE-NODEPORTS - [0:0]
+				-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT
+				-Q 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
+				`),
+			error: `"-Q KUBE-FORWARD`,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			dump, err := ParseIPTablesDump(tc.input)
+			if err == nil {
+				if tc.error != "" {
+					t.Errorf("unexpectedly did not get error")
+				} else if !reflect.DeepEqual(tc.output, dump) {
+					t.Errorf("bad output: expected %#v got %#v", tc.output, dump)
+				}
+			} else {
+				if tc.error == "" {
+					t.Errorf("got unexpected error: %v", err)
+				} else if !strings.Contains(err.Error(), tc.error) {
+					t.Errorf("got wrong error: %v (expected %q)", err, tc.error)
+				}
+			}
+		})
+	}
+}