ipn/ipnlocal: only send AllowsUpdate if clientupdate feature is linked in

Like the earlier RemoteConfig change, gate Hostinfo.AllowsUpdate on
feature.IsRegistered("clientupdate") in addition to the
buildfeatures.HasClientUpdate build-tag const. tsnet binaries don't
import feature/clientupdate even though ts_omit_clientupdate isn't
set, so they shouldn't tell control they can be remotely updated.

Add the previously missing feature.Register call to
feature/clientupdate, document the binary-support requirement on
tailcfg.Hostinfo.AllowsUpdate, and make tsnet's dep test verify it
doesn't depend on feature/clientupdate.

Updates #12614

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

feature/clientupdate/clientupdate.go

@@ -36,6 +36,7 @@ import (
 )
 
 func init() {
+	feature.Register("clientupdate")
 	ipnext.RegisterExtension("clientupdate", newExt)
 
 	// C2N

ipn/ipnlocal/local.go

@@ -6626,7 +6626,9 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip
 	// feature/remoteconfig even though ts_omit_remoteconfig is not
 	// set, so we must not claim RemoteConfig is active there.
 	hi.RemoteConfig = buildfeatures.HasRemoteConfig && prefs.RemoteConfig() && feature.IsRegistered("remoteconfig")
-	hi.AllowsUpdate = buildfeatures.HasClientUpdate && (envknob.AllowsRemoteUpdate() || prefs.AutoUpdate().Apply.EqualBool(true))
+	// Likewise for AllowsUpdate: require the clientupdate feature's init to
+	// have run, not just the build tag being enabled.
+	hi.AllowsUpdate = buildfeatures.HasClientUpdate && (envknob.AllowsRemoteUpdate() || prefs.AutoUpdate().Apply.EqualBool(true)) && feature.IsRegistered("clientupdate")
 
 	if buildfeatures.HasAdvertiseRoutes {
 		b.metrics.advertisedRoutes.Set(float64(tsaddr.WithoutExitRoute(prefs.AdvertiseRoutes()).Len()))

tailcfg/tailcfg.go

@@ -913,9 +913,15 @@ type Hostinfo struct {
 	// away, even if it's disabled most of the time. As an optimization, this is
 	// only sent if IngressEnabled is false, as IngressEnabled implies that this
 	// option is true.
-	WireIngress     bool           `json:",omitzero"`
+	WireIngress    bool `json:",omitzero"`
-	IngressEnabled  bool           `json:",omitzero"`  // if the node has any funnel endpoint enabled
+	IngressEnabled bool `json:",omitzero"` // if the node has any funnel endpoint enabled
-	AllowsUpdate    bool           `json:",omitzero"`  // indicates that the node has opted-in to admin-console-drive remote updates
+
+	// AllowsUpdate reports that the node has opted in to
+	// admin-console-driven remote updates and that the running binary
+	// includes client update support (the feature/clientupdate package,
+	// which tsnet apps don't include).
+	AllowsUpdate bool `json:",omitzero"`
+
 	Machine         string         `json:",omitzero"`  // the current host's machine type (uname -m)
 	GoArch          string         `json:",omitzero"`  // GOARCH value (of the built binary)
 	GoArchVar       string         `json:",omitzero"`  // GOARM, GOAMD64, etc (of the built binary)

tailcfg/tailcfg_view.go

@@ -570,7 +570,10 @@ func (v HostinfoView) WireIngress() bool { return v.ж.WireIngress }
 // if the node has any funnel endpoint enabled
 func (v HostinfoView) IngressEnabled() bool { return v.ж.IngressEnabled }
 
-// indicates that the node has opted-in to admin-console-drive remote updates
+// AllowsUpdate reports that the node has opted in to
+// admin-console-driven remote updates and that the running binary
+// includes client update support (the feature/clientupdate package,
+// which tsnet apps don't include).
 func (v HostinfoView) AllowsUpdate() bool { return v.ж.AllowsUpdate }
 
 // the current host's machine type (uname -m)

tsnet/tsnet_test.go

@@ -3409,6 +3409,7 @@ func TestDeps(t *testing.T) {
 		BadDeps: map[string]string{
 			"golang.org/x/crypto/ssh":                       "tsnet should not depend on SSH",
 			"golang.org/x/crypto/ssh/internal/bcrypt_pbkdf": "tsnet should not depend on SSH",
+			"tailscale.com/feature/clientupdate":            "tsnet should not depend on feature/clientupdate",
 			"tailscale.com/feature/remoteconfig":            "tsnet should not depend on feature/remoteconfig",
 			"tailscale.com/feature/syspolicy":               "tsnet should not depend on syspolicy",
 			"tailscale.com/ipn/store/awsstore":              "tsnet callers wanting AWS state storage should import awsstore themselves",