From 7a7e314096919a235165622ad488cef81daed20e Mon Sep 17 00:00:00 2001 From: Andrew Lytvynov Date: Thu, 6 Jun 2024 16:31:52 -0700 Subject: [PATCH] ipn/ipnlocal,clientupdate: allow auto-updates in contaienrs (#12391) We assume most containers are immutable and don't expect tailscale running in them to auto-update. But there's no reason to prohibit it outright. Ignore the tailnet-wide default auto-update setting in containers, but allow local users to turn on auto-updates via the CLI. RELNOTE=Auto-updates are allowed in containers, but ignore the tailnet-wide default. Fixes #12292 Signed-off-by: Andrew Lytvynov --- clientupdate/clientupdate.go | 28 +++++++++++++--------------- ipn/ipnlocal/local.go | 6 ++++++ ipn/ipnlocal/local_test.go | 22 ++++++++++++++++++++++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/clientupdate/clientupdate.go b/clientupdate/clientupdate.go index c5471956f..47627ddf8 100644 --- a/clientupdate/clientupdate.go +++ b/clientupdate/clientupdate.go @@ -29,7 +29,6 @@ import ( "github.com/google/uuid" "tailscale.com/clientupdate/distsign" - "tailscale.com/hostinfo" "tailscale.com/types/logger" "tailscale.com/util/cmpver" "tailscale.com/util/winutil" @@ -163,10 +162,9 @@ func NewUpdater(args Arguments) (*Updater, error) { type updateFunction func() error func (up *Updater) getUpdateFunction() (fn updateFunction, canAutoUpdate bool) { - canAutoUpdate = !hostinfo.New().Container.EqualBool(true) // EqualBool(false) would return false if the value is not set. switch runtime.GOOS { case "windows": - return up.updateWindows, canAutoUpdate + return up.updateWindows, true case "linux": switch distro.Get() { case distro.NixOS: @@ -180,20 +178,20 @@ func (up *Updater) getUpdateFunction() (fn updateFunction, canAutoUpdate bool) { // auto-update mechanism. return up.updateSynology, false case distro.Debian: // includes Ubuntu - return up.updateDebLike, canAutoUpdate + return up.updateDebLike, true case distro.Arch: if up.archPackageInstalled() { // Arch update func just prints a message about how to update, // it doesn't support auto-updates. return up.updateArchLike, false } - return up.updateLinuxBinary, canAutoUpdate + return up.updateLinuxBinary, true case distro.Alpine: - return up.updateAlpineLike, canAutoUpdate + return up.updateAlpineLike, true case distro.Unraid: - return up.updateUnraid, canAutoUpdate + return up.updateUnraid, true case distro.QNAP: - return up.updateQNAP, canAutoUpdate + return up.updateQNAP, true } switch { case haveExecutable("pacman"): @@ -202,21 +200,21 @@ func (up *Updater) getUpdateFunction() (fn updateFunction, canAutoUpdate bool) { // it doesn't support auto-updates. return up.updateArchLike, false } - return up.updateLinuxBinary, canAutoUpdate + return up.updateLinuxBinary, true case haveExecutable("apt-get"): // TODO(awly): add support for "apt" // The distro.Debian switch case above should catch most apt-based // systems, but add this fallback just in case. - return up.updateDebLike, canAutoUpdate + return up.updateDebLike, true case haveExecutable("dnf"): - return up.updateFedoraLike("dnf"), canAutoUpdate + return up.updateFedoraLike("dnf"), true case haveExecutable("yum"): - return up.updateFedoraLike("yum"), canAutoUpdate + return up.updateFedoraLike("yum"), true case haveExecutable("apk"): - return up.updateAlpineLike, canAutoUpdate + return up.updateAlpineLike, true } // If nothing matched, fall back to tarball updates. if up.Update == nil { - return up.updateLinuxBinary, canAutoUpdate + return up.updateLinuxBinary, true } case "darwin": switch { @@ -232,7 +230,7 @@ func (up *Updater) getUpdateFunction() (fn updateFunction, canAutoUpdate bool) { return nil, false } case "freebsd": - return up.updateFreeBSD, canAutoUpdate + return up.updateFreeBSD, true } return nil, false } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 6483e5a60..0bda0aba1 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2575,6 +2575,12 @@ func (b *LocalBackend) onTailnetDefaultAutoUpdate(au bool) { // user. Tailnet default should not affect us, even if it changes. return } + if au && b.hostinfo != nil && b.hostinfo.Container.EqualBool(true) { + // This is a containerized node, which is usually meant to be + // immutable. Do not enable auto-updates if the tailnet does. But users + // can still manually enable auto-updates on this node. + return + } b.logf("using tailnet default auto-update setting: %v", au) prefsClone := prefs.AsStruct() prefsClone.AutoUpdate.Apply = opt.NewBool(au) diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index cb3b5c00a..025060c72 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -30,6 +30,7 @@ import ( "tailscale.com/drive" "tailscale.com/drive/driveimpl" "tailscale.com/health" + "tailscale.com/hostinfo" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/net/netcheck" @@ -2296,6 +2297,7 @@ func TestPreferencePolicyInfo(t *testing.T) { func TestOnTailnetDefaultAutoUpdate(t *testing.T) { tests := []struct { before, after opt.Bool + container opt.Bool tailnetDefault bool }{ { @@ -2328,10 +2330,30 @@ func TestOnTailnetDefaultAutoUpdate(t *testing.T) { tailnetDefault: false, after: opt.NewBool(true), }, + { + before: opt.Bool(""), + container: opt.NewBool(true), + tailnetDefault: true, + after: opt.Bool(""), + }, + { + before: opt.NewBool(false), + container: opt.NewBool(true), + tailnetDefault: true, + after: opt.NewBool(false), + }, + { + before: opt.NewBool(true), + container: opt.NewBool(true), + tailnetDefault: false, + after: opt.NewBool(true), + }, } for _, tt := range tests { t.Run(fmt.Sprintf("before=%s,after=%s", tt.before, tt.after), func(t *testing.T) { b := newTestBackend(t) + b.hostinfo = hostinfo.New() + b.hostinfo.Container = tt.container p := ipn.NewPrefs() p.AutoUpdate.Apply = tt.before if err := b.pm.setPrefsLocked(p.View()); err != nil {