Skip to content

Commit

Permalink
2068 AutoApprovers tests (#2105)
Browse files Browse the repository at this point in the history 
* replace old suite approved routes test with table driven

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* add test to reproduce issue

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

* add integration test for 2068

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>

---------

Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
  • Loading branch information
@kradalby
kradalby authored Sep 5, 2024
1 parent adc084f commit f368ed0
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 69 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test-integration.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ jobs:
- TestEnablingRoutes
- TestHASubnetRouterFailover
- TestEnableDisableAutoApprovedRoute
- TestAutoApprovedSubRoute2068
- TestSubnetRouteACL
- TestHeadscale
- TestCreateTailscale
Expand Down
150 changes: 101 additions & 49 deletions hscontrol/db/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"math/big"
"net/netip"
"regexp"
"sort"
"strconv"
"sync"
"testing"
Expand Down Expand Up @@ -518,8 +519,37 @@ func TestHeadscale_generateGivenName(t *testing.T) {
}
}

func (s *Suite) TestAutoApproveRoutes(c *check.C) {
acl := []byte(`
func TestAutoApproveRoutes(t *testing.T) {
tests := []struct {
name string
acl string
routes []netip.Prefix
want []netip.Prefix
}{
{
name: "2068-approve-issue-sub",
acl: `
{
"groups": {
"group:k8s": ["test"]
},
"acls": [
{"action": "accept", "users": ["*"], "ports": ["*:*"]},
],
"autoApprovers": {
"routes": {
"10.42.0.0/16": ["test"],
}
}
}`,
routes: []netip.Prefix{netip.MustParsePrefix("10.42.7.0/24")},
want: []netip.Prefix{netip.MustParsePrefix("10.42.7.0/24")},
},
{
name: "2068-approve-issue-sub",
acl: `
{
"tagOwners": {
"tag:exit": ["test"],
Expand All @@ -540,61 +570,83 @@ func (s *Suite) TestAutoApproveRoutes(c *check.C) {
"10.11.0.0/16": ["test"],
}
}
}
`)

pol, err := policy.LoadACLPolicyFromBytes(acl)
c.Assert(err, check.IsNil)
c.Assert(pol, check.NotNil)

user, err := db.CreateUser("test")
c.Assert(err, check.IsNil)

pak, err := db.CreatePreAuthKey(user.Name, false, false, nil, nil)
c.Assert(err, check.IsNil)
}`,
routes: []netip.Prefix{
netip.MustParsePrefix("0.0.0.0/0"),
netip.MustParsePrefix("::/0"),
netip.MustParsePrefix("10.10.0.0/16"),
netip.MustParsePrefix("10.11.0.0/24"),
},
want: []netip.Prefix{
netip.MustParsePrefix("::/0"),
netip.MustParsePrefix("10.11.0.0/24"),
netip.MustParsePrefix("10.10.0.0/16"),
netip.MustParsePrefix("0.0.0.0/0"),
},
},
}

nodeKey := key.NewNode()
machineKey := key.NewMachine()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
adb, err := newTestDB()
assert.NoError(t, err)
pol, err := policy.LoadACLPolicyFromBytes([]byte(tt.acl))

assert.NoError(t, err)
assert.NotNil(t, pol)

user, err := adb.CreateUser("test")
assert.NoError(t, err)

pak, err := adb.CreatePreAuthKey(user.Name, false, false, nil, nil)
assert.NoError(t, err)

nodeKey := key.NewNode()
machineKey := key.NewMachine()

v4 := netip.MustParseAddr("100.64.0.1")
node := types.Node{
ID: 0,
MachineKey: machineKey.Public(),
NodeKey: nodeKey.Public(),
Hostname: "test",
UserID: user.ID,
RegisterMethod: util.RegisterMethodAuthKey,
AuthKeyID: ptr.To(pak.ID),
Hostinfo: &tailcfg.Hostinfo{
RequestTags: []string{"tag:exit"},
RoutableIPs: tt.routes,
},
IPv4: &v4,
}

defaultRouteV4 := netip.MustParsePrefix("0.0.0.0/0")
defaultRouteV6 := netip.MustParsePrefix("::/0")
route1 := netip.MustParsePrefix("10.10.0.0/16")
// Check if a subprefix of an autoapproved route is approved
route2 := netip.MustParsePrefix("10.11.0.0/24")
trx := adb.DB.Save(&node)
assert.NoError(t, trx.Error)

v4 := netip.MustParseAddr("100.64.0.1")
node := types.Node{
ID: 0,
MachineKey: machineKey.Public(),
NodeKey: nodeKey.Public(),
Hostname: "test",
UserID: user.ID,
RegisterMethod: util.RegisterMethodAuthKey,
AuthKeyID: ptr.To(pak.ID),
Hostinfo: &tailcfg.Hostinfo{
RequestTags: []string{"tag:exit"},
RoutableIPs: []netip.Prefix{defaultRouteV4, defaultRouteV6, route1, route2},
},
IPv4: &v4,
}
sendUpdate, err := adb.SaveNodeRoutes(&node)
assert.NoError(t, err)
assert.False(t, sendUpdate)

trx := db.DB.Save(&node)
c.Assert(trx.Error, check.IsNil)
node0ByID, err := adb.GetNodeByID(0)
assert.NoError(t, err)

sendUpdate, err := db.SaveNodeRoutes(&node)
c.Assert(err, check.IsNil)
c.Assert(sendUpdate, check.Equals, false)
// TODO(kradalby): Check state update
err = adb.EnableAutoApprovedRoutes(pol, node0ByID)
assert.NoError(t, err)

node0ByID, err := db.GetNodeByID(0)
c.Assert(err, check.IsNil)
enabledRoutes, err := adb.GetEnabledRoutes(node0ByID)
assert.NoError(t, err)
assert.Len(t, enabledRoutes, len(tt.want))

// TODO(kradalby): Check state update
err = db.EnableAutoApprovedRoutes(pol, node0ByID)
c.Assert(err, check.IsNil)
sort.Slice(enabledRoutes, func(i, j int) bool {
return util.ComparePrefix(enabledRoutes[i], enabledRoutes[j]) > 0
})

enabledRoutes, err := db.GetEnabledRoutes(node0ByID)
c.Assert(err, check.IsNil)
c.Assert(enabledRoutes, check.HasLen, 4)
if diff := cmp.Diff(tt.want, enabledRoutes, util.Comparers...); diff != "" {
t.Errorf("unexpected enabled routes (-want +got):\n%s", diff)
}
})
}
}

func TestEphemeralGarbageCollectorOrder(t *testing.T) {
Expand Down
23 changes: 3 additions & 20 deletions hscontrol/poll.go
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
package hscontrol

import (
"cmp"
"context"
"fmt"
"math/rand/v2"
"net/http"
"net/netip"
"sort"
"strings"
"time"

"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/mapper"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/rs/zerolog/log"
"github.com/sasha-s/go-deadlock"
xslices "golang.org/x/exp/slices"
Expand Down Expand Up @@ -742,10 +741,10 @@ func hostInfoChanged(old, new *tailcfg.Hostinfo) (bool, bool) {
newRoutes := new.RoutableIPs

sort.Slice(oldRoutes, func(i, j int) bool {
return comparePrefix(oldRoutes[i], oldRoutes[j]) > 0
return util.ComparePrefix(oldRoutes[i], oldRoutes[j]) > 0
})
sort.Slice(newRoutes, func(i, j int) bool {
return comparePrefix(newRoutes[i], newRoutes[j]) > 0
return util.ComparePrefix(newRoutes[i], newRoutes[j]) > 0
})

if !xslices.Equal(oldRoutes, newRoutes) {
Expand All @@ -764,19 +763,3 @@ func hostInfoChanged(old, new *tailcfg.Hostinfo) (bool, bool) {

return false, false
}

// TODO(kradalby): Remove after go 1.23, will be in stdlib.
// Compare returns an integer comparing two prefixes.
// The result will be 0 if p == p2, -1 if p < p2, and +1 if p > p2.
// Prefixes sort first by validity (invalid before valid), then
// address family (IPv4 before IPv6), then prefix length, then
// address.
func comparePrefix(p, p2 netip.Prefix) int {
if c := cmp.Compare(p.Addr().BitLen(), p2.Addr().BitLen()); c != 0 {
return c
}
if c := cmp.Compare(p.Bits(), p2.Bits()); c != 0 {
return c
}
return p.Addr().Compare(p2.Addr())
}
19 changes: 19 additions & 0 deletions hscontrol/util/net.go
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
package util

import (
"cmp"
"context"
"net"
"net/netip"
)

func GrpcSocketDialer(ctx context.Context, addr string) (net.Conn, error) {
var d net.Dialer

return d.DialContext(ctx, "unix", addr)
}


// TODO(kradalby): Remove after go 1.24, will be in stdlib.
// Compare returns an integer comparing two prefixes.
// The result will be 0 if p == p2, -1 if p < p2, and +1 if p > p2.
// Prefixes sort first by validity (invalid before valid), then
// address family (IPv4 before IPv6), then prefix length, then
// address.
func ComparePrefix(p, p2 netip.Prefix) int {
if c := cmp.Compare(p.Addr().BitLen(), p2.Addr().BitLen()); c != 0 {
return c
}
if c := cmp.Compare(p.Bits(), p2.Bits()); c != 0 {
return c
}
return p.Addr().Compare(p2.Addr())
}
90 changes: 90 additions & 0 deletions integration/route_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
"github.com/juanfont/headscale/hscontrol/policy"
"github.com/juanfont/headscale/hscontrol/util"
Expand Down Expand Up @@ -957,6 +958,95 @@ func TestEnableDisableAutoApprovedRoute(t *testing.T) {
assert.Equal(t, true, reAdvertisedRoutes[0].GetIsPrimary())
}

func TestAutoApprovedSubRoute2068(t *testing.T) {
IntegrationSkip(t)
t.Parallel()

expectedRoutes := "10.42.7.0/24"

user := "subroute"

scenario, err := NewScenario(dockertestMaxWait())
assertNoErrf(t, "failed to create scenario: %s", err)
defer scenario.Shutdown()

spec := map[string]int{
user: 1,
}

err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{tsic.WithTags([]string{"tag:approve"})}, hsic.WithTestName("clienableroute"), hsic.WithACLPolicy(
&policy.ACLPolicy{
ACLs: []policy.ACL{
{
Action: "accept",
Sources: []string{"*"},
Destinations: []string{"*:*"},
},
},
TagOwners: map[string][]string{
"tag:approve": {user},
},
AutoApprovers: policy.AutoApprovers{
Routes: map[string][]string{
"10.42.0.0/16": {"tag:approve"},
},
},
},
))
assertNoErrHeadscaleEnv(t, err)

allClients, err := scenario.ListTailscaleClients()
assertNoErrListClients(t, err)

err = scenario.WaitForTailscaleSync()
assertNoErrSync(t, err)

headscale, err := scenario.Headscale()
assertNoErrGetHeadscale(t, err)

subRouter1 := allClients[0]

// Initially advertise route
command := []string{
"tailscale",
"set",
"--advertise-routes=" + expectedRoutes,
}
_, _, err = subRouter1.Execute(command)
assertNoErrf(t, "failed to advertise route: %s", err)

time.Sleep(10 * time.Second)

var routes []*v1.Route
err = executeAndUnmarshal(
headscale,
[]string{
"headscale",
"routes",
"list",
"--output",
"json",
},
&routes,
)
assertNoErr(t, err)
assert.Len(t, routes, 1)

want := []*v1.Route{
{
Id: 1,
Prefix: expectedRoutes,
Advertised: true,
Enabled: true,
IsPrimary: true,
},
}

if diff := cmp.Diff(want, routes, cmpopts.IgnoreUnexported(v1.Route{}), cmpopts.IgnoreFields(v1.Route{}, "Node", "CreatedAt", "UpdatedAt", "DeletedAt")); diff != "" {
t.Errorf("unexpected routes (-want +got):\n%s", diff)
}
}

// TestSubnetRouteACL verifies that Subnet routes are distributed
// as expected when ACLs are activated.
// It implements the issue from
Expand Down

0 comments on commit f368ed0

Please sign in to comment.