mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-07 16:40:21 +09:00
cli/policy: route check through gRPC; bypass goes direct to DB
policy check previously ran the full policy engine in-process inside the CLI, building a sandbox PolicyManager from the file and the database. That duplicated the engine's runtime dependencies onto the CLI and forced --bypass-grpc-and-access-database-directly to pull in full server config validation. nblock hit it on PR #3229: passing --bypass with a real config produced a flood of 'Fatal config error:' lines from validateServerConfig because the cobra init early-return for 'policy check' skipped --config registration and OnInitialize. Make 'policy check' a thin frontend for the new CheckPolicy gRPC method. The server-side handler builds a fresh PolicyManager from the request bytes and the state's live users/nodes, runs SetPolicy on the sandbox so the tests block executes, and returns the result through gRPC status. No persistence, no policy_mode coupling. --bypass-grpc-and-access-database-directly keeps doing what its name says — opens the DB directly for cases where the server is not running — but is no longer the only way to evaluate a tests block. Drop the 'policy check' early-return in cmd/headscale/cli/root.go (added in PR #2580 when check was syntax-only). All paths now need either gRPC or direct DB access, both of which want the config and flags the rest of cobra init sets up. integration/cli_policy_test.go covers the matrix nblock asked about: policy_mode={file,database} x fixture={acl-only, acl+passing-tests, acl+failing-tests} x bypass={false,true} = 12 rows. acl-only and acl-plus-passing-tests must pass; acl-plus-failing-tests must surface 'test(s) failed'; the policy_mode axis proves check does not depend on where the server stores its current policy. Updates #1803
This commit is contained in:
+39
-52
@@ -1,7 +1,6 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
@@ -18,10 +17,7 @@ const (
|
||||
bypassFlag = "bypass-grpc-and-access-database-directly" //nolint:gosec // not a credential
|
||||
)
|
||||
|
||||
var (
|
||||
errAborted = errors.New("command aborted by user")
|
||||
errTestsRequireBypass = errors.New("policy contains a tests block; rerun with --" + bypassFlag + " to evaluate it (headscale must not be running)")
|
||||
)
|
||||
var errAborted = errors.New("command aborted by user")
|
||||
|
||||
// bypassDatabase loads the server config and opens the database directly,
|
||||
// bypassing the gRPC server. The caller is responsible for closing the
|
||||
@@ -176,9 +172,10 @@ var checkPolicy = &cobra.Command{
|
||||
Use: "check",
|
||||
Short: "Check the Policy file for errors",
|
||||
Long: `
|
||||
Check parses the policy and validates its structure. If the policy contains a
|
||||
"tests" block, those tests are evaluated against the live users and nodes:
|
||||
pass --` + bypassFlag + ` to open the database directly when headscale is not running.`,
|
||||
Check validates the policy against the server's live users and nodes,
|
||||
running any "tests" block. By default the command is a thin frontend
|
||||
for a gRPC call to a running headscale; pass --` + bypassFlag + ` to
|
||||
open the database directly when headscale is not running.`,
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
policyPath, _ := cmd.Flags().GetString("file")
|
||||
|
||||
@@ -187,56 +184,54 @@ var checkPolicy = &cobra.Command{
|
||||
return fmt.Errorf("reading policy file: %w", err)
|
||||
}
|
||||
|
||||
bypass, _ := cmd.Flags().GetBool(bypassFlag)
|
||||
if bypass, _ := cmd.Flags().GetBool(bypassFlag); bypass {
|
||||
if !confirmAction(cmd, "DO NOT run this command if an instance of headscale is running, are you sure headscale is not running?") {
|
||||
return errAborted
|
||||
}
|
||||
|
||||
// Without bypass we don't have users or nodes to resolve user@
|
||||
// tokens or test aliases against. Validate structure and warn
|
||||
// explicitly if tests are present, rather than running them
|
||||
// with empty data and reporting spurious failures.
|
||||
if !bypass {
|
||||
_, err = policy.NewPolicyManager(policyBytes, nil, views.Slice[types.NodeView]{})
|
||||
d, err := bypassDatabase()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer d.Close()
|
||||
|
||||
users, err := d.ListUsers()
|
||||
if err != nil {
|
||||
return fmt.Errorf("loading users: %w", err)
|
||||
}
|
||||
|
||||
nodes, err := d.ListNodes()
|
||||
if err != nil {
|
||||
return fmt.Errorf("loading nodes: %w", err)
|
||||
}
|
||||
|
||||
// NewPolicyManager validates structure and user references
|
||||
// but intentionally skips test evaluation (boot path).
|
||||
// SetPolicy is the user-write boundary and is what runs the
|
||||
// tests block.
|
||||
pm, err := policy.NewPolicyManager(policyBytes, users, nodes.ViewSlice())
|
||||
if err != nil {
|
||||
return fmt.Errorf("parsing policy file: %w", err)
|
||||
}
|
||||
|
||||
if policyHasTestsBlock(policyBytes) {
|
||||
return errTestsRequireBypass
|
||||
_, err = pm.SetPolicy(policyBytes)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
fmt.Println("Policy syntax is valid (run with --" + bypassFlag + " to also validate user references against the database)")
|
||||
fmt.Println("Policy is valid")
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
if !confirmAction(cmd, "DO NOT run this command if an instance of headscale is running, are you sure headscale is not running?") {
|
||||
return errAborted
|
||||
}
|
||||
|
||||
d, err := bypassDatabase()
|
||||
ctx, client, conn, cancel, err := newHeadscaleCLIWithConfig()
|
||||
if err != nil {
|
||||
return err
|
||||
return fmt.Errorf("connecting to headscale: %w", err)
|
||||
}
|
||||
defer d.Close()
|
||||
defer cancel()
|
||||
defer conn.Close()
|
||||
|
||||
users, err := d.ListUsers()
|
||||
if err != nil {
|
||||
return fmt.Errorf("loading users: %w", err)
|
||||
}
|
||||
|
||||
nodes, err := d.ListNodes()
|
||||
if err != nil {
|
||||
return fmt.Errorf("loading nodes: %w", err)
|
||||
}
|
||||
|
||||
// NewPolicyManager validates structure and user references but
|
||||
// intentionally skips test evaluation (boot path). SetPolicy is
|
||||
// the user-write boundary and is what runs the tests block.
|
||||
pm, err := policy.NewPolicyManager(policyBytes, users, nodes.ViewSlice())
|
||||
if err != nil {
|
||||
return fmt.Errorf("parsing policy file: %w", err)
|
||||
}
|
||||
|
||||
_, err = pm.SetPolicy(policyBytes)
|
||||
_, err = client.CheckPolicy(ctx, &v1.CheckPolicyRequest{Policy: string(policyBytes)})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -246,11 +241,3 @@ var checkPolicy = &cobra.Command{
|
||||
return nil
|
||||
},
|
||||
}
|
||||
|
||||
// policyHasTestsBlock is a cheap textual probe for `"tests"` so callers
|
||||
// can warn the user when they need to rerun with bypass. False positives
|
||||
// (e.g. a comment containing the word) are harmless: the user is told to
|
||||
// pass a flag they could have passed anyway.
|
||||
func policyHasTestsBlock(b []byte) bool {
|
||||
return bytes.Contains(b, []byte(`"tests"`))
|
||||
}
|
||||
|
||||
@@ -3,7 +3,6 @@ package cli
|
||||
import (
|
||||
"os"
|
||||
"runtime"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
@@ -22,11 +21,6 @@ func init() {
|
||||
return
|
||||
}
|
||||
|
||||
if slices.Contains(os.Args, "policy") && slices.Contains(os.Args, "check") {
|
||||
zerolog.SetGlobalLevel(zerolog.Disabled)
|
||||
return
|
||||
}
|
||||
|
||||
cobra.OnInitialize(initConfig)
|
||||
rootCmd.PersistentFlags().
|
||||
StringVarP(&cfgFile, "config", "c", "", "config file (default is /etc/headscale/config.yaml)")
|
||||
|
||||
@@ -26,6 +26,7 @@ import (
|
||||
"tailscale.com/types/views"
|
||||
|
||||
v1 "github.com/juanfont/headscale/gen/go/headscale/v1"
|
||||
policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2"
|
||||
"github.com/juanfont/headscale/hscontrol/state"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
@@ -781,6 +782,35 @@ func (api headscaleV1APIServer) SetPolicy(
|
||||
return response, nil
|
||||
}
|
||||
|
||||
// CheckPolicy validates the given policy against the server's live users
|
||||
// and nodes, running its `tests` block as a sandbox. Nothing is persisted
|
||||
// and the live PolicyManager is not touched. Works regardless of
|
||||
// policy.mode so operators can validate a policy file before storing it.
|
||||
func (api headscaleV1APIServer) CheckPolicy(
|
||||
_ context.Context,
|
||||
request *v1.CheckPolicyRequest,
|
||||
) (*v1.CheckPolicyResponse, error) {
|
||||
polB := []byte(request.GetPolicy())
|
||||
|
||||
users, err := api.h.state.ListAllUsers()
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "loading users: %s", err)
|
||||
}
|
||||
|
||||
nodes := api.h.state.ListNodes()
|
||||
|
||||
pm, err := policyv2.NewPolicyManager(polB, users, nodes)
|
||||
if err != nil {
|
||||
return nil, status.Error(codes.InvalidArgument, err.Error())
|
||||
}
|
||||
|
||||
if _, err := pm.SetPolicy(polB); err != nil {
|
||||
return nil, status.Error(codes.InvalidArgument, err.Error())
|
||||
}
|
||||
|
||||
return &v1.CheckPolicyResponse{}, nil
|
||||
}
|
||||
|
||||
// The following service calls are for testing and debugging
|
||||
func (api headscaleV1APIServer) DebugCreateNode(
|
||||
ctx context.Context,
|
||||
|
||||
@@ -0,0 +1,166 @@
|
||||
package integration
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
policyv2 "github.com/juanfont/headscale/hscontrol/policy/v2"
|
||||
"github.com/juanfont/headscale/integration/hsic"
|
||||
"github.com/juanfont/headscale/integration/tsic"
|
||||
"github.com/stretchr/testify/require"
|
||||
"tailscale.com/tailcfg"
|
||||
)
|
||||
|
||||
// TestPolicyCheckCommand exercises `headscale policy check` across the
|
||||
// matrix that nblock asked about on PR #3229:
|
||||
//
|
||||
// - policyMode: server runs with policy_mode=file vs policy_mode=database.
|
||||
// `check` reads from `--file`, so the server-side mode should not
|
||||
// change the outcome; running both proves that.
|
||||
// - fixture: ACL only, ACL with passing tests, ACL with failing tests.
|
||||
// - bypass: no-bypass talks to the server over gRPC; bypass opens the
|
||||
// database directly.
|
||||
//
|
||||
// Each row spins up its own scenario because policy_mode is fixed at boot
|
||||
// via `HEADSCALE_POLICY_MODE`. The two users + two nodes give the tests
|
||||
// block real `user@` aliases to resolve against.
|
||||
func TestPolicyCheckCommand(t *testing.T) {
|
||||
IntegrationSkip(t)
|
||||
|
||||
type fixture struct {
|
||||
name string
|
||||
policy policyv2.Policy
|
||||
}
|
||||
|
||||
const (
|
||||
user1 = "user1@"
|
||||
user2 = "user2@"
|
||||
)
|
||||
|
||||
aclOnly := policyv2.Policy{
|
||||
ACLs: []policyv2.ACL{
|
||||
{
|
||||
Action: policyv2.ActionAccept,
|
||||
Protocol: "tcp", //nolint:goconst // protocol literal, used inline once
|
||||
Sources: []policyv2.Alias{usernamep(user1)},
|
||||
Destinations: []policyv2.AliasWithPorts{
|
||||
aliasWithPorts(usernamep(user2), tailcfg.PortRange{First: 22, Last: 22}),
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
aclPlusPassingTests := aclOnly
|
||||
aclPlusPassingTests.Tests = []policyv2.PolicyTest{
|
||||
{
|
||||
Src: user1,
|
||||
Accept: []string{user2 + ":22"},
|
||||
},
|
||||
}
|
||||
|
||||
aclPlusFailingTests := aclOnly
|
||||
aclPlusFailingTests.Tests = []policyv2.PolicyTest{
|
||||
{
|
||||
// Reverse direction is not allowed by the ACL; the test
|
||||
// asserts ALLOWED, so it must fail.
|
||||
Src: user2,
|
||||
Accept: []string{user1 + ":22"},
|
||||
},
|
||||
}
|
||||
|
||||
fixtures := []fixture{
|
||||
{name: "acl-only", policy: aclOnly},
|
||||
{name: "acl-plus-passing-tests", policy: aclPlusPassingTests},
|
||||
{name: "acl-plus-failing-tests", policy: aclPlusFailingTests},
|
||||
}
|
||||
|
||||
type row struct {
|
||||
name string
|
||||
policyMode string
|
||||
fixture fixture
|
||||
bypass bool
|
||||
wantErr string
|
||||
wantStdout string
|
||||
}
|
||||
|
||||
modes := []string{"file", "database"} //nolint:goconst // axis labels match HEADSCALE_POLICY_MODE values
|
||||
bypasses := []bool{false, true}
|
||||
rows := make([]row, 0, len(modes)*len(fixtures)*len(bypasses))
|
||||
|
||||
for _, mode := range modes {
|
||||
for _, f := range fixtures {
|
||||
for _, bypass := range bypasses {
|
||||
suffix := "no-bypass"
|
||||
if bypass {
|
||||
suffix = "bypass"
|
||||
}
|
||||
|
||||
r := row{
|
||||
name: mode + "-" + f.name + "-" + suffix,
|
||||
policyMode: mode,
|
||||
fixture: f,
|
||||
bypass: bypass,
|
||||
wantStdout: "Policy is valid",
|
||||
}
|
||||
if f.name == "acl-plus-failing-tests" {
|
||||
r.wantErr = "test(s) failed"
|
||||
r.wantStdout = ""
|
||||
}
|
||||
|
||||
rows = append(rows, r)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, tt := range rows {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
spec := ScenarioSpec{
|
||||
NodesPerUser: 1,
|
||||
Users: []string{"user1", "user2"}, //nolint:goconst // matches usernamep("user1@")/("user2@") above
|
||||
}
|
||||
|
||||
scenario, err := NewScenario(spec)
|
||||
require.NoError(t, err)
|
||||
|
||||
defer scenario.ShutdownAssertNoPanics(t)
|
||||
|
||||
err = scenario.CreateHeadscaleEnv(
|
||||
[]tsic.Option{},
|
||||
hsic.WithTestName("cli-policycheck"),
|
||||
hsic.WithConfigEnv(map[string]string{
|
||||
"HEADSCALE_POLICY_MODE": tt.policyMode, //nolint:goconst // env var name from hscontrol/types/config.go
|
||||
}),
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
headscale, err := scenario.Headscale()
|
||||
require.NoError(t, err)
|
||||
|
||||
pBytes, err := json.Marshal(tt.fixture.policy)
|
||||
require.NoError(t, err)
|
||||
|
||||
policyFilePath := "/etc/headscale/policy.json" //nolint:goconst // standard headscale policy path
|
||||
err = headscale.WriteFile(policyFilePath, pBytes)
|
||||
require.NoError(t, err)
|
||||
|
||||
cmd := []string{"headscale", "policy", "check", "-f", policyFilePath} //nolint:goconst // CLI invocation
|
||||
if tt.bypass {
|
||||
// --force suppresses the "is the server running?"
|
||||
// confirmation prompt so the command can run
|
||||
// non-interactively under the test harness.
|
||||
cmd = append(cmd, "--bypass-grpc-and-access-database-directly", "--force")
|
||||
}
|
||||
|
||||
stdout, err := headscale.Execute(cmd)
|
||||
|
||||
if tt.wantErr != "" {
|
||||
require.ErrorContains(t, err, tt.wantErr)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, stdout, tt.wantStdout)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user