Merge 24f3895b2b into e2d5ee0927

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

hscontrol/app.go

@@ -154,6 +154,10 @@ func NewHeadscale(cfg *types.Config) (*Headscale, error) {
 		}
 	})
 
+	if err = app.loadPolicyManager(); err != nil {
+		return nil, fmt.Errorf("failed to load ACL policy: %w", err)
+	}
+
 	var authProvider AuthProvider
 	authProvider = NewAuthProviderWeb(cfg.ServerURL)
 	if cfg.OIDC.Issuer != "" {
@@ -531,11 +535,6 @@ func (h *Headscale) Serve() error {
 		}
 	}
 
-	var err error
-	if err = h.loadPolicyManager(); err != nil {
-		return fmt.Errorf("failed to load ACL policy: %w", err)
-	}
-
 	if dumpConfig {
 		spew.Dump(h.cfg)
 	}

hscontrol/policy/pm.go

@@ -165,6 +165,9 @@ func (pm *PolicyManagerV1) ApproversForRoute(route netip.Prefix) []string {
 	// TODO(kradalby): This can be a parse error of the address in the policy,
 	// in the new policy this will be typed and not a problem, in this policy
 	// we will just return empty list
+	if pm.pol == nil {
+		return nil
+	}
 	approvers, _ := pm.pol.AutoApprovers.GetRouteApprovers(route)
 	return approvers
 }

integration/cli_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/juanfont/headscale/integration/hsic"
 	"github.com/juanfont/headscale/integration/tsic"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/exp/slices"
 )
 
 func executeAndUnmarshal[T any](headscale ControlServer, command []string, result T) error {
@@ -786,117 +787,85 @@ func TestNodeTagCommand(t *testing.T) {
 	)
 }
 
-func TestNodeAdvertiseTagNoACLCommand(t *testing.T) {
+func TestNodeAdvertiseTagCommand(t *testing.T) {
 	IntegrationSkip(t)
 	t.Parallel()
 
-	scenario, err := NewScenario(dockertestMaxWait())
-	assertNoErr(t, err)
-	defer scenario.ShutdownAssertNoPanics(t)
-
-	spec := map[string]int{
-		"user1": 1,
-	}
-
-	err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{tsic.WithTags([]string{"tag:test"})}, hsic.WithTestName("cliadvtags"))
-	assertNoErr(t, err)
-
-	headscale, err := scenario.Headscale()
-	assertNoErr(t, err)
-
-	// Test list all nodes after added seconds
-	resultMachines := make([]*v1.Node, spec["user1"])
-	err = executeAndUnmarshal(
-		headscale,
-		[]string{
-			"headscale",
-			"nodes",
-			"list",
-			"--tags",
-			"--output", "json",
+	tests := []struct {
+		name    string
+		policy  *policy.ACLPolicy
+		wantTag bool
+	}{
+		{
+			name:    "no-policy",
+			wantTag: false,
 		},
-		&resultMachines,
-	)
-	assert.Nil(t, err)
-	found := false
-	for _, node := range resultMachines {
-		if node.GetInvalidTags() != nil {
-			for _, tag := range node.GetInvalidTags() {
-				if tag == "tag:test" {
-					found = true
-				}
-			}
-		}
-	}
-	assert.Equal(
-		t,
-		true,
-		found,
-		"should not find a node with the tag 'tag:test' in the list of nodes",
-	)
-}
-
-func TestNodeAdvertiseTagWithACLCommand(t *testing.T) {
-	IntegrationSkip(t)
-	t.Parallel()
-
-	scenario, err := NewScenario(dockertestMaxWait())
-	assertNoErr(t, err)
-	defer scenario.ShutdownAssertNoPanics(t)
-
-	spec := map[string]int{
-		"user1": 1,
-	}
-
-	err = scenario.CreateHeadscaleEnv(spec, []tsic.Option{tsic.WithTags([]string{"tag:exists"})}, hsic.WithTestName("cliadvtags"), hsic.WithACLPolicy(
-		&policy.ACLPolicy{
-			ACLs: []policy.ACL{
-				{
-					Action:       "accept",
-					Sources:      []string{"*"},
-					Destinations: []string{"*:*"},
+		{
+			name: "with-policy",
+			policy: &policy.ACLPolicy{
+				ACLs: []policy.ACL{
+					{
+						Action:       "accept",
+						Sources:      []string{"*"},
+						Destinations: []string{"*:*"},
+					},
+				},
+				TagOwners: map[string][]string{
+					"tag:test": {"user1"},
 				},
 			},
-			TagOwners: map[string][]string{
-				"tag:exists": {"user1"},
-			},
+			wantTag: true,
 		},
-	))
-	assertNoErr(t, err)
+	}
 
-	headscale, err := scenario.Headscale()
-	assertNoErr(t, err)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			scenario, err := NewScenario(dockertestMaxWait())
+			assertNoErr(t, err)
+			// defer scenario.ShutdownAssertNoPanics(t)
 
-	// Test list all nodes after added seconds
-	resultMachines := make([]*v1.Node, spec["user1"])
-	err = executeAndUnmarshal(
-		headscale,
-		[]string{
-			"headscale",
-			"nodes",
-			"list",
-			"--tags",
-			"--output", "json",
-		},
-		&resultMachines,
-	)
-	assert.Nil(t, err)
-	found := false
-	for _, node := range resultMachines {
-		if node.GetValidTags() != nil {
-			for _, tag := range node.GetValidTags() {
-				if tag == "tag:exists" {
-					found = true
+			spec := map[string]int{
+				"user1": 1,
+			}
+
+			err = scenario.CreateHeadscaleEnv(spec,
+				[]tsic.Option{tsic.WithTags([]string{"tag:test"})},
+				hsic.WithTestName("cliadvtags"),
+				hsic.WithACLPolicy(tt.policy),
+			)
+			assertNoErr(t, err)
+
+			headscale, err := scenario.Headscale()
+			assertNoErr(t, err)
+
+			// Test list all nodes after added seconds
+			resultMachines := make([]*v1.Node, spec["user1"])
+			err = executeAndUnmarshal(
+				headscale,
+				[]string{
+					"headscale",
+					"nodes",
+					"list",
+					"--tags",
+					"--output", "json",
+				},
+				&resultMachines,
+			)
+			assert.Nil(t, err)
+			found := false
+			for _, node := range resultMachines {
+				if tags := node.GetValidTags(); tags != nil {
+					found = slices.Contains(tags, "tag:test")
 				}
 			}
-		}
+			assert.Equalf(
+				t,
+				tt.wantTag,
+				found,
+				"'tag:test' found(%t) is the list of nodes, expected %t", found, tt.wantTag,
+			)
+		})
 	}
-	assert.Equal(
-		t,
-		true,
-		found,
-		"should not find a node with the tag 'tag:exists' in the list of nodes",
-	)
 }
 
 func TestNodeCommand(t *testing.T) {
@@ -1732,7 +1701,7 @@ func TestPolicyBrokenConfigCommand(t *testing.T) {
 			policyFilePath,
 		},
 	)
-	assert.ErrorContains(t, err, "verifying policy rules: invalid action")
+	assert.ErrorContains(t, err, "compiling filter rules: invalid action")
 
 	// The new policy was invalid, the old one should still be in place, which
 	// is none.

integration/hsic/hsic.go

@@ -81,6 +81,10 @@ type Option = func(c *HeadscaleInContainer)
 // HeadscaleInContainer instance.
 func WithACLPolicy(acl *policy.ACLPolicy) Option {
 	return func(hsic *HeadscaleInContainer) {
+		if acl == nil {
+			return
+		}
+
 		// TODO(kradalby): Move somewhere appropriate
 		hsic.env["HEADSCALE_POLICY_PATH"] = aclPolicyPath