util/syspolicy/{setting,ptype}: move PreferenceOption and Visibility to new leaf package

Step 3 in the series. See earlier cc532efc2000 and d05e6dc09e.

This step moves some types into a new leaf "ptype" package out of the
big "settings" package. The policyclient.Client will later get new
methods to return those things (as well as Duration and Uint64, which
weren't done at the time of the earlier prototype).

Updates #16998
Updates #12614

Change-Id: I4d72d8079de3b5351ed602eaa72863372bd474a2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/derper/depaware.txt

@@ -176,6 +176,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/ipn+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/util/syspolicy/rsop
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy
         tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

cmd/k8s-operator/depaware.txt

@@ -957,6 +957,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/control/controlclient+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/control/controlclient+
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

cmd/tailscale/depaware.txt

@@ -197,6 +197,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/ipn+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/util/syspolicy/rsop
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy
         tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

cmd/tailscaled/depaware.txt

@@ -434,6 +434,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/cmd/tailscaled+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/control/controlclient+
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/setting                         from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

cmd/tsidp/depaware.txt

@@ -386,6 +386,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/control/controlclient+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/control/controlclient+
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/ipn/localapi+
         tailscale.com/util/syspolicy/setting                         from tailscale.com/client/local+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

net/dns/manager_windows.go

@@ -32,7 +32,7 @@ import (
 	"tailscale.com/util/syspolicy"
 	"tailscale.com/util/syspolicy/pkey"
 	"tailscale.com/util/syspolicy/policyclient"
-	"tailscale.com/util/syspolicy/setting"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/winutil"
 )
 
@@ -521,7 +521,7 @@ func (m *windowsManager) reconfigureDNSRegistration() {
 	// Disable DNS registration by default (if the policy setting is not configured).
 	// This is primarily for historical reasons and to avoid breaking existing
 	// setups that rely on this behavior.
-	enableDNSRegistration, err := syspolicy.GetPreferenceOptionOrDefault(pkey.EnableDNSRegistration, setting.NeverByPolicy)
+	enableDNSRegistration, err := syspolicy.GetPreferenceOptionOrDefault(pkey.EnableDNSRegistration, ptype.NeverByPolicy)
 	if err != nil {
 		m.logf("error getting DNSRegistration policy setting: %v", err) // non-fatal; we'll use the default
 	}

tsnet/depaware.txt

@@ -381,6 +381,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
         tailscale.com/util/syspolicy/internal/metrics                from tailscale.com/util/syspolicy/source
         tailscale.com/util/syspolicy/pkey                            from tailscale.com/control/controlclient+
         tailscale.com/util/syspolicy/policyclient                    from tailscale.com/control/controlclient+
+        tailscale.com/util/syspolicy/ptype                           from tailscale.com/util/syspolicy+
         tailscale.com/util/syspolicy/rsop                            from tailscale.com/ipn/localapi+
         tailscale.com/util/syspolicy/setting                         from tailscale.com/client/local+
         tailscale.com/util/syspolicy/source                          from tailscale.com/util/syspolicy+

tstest/deptest/deptest.go

@@ -24,6 +24,7 @@ import (
 type DepChecker struct {
 	GOOS     string            // optional
 	GOARCH   string            // optional
+	OnDep    func(string)      // if non-nil, called per import
 	BadDeps  map[string]string // package => why
 	WantDeps set.Set[string]   // packages expected
 	Tags     string            // comma-separated
@@ -66,6 +67,9 @@ func (c DepChecker) Check(t *testing.T) {
 	})
 
 	for _, dep := range res.Deps {
+		if c.OnDep != nil {
+			c.OnDep(dep)
+		}
 		if why, ok := c.BadDeps[dep]; ok {
 			t.Errorf("package %q is not allowed as a dependency (env: %q); reason: %s", dep, extraEnv, why)
 		}

util/syspolicy/ptype/ptype.go

@@ -1,11 +1,11 @@
 // Copyright (c) Tailscale Inc & AUTHORS
 // SPDX-License-Identifier: BSD-3-Clause
 
-package setting
+// Package ptype contains types used by syspolicy.
-
+//
-import (
+// It's a leaf package for dependency reasons and should not contain much if any
-	"encoding"
+// code, and should not import much (or anything).
-)
+package ptype
 
 // PreferenceOption is a policy that governs whether a boolean variable
 // is forcibly assigned an administrator-defined value, or allowed to receive
@@ -18,9 +18,10 @@ const (
 	AlwaysByPolicy
 )
 
-// Show returns if the UI option that controls the choice administered by this
+// Show reports whether the UI option that controls the choice administered by
-// policy should be shown. Currently this is true if and only if the policy is
+// this policy should be shown (that is, available for users to change).
-// [ShowChoiceByPolicy].
+//
+// Currently this is true if and only if the policy is [ShowChoiceByPolicy].
 func (p PreferenceOption) Show() bool {
 	return p == ShowChoiceByPolicy
 }
@@ -91,11 +92,6 @@ func (p *PreferenceOption) UnmarshalText(text []byte) error {
 // component of a user interface is to be shown.
 type Visibility byte
 
-var (
-	_ encoding.TextMarshaler   = (*Visibility)(nil)
-	_ encoding.TextUnmarshaler = (*Visibility)(nil)
-)
-
 const (
 	VisibleByPolicy Visibility = 'v'
 	HiddenByPolicy  Visibility = 'h'

util/syspolicy/ptype/ptype_test.go

@@ -0,0 +1,24 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package ptype
+
+import (
+	"encoding"
+	"testing"
+
+	"tailscale.com/tstest/deptest"
+)
+
+var (
+	_ encoding.TextMarshaler   = (*Visibility)(nil)
+	_ encoding.TextUnmarshaler = (*Visibility)(nil)
+)
+
+func TestImports(t *testing.T) {
+	deptest.DepChecker{
+		OnDep: func(dep string) {
+			t.Errorf("unexpected dep %q in leaf package; this package should not contain much code", dep)
+		},
+	}.Check(t)
+}

util/syspolicy/rsop/change_callbacks.go

@@ -13,6 +13,7 @@ import (
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/pkey"
 	"tailscale.com/util/syspolicy/policyclient"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/setting"
 )
 
@@ -50,7 +51,7 @@ func (c PolicyChange) HasChanged(key pkey.Key) bool {
 		return true
 	}
 	switch newVal := new.(type) {
-	case bool, uint64, string, setting.Visibility, setting.PreferenceOption, time.Duration:
+	case bool, uint64, string, ptype.Visibility, ptype.PreferenceOption, time.Duration:
 		return newVal != old
 	case []string:
 		oldVal, ok := old.([]string)

util/syspolicy/setting/setting.go

@@ -17,6 +17,7 @@ import (
 	"tailscale.com/types/lazy"
 	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/testenv"
 )
 
@@ -130,7 +131,7 @@ func (t Type) String() string {
 
 // ValueType is a constraint that allows Go types corresponding to [Type].
 type ValueType interface {
-	bool | uint64 | string | []string | Visibility | PreferenceOption | time.Duration
+	bool | uint64 | string | []string | ptype.Visibility | ptype.PreferenceOption | time.Duration
 }
 
 // Definition defines policy key, scope and value type.

util/syspolicy/setting/snapshot_test.go

@@ -12,6 +12,12 @@ import (
 	jsonv2 "github.com/go-json-experiment/json"
 	"tailscale.com/util/syspolicy/internal"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
+)
+
+const (
+	VisibleByPolicy    = ptype.VisibleByPolicy
+	ShowChoiceByPolicy = ptype.ShowChoiceByPolicy
 )
 
 func TestMergeSnapshots(t *testing.T) {

util/syspolicy/source/policy_reader.go

@@ -17,6 +17,7 @@ import (
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/internal/metrics"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/setting"
 )
 
@@ -365,21 +366,21 @@ func readPolicySettingValue(store Store, s *setting.Definition) (value any, err
 	case setting.PreferenceOptionValue:
 		s, err := store.ReadString(key)
 		if err == nil {
-			var value setting.PreferenceOption
+			var value ptype.PreferenceOption
 			if err = value.UnmarshalText([]byte(s)); err == nil {
 				return value, nil
 			}
 		}
-		return setting.ShowChoiceByPolicy, err
+		return ptype.ShowChoiceByPolicy, err
 	case setting.VisibilityValue:
 		s, err := store.ReadString(key)
 		if err == nil {
-			var value setting.Visibility
+			var value ptype.Visibility
 			if err = value.UnmarshalText([]byte(s)); err == nil {
 				return value, nil
 			}
 		}
-		return setting.VisibleByPolicy, err
+		return ptype.VisibleByPolicy, err
 	case setting.DurationValue:
 		s, err := store.ReadString(key)
 		if err == nil {

util/syspolicy/source/policy_reader_test.go

@@ -10,6 +10,7 @@ import (
 
 	"tailscale.com/util/must"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/setting"
 )
 
@@ -139,8 +140,8 @@ func TestReaderLifecycle(t *testing.T) {
 			},
 			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)),
+				"PreferenceOptionValue": setting.RawItemWith(ptype.AlwaysByPolicy, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
-				"VisibilityValue":       setting.RawItemWith(setting.VisibleByPolicy, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
+				"VisibilityValue":       setting.RawItemWith(ptype.VisibleByPolicy, nil, setting.NewNamedOrigin("Test", setting.DeviceScope)),
 			}, setting.NewNamedOrigin("Test", setting.DeviceScope)),
 		},
 		{
@@ -169,8 +170,8 @@ func TestReaderLifecycle(t *testing.T) {
 			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)),
+				"PreferenceOptionValue": setting.RawItemWith(ptype.ShowChoiceByPolicy, nil, setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
-				"VisibilityValue":       setting.RawItemWith(setting.VisibleByPolicy, setting.NewErrorText("type mismatch in ReadString: got uint64"), setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
+				"VisibilityValue":       setting.RawItemWith(ptype.VisibleByPolicy, setting.NewErrorText("type mismatch in ReadString: got uint64"), setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
 			}, setting.NewNamedOrigin("Test", setting.CurrentUserScope)),
 		},
 	}

util/syspolicy/syspolicy.go

@@ -18,6 +18,7 @@ import (
 
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/rsop"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
@@ -111,14 +112,14 @@ func GetStringArray(key pkey.Key, defaultValue []string) ([]string, error) {
 // the authority to set. It describes user-decides/always/never options, where
 // "always" and "never" remove the user's ability to make a selection. If not
 // present or set to a different value, "user-decides" is the default.
-func GetPreferenceOption(name pkey.Key) (setting.PreferenceOption, error) {
+func GetPreferenceOption(name pkey.Key) (ptype.PreferenceOption, error) {
-	return getCurrentPolicySettingValue(name, setting.ShowChoiceByPolicy)
+	return getCurrentPolicySettingValue(name, ptype.ShowChoiceByPolicy)
 }
 
 // GetPreferenceOptionOrDefault is like [GetPreferenceOption], but allows
 // specifying a default value to return if the policy setting is not configured.
 // It can be used in situations where "user-decides" is not the default.
-func GetPreferenceOptionOrDefault(name pkey.Key, defaultValue setting.PreferenceOption) (setting.PreferenceOption, error) {
+func GetPreferenceOptionOrDefault(name pkey.Key, defaultValue ptype.PreferenceOption) (ptype.PreferenceOption, error) {
 	return getCurrentPolicySettingValue(name, defaultValue)
 }
 
@@ -127,8 +128,8 @@ func GetPreferenceOptionOrDefault(name pkey.Key, defaultValue setting.Preference
 // for UI elements. The registry value should be a string set to "show" (return
 // true) or "hide" (return true). If not present or set to a different value,
 // "show" (return false) is the default.
-func GetVisibility(name pkey.Key) (setting.Visibility, error) {
+func GetVisibility(name pkey.Key) (ptype.Visibility, error) {
-	return getCurrentPolicySettingValue(name, setting.VisibleByPolicy)
+	return getCurrentPolicySettingValue(name, ptype.VisibleByPolicy)
 }
 
 // GetDuration loads a policy from the registry that can be managed

util/syspolicy/syspolicy_test.go

@@ -13,6 +13,7 @@ import (
 	"tailscale.com/util/syspolicy/internal/loggerx"
 	"tailscale.com/util/syspolicy/internal/metrics"
 	"tailscale.com/util/syspolicy/pkey"
+	"tailscale.com/util/syspolicy/ptype"
 	"tailscale.com/util/syspolicy/setting"
 	"tailscale.com/util/syspolicy/source"
 	"tailscale.com/util/testenv"
@@ -249,7 +250,7 @@ func TestGetPreferenceOption(t *testing.T) {
 		key          pkey.Key
 		handlerValue string
 		handlerError error
-		wantValue    setting.PreferenceOption
+		wantValue    ptype.PreferenceOption
 		wantError    error
 		wantMetrics  []metrics.TestState
 	}{
@@ -257,7 +258,7 @@ func TestGetPreferenceOption(t *testing.T) {
 			name:         "always by policy",
 			key:          pkey.EnableIncomingConnections,
 			handlerValue: "always",
-			wantValue:    setting.AlwaysByPolicy,
+			wantValue:    ptype.AlwaysByPolicy,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_any", Value: 1},
 				{Name: "$os_syspolicy_AllowIncomingConnections", Value: 1},
@@ -267,7 +268,7 @@ func TestGetPreferenceOption(t *testing.T) {
 			name:         "never by policy",
 			key:          pkey.EnableIncomingConnections,
 			handlerValue: "never",
-			wantValue:    setting.NeverByPolicy,
+			wantValue:    ptype.NeverByPolicy,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_any", Value: 1},
 				{Name: "$os_syspolicy_AllowIncomingConnections", Value: 1},
@@ -277,7 +278,7 @@ func TestGetPreferenceOption(t *testing.T) {
 			name:         "use default",
 			key:          pkey.EnableIncomingConnections,
 			handlerValue: "",
-			wantValue:    setting.ShowChoiceByPolicy,
+			wantValue:    ptype.ShowChoiceByPolicy,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_any", Value: 1},
 				{Name: "$os_syspolicy_AllowIncomingConnections", Value: 1},
@@ -287,13 +288,13 @@ func TestGetPreferenceOption(t *testing.T) {
 			name:         "read non-existing value",
 			key:          pkey.EnableIncomingConnections,
 			handlerError: ErrNotConfigured,
-			wantValue:    setting.ShowChoiceByPolicy,
+			wantValue:    ptype.ShowChoiceByPolicy,
 		},
 		{
 			name:         "other error is returned",
 			key:          pkey.EnableIncomingConnections,
 			handlerError: someOtherError,
-			wantValue:    setting.ShowChoiceByPolicy,
+			wantValue:    ptype.ShowChoiceByPolicy,
 			wantError:    someOtherError,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_errors", Value: 1},
@@ -342,7 +343,7 @@ func TestGetVisibility(t *testing.T) {
 		key          pkey.Key
 		handlerValue string
 		handlerError error
-		wantValue    setting.Visibility
+		wantValue    ptype.Visibility
 		wantError    error
 		wantMetrics  []metrics.TestState
 	}{
@@ -350,7 +351,7 @@ func TestGetVisibility(t *testing.T) {
 			name:         "hidden by policy",
 			key:          pkey.AdminConsoleVisibility,
 			handlerValue: "hide",
-			wantValue:    setting.HiddenByPolicy,
+			wantValue:    ptype.HiddenByPolicy,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_any", Value: 1},
 				{Name: "$os_syspolicy_AdminConsole", Value: 1},
@@ -360,7 +361,7 @@ func TestGetVisibility(t *testing.T) {
 			name:         "visibility default",
 			key:          pkey.AdminConsoleVisibility,
 			handlerValue: "show",
-			wantValue:    setting.VisibleByPolicy,
+			wantValue:    ptype.VisibleByPolicy,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_any", Value: 1},
 				{Name: "$os_syspolicy_AdminConsole", Value: 1},
@@ -371,14 +372,14 @@ func TestGetVisibility(t *testing.T) {
 			key:          pkey.AdminConsoleVisibility,
 			handlerValue: "show",
 			handlerError: ErrNotConfigured,
-			wantValue:    setting.VisibleByPolicy,
+			wantValue:    ptype.VisibleByPolicy,
 		},
 		{
 			name:         "other error is returned",
 			key:          pkey.AdminConsoleVisibility,
 			handlerValue: "show",
 			handlerError: someOtherError,
-			wantValue:    setting.VisibleByPolicy,
+			wantValue:    ptype.VisibleByPolicy,
 			wantError:    someOtherError,
 			wantMetrics: []metrics.TestState{
 				{Name: "$os_syspolicy_errors", Value: 1},