util/syspolicy/*: move syspolicy keys to new const leaf "pkey" package

This is step 1 of ~3, breaking up #14720 into reviewable chunks, with
the aim to make syspolicy be a build-time configurable feature.

In this first (very noisy) step, all the syspolicy string key
constants move to a new constant-only (code-free) package. This will
make future steps more reviewable, without this movement noise.

There are no code or behavior changes here.

The future steps of this series can be seen in #14720: removing global
funcs from syspolicy resolution and using an interface that's plumbed
around instead. Then adding build tags.

Updates #12614

Change-Id: If73bf2c28b9c9b1a408fe868b0b6a25b03eeabd1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick
2025-08-30 08:02:35 -07:00
committed by Brad Fitzpatrick
parent 6d45fcfc93
commit cc532efc20
48 changed files with 601 additions and 554 deletions
+8 -7
View File
@@ -11,6 +11,7 @@ import (
"strings"
"unicode/utf8"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
)
@@ -22,7 +23,7 @@ var _ Store = (*EnvPolicyStore)(nil)
type EnvPolicyStore struct{}
// ReadString implements [Store].
func (s *EnvPolicyStore) ReadString(key setting.Key) (string, error) {
func (s *EnvPolicyStore) ReadString(key pkey.Key) (string, error) {
_, str, err := s.lookupSettingVariable(key)
if err != nil {
return "", err
@@ -31,7 +32,7 @@ func (s *EnvPolicyStore) ReadString(key setting.Key) (string, error) {
}
// ReadUInt64 implements [Store].
func (s *EnvPolicyStore) ReadUInt64(key setting.Key) (uint64, error) {
func (s *EnvPolicyStore) ReadUInt64(key pkey.Key) (uint64, error) {
name, str, err := s.lookupSettingVariable(key)
if err != nil {
return 0, err
@@ -47,7 +48,7 @@ func (s *EnvPolicyStore) ReadUInt64(key setting.Key) (uint64, error) {
}
// ReadBoolean implements [Store].
func (s *EnvPolicyStore) ReadBoolean(key setting.Key) (bool, error) {
func (s *EnvPolicyStore) ReadBoolean(key pkey.Key) (bool, error) {
name, str, err := s.lookupSettingVariable(key)
if err != nil {
return false, err
@@ -63,7 +64,7 @@ func (s *EnvPolicyStore) ReadBoolean(key setting.Key) (bool, error) {
}
// ReadStringArray implements [Store].
func (s *EnvPolicyStore) ReadStringArray(key setting.Key) ([]string, error) {
func (s *EnvPolicyStore) ReadStringArray(key pkey.Key) ([]string, error) {
_, str, err := s.lookupSettingVariable(key)
if err != nil || str == "" {
return nil, err
@@ -79,7 +80,7 @@ func (s *EnvPolicyStore) ReadStringArray(key setting.Key) ([]string, error) {
return res[0:dst], nil
}
func (s *EnvPolicyStore) lookupSettingVariable(key setting.Key) (name, value string, err error) {
func (s *EnvPolicyStore) lookupSettingVariable(key pkey.Key) (name, value string, err error) {
name, err = keyToEnvVarName(key)
if err != nil {
return "", "", err
@@ -103,7 +104,7 @@ var (
//
// It's fine to use this in [EnvPolicyStore] without caching variable names since it's not a hot path.
// [EnvPolicyStore] is not a [Changeable] policy store, so the conversion will only happen once.
func keyToEnvVarName(key setting.Key) (string, error) {
func keyToEnvVarName(key pkey.Key) (string, error) {
if len(key) == 0 {
return "", errEmptyKey
}
@@ -135,7 +136,7 @@ func keyToEnvVarName(key setting.Key) (string, error) {
}
case isDigit(c):
split = currentWord.Len() > 0 && !isDigit(key[i-1])
case c == setting.KeyPathSeparator:
case c == pkey.KeyPathSeparator:
words = append(words, currentWord.String())
currentWord.Reset()
continue
@@ -11,13 +11,14 @@ import (
"strconv"
"testing"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
)
func TestKeyToEnvVarName(t *testing.T) {
tests := []struct {
name string
key setting.Key
key pkey.Key
want string // suffix after "TS_DEBUGSYSPOLICY_"
wantErr error
}{
@@ -166,7 +167,7 @@ func TestEnvPolicyStore(t *testing.T) {
}
tests := []struct {
name string
key setting.Key
key pkey.Key
lookup func(string) (string, bool)
want any
wantErr error
+3 -2
View File
@@ -16,6 +16,7 @@ import (
"tailscale.com/util/set"
"tailscale.com/util/syspolicy/internal/loggerx"
"tailscale.com/util/syspolicy/internal/metrics"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
)
@@ -138,9 +139,9 @@ func (r *Reader) reload(force bool) (*setting.Snapshot, error) {
metrics.Reset(r.origin)
var m map[setting.Key]setting.RawItem
var m map[pkey.Key]setting.RawItem
if lastPolicyCount := r.lastPolicy.Len(); lastPolicyCount > 0 {
m = make(map[setting.Key]setting.RawItem, lastPolicyCount)
m = make(map[pkey.Key]setting.RawItem, lastPolicyCount)
}
for _, s := range r.settings {
if !r.origin.Scope().IsConfigurableSetting(s) {
+5 -4
View File
@@ -9,6 +9,7 @@ import (
"time"
"tailscale.com/util/must"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
)
@@ -72,7 +73,7 @@ func TestReaderLifecycle(t *testing.T) {
initWant: setting.NewSnapshot(nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
addStrings: []TestSetting[string]{TestSettingOf("StringValue", "S1")},
addStringLists: []TestSetting[[]string]{TestSettingOf("StringListValue", []string{"S1", "S2", "S3"})},
newWant: setting.NewSnapshot(map[setting.Key]setting.RawItem{
newWant: setting.NewSnapshot(map[pkey.Key]setting.RawItem{
"StringValue": setting.RawItemWith("S1", nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
"StringListValue": setting.RawItemWith([]string{"S1", "S2", "S3"}, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
}, setting.NewNamedOrigin("Test", setting.DeviceScope)),
@@ -136,7 +137,7 @@ func TestReaderLifecycle(t *testing.T) {
TestSettingOf("PreferenceOptionValue", "always"),
TestSettingOf("VisibilityValue", "show"),
},
initWant: setting.NewSnapshot(map[setting.Key]setting.RawItem{
initWant: setting.NewSnapshot(map[pkey.Key]setting.RawItem{
"DurationValue": setting.RawItemWith(must.Get(time.ParseDuration("2h30m")), nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
"PreferenceOptionValue": setting.RawItemWith(setting.AlwaysByPolicy, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
"VisibilityValue": setting.RawItemWith(setting.VisibleByPolicy, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
@@ -165,7 +166,7 @@ func TestReaderLifecycle(t *testing.T) {
initUInt64s: []TestSetting[uint64]{
TestSettingOf[uint64]("VisibilityValue", 42), // type mismatch
},
initWant: setting.NewSnapshot(map[setting.Key]setting.RawItem{
initWant: setting.NewSnapshot(map[pkey.Key]setting.RawItem{
"DurationValue1": setting.RawItemWith(nil, setting.NewErrorText("time: invalid duration \"soon\""), setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
"DurationValue2": setting.RawItemWith(nil, setting.NewErrorText("bang!"), setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
"PreferenceOptionValue": setting.RawItemWith(setting.ShowChoiceByPolicy, nil, setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
@@ -277,7 +278,7 @@ func TestReadingSession(t *testing.T) {
t.Fatalf("the session was closed prematurely")
}
want := setting.NewSnapshot(map[setting.Key]setting.RawItem{
want := setting.NewSnapshot(map[pkey.Key]setting.RawItem{
"StringValue": setting.RawItemWith("S1", nil, origin),
}, origin)
if got := session.GetSettings(); !got.Equal(want) {
+5 -4
View File
@@ -13,6 +13,7 @@ import (
"io"
"tailscale.com/types/lazy"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
)
@@ -31,19 +32,19 @@ type Store interface {
// ReadString returns the value of a [setting.StringValue] with the specified key,
// an [setting.ErrNotConfigured] if the policy setting is not configured, or
// an error on failure.
ReadString(key setting.Key) (string, error)
ReadString(key pkey.Key) (string, error)
// ReadUInt64 returns the value of a [setting.IntegerValue] with the specified key,
// an [setting.ErrNotConfigured] if the policy setting is not configured, or
// an error on failure.
ReadUInt64(key setting.Key) (uint64, error)
ReadUInt64(key pkey.Key) (uint64, error)
// ReadBoolean returns the value of a [setting.BooleanValue] with the specified key,
// an [setting.ErrNotConfigured] if the policy setting is not configured, or
// an error on failure.
ReadBoolean(key setting.Key) (bool, error)
ReadBoolean(key pkey.Key) (bool, error)
// ReadStringArray returns the value of a [setting.StringListValue] with the specified key,
// an [setting.ErrNotConfigured] if the policy setting is not configured, or
// an error on failure.
ReadStringArray(key setting.Key) ([]string, error)
ReadStringArray(key pkey.Key) ([]string, error)
}
// Lockable is an optional interface that [Store] implementations may support.
+15 -14
View File
@@ -13,6 +13,7 @@ import (
"golang.org/x/sys/windows/registry"
"tailscale.com/util/set"
"tailscale.com/util/syspolicy/internal/loggerx"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
"tailscale.com/util/winutil/gp"
)
@@ -251,7 +252,7 @@ func (ps *PlatformPolicyStore) onChange() {
// ReadString retrieves a string policy with the specified key.
// It returns [setting.ErrNotConfigured] if the policy setting does not exist.
func (ps *PlatformPolicyStore) ReadString(key setting.Key) (val string, err error) {
func (ps *PlatformPolicyStore) ReadString(key pkey.Key) (val string, err error) {
return getPolicyValue(ps, key,
func(key registry.Key, valueName string) (string, error) {
val, _, err := key.GetStringValue(valueName)
@@ -261,7 +262,7 @@ func (ps *PlatformPolicyStore) ReadString(key setting.Key) (val string, err erro
// ReadUInt64 retrieves an integer policy with the specified key.
// It returns [setting.ErrNotConfigured] if the policy setting does not exist.
func (ps *PlatformPolicyStore) ReadUInt64(key setting.Key) (uint64, error) {
func (ps *PlatformPolicyStore) ReadUInt64(key pkey.Key) (uint64, error) {
return getPolicyValue(ps, key,
func(key registry.Key, valueName string) (uint64, error) {
val, _, err := key.GetIntegerValue(valueName)
@@ -271,7 +272,7 @@ func (ps *PlatformPolicyStore) ReadUInt64(key setting.Key) (uint64, error) {
// ReadBoolean retrieves a boolean policy with the specified key.
// It returns [setting.ErrNotConfigured] if the policy setting does not exist.
func (ps *PlatformPolicyStore) ReadBoolean(key setting.Key) (bool, error) {
func (ps *PlatformPolicyStore) ReadBoolean(key pkey.Key) (bool, error) {
return getPolicyValue(ps, key,
func(key registry.Key, valueName string) (bool, error) {
val, _, err := key.GetIntegerValue(valueName)
@@ -283,8 +284,8 @@ func (ps *PlatformPolicyStore) ReadBoolean(key setting.Key) (bool, error) {
}
// ReadString retrieves a multi-string policy with the specified key.
// It returns [setting.ErrNotConfigured] if the policy setting does not exist.
func (ps *PlatformPolicyStore) ReadStringArray(key setting.Key) ([]string, error) {
// It returns [pkey.ErrNotConfigured] if the policy setting does not exist.
func (ps *PlatformPolicyStore) ReadStringArray(key pkey.Key) ([]string, error) {
return getPolicyValue(ps, key,
func(key registry.Key, valueName string) ([]string, error) {
val, _, err := key.GetStringsValue(valueName)
@@ -322,25 +323,25 @@ func (ps *PlatformPolicyStore) ReadStringArray(key setting.Key) ([]string, error
})
}
// splitSettingKey extracts the registry key name and value name from a [setting.Key].
// The [setting.Key] format allows grouping settings into nested categories using one
// or more [setting.KeyPathSeparator]s in the path. How individual policy settings are
// splitSettingKey extracts the registry key name and value name from a [pkey.Key].
// The [pkey.Key] format allows grouping settings into nested categories using one
// or more [pkey.KeyPathSeparator]s in the path. How individual policy settings are
// stored is an implementation detail of each [Store]. In the [PlatformPolicyStore]
// for Windows, we map nested policy categories onto the Registry key hierarchy.
// The last component after a [setting.KeyPathSeparator] is treated as the value name,
// The last component after a [pkey.KeyPathSeparator] is treated as the value name,
// while everything preceding it is considered a subpath (relative to the {HKLM,HKCU}\Software\Policies\Tailscale key).
// If there are no [setting.KeyPathSeparator]s in the key, the policy setting value
// If there are no [pkey.KeyPathSeparator]s in the key, the policy setting value
// is meant to be stored directly under {HKLM,HKCU}\Software\Policies\Tailscale.
func splitSettingKey(key setting.Key) (path, valueName string) {
if idx := strings.LastIndexByte(string(key), setting.KeyPathSeparator); idx != -1 {
path = strings.ReplaceAll(string(key[:idx]), string(setting.KeyPathSeparator), `\`)
func splitSettingKey(key pkey.Key) (path, valueName string) {
if idx := strings.LastIndexByte(string(key), pkey.KeyPathSeparator); idx != -1 {
path = strings.ReplaceAll(string(key[:idx]), string(pkey.KeyPathSeparator), `\`)
valueName = string(key[idx+1:])
return path, valueName
}
return "", string(key)
}
func getPolicyValue[T any](ps *PlatformPolicyStore, key setting.Key, getter registryValueGetter[T]) (T, error) {
func getPolicyValue[T any](ps *PlatformPolicyStore, key pkey.Key, getter registryValueGetter[T]) (T, error) {
var zero T
ps.mu.Lock()
@@ -19,6 +19,7 @@ import (
"tailscale.com/tstest"
"tailscale.com/util/cibuild"
"tailscale.com/util/mak"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
"tailscale.com/util/winutil"
"tailscale.com/util/winutil/gp"
@@ -31,7 +32,7 @@ import (
type subkeyStrings []string
type testPolicyValue struct {
name setting.Key
name pkey.Key
value any
}
@@ -100,7 +101,7 @@ func TestReadPolicyStore(t *testing.T) {
t.Skipf("test requires running as elevated user")
}
tests := []struct {
name setting.Key
name pkey.Key
newValue any
legacyValue any
want any
@@ -269,7 +270,7 @@ func TestPolicyStoreChangeNotifications(t *testing.T) {
func TestSplitSettingKey(t *testing.T) {
tests := []struct {
name string
key setting.Key
key pkey.Key
wantPath string
wantValue string
}{
+15 -14
View File
@@ -12,6 +12,7 @@ import (
"tailscale.com/util/mak"
"tailscale.com/util/set"
"tailscale.com/util/slicesx"
"tailscale.com/util/syspolicy/pkey"
"tailscale.com/util/syspolicy/setting"
"tailscale.com/util/testenv"
)
@@ -31,7 +32,7 @@ type TestValueType interface {
// TestSetting is a policy setting in a [TestStore].
type TestSetting[T TestValueType] struct {
// Key is the setting's unique identifier.
Key setting.Key
Key pkey.Key
// Error is the error to be returned by the [TestStore] when reading
// a policy setting with the specified key.
Error error
@@ -43,20 +44,20 @@ type TestSetting[T TestValueType] struct {
// TestSettingOf returns a [TestSetting] representing a policy setting
// configured with the specified key and value.
func TestSettingOf[T TestValueType](key setting.Key, value T) TestSetting[T] {
func TestSettingOf[T TestValueType](key pkey.Key, value T) TestSetting[T] {
return TestSetting[T]{Key: key, Value: value}
}
// TestSettingWithError returns a [TestSetting] representing a policy setting
// with the specified key and error.
func TestSettingWithError[T TestValueType](key setting.Key, err error) TestSetting[T] {
func TestSettingWithError[T TestValueType](key pkey.Key, err error) TestSetting[T] {
return TestSetting[T]{Key: key, Error: err}
}
// testReadOperation describes a single policy setting read operation.
type testReadOperation struct {
// Key is the setting's unique identifier.
Key setting.Key
Key pkey.Key
// Type is a value type of a read operation.
// [setting.BooleanValue], [setting.IntegerValue], [setting.StringValue] or [setting.StringListValue]
Type setting.Type
@@ -65,7 +66,7 @@ type testReadOperation struct {
// TestExpectedReads is the number of read operations with the specified details.
type TestExpectedReads struct {
// Key is the setting's unique identifier.
Key setting.Key
Key pkey.Key
// Type is a value type of a read operation.
// [setting.BooleanValue], [setting.IntegerValue], [setting.StringValue] or [setting.StringListValue]
Type setting.Type
@@ -87,8 +88,8 @@ type TestStore struct {
storeLockCount atomic.Int32
mu sync.RWMutex
suspendCount int // change callback are suspended if > 0
mr, mw map[setting.Key]any // maps for reading and writing; they're the same unless the store is suspended.
suspendCount int // change callback are suspended if > 0
mr, mw map[pkey.Key]any // maps for reading and writing; they're the same unless the store is suspended.
cbs set.HandleSet[func()]
closed bool
@@ -99,7 +100,7 @@ type TestStore struct {
// NewTestStore returns a new [TestStore].
// The tb will be used to report coding errors detected by the [TestStore].
func NewTestStore(tb testenv.TB) *TestStore {
m := make(map[setting.Key]any)
m := make(map[pkey.Key]any)
store := &TestStore{
tb: tb,
done: make(chan struct{}),
@@ -162,7 +163,7 @@ func (s *TestStore) IsEmpty() bool {
}
// ReadString implements [Store].
func (s *TestStore) ReadString(key setting.Key) (string, error) {
func (s *TestStore) ReadString(key pkey.Key) (string, error) {
defer s.recordRead(key, setting.StringValue)
s.mu.RLock()
defer s.mu.RUnlock()
@@ -181,7 +182,7 @@ func (s *TestStore) ReadString(key setting.Key) (string, error) {
}
// ReadUInt64 implements [Store].
func (s *TestStore) ReadUInt64(key setting.Key) (uint64, error) {
func (s *TestStore) ReadUInt64(key pkey.Key) (uint64, error) {
defer s.recordRead(key, setting.IntegerValue)
s.mu.RLock()
defer s.mu.RUnlock()
@@ -200,7 +201,7 @@ func (s *TestStore) ReadUInt64(key setting.Key) (uint64, error) {
}
// ReadBoolean implements [Store].
func (s *TestStore) ReadBoolean(key setting.Key) (bool, error) {
func (s *TestStore) ReadBoolean(key pkey.Key) (bool, error) {
defer s.recordRead(key, setting.BooleanValue)
s.mu.RLock()
defer s.mu.RUnlock()
@@ -219,7 +220,7 @@ func (s *TestStore) ReadBoolean(key setting.Key) (bool, error) {
}
// ReadStringArray implements [Store].
func (s *TestStore) ReadStringArray(key setting.Key) ([]string, error) {
func (s *TestStore) ReadStringArray(key pkey.Key) ([]string, error) {
defer s.recordRead(key, setting.StringListValue)
s.mu.RLock()
defer s.mu.RUnlock()
@@ -237,7 +238,7 @@ func (s *TestStore) ReadStringArray(key setting.Key) ([]string, error) {
return slice, nil
}
func (s *TestStore) recordRead(key setting.Key, typ setting.Type) {
func (s *TestStore) recordRead(key pkey.Key, typ setting.Type) {
s.readsMu.Lock()
op := testReadOperation{key, typ}
num := s.reads[op]
@@ -399,7 +400,7 @@ func (s *TestStore) SetStringLists(settings ...TestSetting[[]string]) {
}
// Delete deletes the specified settings from s.
func (s *TestStore) Delete(keys ...setting.Key) {
func (s *TestStore) Delete(keys ...pkey.Key) {
s.storeLock.Lock()
for _, key := range keys {
s.mu.Lock()