ensure renabled auto-approve routes works (#1670)

.github/workflows/test-integration-v2-TestEnableDisableAutoApprovedRoute.yaml

@@ -0,0 +1,67 @@
+# DO NOT EDIT, generated with cmd/gh-action-integration-generator/main.go
+# To regenerate, run "go generate" in cmd/gh-action-integration-generator/
+
+name: Integration Test v2 - TestEnableDisableAutoApprovedRoute
+
+on: [pull_request]
+
+concurrency:
+  group: ${{ github.workflow }}-$${{ github.head_ref || github.run_id }}
+  cancel-in-progress: true
+
+jobs:
+  TestEnableDisableAutoApprovedRoute:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 2
+
+      - uses: DeterminateSystems/nix-installer-action@main
+      - uses: DeterminateSystems/magic-nix-cache-action@main
+      - uses: satackey/action-docker-layer-caching@main
+        continue-on-error: true
+
+      - name: Get changed files
+        id: changed-files
+        uses: tj-actions/changed-files@v34
+        with:
+          files: |
+            *.nix
+            go.*
+            **/*.go
+            integration_test/
+            config-example.yaml
+
+      - name: Run TestEnableDisableAutoApprovedRoute
+        uses: Wandalen/wretry.action@master
+        if: steps.changed-files.outputs.any_changed == 'true'
+        with:
+          attempt_limit: 5
+          command: |
+            nix develop --command -- docker run \
+              --tty --rm \
+              --volume ~/.cache/hs-integration-go:/go \
+              --name headscale-test-suite \
+              --volume $PWD:$PWD -w $PWD/integration \
+              --volume /var/run/docker.sock:/var/run/docker.sock \
+              --volume $PWD/control_logs:/tmp/control \
+              golang:1 \
+                go run gotest.tools/gotestsum@latest -- ./... \
+                  -failfast \
+                  -timeout 120m \
+                  -parallel 1 \
+                  -run "^TestEnableDisableAutoApprovedRoute$"
+
+      - uses: actions/upload-artifact@v3
+        if: always() && steps.changed-files.outputs.any_changed == 'true'
+        with:
+          name: logs
+          path: "control_logs/*.log"
+
+      - uses: actions/upload-artifact@v3
+        if: always() && steps.changed-files.outputs.any_changed == 'true'
+        with:
+          name: pprof
+          path: "control_logs/*.pprof.tar"

hscontrol/db/routes.go

@@ -639,13 +639,19 @@ func (hsdb *HSDatabase) EnableAutoApprovedRoutes(
 	aclPolicy *policy.ACLPolicy,
 	node *types.Node,
 ) error {
-	hsdb.mu.Lock()
+	if len(aclPolicy.AutoApprovers.ExitNode) == 0 && len(aclPolicy.AutoApprovers.Routes) == 0 {
-	defer hsdb.mu.Unlock()
+		// No autoapprovers configured
+		return nil
+	}
 
 	if len(node.IPAddresses) == 0 {
-		return nil // This node has no IPAddresses, so can't possibly match any autoApprovers ACLs
+		// This node has no IPAddresses, so can't possibly match any autoApprovers ACLs
+		return nil
 	}
 
+	hsdb.mu.Lock()
+	defer hsdb.mu.Unlock()
+
 	routes, err := hsdb.getNodeAdvertisedRoutes(node)
 	if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
 		log.Error().
@@ -657,6 +663,8 @@ func (hsdb *HSDatabase) EnableAutoApprovedRoutes(
 		return err
 	}
 
+	log.Trace().Interface("routes", routes).Msg("routes for autoapproving")
+
 	approvedRoutes := types.Routes{}
 
 	for _, advertisedRoute := range routes {
@@ -676,6 +684,13 @@ func (hsdb *HSDatabase) EnableAutoApprovedRoutes(
 			return err
 		}
 
+		log.Trace().
+			Str("node", node.Hostname).
+			Str("user", node.User.Name).
+			Strs("routeApprovers", routeApprovers).
+			Str("prefix", netip.Prefix(advertisedRoute.Prefix).String()).
+			Msg("looking up route for autoapproving")
+
 		for _, approvedAlias := range routeApprovers {
 			if approvedAlias == node.User.Name {
 				approvedRoutes = append(approvedRoutes, advertisedRoute)

hscontrol/poll.go

@@ -125,6 +125,14 @@ func (h *Headscale) handlePoll(
 
 					return
 				}
+
+				if h.ACLPolicy != nil {
+					// update routes with peer information
+					err = h.db.EnableAutoApprovedRoutes(h.ACLPolicy, node)
+					if err != nil {
+						logErr(err, "Error running auto approved routes")
+					}
+				}
 			}
 
 			// Services is mostly useful for discovery and not critical,

integration/route_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
+	"github.com/juanfont/headscale/hscontrol/policy"
 	"github.com/juanfont/headscale/integration/hsic"
 	"github.com/juanfont/headscale/integration/tsic"
 	"github.com/stretchr/testify/assert"
@@ -778,3 +779,145 @@ func TestHASubnetRouterFailover(t *testing.T) {
 		)
 	}
 }
+
+func TestEnableDisableAutoApprovedRoute(t *testing.T) {
+	IntegrationSkip(t)
+	t.Parallel()
+
+	expectedRoutes := "172.0.0.0/24"
+
+	user := "enable-disable-routing"
+
+	scenario, err := NewScenario()
+	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{
+					expectedRoutes: {"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)
+
+	// All routes should be auto approved and enabled
+	assert.Equal(t, true, routes[0].GetAdvertised())
+	assert.Equal(t, true, routes[0].GetEnabled())
+	assert.Equal(t, true, routes[0].GetIsPrimary())
+
+	// Stop advertising route
+	command = []string{
+		"tailscale",
+		"set",
+		"--advertise-routes=",
+	}
+	_, _, err = subRouter1.Execute(command)
+	assertNoErrf(t, "failed to remove advertised route: %s", err)
+
+	time.Sleep(10 * time.Second)
+
+	var notAdvertisedRoutes []*v1.Route
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"routes",
+			"list",
+			"--output",
+			"json",
+		},
+		&notAdvertisedRoutes,
+	)
+	assertNoErr(t, err)
+	assert.Len(t, notAdvertisedRoutes, 1)
+
+	// Route is no longer advertised
+	assert.Equal(t, false, notAdvertisedRoutes[0].GetAdvertised())
+	assert.Equal(t, false, notAdvertisedRoutes[0].GetEnabled())
+	assert.Equal(t, true, notAdvertisedRoutes[0].GetIsPrimary())
+
+	// Advertise route again
+	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 reAdvertisedRoutes []*v1.Route
+	err = executeAndUnmarshal(
+		headscale,
+		[]string{
+			"headscale",
+			"routes",
+			"list",
+			"--output",
+			"json",
+		},
+		&reAdvertisedRoutes,
+	)
+	assertNoErr(t, err)
+	assert.Len(t, reAdvertisedRoutes, 1)
+
+	// All routes should be auto approved and enabled
+	assert.Equal(t, true, reAdvertisedRoutes[0].GetAdvertised())
+	assert.Equal(t, true, reAdvertisedRoutes[0].GetEnabled())
+	assert.Equal(t, true, reAdvertisedRoutes[0].GetIsPrimary())
+}