diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index d3df81ca430..d0fa9bc7c90 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -675,18 +675,22 @@ func TestParseIPTablesData(t *testing.T) { } } -func countRules(tableName string, ruleData string) int { - tables, err := parseIPTablesData(ruleData) +func countRules(tableName utiliptables.Table, ruleData string) int { + dump, err := iptablestest.ParseIPTablesDump(ruleData) if err != nil { klog.ErrorS(err, "error parsing iptables rules") return -1 } rules := 0 - for _, line := range tables[tableName] { - if line[0] == '-' { - rules++ - } + table, err := dump.GetTable(tableName) + if err != nil { + 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)) - for tableName := range tables { - tableNames = append(tableNames, tableName) - } - sort.Strings(tableNames) + // Sort tables + sort.Slice(dump.Tables, func(i, j int) bool { + return dump.Tables[i].Name < dump.Tables[j].Name + }) - var output []string - for _, name := range tableNames { - lines := tables[name] - - // Move "*TABLENAME" line - lines, output = moveMatchingLines(`^\*`, lines, output) - - // findAllMatches() returns a sorted list of unique matches. So for - // each of the following, we find all the matches for the regex, then - // for each unique match (in sorted order), move all of the lines that - // contain that match. - - // Move and sort ":CHAINNAME" lines (in the same order we will sort - // the chains themselves below). - lines, output = moveMatchingLines(`:KUBE-NODEPORTS`, lines, output) - lines, output = moveMatchingLines(`:KUBE-SERVICES`, lines, output) - for _, chainName := range findAllMatches(lines, `^(:KUBE-[^ ]*) `) { - lines, output = moveMatchingLines(chainName, lines, output) - } - for _, chainName := range findAllMatches(lines, `^(:[^ ]*) `) { - lines, output = moveMatchingLines(chainName, lines, output) - } - - // Move KUBE-NODEPORTS rules for each service, sorted by service name - for _, nextNodePortService := range findAllMatches(lines, `-A KUBE-NODEPORTS.*--comment "?([^ ]*)`) { - lines, output = moveMatchingLines(fmt.Sprintf(`^-A KUBE-NODEPORTS.*%s`, nextNodePortService), lines, output) - } - - // 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) + // Sort chains + for t := range dump.Tables { + table := &dump.Tables[t] + sort.Slice(table.Chains, func(i, j int) bool { + switch { + case table.Chains[i].Name == kubeNodePortsChain: + // KUBE-NODEPORTS comes before anything + return true + case table.Chains[j].Name == kubeNodePortsChain: + // anything goes after KUBE-NODEPORTS + return false + case table.Chains[i].Name == kubeServicesChain: + // KUBE-SERVICES comes before anything (except KUBE-NODEPORTS) + return true + case table.Chains[j].Name == kubeServicesChain: + // anything (except KUBE-NODEPORTS) goes after KUBE-SERVICES + return false + case strings.HasPrefix(string(table.Chains[i].Name), "KUBE-") && !strings.HasPrefix(string(table.Chains[j].Name), "KUBE-"): + // KUBE-* comes before non-KUBE-* + return true + case !strings.HasPrefix(string(table.Chains[i].Name), "KUBE-") && strings.HasPrefix(string(table.Chains[j].Name), "KUBE-"): + // non-KUBE-* goes after KUBE-* + return false + default: + // We have two KUBE-* chains or two non-KUBE-* chains; either + // way they sort alphabetically + return table.Chains[i].Name < table.Chains[j].Name + } + }) } - // Input ended with a "\n", so make sure the output does too - output = append(output, "") + // Sort KUBE-NODEPORTS chains by service name + 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) diff --git a/pkg/util/iptables/testing/parse.go b/pkg/util/iptables/testing/parse.go index 9d7cb86b627..536cf3724ed 100644 --- a/pkg/util/iptables/testing/parse.go +++ b/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.) // diff --git a/pkg/util/iptables/testing/parse_test.go b/pkg/util/iptables/testing/parse_test.go index 732146d2dfc..c439ef61fcd 100644 --- a/pkg/util/iptables/testing/parse_test.go +++ b/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) + } + } + }) + } +}