pkg/route,pkg/mesh: replace NAT with ip rules

This commit entirely replaces NAT in Kilo with a few iproute2 rules. Previously, Kilo would source-NAT the majority of packets in order to avoid problems with strict source checks in cloud providers causing packets to be considered martians. This source-NAT-ing made it difficult to correctly apply Kuberenetes NetworkPolicies based on source IPs. This rewrite instead relies on a handful of iproute2 rules to ensure that packets get encapsulated in certain scenarios based on the source network and/or source interface. This has the benefit of avoiding extra iptables bloat as well as enabling better compatibility with NetworkPolicies. Signed-off-by: Lucas Servén Marín <lserven@gmail.com>
2020-02-20 21:27:50 +01:00
parent 4857d10da1
commit 134cbe90be
5 changed files with 605 additions and 151 deletions
--- a/pkg/route/route.go
+++ b/pkg/route/route.go
@@ -28,22 +28,24 @@ import (
 type Table struct {
 	errors     chan error
 	mu         sync.Mutex
-	routes     map[string]*netlink.Route
+	rs         map[string]interface{}
 	subscribed bool

 	// Make these functions fields to allow
 	// for testing.
-	add func(*netlink.Route) error
-	del func(*netlink.Route) error
+	addRoute func(*netlink.Route) error
+	delRoute func(*netlink.Route) error
+	addRule  func(*netlink.Rule) error
+	delRule  func(*netlink.Rule) error
 }

 // NewTable generates a new table.
 func NewTable() *Table {
 	return &Table{
-		errors: make(chan error),
-		routes: make(map[string]*netlink.Route),
-		add:    netlink.RouteReplace,
-		del: func(r *netlink.Route) error {
+		errors:   make(chan error),
+		rs:       make(map[string]interface{}),
+		addRoute: netlink.RouteReplace,
+		delRoute: func(r *netlink.Route) error {
 			name := routeToString(r)
 			if name == "" {
 				return errors.New("attempting to delete invalid route")
@@ -59,10 +61,27 @@ func NewTable() *Table {
 			}
 			return nil
 		},
+		addRule: netlink.RuleAdd,
+		delRule: func(r *netlink.Rule) error {
+			name := ruleToString(r)
+			if name == "" {
+				return errors.New("attempting to delete invalid rule")
+			}
+			rules, err := netlink.RuleList(netlink.FAMILY_ALL)
+			if err != nil {
+				return fmt.Errorf("failed to list rules before deletion: %v", err)
+			}
+			for _, rule := range rules {
+				if ruleToString(&rule) == name {
+					return netlink.RuleDel(r)
+				}
+			}
+			return nil
+		},
 	}
 }

-// Run watches for changes to routes in the table and reconciles
+// Run watches for changes to routes and rules in the table and reconciles
 // the table against the desired state.
 func (t *Table) Run(stop <-chan struct{}) (<-chan error, error) {
 	t.mu.Lock()
@@ -90,16 +109,19 @@ func (t *Table) Run(stop <-chan struct{}) (<-chan error, error) {
 			// Watch for deleted routes to reconcile this table's routes.
 			case unix.RTM_DELROUTE:
 				t.mu.Lock()
-				for _, r := range t.routes {
-					// Filter out invalid routes.
-					if r == nil || r.Dst == nil {
-						continue
-					}
-					// If any deleted route's destination matches a destination
-					// in the table, reset the corresponding route just in case.
-					if r.Dst.IP.Equal(e.Route.Dst.IP) && r.Dst.Mask.String() == e.Route.Dst.Mask.String() {
-						if err := t.add(r); err != nil {
-							nonBlockingSend(t.errors, fmt.Errorf("failed add route: %v", err))
+				for k := range t.rs {
+					switch r := t.rs[k].(type) {
+					case *netlink.Route:
+						// Filter out invalid routes.
+						if r == nil || r.Dst == nil {
+							continue
+						}
+						// If any deleted route's destination matches a destination
+						// in the table, reset the corresponding route just in case.
+						if r.Dst.IP.Equal(e.Route.Dst.IP) && r.Dst.Mask.String() == e.Route.Dst.Mask.String() {
+							if err := t.addRoute(r); err != nil {
+								nonBlockingSend(t.errors, fmt.Errorf("failed add route: %v", err))
+							}
 						}
 					}
 				}
@@ -110,46 +132,66 @@ func (t *Table) Run(stop <-chan struct{}) (<-chan error, error) {
 	return t.errors, nil
 }

-// CleanUp will clean up any routes created by the instance.
+// CleanUp will clean up any routes and rules created by the instance.
 func (t *Table) CleanUp() error {
 	t.mu.Lock()
 	defer t.mu.Unlock()
-	for k, route := range t.routes {
-		if err := t.del(route); err != nil {
-			return fmt.Errorf("failed to delete route: %v", err)
+	for k := range t.rs {
+		switch r := t.rs[k].(type) {
+		case *netlink.Route:
+			if err := t.delRoute(r); err != nil {
+				return fmt.Errorf("failed to delete route: %v", err)
+			}
+		case *netlink.Rule:
+			if err := t.delRule(r); err != nil {
+				return fmt.Errorf("failed to delete rule: %v", err)
+			}
 		}
-		delete(t.routes, k)
+		delete(t.rs, k)
 	}
 	return nil
 }

-// Set idempotently overwrites any routes previously defined
-// for the table with the given set of routes.
-func (t *Table) Set(routes []*netlink.Route) error {
-	r := make(map[string]*netlink.Route)
+// Set idempotently overwrites any routes and rules previously defined
+// for the table with the given set of routes and rules.
+func (t *Table) Set(routes []*netlink.Route, rules []*netlink.Rule) error {
+	rs := make(map[string]interface{})
 	for _, route := range routes {
 		if route == nil {
 			continue
 		}
-		r[routeToString(route)] = route
+		rs[routeToString(route)] = route
+	}
+	for _, rule := range rules {
+		if rule == nil {
+			continue
+		}
+		rs[ruleToString(rule)] = rule
 	}
 	t.mu.Lock()
 	defer t.mu.Unlock()
-	for k := range t.routes {
-		if _, ok := r[k]; !ok {
-			if err := t.del(t.routes[k]); err != nil {
-				return fmt.Errorf("failed to delete route: %v", err)
+	for k := range t.rs {
+		if _, ok := rs[k]; !ok {
+			switch r := t.rs[k].(type) {
+			case *netlink.Route:
+				if err := t.delRoute(r); err != nil {
+					return fmt.Errorf("failed to delete route: %v", err)
+				}
+			case *netlink.Rule:
+				if err := t.delRule(r); err != nil {
+					return fmt.Errorf("failed to delete rule: %v", err)
+				}
 			}
-			delete(t.routes, k)
+			delete(t.rs, k)
 		}
 	}

-	// When adding routes, we need to compare against what is
+	// When adding routes/rules, we need to compare against what is
 	// actually on the Linux routing table. This is because
-	// routes can be deleted by the kernel due to interface churn
-	// causing a situation where the controller thinks it has a route
+	// routes/rules can be deleted by the kernel due to interface churn
+	// causing a situation where the controller thinks it has an item
 	// that is not actually there.
-	existing := make(map[string]*netlink.Route)
+	existing := make(map[string]interface{})
 	existingRoutes, err := netlink.RouteList(nil, netlink.FAMILY_ALL)
 	if err != nil {
 		return fmt.Errorf("failed to list existing routes: %v", err)
@@ -158,12 +200,27 @@ func (t *Table) Set(routes []*netlink.Route) error {
 		existing[routeToString(&existingRoutes[k])] = &existingRoutes[k]
 	}

-	for k := range r {
+	existingRules, err := netlink.RuleList(netlink.FAMILY_ALL)
+	if err != nil {
+		return fmt.Errorf("failed to list existing rules: %v", err)
+	}
+	for k := range existingRules {
+		existing[ruleToString(&existingRules[k])] = &existingRules[k]
+	}
+
+	for k := range rs {
 		if _, ok := existing[k]; !ok {
-			if err := t.add(r[k]); err != nil {
-				return fmt.Errorf("failed to add route %q: %v", routeToString(r[k]), err)
+			switch r := rs[k].(type) {
+			case *netlink.Route:
+				if err := t.addRoute(r); err != nil {
+					return fmt.Errorf("failed to add route %q: %v", k, err)
+				}
+			case *netlink.Rule:
+				if err := t.addRule(r); err != nil {
+					return fmt.Errorf("failed to add rule %q: %v", k, err)
+				}
 			}
-			t.routes[k] = r[k]
+			t.rs[k] = rs[k]
 		}
 	}
 	return nil
@@ -190,3 +247,18 @@ func routeToString(route *netlink.Route) string {
 	}
 	return fmt.Sprintf("dst: %s, via: %s, src: %s, dev: %d", route.Dst.String(), gw, src, route.LinkIndex)
 }
+
+func ruleToString(rule *netlink.Rule) string {
+	if rule == nil || (rule.Src == nil && rule.Dst == nil) {
+		return ""
+	}
+	src := "-"
+	if rule.Src != nil {
+		src = rule.Src.String()
+	}
+	dst := "-"
+	if rule.Dst != nil {
+		dst = rule.Dst.String()
+	}
+	return fmt.Sprintf("src: %s, dst: %s, table: %d, input: %s", src, dst, rule.Table, rule.IifName)
+}
--- a/pkg/route/route_test.go
+++ b/pkg/route/route_test.go
@@ -31,36 +31,54 @@ func TestSet(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to parse CIDR: %v", err)
 	}
-	add := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	addRoute := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			backend[routeToString(r)] = r
 			return nil
 		}
 	}
-	del := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	delRoute := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			delete(backend, routeToString(r))
 			return nil
 		}
 	}
-	adderr := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	addRule := func(backend map[string]interface{}) func(*netlink.Rule) error {
+		return func(r *netlink.Rule) error {
+			backend[ruleToString(r)] = r
+			return nil
+		}
+	}
+	delRule := func(backend map[string]interface{}) func(*netlink.Rule) error {
+		return func(r *netlink.Rule) error {
+			delete(backend, ruleToString(r))
+			return nil
+		}
+	}
+	adderr := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			return errors.New(routeToString(r))
 		}
 	}
 	for _, tc := range []struct {
-		name   string
-		routes []*netlink.Route
-		err    bool
-		add    func(map[string]*netlink.Route) func(*netlink.Route) error
-		del    func(map[string]*netlink.Route) func(*netlink.Route) error
+		name     string
+		routes   []*netlink.Route
+		rules    []*netlink.Rule
+		err      bool
+		addRoute func(map[string]interface{}) func(*netlink.Route) error
+		delRoute func(map[string]interface{}) func(*netlink.Route) error
+		addRule  func(map[string]interface{}) func(*netlink.Rule) error
+		delRule  func(map[string]interface{}) func(*netlink.Rule) error
 	}{
 		{
-			name:   "empty",
-			routes: nil,
-			err:    false,
-			add:    add,
-			del:    del,
+			name:     "empty",
+			routes:   nil,
+			rules:    nil,
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "single",
@@ -70,9 +88,17 @@ func TestSet(t *testing.T) {
 					Gw:  net.ParseIP("10.1.0.1"),
 				},
 			},
-			err: false,
-			add: add,
-			del: del,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+			},
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "multiple",
@@ -86,16 +112,30 @@ func TestSet(t *testing.T) {
 					Gw:  net.ParseIP("127.0.0.1"),
 				},
 			},
-			err: false,
-			add: add,
-			del: del,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+				{
+					Src:   c2,
+					Table: 2,
+				},
+			},
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
-			name:   "err empty",
-			routes: nil,
-			err:    false,
-			add:    adderr,
-			del:    del,
+			name:     "err empty",
+			routes:   nil,
+			err:      false,
+			addRoute: adderr,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "err",
@@ -109,18 +149,30 @@ func TestSet(t *testing.T) {
 					Gw:  net.ParseIP("127.0.0.1"),
 				},
 			},
-			err: true,
-			add: adderr,
-			del: del,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+				{
+					Src:   c2,
+					Table: 2,
+				},
+			},
+			err:      true,
+			addRoute: adderr,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 	} {
-		backend := make(map[string]*netlink.Route)
-		a := tc.add(backend)
-		d := tc.del(backend)
+		backend := make(map[string]interface{})
 		table := NewTable()
-		table.add = a
-		table.del = d
-		if err := table.Set(tc.routes); (err != nil) != tc.err {
+		table.addRoute = tc.addRoute(backend)
+		table.delRoute = tc.delRoute(backend)
+		table.addRule = tc.addRule(backend)
+		table.delRule = tc.delRule(backend)
+		if err := table.Set(tc.routes, tc.rules); (err != nil) != tc.err {
 			no := "no"
 			if tc.err {
 				no = "an"
@@ -131,11 +183,18 @@ func TestSet(t *testing.T) {
 		if !tc.err {
 			for _, r := range tc.routes {
 				r1 := backend[routeToString(r)]
-				r2 := table.routes[routeToString(r)]
+				r2 := table.rs[routeToString(r)]
 				if r != r1 || r != r2 {
 					t.Errorf("test case %q: expected all routes to be equal: expected %v, got %v and %v", tc.name, r, r1, r2)
 				}
 			}
+			for _, r := range tc.rules {
+				r1 := backend[ruleToString(r)]
+				r2 := table.rs[ruleToString(r)]
+				if r != r1 || r != r2 {
+					t.Errorf("test case %q: expected all rules to be equal: expected %v, got %v and %v", tc.name, r, r1, r2)
+				}
+			}
 		}
 	}
 }
@@ -149,36 +208,53 @@ func TestCleanUp(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to parse CIDR: %v", err)
 	}
-	add := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	addRoute := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			backend[routeToString(r)] = r
 			return nil
 		}
 	}
-	del := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	delRoute := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			delete(backend, routeToString(r))
 			return nil
 		}
 	}
-	delerr := func(backend map[string]*netlink.Route) func(*netlink.Route) error {
+	addRule := func(backend map[string]interface{}) func(*netlink.Rule) error {
+		return func(r *netlink.Rule) error {
+			backend[ruleToString(r)] = r
+			return nil
+		}
+	}
+	delRule := func(backend map[string]interface{}) func(*netlink.Rule) error {
+		return func(r *netlink.Rule) error {
+			delete(backend, ruleToString(r))
+			return nil
+		}
+	}
+	delerr := func(backend map[string]interface{}) func(*netlink.Route) error {
 		return func(r *netlink.Route) error {
 			return errors.New(routeToString(r))
 		}
 	}
 	for _, tc := range []struct {
-		name   string
-		routes []*netlink.Route
-		err    bool
-		add    func(map[string]*netlink.Route) func(*netlink.Route) error
-		del    func(map[string]*netlink.Route) func(*netlink.Route) error
+		name     string
+		routes   []*netlink.Route
+		rules    []*netlink.Rule
+		err      bool
+		addRoute func(map[string]interface{}) func(*netlink.Route) error
+		delRoute func(map[string]interface{}) func(*netlink.Route) error
+		addRule  func(map[string]interface{}) func(*netlink.Rule) error
+		delRule  func(map[string]interface{}) func(*netlink.Rule) error
 	}{
 		{
-			name:   "empty",
-			routes: nil,
-			err:    false,
-			add:    add,
-			del:    del,
+			name:     "empty",
+			routes:   nil,
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "single",
@@ -188,9 +264,17 @@ func TestCleanUp(t *testing.T) {
 					Gw:  net.ParseIP("10.1.0.1"),
 				},
 			},
-			err: false,
-			add: add,
-			del: del,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+			},
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "multiple",
@@ -204,16 +288,30 @@ func TestCleanUp(t *testing.T) {
 					Gw:  net.ParseIP("127.0.0.1"),
 				},
 			},
-			err: false,
-			add: add,
-			del: del,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+				{
+					Src:   c2,
+					Table: 2,
+				},
+			},
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
-			name:   "err empty",
-			routes: nil,
-			err:    false,
-			add:    add,
-			del:    delerr,
+			name:     "err empty",
+			routes:   nil,
+			err:      false,
+			addRoute: addRoute,
+			delRoute: delRoute,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 		{
 			name: "err",
@@ -227,18 +325,30 @@ func TestCleanUp(t *testing.T) {
 					Gw:  net.ParseIP("127.0.0.1"),
 				},
 			},
-			err: true,
-			add: add,
-			del: delerr,
+			rules: []*netlink.Rule{
+				{
+					Src:   c1,
+					Table: 1,
+				},
+				{
+					Src:   c2,
+					Table: 2,
+				},
+			},
+			err:      true,
+			addRoute: addRoute,
+			delRoute: delerr,
+			addRule:  addRule,
+			delRule:  delRule,
 		},
 	} {
-		backend := make(map[string]*netlink.Route)
-		a := tc.add(backend)
-		d := tc.del(backend)
+		backend := make(map[string]interface{})
 		table := NewTable()
-		table.add = a
-		table.del = d
-		if err := table.Set(tc.routes); err != nil {
+		table.addRoute = tc.addRoute(backend)
+		table.delRoute = tc.delRoute(backend)
+		table.addRule = tc.addRule(backend)
+		table.delRule = tc.delRule(backend)
+		if err := table.Set(tc.routes, tc.rules); err != nil {
 			t.Fatalf("test case %q: Set should not fail: %v", tc.name, err)
 		}
 		if err := table.CleanUp(); (err != nil) != tc.err {
@@ -252,9 +362,18 @@ func TestCleanUp(t *testing.T) {
 		if !tc.err {
 			for _, r := range tc.routes {
 				r1 := backend[routeToString(r)]
-				r2 := table.routes[routeToString(r)]
+				r2 := table.rs[routeToString(r)]
 				if r1 != nil || r2 != nil {
-					t.Errorf("test case %q: expected all routes to be nil: expected got %v and %v", tc.name, r1, r2)
+					t.Errorf("test case %q: expected all routes to be nil: expected nil, got %v and %v", tc.name, r1, r2)
+				}
+			}
+		}
+		if !tc.err {
+			for _, r := range tc.rules {
+				r1 := backend[ruleToString(r)]
+				r2 := table.rs[ruleToString(r)]
+				if r1 != nil || r2 != nil {
+					t.Errorf("test case %q: expected all rules to be nil: expected nil, got %v and %v", tc.name, r1, r2)
 				}
 			}
 		}