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

@@ -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)
 			}
 		}
 	}

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

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
+}

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)
+				}
+			}
+		})
+	}
+}