From b09a07023a01d4210466a763081d548bb0134b98 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 8 Jun 2026 17:44:38 +0200 Subject: [PATCH] Tests MDM config reload via ticker --- client/mdm/ticker.go | 50 +++++++++++++------ client/mdm/ticker_test.go | 100 ++++++++++++++++++++++++++++++++++++++ client/server/server.go | 2 +- 3 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 client/mdm/ticker_test.go diff --git a/client/mdm/ticker.go b/client/mdm/ticker.go index 6ea374b1e..b0113ff54 100644 --- a/client/mdm/ticker.go +++ b/client/mdm/ticker.go @@ -4,16 +4,40 @@ import ( "context" "reflect" "sort" + "testing" "time" log "github.com/sirupsen/logrus" ) -// DefaultReloadInterval is the cadence at which the desktop daemon re-reads -// the OS-native MDM policy. Picked to balance responsiveness against +// defaultReloadInterval is the production cadence at which the desktop daemon +// re-reads the OS-native MDM policy. Picked to balance responsiveness against // registry/plist I/O overhead. Mobile builds use OS-side notifications -// instead and bypass this ticker entirely. -const DefaultReloadInterval = 1 * time.Minute +// instead and bypass this ticker entirely. Unexported on purpose: callers do +// not pass it — NewTicker owns the default (see reloadInterval). +const defaultReloadInterval = 1 * time.Minute + +// testReloadInterval is the cadence used under `go test` (detected via +// testing.Testing()) so the reload path is exercised in seconds rather than +// minutes. It has no effect on production builds, where testing.Testing() +// always returns false. +const testReloadInterval = 1 * time.Second + +// reloadInterval returns the production cadence, or the accelerated test +// cadence when running under `go test`. Centralising the choice here keeps +// the prod/test split in one place and out of the ticker's call sites. +func reloadInterval() time.Duration { + if testing.Testing() { + return testReloadInterval + } + return defaultReloadInterval +} + +// policyLoader is the indirection through which the ticker reads the +// OS-native policy, both for the initial observation and on every tick. +// Production points it at LoadPolicy; tests in this package override it to +// feed a scripted sequence of policies without touching the real OS store. +var policyLoader = LoadPolicy // Ticker periodically re-reads the OS-native MDM policy via LoadPolicy and // invokes onChange whenever the observed Policy diverges from the last @@ -25,17 +49,15 @@ type Ticker struct { prev *Policy } -// NewTicker constructs a Ticker that calls LoadPolicy every `interval` and -// invokes `onChange` on any diff. Pass a zero interval to fall back to -// DefaultReloadInterval. onChange may be nil for a log-only ticker. -func NewTicker(interval time.Duration, onChange func(prev, curr *Policy)) *Ticker { - if interval <= 0 { - interval = DefaultReloadInterval - } +// NewTicker constructs a Ticker that re-reads the OS-native policy every +// reloadInterval() and invokes onChange on any diff. The cadence is owned by +// reloadInterval (production default, accelerated under `go test`); callers +// do not supply it. onChange may be nil for a log-only ticker. +func NewTicker(onChange func(prev, curr *Policy)) *Ticker { return &Ticker{ - interval: interval, + interval: reloadInterval(), onChange: onChange, - prev: LoadPolicy(), + prev: policyLoader(), } } @@ -52,7 +74,7 @@ func (t *Ticker) Run(ctx context.Context) { log.Info("MDM policy reload ticker stopped") return case <-tk.C: - curr := LoadPolicy() + curr := policyLoader() if PoliciesEqual(t.prev, curr) { continue } diff --git a/client/mdm/ticker_test.go b/client/mdm/ticker_test.go new file mode 100644 index 000000000..594f6a115 --- /dev/null +++ b/client/mdm/ticker_test.go @@ -0,0 +1,100 @@ +package mdm + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// withPolicyLoader overrides the package-level policyLoader for the duration +// of the test so the ticker observes a scripted policy instead of the real +// OS-native store. The original loader is restored on cleanup. +func withPolicyLoader(t *testing.T, fn func() *Policy) { + t.Helper() + prev := policyLoader + policyLoader = fn + t.Cleanup(func() { policyLoader = prev }) +} + +func TestTicker_UsesTestCadenceUnderGoTest(t *testing.T) { + withPolicyLoader(t, func() *Policy { return NewPolicy(nil) }) + + // Under `go test`, testing.Testing() is true so reloadInterval() returns + // the accelerated 1s cadence instead of the minute-long production + // default — this is what makes the reload path observable without a real + // wall-clock wait. + assert.Equal(t, testReloadInterval, reloadInterval()) + assert.Equal(t, testReloadInterval, NewTicker(nil).interval) +} + +func TestTicker_FiresOnChangeWithDelta(t *testing.T) { + var mu sync.Mutex + current := NewPolicy(nil) // initial observation: empty (no enforcement) + withPolicyLoader(t, func() *Policy { + mu.Lock() + defer mu.Unlock() + return current + }) + + type change struct{ prev, curr *Policy } + changes := make(chan change, 1) + tk := NewTicker(func(prev, curr *Policy) { + select { + case changes <- change{prev, curr}: + default: + } + }) + require.Equal(t, testReloadInterval, tk.interval) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { tk.Run(ctx); close(done) }() + // Stop Run and wait for it to exit before returning, so the policyLoader + // restore in t.Cleanup can't race the ticker goroutine still reading it. + defer func() { cancel(); <-done }() + + // Flip the OS-observed policy from empty to one managed key. The next + // tick must detect the diff and invoke onChange. + mu.Lock() + current = NewPolicy(map[string]any{KeyManagementURL: "https://mdm.example.com:443"}) + mu.Unlock() + + select { + case c := <-changes: + assert.True(t, c.prev.IsEmpty(), "prev should be the initial empty policy") + assert.True(t, c.curr.HasKey(KeyManagementURL), "curr should carry the newly-pushed managed key") + case <-time.After(5 * time.Second): + t.Fatal("onChange not invoked within 5s; ticker should fire every 1s under test") + } +} + +func TestTicker_NoCallbackWhenPolicyUnchanged(t *testing.T) { + withPolicyLoader(t, func() *Policy { + return NewPolicy(map[string]any{KeyBlockInbound: true}) + }) + + fired := make(chan struct{}, 1) + tk := NewTicker(func(_, _ *Policy) { + select { + case fired <- struct{}{}: + default: + } + }) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { tk.Run(ctx); close(done) }() + defer func() { cancel(); <-done }() + + // Over ~2 ticks at the 1s test cadence the policy never changes, so the + // diff guard must suppress the callback entirely. + select { + case <-fired: + t.Fatal("onChange fired despite an unchanged policy") + case <-time.After(2500 * time.Millisecond): + } +} diff --git a/client/server/server.go b/client/server/server.go index c06b05c26..6d6d58b37 100644 --- a/client/server/server.go +++ b/client/server/server.go @@ -168,7 +168,7 @@ func (s *Server) Start() error { // applies the freshly-read MDM policy as the last layer) and brings // the engine back with the new values. if s.mdmTicker == nil { - s.mdmTicker = mdm.NewTicker(mdm.DefaultReloadInterval, s.onMDMPolicyChange) + s.mdmTicker = mdm.NewTicker(s.onMDMPolicyChange) go s.mdmTicker.Run(s.rootCtx) }