state: let a node's name follow a new hostname at login

As SaaS. It stayed old, and the mismatch then read as an admin-set name,
which stopped later hostname changes from renaming the node too.

hscontrol/servertest/lifecycle_compat_test.go

@@ -187,6 +187,14 @@ func runRouteLifecycleCompat(t *testing.T, id, observer string, skips map[string
 				{"filter-routes", filterRouteSources(step.Captures[observer].Netmap), func() string {
 					return filterRouteSources(r.clients[observer].Netmap())
 				}},
+				{"name", firstLabel(dev.Name), func() string {
+					nodes := r.machineNodes(router)
+					if len(nodes) != 1 {
+						return fmt.Sprintf("devices %d", len(nodes))
+					}
+
+					return nodes[0].GivenName()
+				}},
 				{"device", deviceState(
 					countDevicesOnMachine(t, step, dev.MachineKey),
 					dev.ID == join.saasID, sortedStrings(dev.Addresses) == join.saasAddrs,
@@ -390,6 +398,12 @@ func filterRouteSources(nm *netmap.NetworkMap) string {
 	return fmt.Sprint(slices.Sorted(maps.Keys(seen)))
 }
 
+func firstLabel(name string) string {
+	label, _, _ := strings.Cut(name, ".")
+
+	return label
+}
+
 // saasDevice is the part of a SaaS device-list entry the checks read.
 // Keys and IDs are the capture's labels.
 type saasDevice struct {

hscontrol/servertest/lifecycle_test.go

@@ -679,6 +679,50 @@ func TestNodeExpiryRouteFailover(t *testing.T) {
 	}
 }
 
+// TestReloginFollowsNewHostname: a node's name follows a new hostname at a login
+// as at a map request, and later hostname changes still rename it. An admin-set
+// name stays.
+func TestReloginFollowsNewHostname(t *testing.T) {
+	t.Parallel()
+
+	hostinfo := func(name string) *tailcfg.Hostinfo {
+		return &tailcfg.Hostinfo{Hostname: name, BackendLogID: "servertest"}
+	}
+
+	srv := servertest.NewServer(t)
+	owner := srv.CreateUser(t, "owner")
+	id := types.UserID(owner.ID)
+	client := servertest.NewClient(t, srv, "first", servertest.WithUser(owner))
+
+	client.WaitForCondition(t, "a first map", 20*time.Second, func(nm *netmap.NetworkMap) bool {
+		return nm != nil && nm.SelfNode.Valid()
+	})
+
+	givenName := func() string {
+		nodes := srv.State().ListNodes()
+		require.Equal(t, 1, nodes.Len())
+
+		return nodes.At(0).GivenName()
+	}
+
+	require.NoError(t, client.LogoutAndDisconnect(t.Context()))
+	require.NoError(t, client.Relogin(t.Context(), srv.CreatePreAuthKey(t, id), false, hostinfo("second")))
+	assert.Equal(t, "second", givenName())
+
+	client.Direct().SetHostinfo(hostinfo("third"))
+	require.NoError(t, client.Direct().SendUpdate(t.Context()))
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		assert.Equal(c, "third", givenName())
+	}, 10*time.Second, 100*time.Millisecond, "a later hostname change renames too")
+
+	_, _, err := srv.State().RenameNode(srv.State().ListNodes().At(0).ID(), "chosen")
+	require.NoError(t, err)
+
+	require.NoError(t, client.LogoutAndDisconnect(t.Context()))
+	require.NoError(t, client.Relogin(t.Context(), srv.CreatePreAuthKey(t, id), false, hostinfo("fourth")))
+	assert.Equal(t, "chosen", givenName(), "a login keeps an admin-set name")
+}
+
 // TestLogoutOfDeletedDeviceRegistersNothing: a client sends its auth key with a
 // logout, and that must not bring back the device an admin just deleted.
 func TestLogoutOfDeletedDeviceRegistersNothing(t *testing.T) {

hscontrol/state/state.go

@@ -2578,7 +2578,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 			node.DiscoKey = regData.DiscoKey
 		}
 
-		node.Hostname = params.Hostname
+		setHostname(node, params.Hostname)
 		node.Hostinfo = hostinfo
 
 		// Preserve the node's live endpoints when the register request carried
@@ -3699,7 +3699,7 @@ func (s *State) reregisterNodeWithPAK(
 			return
 		}
 
-		node.Hostname = hostname
+		setHostname(node, hostname)
 
 		// TODO(kradalby): We should ensure we use the same hostinfo and node merge semantics
 		// when a node re-registers as we do when it sends a map request (UpdateNodeFromMapRequest).
@@ -4114,6 +4114,23 @@ func (s *State) PingDB(ctx context.Context) error {
 	return s.db.PingDB(ctx)
 }
 
+// setHostname stores a node's hostname, and its name follows, as on SaaS. An
+// admin-set name stays: the name only follows while it is still what the old
+// hostname gives (possibly with a "-N" collision bump). [NodeStore.UpdateNode]
+// bumps a name that collides.
+func setHostname(node *types.Node, hostname string) {
+	if hostname == node.Hostname {
+		return
+	}
+
+	autoDerived := isAutoDerivedGivenName(node.GivenName, node.Hostname)
+
+	node.Hostname = hostname
+	if autoDerived {
+		node.GivenName = dnsname.SanitizeHostname(hostname)
+	}
+}
+
 // isAutoDerivedGivenName reports whether given matches what
 // dnsname.SanitizeHostname(hostname) would produce, optionally with a
 // [NodeStore] collision-bump "-N" suffix ([bumpedLabel]). It is used to
@@ -4271,17 +4288,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 		// Only a real hostname change may re-derive GivenName: it is peer
 		// visible, so the whole node is resent and peers learn the name.
 		if hasReportedHostnameChange(currentNode.Hostname, newHostinfo) {
-			// Preserve an admin-renamed GivenName: only auto-derive
-			// when the current GivenName is still what
-			// SanitizeHostname of the old Hostname would produce
-			// (possibly with a "-N" collision bump).
-			autoDerived := isAutoDerivedGivenName(currentNode.GivenName, currentNode.Hostname)
-
-			currentNode.Hostname = newHostinfo.Hostname
-			if autoDerived {
-				currentNode.GivenName = dnsname.SanitizeHostname(newHostinfo.Hostname)
-				// [NodeStore.UpdateNode] auto-bumps GivenName on collision.
-			}
+			setHostname(currentNode, newHostinfo.Hostname)
 		}
 
 		// Approval follows the announced set, only when it changes: any