From f2fa1033d00b5cb3fecf3942b6c12764045fae51 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 8 Apr 2022 16:28:59 -0400 Subject: [PATCH 1/5] pkg/util/iptables/testing: Add better IPTables rule-parsing helpers There were previously some strange iptables-rule-parsing functions that were only used by two unit tests in pkg/proxy/ipvs. Get rid of them and replace them with some much better iptables-rule-parsing functions. --- pkg/proxy/ipvs/proxier_test.go | 65 ++++--- pkg/util/iptables/testing/fake.go | 57 ------ pkg/util/iptables/testing/parse.go | 206 ++++++++++++++++++++++ pkg/util/iptables/testing/parse_test.go | 221 ++++++++++++++++++++++++ 4 files changed, 468 insertions(+), 81 deletions(-) create mode 100644 pkg/util/iptables/testing/parse.go create mode 100644 pkg/util/iptables/testing/parse_test.go diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index 79315d0dd8e..d084c76506c 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -1664,11 +1664,25 @@ func TestMasqueradeRule(t *testing.T) { makeServiceMap(fp) fp.syncProxyRules() - postRoutingRules := ipt.GetRules(string(kubePostroutingChain)) - if !hasJump(postRoutingRules, "MASQUERADE", "") { + buf := bytes.NewBuffer(nil) + _ = ipt.SaveInto(utiliptables.TableNAT, buf) + natRules := strings.Split(string(buf.Bytes()), "\n") + var hasMasqueradeJump, hasMasqRandomFully bool + for _, line := range natRules { + rule, _ := iptablestest.ParseRule(line, false) + if rule != nil && rule.Chain == kubePostroutingChain && rule.Jump != nil && rule.Jump.Value == "MASQUERADE" { + hasMasqueradeJump = true + if rule.RandomFully != nil { + hasMasqRandomFully = true + } + break + } + } + + if !hasMasqueradeJump { t.Errorf("Failed to find -j MASQUERADE in %s chain", kubePostroutingChain) } - if hasMasqRandomFully(postRoutingRules) != testcase { + if hasMasqRandomFully != testcase { probs := map[bool]string{false: "found", true: "did not find"} t.Errorf("%s --random-fully in -j MASQUERADE rule in %s chain for HasRandomFully()=%v", probs[testcase], kubePostroutingChain, testcase) } @@ -3817,42 +3831,45 @@ func buildFakeProxier() (*iptablestest.FakeIPTables, *Proxier) { return ipt, NewFakeProxier(ipt, ipvs, ipset, nil, nil, v1.IPv4Protocol) } -func hasJump(rules []iptablestest.Rule, destChain, ipSet string) bool { - for _, r := range rules { - if r[iptablestest.Jump] == destChain { - if ipSet == "" { - return true - } - if strings.Contains(r[iptablestest.MatchSet], ipSet) { - return true - } - } - } - return false -} +func getRules(ipt *iptablestest.FakeIPTables, chain utiliptables.Chain) []*iptablestest.Rule { + var rules []*iptablestest.Rule -func hasMasqRandomFully(rules []iptablestest.Rule) bool { - for _, r := range rules { - if r[iptablestest.Masquerade] == "--random-fully" { - return true + buf := bytes.NewBuffer(nil) + // FIXME: FakeIPTables.SaveInto is currently broken and ignores the "table" + // argument and just echoes whatever was last passed to RestoreAll(), so even + // though we want to see the rules from both "nat" and "filter", we have to + // only request one of them, or else we'll get all the rules twice... + _ = ipt.SaveInto(utiliptables.TableNAT, buf) + // _ = ipt.SaveInto(utiliptable.TableFilter, buf) + lines := strings.Split(string(buf.Bytes()), "\n") + for _, l := range lines { + if !strings.HasPrefix(l, "-A ") { + continue + } + rule, _ := iptablestest.ParseRule(l, false) + if rule != nil && rule.Chain == chain { + rules = append(rules, rule) } } - return false + return rules } // checkIptables to check expected iptables chain and rules. The got rules must have same number and order as the // expected rules. func checkIptables(t *testing.T, ipt *iptablestest.FakeIPTables, epIpt netlinktest.ExpectedIptablesChain) { for epChain, epRules := range epIpt { - rules := ipt.GetRules(epChain) + rules := getRules(ipt, utiliptables.Chain(epChain)) if len(rules) != len(epRules) { t.Errorf("Expected %d iptables rule in chain %s, got %d", len(epRules), epChain, len(rules)) continue } for i, epRule := range epRules { rule := rules[i] - if rule[iptablestest.Jump] != epRule.JumpChain || !strings.Contains(rule[iptablestest.MatchSet], epRule.MatchSet) { - t.Errorf("Expected MatchSet=%s JumpChain=%s, got MatchSet=%s JumpChain=%s", epRule.MatchSet, epRule.JumpChain, rule[iptablestest.MatchSet], rule[iptablestest.Jump]) + if rule.Jump == nil || rule.Jump.Value != epRule.JumpChain { + t.Errorf("Expected MatchSet=%s JumpChain=%s, got %s", epRule.MatchSet, epRule.JumpChain, rule.Raw) + } + if (epRule.MatchSet == "" && rule.MatchSet != nil) || (epRule.MatchSet != "" && (rule.MatchSet == nil || rule.MatchSet.Value != epRule.MatchSet)) { + t.Errorf("Expected MatchSet=%s JumpChain=%s, got %s", epRule.MatchSet, epRule.JumpChain, rule.Raw) } } } diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index 55e642238fa..e832037b536 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -18,43 +18,11 @@ package testing import ( "bytes" - "fmt" - "strings" "time" "k8s.io/kubernetes/pkg/util/iptables" ) -const ( - // Destination represents the destination address flag - Destination = "-d " - // Source represents the source address flag - Source = "-s " - // DPort represents the destination port flag - DPort = "--dport " - // Protocol represents the protocol flag - Protocol = "-p " - // Jump represents jump flag specifies the jump target - Jump = "-j " - // Reject specifies the reject target - Reject = "REJECT" - // Accept specifies the accept target - Accept = "ACCEPT" - // ToDest represents the flag used to specify the destination address in DNAT - ToDest = "--to-destination " - // Recent represents the sub-command recent that allows to dynamically create list of IP address to match against - Recent = "recent " - // MatchSet represents the flag which match packets against the specified set - MatchSet = "--match-set " - // SrcType represents the --src-type flag which matches if the source address is of given type - SrcType = "--src-type " - // Masquerade represents the target that is used in nat table. - Masquerade = "MASQUERADE " -) - -// Rule holds a map of rules. -type Rule map[string]string - // FakeIPTables is no-op implementation of iptables Interface. type FakeIPTables struct { hasRandomFully bool @@ -146,31 +114,6 @@ func (f *FakeIPTables) RestoreAll(data []byte, flush iptables.FlushFlag, counter func (f *FakeIPTables) Monitor(canary iptables.Chain, tables []iptables.Table, reloadFunc func(), interval time.Duration, stopCh <-chan struct{}) { } -func getToken(line, separator string) string { - tokens := strings.Split(line, separator) - if len(tokens) == 2 { - return strings.Split(tokens[1], " ")[0] - } - return "" -} - -// GetRules is part of iptables.Interface -func (f *FakeIPTables) GetRules(chainName string) (rules []Rule) { - for _, l := range strings.Split(string(f.Lines), "\n") { - if strings.Contains(l, fmt.Sprintf("-A %v", chainName)) { - newRule := Rule(map[string]string{}) - for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest, Recent, MatchSet, SrcType, Masquerade} { - tok := getToken(l, arg) - if tok != "" { - newRule[arg] = tok - } - } - rules = append(rules, newRule) - } - } - return -} - // HasRandomFully is part of iptables.Interface func (f *FakeIPTables) HasRandomFully() bool { return f.hasRandomFully diff --git a/pkg/util/iptables/testing/parse.go b/pkg/util/iptables/testing/parse.go new file mode 100644 index 00000000000..9d7cb86b627 --- /dev/null +++ b/pkg/util/iptables/testing/parse.go @@ -0,0 +1,206 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "fmt" + "reflect" + "regexp" + "strings" + + "k8s.io/kubernetes/pkg/util/iptables" +) + +// 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.) +// +// The parsing is mostly-automated based on type reflection. The `param` tag on a field +// indicates the parameter whose value will be placed into that field. (The code assumes +// that we don't use both the short and long forms of any parameter names (eg, "-s" vs +// "--source"), which is currently true, but it could be extended if necessary.) The +// `negatable` tag indicates if a parameter is allowed to be preceded by "!". +// +// Parameters that take a value are stored as type `*IPTablesValue`, which encapsulates a +// string value and whether the rule was negated (ie, whether the rule requires that we +// *match* or *don't match* that value). But string-valued parameters that can't be +// negated use `IPTablesValue` rather than `string` too, just for API consistency. +// +// Parameters that don't take a value are stored as `*bool`, where the value is `nil` if +// the parameter was not present, `&true` if the parameter was present, or `&false` if the +// parameter was present but negated. +// +// Parsing skips over "-m MODULE" parameters because most parameters have unique names +// anyway even ignoring the module name, and in the cases where they don't (eg "-m tcp +// --sport" vs "-m udp --sport") the parameters are mutually-exclusive and it's more +// convenient to store them in the same struct field anyway. +type Rule struct { + // Raw contains the original raw rule string + Raw string + + Chain iptables.Chain `param:"-A"` + Comment *IPTablesValue `param:"--comment"` + + Protocol *IPTablesValue `param:"-p" negatable:"true"` + + SourceAddress *IPTablesValue `param:"-s" negatable:"true"` + SourceType *IPTablesValue `param:"--src-type" negatable:"true"` + SourcePort *IPTablesValue `param:"--sport" negatable:"true"` + + DestinationAddress *IPTablesValue `param:"-d" negatable:"true"` + DestinationType *IPTablesValue `param:"--dst-type" negatable:"true"` + DestinationPort *IPTablesValue `param:"--dport" negatable:"true"` + + MatchSet *IPTablesValue `param:"--match-set" negatable:"true"` + + Jump *IPTablesValue `param:"-j"` + RandomFully *bool `param:"--random-fully"` + Probability *IPTablesValue `param:"--probability"` + DNATDestination *IPTablesValue `param:"--to-destination"` + + // We don't actually use the values of these, but we care if they are present + AffinityCheck *bool `param:"--rcheck" negatable:"true"` + MarkCheck *IPTablesValue `param:"--mark" negatable:"true"` + CTStateCheck *IPTablesValue `param:"--ctstate" negatable:"true"` + + // We don't currently care about any of these in the unit tests, but we expect + // them to be present in some rules that we parse, so we define how to parse them. + AffinityName *IPTablesValue `param:"--name"` + AffinitySeconds *IPTablesValue `param:"--seconds"` + AffinitySet *bool `param:"--set" negatable:"true"` + AffinityReap *bool `param:"--reap"` + StatisticMode *IPTablesValue `param:"--mode"` +} + +// IPTablesValue is a value of a parameter in an Rule, where the parameter is +// possibly negated. +type IPTablesValue struct { + Negated bool + Value string +} + +// for debugging; otherwise %v will just print the pointer value +func (v *IPTablesValue) String() string { + if v.Negated { + return fmt.Sprintf("NOT %q", v.Value) + } else { + return fmt.Sprintf("%q", v.Value) + } +} + +// Matches returns true if cmp equals / doesn't equal v.Value (depending on +// v.Negated). +func (v *IPTablesValue) Matches(cmp string) bool { + if v.Negated { + return v.Value != cmp + } else { + return v.Value == cmp + } +} + +// findParamField finds a field in value with the struct tag "param:${param}" and if found, +// returns a pointer to the Value of that field, and the value of its "negatable" tag. +func findParamField(value reflect.Value, param string) (*reflect.Value, bool) { + typ := value.Type() + for i := 0; i < typ.NumField(); i++ { + field := typ.Field(i) + if field.Tag.Get("param") == param { + fValue := value.Field(i) + return &fValue, field.Tag.Get("negatable") == "true" + } + } + return nil, false +} + +// wordRegex matches a single word or a quoted string (at the start of the string, or +// preceded by whitespace) +var wordRegex = regexp.MustCompile(`(?:^|\s)("[^"]*"|[^"]\S*)`) + +// Used by ParseRule +var boolPtrType = reflect.PtrTo(reflect.TypeOf(true)) +var ipTablesValuePtrType = reflect.TypeOf((*IPTablesValue)(nil)) + +// ParseRule parses rule. If strict is false, it will parse the recognized +// parameters and ignore unrecognized ones. If it is true, parsing will fail if there are +// unrecognized parameters. +func ParseRule(rule string, strict bool) (*Rule, error) { + parsed := &Rule{Raw: rule} + + // Split rule into "words" (where a quoted string is a single word). + var words []string + for _, match := range wordRegex.FindAllStringSubmatch(rule, -1) { + words = append(words, strings.Trim(match[1], `"`)) + } + + // The chain name must come first (and can't be the only thing there) + if len(words) < 2 || words[0] != "-A" { + return nil, fmt.Errorf(`bad iptables rule (does not start with "-A CHAIN")`) + } else if len(words) < 3 { + return nil, fmt.Errorf("bad iptables rule (no match rules)") + } + + // For each word, see if it is a known iptables parameter, based on the struct + // field tags in Rule. Note that in the non-strict case we implicitly assume that + // no unrecognized parameter will take an argument that could be mistaken for + // another parameter. + v := reflect.ValueOf(parsed).Elem() + negated := false + for w := 0; w < len(words); { + if words[w] == "-m" && w < len(words)-1 { + // Skip "-m MODULE"; we don't pay attention to that since the + // parameter names are unique enough anyway. + w += 2 + continue + } + + if words[w] == "!" { + negated = true + w++ + continue + } + + // For everything else, see if it corresponds to a field of Rule + if field, negatable := findParamField(v, words[w]); field != nil { + if negated && !negatable { + return nil, fmt.Errorf("cannot negate parameter %q", words[w]) + } + if field.Type() != boolPtrType && w == len(words)-1 { + return nil, fmt.Errorf("parameter %q requires an argument", words[w]) + } + switch field.Type() { + case boolPtrType: + boolVal := !negated + field.Set(reflect.ValueOf(&boolVal)) + w++ + case ipTablesValuePtrType: + field.Set(reflect.ValueOf(&IPTablesValue{Negated: negated, Value: words[w+1]})) + w += 2 + default: + field.SetString(words[w+1]) + w += 2 + } + } else if strict { + return nil, fmt.Errorf("unrecognized parameter %q", words[w]) + } else { + // skip + w++ + } + + negated = false + } + + return parsed, nil +} diff --git a/pkg/util/iptables/testing/parse_test.go b/pkg/util/iptables/testing/parse_test.go new file mode 100644 index 00000000000..732146d2dfc --- /dev/null +++ b/pkg/util/iptables/testing/parse_test.go @@ -0,0 +1,221 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "reflect" + "strings" + "testing" + + "k8s.io/kubernetes/pkg/util/iptables" + utilpointer "k8s.io/utils/pointer" +) + +func TestParseRule(t *testing.T) { + testCases := []struct { + name string + rule string + parsed *Rule + nonStrict bool + err string + }{ + { + name: "basic rule", + rule: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, + parsed: &Rule{ + Raw: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, + Chain: iptables.Chain("KUBE-NODEPORTS"), + Comment: &IPTablesValue{Value: "ns2/svc2:p80 health check node port"}, + Protocol: &IPTablesValue{Value: "tcp"}, + DestinationPort: &IPTablesValue{Value: "30000"}, + Jump: &IPTablesValue{Value: "ACCEPT"}, + }, + }, + { + name: "addRuleToChainRegex requires an actual rule, not just a chain name", + rule: `-A KUBE-NODEPORTS`, + err: `(no match rules)`, + }, + { + name: "ParseRule only parses adds", + rule: `-D KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, + err: `(does not start with "-A CHAIN")`, + }, + { + name: "unquoted comment", + rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + parsed: &Rule{ + Raw: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + Chain: iptables.Chain("KUBE-SVC-XPGD46QRK7WJZT7O"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80"}, + Jump: &IPTablesValue{Value: "KUBE-SEP-SXIVWICOYRO3J4NJ"}, + }, + }, + { + name: "local source", + rule: `-A KUBE-XLB-GNZBNJ2PO5MGZ6GT -m comment --comment "masquerade LOCAL traffic for ns2/svc2:p80 LB IP" -m addrtype --src-type LOCAL -j KUBE-MARK-MASQ`, + parsed: &Rule{ + Raw: `-A KUBE-XLB-GNZBNJ2PO5MGZ6GT -m comment --comment "masquerade LOCAL traffic for ns2/svc2:p80 LB IP" -m addrtype --src-type LOCAL -j KUBE-MARK-MASQ`, + Chain: iptables.Chain("KUBE-XLB-GNZBNJ2PO5MGZ6GT"), + Comment: &IPTablesValue{Value: "masquerade LOCAL traffic for ns2/svc2:p80 LB IP"}, + SourceType: &IPTablesValue{Value: "LOCAL"}, + Jump: &IPTablesValue{Value: "KUBE-MARK-MASQ"}, + }, + }, + { + name: "not local destination", + rule: `-A RULE-TYPE-NOT-CURRENTLY-USED-BY-KUBE-PROXY -m addrtype ! --dst-type LOCAL -j KUBE-MARK-MASQ`, + parsed: &Rule{ + Raw: `-A RULE-TYPE-NOT-CURRENTLY-USED-BY-KUBE-PROXY -m addrtype ! --dst-type LOCAL -j KUBE-MARK-MASQ`, + Chain: iptables.Chain("RULE-TYPE-NOT-CURRENTLY-USED-BY-KUBE-PROXY"), + DestinationType: &IPTablesValue{Negated: true, Value: "LOCAL"}, + Jump: &IPTablesValue{Value: "KUBE-MARK-MASQ"}, + }, + }, + { + name: "destination IP/port", + rule: `-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O`, + parsed: &Rule{ + Raw: `-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O`, + Chain: iptables.Chain("KUBE-SERVICES"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80 cluster IP"}, + Protocol: &IPTablesValue{Value: "tcp"}, + DestinationAddress: &IPTablesValue{Value: "172.30.0.41"}, + DestinationPort: &IPTablesValue{Value: "80"}, + Jump: &IPTablesValue{Value: "KUBE-SVC-XPGD46QRK7WJZT7O"}, + }, + }, + { + name: "source IP", + rule: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ`, + parsed: &Rule{ + Raw: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ`, + Chain: iptables.Chain("KUBE-SEP-SXIVWICOYRO3J4NJ"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80"}, + SourceAddress: &IPTablesValue{Value: "10.180.0.1"}, + Jump: &IPTablesValue{Value: "KUBE-MARK-MASQ"}, + }, + }, + { + name: "not source IP", + rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 ! -s 10.0.0.0/8 -j KUBE-MARK-MASQ`, + parsed: &Rule{ + Raw: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 ! -s 10.0.0.0/8 -j KUBE-MARK-MASQ`, + Chain: iptables.Chain("KUBE-SVC-XPGD46QRK7WJZT7O"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80 cluster IP"}, + Protocol: &IPTablesValue{Value: "tcp"}, + DestinationAddress: &IPTablesValue{Value: "172.30.0.41"}, + DestinationPort: &IPTablesValue{Value: "80"}, + SourceAddress: &IPTablesValue{Negated: true, Value: "10.0.0.0/8"}, + Jump: &IPTablesValue{Value: "KUBE-MARK-MASQ"}, + }, + }, + { + name: "affinity", + rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -m recent --name KUBE-SEP-SXIVWICOYRO3J4NJ --rcheck --seconds 10800 --reap -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + parsed: &Rule{ + Raw: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -m recent --name KUBE-SEP-SXIVWICOYRO3J4NJ --rcheck --seconds 10800 --reap -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + Chain: iptables.Chain("KUBE-SVC-XPGD46QRK7WJZT7O"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80"}, + AffinityName: &IPTablesValue{Value: "KUBE-SEP-SXIVWICOYRO3J4NJ"}, + AffinitySeconds: &IPTablesValue{Value: "10800"}, + AffinityCheck: utilpointer.Bool(true), + AffinityReap: utilpointer.Bool(true), + Jump: &IPTablesValue{Value: "KUBE-SEP-SXIVWICOYRO3J4NJ"}, + }, + }, + { + name: "jump to DNAT", + rule: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80`, + parsed: &Rule{ + Raw: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80`, + Chain: iptables.Chain("KUBE-SEP-SXIVWICOYRO3J4NJ"), + Comment: &IPTablesValue{Value: "ns1/svc1:p80"}, + Protocol: &IPTablesValue{Value: "tcp"}, + Jump: &IPTablesValue{Value: "DNAT"}, + DNATDestination: &IPTablesValue{Value: "10.180.0.1:80"}, + }, + }, + { + name: "jump to endpoint", + rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -m statistic --mode random --probability 0.5000000000 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + parsed: &Rule{ + Raw: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -m statistic --mode random --probability 0.5000000000 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + Chain: iptables.Chain("KUBE-SVC-4SW47YFZTEDKD3PK"), + Comment: &IPTablesValue{Value: "ns4/svc4:p80"}, + Probability: &IPTablesValue{Value: "0.5000000000"}, + StatisticMode: &IPTablesValue{Value: "random"}, + Jump: &IPTablesValue{Value: "KUBE-SEP-UKSFD7AGPMPPLUHC"}, + }, + }, + { + name: "unrecognized arguments", + rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -i eth0 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + err: `unrecognized parameter "-i"`, + }, + { + name: "unrecognized arguments with strict=false", + rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -i eth0 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + nonStrict: true, + parsed: &Rule{ + Raw: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -i eth0 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + Chain: iptables.Chain("KUBE-SVC-4SW47YFZTEDKD3PK"), + Comment: &IPTablesValue{Value: "ns4/svc4:p80"}, + Jump: &IPTablesValue{Value: "KUBE-SEP-UKSFD7AGPMPPLUHC"}, + }, + }, + { + name: "bad use of !", + rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 ! -j KUBE-SEP-UKSFD7AGPMPPLUHC`, + err: `cannot negate parameter "-j"`, + }, + { + name: "missing argument", + rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -j`, + err: `parameter "-j" requires an argument`, + }, + { + name: "negated bool arg", + rule: `-A TEST -m recent ! --rcheck -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + parsed: &Rule{ + Raw: `-A TEST -m recent ! --rcheck -j KUBE-SEP-SXIVWICOYRO3J4NJ`, + Chain: iptables.Chain("TEST"), + AffinityCheck: utilpointer.Bool(false), + Jump: &IPTablesValue{Value: "KUBE-SEP-SXIVWICOYRO3J4NJ"}, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + rule, err := ParseRule(testCase.rule, !testCase.nonStrict) + if err != nil { + if testCase.err == "" { + t.Errorf("expected %+v, got error %q", testCase.parsed, err) + } else if !strings.Contains(err.Error(), testCase.err) { + t.Errorf("wrong error, expected %q got %q", testCase.err, err) + } + } else { + if testCase.err != "" { + t.Errorf("expected error %q, got %+v", testCase.err, rule) + } else if !reflect.DeepEqual(rule, testCase.parsed) { + t.Errorf("bad match: expected\n%+v\ngot\n%+v", testCase.parsed, rule) + } + } + }) + } +} From f0f47ae590a83105871366d47792d3db99ef45d2 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 7 Apr 2022 08:31:27 -0400 Subject: [PATCH 2/5] proxy/iptables: tweak sortIPTablesRules some more Sort the ":CHAINNAME" lines in the same order as the "-A CHAINNAME" lines (meaning, KUBE-NODEPORTS and KUBE-SERVICES come first). (This will simplify IPTablesDump because it won't need to keep track of the declaration order and the rule order separately.) --- pkg/proxy/iptables/proxier_test.go | 278 +++++++++++++++-------------- 1 file changed, 142 insertions(+), 136 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 8dac0068467..d3df81ca430 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -936,7 +936,13 @@ func sortIPTablesRules(ruleData string) (string, error) { // for each unique match (in sorted order), move all of the lines that // contain that match. - // Move and sort ":CHAINNAME" lines + // 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) } @@ -1059,27 +1065,27 @@ func TestSortIPTablesRules(t *testing.T) { `), output: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-FW-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-C6EBXVWJJZMIWKLZ - [0:0] :KUBE-SEP-OYPFS5VJICHGATKP - [0:0] :KUBE-SEP-RS4RBKLTHTF2IUXJ - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-UKSFD7AGPMPPLUHC - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-4SW47YFZTEDKD3PK - [0:0] :KUBE-SVC-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-SVC-X27LE4BHSL4DOUIK - [0:0] @@ -1182,30 +1188,30 @@ func TestSortIPTablesRules(t *testing.T) { `), output: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [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 *mangle - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [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-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [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 @@ -1278,9 +1284,9 @@ func TestSortIPTablesRules(t *testing.T) { *filter COMMIT *nat + :KUBE-SERVICES - [0:0] :KUBE-AAAAA - [0:0] :KUBE-SEP-RS4RBKLTHTF2IUXJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-ZZZZZ - [0:0] :WHY-IS-THIS-CHAIN-HERE - [0:0] -A KUBE-SERVICES -m comment --comment "ns2/svc2:p80 cluster IP" svc2 line 1 @@ -2112,23 +2118,24 @@ func TestOverallIPTablesRulesWithMultipleServices(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-4SW47YFZTEDKD3PK - [0:0] :KUBE-EXT-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-EXT-PAZTZYUUMV5KCDZL - [0:0] :KUBE-EXT-X27LE4BHSL4DOUIK - [0:0] :KUBE-FW-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-C6EBXVWJJZMIWKLZ - [0:0] :KUBE-SEP-OYPFS5VJICHGATKP - [0:0] @@ -2136,7 +2143,6 @@ func TestOverallIPTablesRulesWithMultipleServices(t *testing.T) { :KUBE-SEP-RS4RBKLTHTF2IUXJ - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-UKSFD7AGPMPPLUHC - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-4SW47YFZTEDKD3PK - [0:0] :KUBE-SVC-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-SVC-PAZTZYUUMV5KCDZL - [0:0] @@ -2238,20 +2244,20 @@ func TestClusterIPReject(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j REJECT -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -2315,20 +2321,20 @@ func TestClusterIPEndpointsJump(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] + :KUBE-MARK-MASQ - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O -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 @@ -2421,22 +2427,22 @@ func TestLoadBalancer(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-FW-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] -A KUBE-NODEPORTS -m comment --comment ns1/svc1:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-XPGD46QRK7WJZT7O -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O @@ -2617,21 +2623,21 @@ func TestNodePort(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] -A KUBE-NODEPORTS -m comment --comment ns1/svc1:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-XPGD46QRK7WJZT7O -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O @@ -2718,10 +2724,10 @@ func TestHealthCheckNodePort(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.42 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m addrtype --dst-type LOCAL -m tcp -p tcp --dport 3001 -j REJECT @@ -2730,10 +2736,10 @@ func TestHealthCheckNodePort(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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" -d 127.0.0.1 -j KUBE-NODEPORTS -A KUBE-MARK-MASQ -j MARK --or-mark 0x4000 -A KUBE-POSTROUTING -m mark ! --mark 0x4000/0x4000 -j RETURN @@ -2777,19 +2783,19 @@ func TestMasqueradeRule(t *testing.T) { expectedFmt := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -2836,10 +2842,10 @@ func TestExternalIPsReject(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 192.168.99.11 --dport 80 -j REJECT -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP @@ -2847,10 +2853,10 @@ func TestExternalIPsReject(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -2927,22 +2933,22 @@ func TestOnlyLocalExternalIPs(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-ZX7GRIZKSNUQ3LAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] :KUBE-SVL-XPGD46QRK7WJZT7O - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O @@ -3038,22 +3044,22 @@ func TestNonLocalExternalIPs(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-ZX7GRIZKSNUQ3LAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 external IP" -m tcp -p tcp -d 192.168.99.11 --dport 80 -j KUBE-EXT-XPGD46QRK7WJZT7O @@ -3123,10 +3129,10 @@ func TestNodePortReject(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m addrtype --dst-type LOCAL -m tcp -p tcp --dport 3001 -j REJECT -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP @@ -3134,10 +3140,10 @@ func TestNodePortReject(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -3212,10 +3218,10 @@ func TestLoadBalancerReject(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1:p80 has no endpoints" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j REJECT @@ -3225,10 +3231,10 @@ func TestLoadBalancerReject(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -3324,23 +3330,23 @@ func TestOnlyLocalLoadBalancing(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1: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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-ZX7GRIZKSNUQ3LAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] :KUBE-SVL-XPGD46QRK7WJZT7O - [0:0] -A KUBE-NODEPORTS -m comment --comment ns1/svc1:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-XPGD46QRK7WJZT7O @@ -3406,22 +3412,22 @@ func TestOnlyLocalNodePortsNoClusterCIDR(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-ZX7GRIZKSNUQ3LAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] :KUBE-SVL-XPGD46QRK7WJZT7O - [0:0] -A KUBE-NODEPORTS -m comment --comment ns1/svc1:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-XPGD46QRK7WJZT7O @@ -3453,22 +3459,22 @@ func TestOnlyLocalNodePorts(t *testing.T) { expected := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-XPGD46QRK7WJZT7O - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-SXIVWICOYRO3J4NJ - [0:0] :KUBE-SEP-ZX7GRIZKSNUQ3LAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] :KUBE-SVL-XPGD46QRK7WJZT7O - [0:0] -A KUBE-NODEPORTS -m comment --comment ns1/svc1:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-XPGD46QRK7WJZT7O @@ -4801,22 +4807,22 @@ func TestUpdateEndpointsMap(t *testing.T) { func TestEndpointSliceE2E(t *testing.T) { expectedIPTablesWithSlice := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] + :KUBE-MARK-MASQ - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] :KUBE-SEP-XGJFVO3L2O5SRFNT - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 0 -j KUBE-SVC-AQI2S6QIMU7PVVRP -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 @@ -5250,22 +5256,22 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { clusterExpectedIPTables := dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] + :KUBE-MARK-MASQ - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] :KUBE-SEP-XGJFVO3L2O5SRFNT - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP -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 @@ -5332,20 +5338,20 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { expectEndpointRule: true, expectedIPTablesWithSlice: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] + :KUBE-MARK-MASQ - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVL-AQI2S6QIMU7PVVRP -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 @@ -5383,19 +5389,19 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { expectEndpointRule: false, expectedIPTablesWithSlice: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-MARK-MASQ - [0:0] + :KUBE-POSTROUTING - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVL-AQI2S6QIMU7PVVRP -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 @@ -5629,24 +5635,24 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP @@ -5767,24 +5773,24 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP @@ -5897,24 +5903,24 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] :KUBE-SEP-XGJFVO3L2O5SRFNT - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP @@ -6028,22 +6034,22 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP @@ -6115,22 +6121,22 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 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-NODEPORTS - [0:0] + :KUBE-SERVICES - [0:0] :KUBE-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] :KUBE-SVL-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP @@ -6211,10 +6217,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { noUsableEndpoints: true, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-NODEPORTS -m comment --comment "ns1/svc1 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT -A KUBE-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j REJECT @@ -6223,10 +6229,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -6411,23 +6417,23 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP -A KUBE-SERVICES -m comment --comment "ns1/svc1 loadbalancer IP" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j KUBE-EXT-AQI2S6QIMU7PVVRP @@ -6540,23 +6546,23 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-3JOIVZTXZZRGORX4 - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP -A KUBE-SERVICES -m comment --comment "ns1/svc1 loadbalancer IP" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j KUBE-EXT-AQI2S6QIMU7PVVRP @@ -6662,23 +6668,23 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] :KUBE-SEP-IO5XOSKPAXIFQXAJ - [0:0] :KUBE-SEP-XGJFVO3L2O5SRFNT - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP -A KUBE-SERVICES -m comment --comment "ns1/svc1 loadbalancer IP" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j KUBE-EXT-AQI2S6QIMU7PVVRP @@ -6790,10 +6796,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) noUsableEndpoints: true, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j REJECT -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP @@ -6801,10 +6807,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 @@ -6859,21 +6865,21 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) }, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -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-EXT-AQI2S6QIMU7PVVRP - [0:0] :KUBE-MARK-MASQ - [0:0] - :KUBE-NODEPORTS - [0:0] :KUBE-POSTROUTING - [0:0] :KUBE-SEP-EQCHZ7S2PJ72OHAY - [0:0] - :KUBE-SERVICES - [0:0] :KUBE-SVC-AQI2S6QIMU7PVVRP - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 cluster IP" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j KUBE-SVC-AQI2S6QIMU7PVVRP -A KUBE-SERVICES -m comment --comment "ns1/svc1 loadbalancer IP" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j KUBE-EXT-AQI2S6QIMU7PVVRP @@ -6952,10 +6958,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) noUsableEndpoints: true, expectedIPTables: dedent.Dedent(` *filter - :KUBE-EXTERNAL-SERVICES - [0:0] - :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + :KUBE-EXTERNAL-SERVICES - [0:0] + :KUBE-FORWARD - [0:0] -A KUBE-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 172.30.1.1 --dport 80 -j REJECT -A KUBE-EXTERNAL-SERVICES -m comment --comment "ns1/svc1 has no endpoints" -m tcp -p tcp -d 1.2.3.4 --dport 80 -j REJECT -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP @@ -6963,10 +6969,10 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat - :KUBE-MARK-MASQ - [0:0] :KUBE-NODEPORTS - [0:0] - :KUBE-POSTROUTING - [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 From 10a72a9e034f4904bce1c0ed040ab0543b426b2a Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 7 Apr 2022 11:29:00 -0400 Subject: [PATCH 3/5] pkg/util/iptables/testing: Add IPTables dump-parsing helpers --- pkg/proxy/iptables/proxier_test.go | 205 ++++++------- pkg/util/iptables/testing/parse.go | 169 +++++++++++ pkg/util/iptables/testing/parse_test.go | 374 ++++++++++++++++++++++++ 3 files changed, 648 insertions(+), 100 deletions(-) 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) + } + } + }) + } +} From 913f4bc0bab12534d91211703abff242cd57654f Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 7 Apr 2022 16:13:34 -0400 Subject: [PATCH 4/5] pkg/util/iptables/testing: Fix FakeIPTables FakeIPTables barely implemented any of the iptables interface, and the main part that it did implement, it implemented incorrectly. Fix it: - Implement EnsureChain, DeleteChain, EnsureRule, and DeleteRule, not just SaveInto/Restore/RestoreAll. - Restore/RestoreAll now correctly merge the provided state with the existing state, rather than simply overwriting it. - SaveInto now returns the table that was requested, rather than just echoing back the Restore/RestoreAll. --- pkg/proxy/ipvs/proxier_test.go | 6 +- pkg/util/iptables/testing/fake.go | 214 +++++++++++++++-- pkg/util/iptables/testing/fake_test.go | 315 +++++++++++++++++++++++++ 3 files changed, 508 insertions(+), 27 deletions(-) create mode 100644 pkg/util/iptables/testing/fake_test.go diff --git a/pkg/proxy/ipvs/proxier_test.go b/pkg/proxy/ipvs/proxier_test.go index d084c76506c..baf08b00a91 100644 --- a/pkg/proxy/ipvs/proxier_test.go +++ b/pkg/proxy/ipvs/proxier_test.go @@ -3835,12 +3835,8 @@ func getRules(ipt *iptablestest.FakeIPTables, chain utiliptables.Chain) []*iptab var rules []*iptablestest.Rule buf := bytes.NewBuffer(nil) - // FIXME: FakeIPTables.SaveInto is currently broken and ignores the "table" - // argument and just echoes whatever was last passed to RestoreAll(), so even - // though we want to see the rules from both "nat" and "filter", we have to - // only request one of them, or else we'll get all the rules twice... _ = ipt.SaveInto(utiliptables.TableNAT, buf) - // _ = ipt.SaveInto(utiliptable.TableFilter, buf) + _ = ipt.SaveInto(utiliptables.TableFilter, buf) lines := strings.Split(string(buf.Bytes()), "\n") for _, l := range lines { if !strings.HasPrefix(l, "-A ") { diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index e832037b536..7b134a13cb9 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -18,6 +18,8 @@ package testing import ( "bytes" + "fmt" + "strings" "time" "k8s.io/kubernetes/pkg/util/iptables" @@ -26,53 +28,146 @@ import ( // FakeIPTables is no-op implementation of iptables Interface. type FakeIPTables struct { hasRandomFully bool - Lines []byte protocol iptables.Protocol + + Dump *IPTablesDump } // NewFake returns a no-op iptables.Interface func NewFake() *FakeIPTables { - return &FakeIPTables{protocol: iptables.ProtocolIPv4} + f := &FakeIPTables{ + protocol: iptables.ProtocolIPv4, + Dump: &IPTablesDump{ + Tables: []Table{ + { + Name: iptables.TableNAT, + Chains: []Chain{ + {Name: iptables.ChainPrerouting}, + {Name: iptables.ChainInput}, + {Name: iptables.ChainOutput}, + {Name: iptables.ChainPostrouting}, + }, + }, + { + Name: iptables.TableFilter, + Chains: []Chain{ + {Name: iptables.ChainInput}, + {Name: iptables.ChainForward}, + {Name: iptables.ChainOutput}, + }, + }, + { + Name: iptables.TableMangle, + Chains: []Chain{}, + }, + }, + }, + } + + return f } // NewIPv6Fake returns a no-op iptables.Interface with IsIPv6() == true func NewIPv6Fake() *FakeIPTables { - return &FakeIPTables{protocol: iptables.ProtocolIPv6} + f := NewFake() + f.protocol = iptables.ProtocolIPv6 + return f } -// SetHasRandomFully is part of iptables.Interface +// SetHasRandomFully sets f's return value for HasRandomFully() func (f *FakeIPTables) SetHasRandomFully(can bool) *FakeIPTables { f.hasRandomFully = can return f } // EnsureChain is part of iptables.Interface -func (*FakeIPTables) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) { - return true, nil +func (f *FakeIPTables) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) { + t, err := f.Dump.GetTable(table) + if err != nil { + return false, err + } + if c, _ := f.Dump.GetChain(table, chain); c != nil { + return true, nil + } + t.Chains = append(t.Chains, Chain{Name: chain}) + return false, nil } // FlushChain is part of iptables.Interface -func (*FakeIPTables) FlushChain(table iptables.Table, chain iptables.Chain) error { +func (f *FakeIPTables) FlushChain(table iptables.Table, chain iptables.Chain) error { + if c, _ := f.Dump.GetChain(table, chain); c != nil { + c.Rules = nil + } return nil } // DeleteChain is part of iptables.Interface -func (*FakeIPTables) DeleteChain(table iptables.Table, chain iptables.Chain) error { +func (f *FakeIPTables) DeleteChain(table iptables.Table, chain iptables.Chain) error { + t, err := f.Dump.GetTable(table) + if err != nil { + return err + } + for i := range t.Chains { + if t.Chains[i].Name == chain { + t.Chains = append(t.Chains[:i], t.Chains[i+1:]...) + return nil + } + } return nil } // ChainExists is part of iptables.Interface -func (*FakeIPTables) ChainExists(table iptables.Table, chain iptables.Chain) (bool, error) { - return true, nil +func (f *FakeIPTables) ChainExists(table iptables.Table, chain iptables.Chain) (bool, error) { + if _, err := f.Dump.GetTable(table); err != nil { + return false, err + } + if c, _ := f.Dump.GetChain(table, chain); c != nil { + return true, nil + } + return false, nil } // EnsureRule is part of iptables.Interface -func (*FakeIPTables) EnsureRule(position iptables.RulePosition, table iptables.Table, chain iptables.Chain, args ...string) (bool, error) { - return true, nil +func (f *FakeIPTables) EnsureRule(position iptables.RulePosition, table iptables.Table, chain iptables.Chain, args ...string) (bool, error) { + c, err := f.Dump.GetChain(table, chain) + if err != nil { + return false, err + } + + rule := "-A " + string(chain) + " " + strings.Join(args, " ") + for _, r := range c.Rules { + if r.Raw == rule { + return true, nil + } + } + + parsed, err := ParseRule(rule, false) + if err != nil { + return false, err + } + + if position == iptables.Append { + c.Rules = append(c.Rules, parsed) + } else { + c.Rules = append([]*Rule{parsed}, c.Rules...) + } + return false, nil } // DeleteRule is part of iptables.Interface -func (*FakeIPTables) DeleteRule(table iptables.Table, chain iptables.Chain, args ...string) error { +func (f *FakeIPTables) DeleteRule(table iptables.Table, chain iptables.Chain, args ...string) error { + c, err := f.Dump.GetChain(table, chain) + if err != nil { + return err + } + + rule := "-A " + string(chain) + " " + strings.Join(args, " ") + for i, r := range c.Rules { + if r.Raw == rule { + c.Rules = append(c.Rules[:i], c.Rules[i+1:]...) + break + } + } return nil } @@ -86,27 +181,102 @@ func (f *FakeIPTables) Protocol() iptables.Protocol { return f.protocol } -// Save is part of iptables.Interface -func (f *FakeIPTables) Save(table iptables.Table) ([]byte, error) { - lines := make([]byte, len(f.Lines)) - copy(lines, f.Lines) - return lines, nil +func (f *FakeIPTables) saveTable(table iptables.Table, buffer *bytes.Buffer) error { + t, err := f.Dump.GetTable(table) + if err != nil { + return err + } + + fmt.Fprintf(buffer, "*%s\n", table) + 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) + } + } + fmt.Fprintf(buffer, "COMMIT\n") + return nil } // SaveInto is part of iptables.Interface func (f *FakeIPTables) SaveInto(table iptables.Table, buffer *bytes.Buffer) error { - buffer.Write(f.Lines) + if table == "" { + // As a secret extension to the API, FakeIPTables treats table="" as + // meaning "all tables" + for i := range f.Dump.Tables { + err := f.saveTable(f.Dump.Tables[i].Name, buffer) + if err != nil { + return err + } + } + return nil + } + + return f.saveTable(table, buffer) +} + +func (f *FakeIPTables) restoreTable(newTable *Table, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error { + oldTable, err := f.Dump.GetTable(newTable.Name) + if err != nil { + return err + } + + if flush == iptables.FlushTables { + oldTable.Chains = make([]Chain, 0, len(newTable.Chains)) + } + + for _, newChain := range newTable.Chains { + oldChain, _ := f.Dump.GetChain(newTable.Name, newChain.Name) + switch { + case oldChain == nil && newChain.Deleted: + // no-op + case oldChain == nil && !newChain.Deleted: + oldTable.Chains = append(oldTable.Chains, newChain) + case oldChain != nil && newChain.Deleted: + // FIXME: should make sure chain is not referenced from other jumps + _ = f.DeleteChain(newTable.Name, newChain.Name) + case oldChain != nil && !newChain.Deleted: + // replace old data with new + oldChain.Rules = newChain.Rules + if counters == iptables.RestoreCounters { + oldChain.Packets = newChain.Packets + oldChain.Bytes = newChain.Bytes + } + } + } return nil } // Restore is part of iptables.Interface -func (*FakeIPTables) Restore(table iptables.Table, data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error { - return nil +func (f *FakeIPTables) Restore(table iptables.Table, data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error { + dump, err := ParseIPTablesDump(string(data)) + if err != nil { + return err + } + + newTable, err := dump.GetTable(table) + if err != nil { + return err + } + + return f.restoreTable(newTable, flush, counters) } // RestoreAll is part of iptables.Interface func (f *FakeIPTables) RestoreAll(data []byte, flush iptables.FlushFlag, counters iptables.RestoreCountersFlag) error { - f.Lines = data + dump, err := ParseIPTablesDump(string(data)) + if err != nil { + return err + } + + for i := range dump.Tables { + err = f.restoreTable(&dump.Tables[i], flush, counters) + if err != nil { + return err + } + } return nil } diff --git a/pkg/util/iptables/testing/fake_test.go b/pkg/util/iptables/testing/fake_test.go new file mode 100644 index 00000000000..933121bef5d --- /dev/null +++ b/pkg/util/iptables/testing/fake_test.go @@ -0,0 +1,315 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "bytes" + "strings" + "testing" + + "github.com/lithammer/dedent" + + "k8s.io/kubernetes/pkg/util/iptables" +) + +func TestFakeIPTables(t *testing.T) { + fake := NewFake() + buf := bytes.NewBuffer(nil) + + err := fake.SaveInto("", buf) + if err != nil { + t.Fatalf("unexpected error from SaveInto: %v", err) + } + expected := dedent.Dedent(strings.Trim(` + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + COMMIT + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + COMMIT + *mangle + COMMIT + `, "\n")) + if string(buf.Bytes()) != expected { + t.Fatalf("bad initial dump. expected:\n%s\n\ngot:\n%s\n", expected, buf.Bytes()) + } + + // EnsureChain + existed, err := fake.EnsureChain(iptables.Table("blah"), iptables.Chain("KUBE-TEST")) + if err == nil { + t.Errorf("did not get expected error creating chain in non-existent table") + } else if existed { + t.Errorf("wrong return value from EnsureChain with non-existent table") + } + existed, err = fake.EnsureChain(iptables.TableNAT, iptables.Chain("KUBE-TEST")) + if err != nil { + t.Errorf("unexpected error creating chain: %v", err) + } else if existed { + t.Errorf("wrong return value from EnsureChain with non-existent chain") + } + existed, err = fake.EnsureChain(iptables.TableNAT, iptables.Chain("KUBE-TEST")) + if err != nil { + t.Errorf("unexpected error creating chain: %v", err) + } else if !existed { + t.Errorf("wrong return value from EnsureChain with existing chain") + } + + // ChainExists + exists, err := fake.ChainExists(iptables.TableNAT, iptables.Chain("KUBE-TEST")) + if err != nil { + t.Errorf("unexpected error checking chain: %v", err) + } else if !exists { + t.Errorf("wrong return value from ChainExists with existing chain") + } + exists, err = fake.ChainExists(iptables.TableNAT, iptables.Chain("KUBE-TEST-NOT")) + if err != nil { + t.Errorf("unexpected error checking chain: %v", err) + } else if exists { + t.Errorf("wrong return value from ChainExists with non-existent chain") + } + + // EnsureRule + existed, err = fake.EnsureRule(iptables.Append, iptables.Table("blah"), iptables.Chain("KUBE-TEST"), "-j", "ACCEPT") + if err == nil { + t.Errorf("did not get expected error creating rule in non-existent table") + } else if existed { + t.Errorf("wrong return value from EnsureRule with non-existent table") + } + existed, err = fake.EnsureRule(iptables.Append, iptables.TableNAT, iptables.Chain("KUBE-TEST-NOT"), "-j", "ACCEPT") + if err == nil { + t.Errorf("did not get expected error creating rule in non-existent chain") + } else if existed { + t.Errorf("wrong return value from EnsureRule with non-existent chain") + } + existed, err = fake.EnsureRule(iptables.Append, iptables.TableNAT, iptables.Chain("KUBE-TEST"), "-j", "ACCEPT") + if err != nil { + t.Errorf("unexpected error creating rule: %v", err) + } else if existed { + t.Errorf("wrong return value from EnsureRule with non-existent rule") + } + existed, err = fake.EnsureRule(iptables.Prepend, iptables.TableNAT, iptables.Chain("KUBE-TEST"), "-j", "DROP") + if err != nil { + t.Errorf("unexpected error creating rule: %v", err) + } else if existed { + t.Errorf("wrong return value from EnsureRule with non-existent rule") + } + existed, err = fake.EnsureRule(iptables.Append, iptables.TableNAT, iptables.Chain("KUBE-TEST"), "-j", "DROP") + if err != nil { + t.Errorf("unexpected error creating rule: %v", err) + } else if !existed { + t.Errorf("wrong return value from EnsureRule with already-existing rule") + } + + // Sanity-check... + buf.Reset() + err = fake.SaveInto("", buf) + if err != nil { + t.Fatalf("unexpected error from SaveInto: %v", err) + } + expected = dedent.Dedent(strings.Trim(` + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + :KUBE-TEST - [0:0] + -A KUBE-TEST -j DROP + -A KUBE-TEST -j ACCEPT + COMMIT + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + COMMIT + *mangle + COMMIT + `, "\n")) + if string(buf.Bytes()) != expected { + t.Fatalf("bad sanity-check dump. expected:\n%s\n\ngot:\n%s\n", expected, buf.Bytes()) + } + + // DeleteRule + err = fake.DeleteRule(iptables.Table("blah"), iptables.Chain("KUBE-TEST"), "-j", "DROP") + if err == nil { + t.Errorf("did not get expected error deleting rule in non-existent table") + } + err = fake.DeleteRule(iptables.TableNAT, iptables.Chain("KUBE-TEST-NOT"), "-j", "DROP") + if err == nil { + t.Errorf("did not get expected error deleting rule in non-existent chain") + } + err = fake.DeleteRule(iptables.TableNAT, iptables.Chain("KUBE-TEST"), "-j", "DROPLET") + if err != nil { + t.Errorf("unexpected error deleting non-existent rule: %v", err) + } + err = fake.DeleteRule(iptables.TableNAT, iptables.Chain("KUBE-TEST"), "-j", "DROP") + if err != nil { + t.Errorf("unexpected error deleting rule: %v", err) + } + + // Restore + rules := dedent.Dedent(strings.Trim(` + *nat + :KUBE-RESTORED - [0:0] + :KUBE-MISC-CHAIN - [0:0] + :KUBE-EMPTY - [0:0] + -A KUBE-RESTORED -m comment --comment "restored chain" -j ACCEPT + -A KUBE-MISC-CHAIN -s 1.2.3.4 -j DROP + -A KUBE-MISC-CHAIN -d 5.6.7.8 -j MASQUERADE + COMMIT + `, "\n")) + err = fake.Restore(iptables.TableNAT, []byte(rules), iptables.NoFlushTables, iptables.NoRestoreCounters) + if err != nil { + t.Fatalf("unexpected error from Restore: %v", err) + } + + // We used NoFlushTables, so this should leave KUBE-TEST unchanged + buf.Reset() + err = fake.SaveInto("", buf) + if err != nil { + t.Fatalf("unexpected error from SaveInto: %v", err) + } + expected = dedent.Dedent(strings.Trim(` + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + :KUBE-TEST - [0:0] + :KUBE-RESTORED - [0:0] + :KUBE-MISC-CHAIN - [0:0] + :KUBE-EMPTY - [0:0] + -A KUBE-TEST -j ACCEPT + -A KUBE-RESTORED -m comment --comment "restored chain" -j ACCEPT + -A KUBE-MISC-CHAIN -s 1.2.3.4 -j DROP + -A KUBE-MISC-CHAIN -d 5.6.7.8 -j MASQUERADE + COMMIT + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + COMMIT + *mangle + COMMIT + `, "\n")) + if string(buf.Bytes()) != expected { + t.Fatalf("bad post-restore dump. expected:\n%s\n\ngot:\n%s\n", expected, buf.Bytes()) + } + + // more Restore; empty out one chain and delete another, but also update its counters + rules = dedent.Dedent(strings.Trim(` + *nat + :KUBE-RESTORED - [0:0] + :KUBE-TEST - [99:9999] + -X KUBE-RESTORED + COMMIT + `, "\n")) + err = fake.Restore(iptables.TableNAT, []byte(rules), iptables.NoFlushTables, iptables.RestoreCounters) + if err != nil { + t.Fatalf("unexpected error from Restore: %v", err) + } + + buf.Reset() + err = fake.SaveInto("", buf) + if err != nil { + t.Fatalf("unexpected error from SaveInto: %v", err) + } + expected = dedent.Dedent(strings.Trim(` + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + :KUBE-TEST - [99:9999] + :KUBE-MISC-CHAIN - [0:0] + :KUBE-EMPTY - [0:0] + -A KUBE-MISC-CHAIN -s 1.2.3.4 -j DROP + -A KUBE-MISC-CHAIN -d 5.6.7.8 -j MASQUERADE + COMMIT + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + COMMIT + *mangle + COMMIT + `, "\n")) + if string(buf.Bytes()) != expected { + t.Fatalf("bad post-second-restore dump. expected:\n%s\n\ngot:\n%s\n", expected, buf.Bytes()) + } + + // RestoreAll, FlushTables + rules = dedent.Dedent(strings.Trim(` + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + :KUBE-TEST - [0:0] + -A KUBE-TEST -m comment --comment "filter table KUBE-TEST" -j ACCEPT + COMMIT + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + :KUBE-TEST - [88:8888] + :KUBE-NEW-CHAIN - [0:0] + -A KUBE-NEW-CHAIN -d 172.30.0.1 -j DNAT --to-destination 10.0.0.1 + -A KUBE-NEW-CHAIN -d 172.30.0.2 -j DNAT --to-destination 10.0.0.2 + -A KUBE-NEW-CHAIN -d 172.30.0.3 -j DNAT --to-destination 10.0.0.3 + COMMIT + `, "\n")) + err = fake.RestoreAll([]byte(rules), iptables.FlushTables, iptables.NoRestoreCounters) + if err != nil { + t.Fatalf("unexpected error from RestoreAll: %v", err) + } + + buf.Reset() + err = fake.SaveInto("", buf) + if err != nil { + t.Fatalf("unexpected error from SaveInto: %v", err) + } + expected = dedent.Dedent(strings.Trim(` + *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] + :KUBE-TEST - [88:8888] + :KUBE-NEW-CHAIN - [0:0] + -A KUBE-NEW-CHAIN -d 172.30.0.1 -j DNAT --to-destination 10.0.0.1 + -A KUBE-NEW-CHAIN -d 172.30.0.2 -j DNAT --to-destination 10.0.0.2 + -A KUBE-NEW-CHAIN -d 172.30.0.3 -j DNAT --to-destination 10.0.0.3 + COMMIT + *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] + :KUBE-TEST - [0:0] + -A KUBE-TEST -m comment --comment "filter table KUBE-TEST" -j ACCEPT + COMMIT + *mangle + COMMIT + `, "\n")) + if string(buf.Bytes()) != expected { + t.Fatalf("bad post-restore-all dump. expected:\n%s\n\ngot:\n%s\n", expected, buf.Bytes()) + } +} From 24e1e3d9ee1ae13f86ac04da9133270b75fbf365 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 10 Mar 2022 12:08:04 -0500 Subject: [PATCH 5/5] proxy/iptables: port packet-flow tests to use new parsing stuff --- pkg/proxy/iptables/proxier_test.go | 445 ++++++++--------------------- 1 file changed, 122 insertions(+), 323 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index d0fa9bc7c90..b9c2ef06d6e 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -1387,202 +1387,36 @@ func assertIPTablesRulesNotEqual(t *testing.T, line int, expected, result string } } -// ruleMatchesIP helps test whether an iptables rule such as "! -s 192.168.0.0/16" matches -// ipStr. ruleAddress is either an IP address ("1.2.3.4") or a CIDR string -// ("1.2.3.0/24"). negated is whether the iptables rule negates the match. -func ruleMatchesIP(t *testing.T, negated bool, ruleAddress, ipStr string) bool { +// 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"). +func addressMatches(t *testing.T, address *iptablestest.IPTablesValue, ipStr string) bool { ip := netutils.ParseIPSloppy(ipStr) if ip == nil { t.Fatalf("Bad IP in test case: %s", ipStr) } var matches bool - if strings.Contains(ruleAddress, "/") { - _, cidr, err := netutils.ParseCIDRSloppy(ruleAddress) + if strings.Contains(address.Value, "/") { + _, cidr, err := netutils.ParseCIDRSloppy(address.Value) if err != nil { t.Errorf("Bad CIDR in kube-proxy output: %v", err) } matches = cidr.Contains(ip) } else { - ip2 := netutils.ParseIPSloppy(ruleAddress) + ip2 := netutils.ParseIPSloppy(address.Value) if ip2 == nil { - t.Errorf("Bad IP/CIDR in kube-proxy output: %s", ruleAddress) + t.Errorf("Bad IP/CIDR in kube-proxy output: %s", address.Value) } matches = ip.Equal(ip2) } - return (!negated && matches) || (negated && !matches) + return (!address.Negated && matches) || (address.Negated && !matches) } -// Regular expressions used by iptablesTracer. Note that these are not fully general-purpose -// and may need to be updated if we make large changes to our iptable rules. -var addRuleToChainRegex = regexp.MustCompile(`-A ([^ ]*) `) -var moduleRegex = regexp.MustCompile("-m ([^ ]*)") -var commentRegex = regexp.MustCompile(`-m comment --comment ("[^"]*"|[^" ]*) `) -var srcLocalRegex = regexp.MustCompile("(!)? --src-type LOCAL") -var destLocalRegex = regexp.MustCompile("(!)? --dst-type LOCAL") -var destIPRegex = regexp.MustCompile("(!)? -d ([^ ]*) ") -var destPortRegex = regexp.MustCompile(" --dport ([^ ]*) ") -var sourceIPRegex = regexp.MustCompile("(!)? -s ([^ ]*) ") -var affinityRegex = regexp.MustCompile(" --rcheck ") - -// (If `--probability` appears, it can only appear before the `-j`, and if `--to-destination` -// appears it can only appear after the `-j`, so this is not as fragile as it looks. -var jumpRegex = regexp.MustCompile("(--probability.*)? -j ([^ ]*)( --to-destination (.*))?$") - -func Test_iptablesTracerRegexps(t *testing.T) { - testCases := []struct { - name string - regex *regexp.Regexp - rule string - matches []string - }{ - { - name: "addRuleToChainRegex", - regex: addRuleToChainRegex, - rule: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, - matches: []string{`-A KUBE-NODEPORTS `, "KUBE-NODEPORTS"}, - }, - { - name: "addRuleToChainRegex requires an actual rule, not just a chain name", - regex: addRuleToChainRegex, - rule: `-A KUBE-NODEPORTS`, - matches: nil, - }, - { - name: "addRuleToChainRegex only matches adds", - regex: addRuleToChainRegex, - rule: `-D KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, - matches: nil, - }, - { - name: "commentRegex with quoted comment", - regex: commentRegex, - rule: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, - matches: []string{`-m comment --comment "ns2/svc2:p80 health check node port" `, `"ns2/svc2:p80 health check node port"`}, - }, - { - name: "commentRegex with unquoted comment", - regex: commentRegex, - rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ`, - matches: []string{`-m comment --comment ns1/svc1:p80 `, "ns1/svc1:p80"}, - }, - { - name: "no comment", - regex: commentRegex, - rule: `-A KUBE-POSTROUTING -j MARK --xor-mark 0x4000`, - matches: nil, - }, - { - name: "moduleRegex", - regex: moduleRegex, - rule: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, - matches: []string{"-m comment", "comment"}, - }, - { - name: "local source", - regex: srcLocalRegex, - rule: `-A KUBE-XLB-GNZBNJ2PO5MGZ6GT -m comment --comment "masquerade LOCAL traffic for ns2/svc2:p80 LB IP" -m addrtype --src-type LOCAL -j KUBE-MARK-MASQ`, - matches: []string{" --src-type LOCAL", ""}, - }, - { - name: "not local destination", - regex: destLocalRegex, - rule: `-A RULE-TYPE-NOT-CURRENTLY-USED-BY-KUBE-PROXY -m addrtype ! --dst-type LOCAL -j KUBE-MARK-MASQ`, - matches: []string{"! --dst-type LOCAL", "!"}, - }, - { - name: "destination IP", - regex: destIPRegex, - rule: `-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O`, - matches: []string{" -d 172.30.0.41 ", "", "172.30.0.41"}, - }, - { - name: "destination port", - regex: destPortRegex, - rule: `-A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O`, - matches: []string{" --dport 80 ", "80"}, - }, - { - name: "destination IP but no port", - regex: destPortRegex, - rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -d 172.30.0.41 ! -s 10.0.0.0/8 -j KUBE-MARK-MASQ`, - matches: nil, - }, - { - name: "source IP", - regex: sourceIPRegex, - rule: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -s 10.180.0.1 -j KUBE-MARK-MASQ`, - matches: []string{" -s 10.180.0.1 ", "", "10.180.0.1"}, - }, - { - name: "not source IP", - regex: sourceIPRegex, - rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 ! -s 10.0.0.0/8 -j KUBE-MARK-MASQ`, - matches: []string{"! -s 10.0.0.0/8 ", "!", "10.0.0.0/8"}, - }, - { - name: "affinityRegex", - regex: affinityRegex, - rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -m recent --name KUBE-SEP-SXIVWICOYRO3J4NJ --rcheck --seconds 10800 --reap -j KUBE-SEP-SXIVWICOYRO3J4NJ`, - matches: []string{" --rcheck "}, - }, - { - name: "jump to internal target", - regex: jumpRegex, - rule: `-A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT`, - matches: []string{" -j ACCEPT", "", "ACCEPT", "", ""}, - }, - { - name: "jump to KUBE chain", - regex: jumpRegex, - rule: `-A KUBE-SVC-XPGD46QRK7WJZT7O -m comment --comment ns1/svc1:p80 -j KUBE-SEP-SXIVWICOYRO3J4NJ`, - matches: []string{" -j KUBE-SEP-SXIVWICOYRO3J4NJ", "", "KUBE-SEP-SXIVWICOYRO3J4NJ", "", ""}, - }, - { - name: "jump to DNAT", - regex: jumpRegex, - rule: `-A KUBE-SEP-SXIVWICOYRO3J4NJ -m comment --comment ns1/svc1:p80 -m tcp -p tcp -j DNAT --to-destination 10.180.0.1:80`, - matches: []string{" -j DNAT --to-destination 10.180.0.1:80", "", "DNAT", " --to-destination 10.180.0.1:80", "10.180.0.1:80"}, - }, - { - name: "jump to endpoint", - regex: jumpRegex, - rule: `-A KUBE-SVC-4SW47YFZTEDKD3PK -m comment --comment ns4/svc4:p80 -m statistic --mode random --probability 0.5000000000 -j KUBE-SEP-UKSFD7AGPMPPLUHC`, - matches: []string{"--probability 0.5000000000 -j KUBE-SEP-UKSFD7AGPMPPLUHC", "--probability 0.5000000000", "KUBE-SEP-UKSFD7AGPMPPLUHC", "", ""}, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - matches := testCase.regex.FindStringSubmatch(testCase.rule) - if !reflect.DeepEqual(matches, testCase.matches) { - t.Errorf("bad match: expected %#v, got %#v", testCase.matches, matches) - } - }) - } -} - -// knownModules is the set of modules (ie "-m foo") that we allow to be present in rules passed -// to an iptablesTracer. If a rule using another module is found in a rule, the test will -// fail. -// -// If a module is in knownModules but is not in noMatchModules and is not handled by -// ruleMatches, then the result is that match rules using that module will have no effect -// for tracing purposes. -var knownModules = sets.NewString("addrtype", "comment", "conntrack", "mark", "recent", "statistic", "tcp", "udp") - -// noMatchModules is the list of modules that if we see them in a rule, we just -// assume the rule doesn't match and ignore it (because rules with these modules exist -// in the data we are testing against, but aren't relevant to what we're testing). -var noMatchModules = sets.NewString("conntrack", "mark") - -type iptablesChain []string -type iptablesTable map[string]iptablesChain - // iptablesTracer holds data used while virtually tracing a packet through a set of // iptables rules type iptablesTracer struct { - tables map[string]iptablesTable + ipt *iptablestest.FakeIPTables nodeIP string t *testing.T @@ -1600,109 +1434,56 @@ type iptablesTracer struct { markDrop bool } -// newIPTablesTracer creates an iptablesTracer. ruleData is an iptables rule dump (as with -// "iptables-save"). nodeIP is the IP to treat as the local node IP (for determining -// whether rules with "--src-type LOCAL" or "--dst-type LOCAL" match). -func newIPTablesTracer(t *testing.T, ruleData, nodeIP string) (*iptablesTracer, error) { - tables, err := parseIPTablesData(ruleData) - if err != nil { - return nil, err - } - - tracer := &iptablesTracer{ - tables: make(map[string]iptablesTable), +// newIPTablesTracer creates an iptablesTracer. nodeIP is the IP to treat as the local +// node IP (for determining whether rules with "--src-type LOCAL" or "--dst-type LOCAL" +// match). +func newIPTablesTracer(t *testing.T, ipt *iptablestest.FakeIPTables, nodeIP string) *iptablesTracer { + return &iptablesTracer{ + ipt: ipt, nodeIP: nodeIP, t: t, } - - for name, rules := range tables { - table := make(iptablesTable) - for _, rule := range rules { - match := addRuleToChainRegex.FindStringSubmatch(rule) - if match != nil { - chainName := match[1] - table[chainName] = append(table[chainName], rule) - } - } - tracer.tables[name] = table - } - - return tracer, nil } // ruleMatches checks if the given iptables rule matches (at least probabilistically) a // packet with the given sourceIP, destIP, and destPort. (Note that protocol is currently // ignored.) -func (tracer *iptablesTracer) ruleMatches(rule, sourceIP, destIP, destPort string) bool { - var match []string - - // Delete comments so we don't mistakenly match something in a comment string later - rule = commentRegex.ReplaceAllString(rule, "") - - // Make sure the rule only uses modules ("-m foo") that we are aware of - for _, matches := range moduleRegex.FindAllStringSubmatch(rule, -1) { - moduleName := matches[1] - if !knownModules.Has(moduleName) { - tracer.t.Errorf("Rule %q uses unknown iptables module %q", rule, moduleName) - } - if noMatchModules.Has(moduleName) { - // This rule is doing something irrelevant to iptablesTracer - return false - } - } - +func (tracer *iptablesTracer) ruleMatches(rule *iptablestest.Rule, sourceIP, destIP, destPort string) bool { // The sub-rules within an iptables rule are ANDed together, so the rule only // matches if all of them match. So go through the subrules, and if any of them // DON'T match, then fail. - // Match local/non-local. - match = srcLocalRegex.FindStringSubmatch(rule) - if match != nil { - wantLocal := (match[1] != "!") - sourceIsLocal := (sourceIP == tracer.nodeIP || sourceIP == "127.0.0.1") - if wantLocal != sourceIsLocal { - return false - } + if rule.SourceAddress != nil && !addressMatches(tracer.t, rule.SourceAddress, sourceIP) { + return false } - match = destLocalRegex.FindStringSubmatch(rule) - if match != nil { - wantLocal := (match[1] != "!") - destIsLocal := (destIP == tracer.nodeIP || destIP == "127.0.0.1") - if wantLocal != destIsLocal { + if rule.SourceType != nil { + addrtype := "not-matched" + if sourceIP == tracer.nodeIP || sourceIP == "127.0.0.1" { + addrtype = "LOCAL" + } + if !rule.SourceType.Matches(addrtype) { return false } } - // Match destination IP/port. - match = destIPRegex.FindStringSubmatch(rule) - if match != nil { - negated := match[1] == "!" - ruleAddress := match[2] - if !ruleMatchesIP(tracer.t, negated, ruleAddress, destIP) { + if rule.DestinationAddress != nil && !addressMatches(tracer.t, rule.DestinationAddress, destIP) { + return false + } + if rule.DestinationType != nil { + addrtype := "not-matched" + if destIP == tracer.nodeIP || destIP == "127.0.0.1" { + addrtype = "LOCAL" + } + if !rule.DestinationType.Matches(addrtype) { return false } } - match = destPortRegex.FindStringSubmatch(rule) - if match != nil { - rulePort := match[1] - if rulePort != destPort { - return false - } + if rule.DestinationPort != nil && !rule.DestinationPort.Matches(destPort) { + return false } - // Match source IP (but not currently port) - match = sourceIPRegex.FindStringSubmatch(rule) - if match != nil { - negated := match[1] == "!" - ruleAddress := match[2] - if !ruleMatchesIP(tracer.t, negated, ruleAddress, sourceIP) { - return false - } - } - - // The iptablesTracer has no state/history, so any rule that checks whether affinity - // has been established for a particular endpoint must not match. - if affinityRegex.MatchString(rule) { + // Any rule that checks for past state/history does not match + if rule.AffinityCheck != nil || rule.MarkCheck != nil || rule.CTStateCheck != nil { return false } @@ -1712,25 +1493,26 @@ func (tracer *iptablesTracer) ruleMatches(rule, sourceIP, destIP, destPort strin // runChain runs the given packet through the rules in the given table and chain, updating // tracer's internal state accordingly. It returns true if it hits a terminal action. -func (tracer *iptablesTracer) runChain(table, chain, sourceIP, destIP, destPort string) bool { - for _, rule := range tracer.tables[table][chain] { - match := jumpRegex.FindStringSubmatch(rule) - if match == nil { +func (tracer *iptablesTracer) runChain(table utiliptables.Table, chain utiliptables.Chain, sourceIP, destIP, destPort string) bool { + c, _ := tracer.ipt.Dump.GetChain(table, chain) + if c == nil { + return false + } + + for _, rule := range c.Rules { + if rule.Jump == nil { // You _can_ have rules that don't end in `-j`, but we don't currently // do that. - tracer.t.Errorf("Could not find jump target in rule %q", rule) + tracer.t.Errorf("Could not find jump target in rule %q", rule.Raw) } - isProbabilisticMatch := (match[1] != "") - target := match[2] - natDestination := match[4] if !tracer.ruleMatches(rule, sourceIP, destIP, destPort) { continue } // record the matched rule for debugging purposes - tracer.matches = append(tracer.matches, rule) + tracer.matches = append(tracer.matches, rule.Raw) - switch target { + switch rule.Jump.Value { case "KUBE-MARK-MASQ": tracer.markMasq = true continue @@ -1741,24 +1523,24 @@ func (tracer *iptablesTracer) runChain(table, chain, sourceIP, destIP, destPort case "ACCEPT", "REJECT": // (only valid in filter) - tracer.outputs = append(tracer.outputs, target) + tracer.outputs = append(tracer.outputs, rule.Jump.Value) return true case "DNAT": // (only valid in nat) - tracer.outputs = append(tracer.outputs, natDestination) + tracer.outputs = append(tracer.outputs, rule.DNATDestination.Value) return true default: // We got a "-j KUBE-SOMETHING", so process that chain - terminated := tracer.runChain(table, target, sourceIP, destIP, destPort) + terminated := tracer.runChain(table, utiliptables.Chain(rule.Jump.Value), sourceIP, destIP, destPort) // If the subchain hit a terminal rule AND the rule that sent us // to that chain was non-probabilistic, then this chain terminates // as well. But if we went there because of a --probability rule, // then we want to keep accumulating further matches against this // chain. - if terminated && !isProbabilisticMatch { + if terminated && rule.Probability == nil { return true } } @@ -1774,42 +1556,33 @@ func (tracer *iptablesTracer) runChain(table, chain, sourceIP, destIP, destPort // The return values are: an array of matched rules (for debugging), the final packet // destinations (a comma-separated list of IPs, or one of the special targets "ACCEPT", // "DROP", or "REJECT"), and whether the packet would be masqueraded. -func tracePacket(t *testing.T, ruleData, sourceIP, destIP, destPort, nodeIP string) ([]string, string, bool) { - tracer, err := newIPTablesTracer(t, ruleData, nodeIP) - if err != nil { - t.Errorf("Bad iptables ruleData: %v", err) - } +func tracePacket(t *testing.T, ipt *iptablestest.FakeIPTables, sourceIP, destIP, destPort, nodeIP string) ([]string, string, bool) { + tracer := newIPTablesTracer(t, ipt, nodeIP) - // nat:PREROUTING goes first, then the filter chains, then nat:POSTROUTING. For our - // purposes that means we run through the "nat" chains first, starting from the top of - // KUBE-SERVICES, then we do the "filter" chains. The only interesting thing that - // happens in nat:POSTROUTING is that the masquerade mark gets turned into actual - // masquerading. + // nat:PREROUTING goes first + tracer.runChain(utiliptables.TableNAT, utiliptables.ChainPrerouting, sourceIP, destIP, destPort) - // FIXME: we ought to be able to say - // trace.runChain("nat", "PREROUTING", ...) - // here instead of - // trace.runChain("nat", "KUBE-SERVICES", ...) - // (and similarly below with the "filter" chains) but this doesn't work because the - // rules like "-A PREROUTING -j KUBE-SERVICES" are created with iptables.EnsureRule(), - // which iptablestest.FakeIPTables doesn't implement, so those rules will be missing - // from the ruleData we have. So we have to explicitly specify each kube-proxy chain - // we want to run through here. - tracer.runChain("nat", "KUBE-SERVICES", sourceIP, destIP, destPort) - - // Process pending DNAT (which theoretically might affect REJECT/ACCEPT filter rules) + // After the PREROUTING rules run, pending DNATs are processed (which would affect + // the destination IP that later rules match against). if len(tracer.outputs) != 0 { destIP = strings.Split(tracer.outputs[0], ":")[0] } - // Now run the filter rules to see if the packet is REJECTed or ACCEPTed. The DROP - // rule is created by kubelet, not us, so we have to simulate that manually + // Now the filter rules get run; exactly which ones depend on whether this is an + // inbound, outbound, or intra-host packet, which we don't know. So we just run + // the interesting tables manually. (Theoretically this could cause conflicts in + // the future in which case we'd have to do something more complicated.) + + // The DROP rule is created by kubelet, not us, so we have to simulate that manually. if tracer.markDrop { return tracer.matches, "DROP", false } - tracer.runChain("filter", "KUBE-SERVICES", sourceIP, destIP, destPort) - tracer.runChain("filter", "KUBE-EXTERNAL-SERVICES", sourceIP, destIP, destPort) - tracer.runChain("filter", "KUBE-NODEPORTS", sourceIP, destIP, destPort) + tracer.runChain(utiliptables.TableFilter, kubeServicesChain, sourceIP, destIP, destPort) + tracer.runChain(utiliptables.TableFilter, kubeExternalServicesChain, sourceIP, destIP, destPort) + tracer.runChain(utiliptables.TableFilter, kubeNodePortsChain, sourceIP, destIP, destPort) + + // Finally, the nat:POSTROUTING rules run, but the only interesting thing that + // happens there is that the masquerade mark gets turned into actual masquerading. return tracer.matches, strings.Join(tracer.outputs, ", "), tracer.markMasq } @@ -1823,14 +1596,14 @@ type packetFlowTest struct { masq bool } -func runPacketFlowTests(t *testing.T, line int, ruleData, nodeIP string, testCases []packetFlowTest) { +func runPacketFlowTests(t *testing.T, line int, ipt *iptablestest.FakeIPTables, nodeIP string, testCases []packetFlowTest) { lineStr := "" if line != 0 { lineStr = fmt.Sprintf(" (from line %d)", line) } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - matches, output, masq := tracePacket(t, ruleData, tc.sourceIP, tc.destIP, fmt.Sprintf("%d", tc.destPort), nodeIP) + matches, output, masq := tracePacket(t, ipt, tc.sourceIP, tc.destIP, fmt.Sprintf("%d", tc.destPort), nodeIP) var errors []string if output != tc.output { errors = append(errors, fmt.Sprintf("wrong output: expected %q got %q", tc.output, output)) @@ -1851,10 +1624,19 @@ func runPacketFlowTests(t *testing.T, line int, ruleData, nodeIP string, testCas func TestTracePackets(t *testing.T) { rules := dedent.Dedent(` *filter + :INPUT - [0:0] + :FORWARD - [0:0] + :OUTPUT - [0:0] :KUBE-EXTERNAL-SERVICES - [0:0] :KUBE-FORWARD - [0:0] :KUBE-NODEPORTS - [0:0] :KUBE-SERVICES - [0:0] + -A INPUT -m comment --comment kubernetes health check service ports -j KUBE-NODEPORTS + -A INPUT -m conntrack --ctstate NEW -m comment --comment kubernetes externally-visible service portals -j KUBE-EXTERNAL-SERVICES + -A FORWARD -m comment --comment kubernetes forwarding rules -j KUBE-FORWARD + -A FORWARD -m conntrack --ctstate NEW -m comment --comment kubernetes service portals -j KUBE-SERVICES + -A FORWARD -m conntrack --ctstate NEW -m comment --comment kubernetes externally-visible service portals -j KUBE-EXTERNAL-SERVICES + -A OUTPUT -m conntrack --ctstate NEW -m comment --comment kubernetes service portals -j KUBE-SERVICES -A KUBE-NODEPORTS -m comment --comment "ns2/svc2:p80 health check node port" -m tcp -p tcp --dport 30000 -j ACCEPT -A KUBE-SERVICES -m comment --comment "ns3/svc3:p80 has no endpoints" -m tcp -p tcp -d 172.30.0.43 --dport 80 -j REJECT -A KUBE-FORWARD -m conntrack --ctstate INVALID -j DROP @@ -1862,6 +1644,10 @@ func TestTracePackets(t *testing.T) { -A KUBE-FORWARD -m comment --comment "kubernetes forwarding conntrack rule" -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT COMMIT *nat + :PREROUTING - [0:0] + :INPUT - [0:0] + :OUTPUT - [0:0] + :POSTROUTING - [0:0] :KUBE-EXT-4SW47YFZTEDKD3PK - [0:0] :KUBE-EXT-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-EXT-PAZTZYUUMV5KCDZL - [0:0] @@ -1879,6 +1665,13 @@ func TestTracePackets(t *testing.T) { :KUBE-SVC-GNZBNJ2PO5MGZ6GT - [0:0] :KUBE-SVC-XPGD46QRK7WJZT7O - [0:0] :KUBE-SVL-GNZBNJ2PO5MGZ6GT - [0:0] + -A PREROUTING -m comment --comment kubernetes service portals -j KUBE-SERVICES + -A OUTPUT -m comment --comment kubernetes service portals -j KUBE-SERVICES + -A POSTROUTING -m comment --comment kubernetes postrouting rules -j KUBE-POSTROUTING + -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-NODEPORTS -m comment --comment ns2/svc2:p80 -m tcp -p tcp --dport 3001 -j KUBE-EXT-GNZBNJ2PO5MGZ6GT -A KUBE-SERVICES -m comment --comment "ns1/svc1:p80 cluster IP" -m tcp -p tcp -d 172.30.0.41 --dport 80 -j KUBE-SVC-XPGD46QRK7WJZT7O -A KUBE-SERVICES -m comment --comment "ns2/svc2:p80 cluster IP" -m tcp -p tcp -d 172.30.0.42 --dport 80 -j KUBE-SVC-GNZBNJ2PO5MGZ6GT @@ -1918,7 +1711,13 @@ func TestTracePackets(t *testing.T) { COMMIT `) - runPacketFlowTests(t, getLine(), rules, testNodeIP, []packetFlowTest{ + ipt := iptablestest.NewFake() + err := ipt.RestoreAll([]byte(rules), utiliptables.NoFlushTables, utiliptables.RestoreCounters) + if err != nil { + t.Fatalf("Restore of test data failed: %v", err) + } + + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "no match", sourceIP: "10.0.0.2", @@ -2273,7 +2072,7 @@ func TestClusterIPReject(t *testing.T) { assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "cluster IP rejected", sourceIP: "10.0.0.2", @@ -2355,7 +2154,7 @@ func TestClusterIPEndpointsJump(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "cluster IP accepted", sourceIP: "10.180.0.2", @@ -2474,7 +2273,7 @@ func TestLoadBalancer(t *testing.T) { assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP", sourceIP: "10.0.0.2", @@ -2661,7 +2460,7 @@ func TestNodePort(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP", sourceIP: "10.0.0.2", @@ -2755,7 +2554,7 @@ func TestHealthCheckNodePort(t *testing.T) { assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "firewall accepts HealthCheckNodePort", sourceIP: "1.2.3.4", @@ -2769,7 +2568,7 @@ func TestHealthCheckNodePort(t *testing.T) { fp.OnServiceDelete(svc) fp.syncProxyRules() - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "HealthCheckNodePort no longer has any rule", sourceIP: "1.2.3.4", @@ -2871,7 +2670,7 @@ func TestExternalIPsReject(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "cluster IP with no endpoints", sourceIP: "10.0.0.2", @@ -2979,7 +2778,7 @@ func TestOnlyLocalExternalIPs(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "cluster IP hits both endpoints", sourceIP: "10.0.0.2", @@ -3086,7 +2885,7 @@ func TestNonLocalExternalIPs(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP", sourceIP: "10.0.0.2", @@ -3158,7 +2957,7 @@ func TestNodePortReject(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP", sourceIP: "10.0.0.2", @@ -3249,7 +3048,7 @@ func TestLoadBalancerReject(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP", sourceIP: "10.0.0.2", @@ -3381,7 +3180,7 @@ func TestOnlyLocalLoadBalancing(t *testing.T) { `) assertIPTablesRulesEqual(t, getLine(), expected, fp.iptablesData.String()) - runPacketFlowTests(t, getLine(), fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, getLine(), ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP hits both endpoints", sourceIP: "10.0.0.2", @@ -3555,7 +3354,7 @@ func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTable assertIPTablesRulesEqual(t, line, expected, fp.iptablesData.String()) - runPacketFlowTests(t, line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, line, ipt, testNodeIP, []packetFlowTest{ { name: "pod to cluster IP hit both endpoints", sourceIP: "10.0.0.2", @@ -3583,7 +3382,7 @@ func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTable if fp.localDetector.IsImplemented() { // pod-to-NodePort is treated as internal traffic, so we see both endpoints - runPacketFlowTests(t, line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, line, ipt, testNodeIP, []packetFlowTest{ { name: "pod to NodePort hits both endpoints", sourceIP: "10.0.0.2", @@ -3596,7 +3395,7 @@ func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTable } else { // pod-to-NodePort is (incorrectly) treated as external traffic // when there is no LocalTrafficDetector. - runPacketFlowTests(t, line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, line, ipt, testNodeIP, []packetFlowTest{ { name: "pod to NodePort hits only local endpoint", sourceIP: "10.0.0.2", @@ -5503,7 +5302,7 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { fp.OnEndpointSliceAdd(endpointSlice) fp.syncProxyRules() assertIPTablesRulesEqual(t, tc.line, tc.expectedIPTablesWithSlice, fp.iptablesData.String()) - runPacketFlowTests(t, tc.line, fp.iptablesData.String(), testNodeIP, tc.flowTests) + runPacketFlowTests(t, tc.line, ipt, testNodeIP, tc.flowTests) fp.OnEndpointSliceDelete(endpointSlice) fp.syncProxyRules() @@ -5512,7 +5311,7 @@ func TestInternalTrafficPolicyE2E(t *testing.T) { fp.syncProxyRules() assertIPTablesRulesNotEqual(t, tc.line, tc.expectedIPTablesWithSlice, fp.iptablesData.String()) } - runPacketFlowTests(t, tc.line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, tc.line, ipt, testNodeIP, []packetFlowTest{ { name: "endpoints deleted", sourceIP: "10.0.0.2", @@ -6278,7 +6077,7 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { fp.OnEndpointSliceAdd(testcase.endpointslice) fp.syncProxyRules() assertIPTablesRulesEqual(t, testcase.line, testcase.expectedIPTables, fp.iptablesData.String()) - runPacketFlowTests(t, testcase.line, fp.iptablesData.String(), testNodeIP, testcase.flowTests) + runPacketFlowTests(t, testcase.line, ipt, testNodeIP, testcase.flowTests) fp.OnEndpointSliceDelete(testcase.endpointslice) fp.syncProxyRules() @@ -6288,7 +6087,7 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyLocal(t *testing.T) { } else { assertIPTablesRulesNotEqual(t, testcase.line, testcase.expectedIPTables, fp.iptablesData.String()) } - runPacketFlowTests(t, testcase.line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, testcase.line, ipt, testNodeIP, []packetFlowTest{ { name: "pod to clusterIP after endpoints deleted", sourceIP: "10.0.0.2", @@ -7018,7 +6817,7 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) fp.OnEndpointSliceAdd(testcase.endpointslice) fp.syncProxyRules() assertIPTablesRulesEqual(t, testcase.line, testcase.expectedIPTables, fp.iptablesData.String()) - runPacketFlowTests(t, testcase.line, fp.iptablesData.String(), testNodeIP, testcase.flowTests) + runPacketFlowTests(t, testcase.line, ipt, testNodeIP, testcase.flowTests) fp.OnEndpointSliceDelete(testcase.endpointslice) fp.syncProxyRules() @@ -7028,7 +6827,7 @@ func TestEndpointSliceWithTerminatingEndpointsTrafficPolicyCluster(t *testing.T) } else { assertIPTablesRulesNotEqual(t, testcase.line, testcase.expectedIPTables, fp.iptablesData.String()) } - runPacketFlowTests(t, testcase.line, fp.iptablesData.String(), testNodeIP, []packetFlowTest{ + runPacketFlowTests(t, testcase.line, ipt, testNodeIP, []packetFlowTest{ { name: "pod to clusterIP after endpoints deleted", sourceIP: "10.0.0.2", @@ -7613,7 +7412,7 @@ func TestInternalExternalMasquerade(t *testing.T) { if overridesApplied != len(tc.overrides) { t.Errorf("%d overrides did not match any test case name!", len(tc.overrides)-overridesApplied) } - runPacketFlowTests(t, tc.line, fp.iptablesData.String(), testNodeIP, tcFlowTests) + runPacketFlowTests(t, tc.line, ipt, testNodeIP, tcFlowTests) }) } }