From 7074c49db8bf70bc3516c2b699bc6d3d03a08c98 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 21 Mar 2024 12:19:57 -0700 Subject: [PATCH] control/controlclient: fix panic regression from earlier load balancer hint header In the recent 20e9f3369 we made HealthChangeRequest machine requests include a NodeKey, as it was the oddball machine request that didn't include one. Unfortunately, that code was sometimes being called (at least in some of our integration tests) without a node key due to its registration with health.RegisterWatcher(direct.ReportHealthChange). Fortunately tests in corp caught this before we cut a release. It's possible this only affects this particular integration test's environment, but still worth fixing. Updates tailscale/corp#1297 Change-Id: I84046779955105763dc1be5121c69fec3c138672 Signed-off-by: Brad Fitzpatrick (cherry picked from commit 8444937c89eb08e41359da38921f90fcae823336) --- control/controlclient/direct.go | 5 ++++- types/persist/persist.go | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index 5eb025423..e1a48aad7 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -1719,7 +1719,10 @@ func (c *Direct) ReportHealthChange(sys health.Subsystem, sysErr error) { // Don't report errors to control if the server doesn't support noise. return } - nodeKey := c.GetPersist().PublicNodeKey() + nodeKey, ok := c.GetPersist().PublicNodeKeyOK() + if !ok { + return + } req := &tailcfg.HealthChangeRequest{ Subsys: string(sys), NodeKey: nodeKey, diff --git a/types/persist/persist.go b/types/persist/persist.go index 19df45dcb..60c9438e5 100644 --- a/types/persist/persist.go +++ b/types/persist/persist.go @@ -51,11 +51,32 @@ func (p *Persist) PublicNodeKey() key.NodePublic { return p.PrivateNodeKey.Public() } +// PublicNodeKeyOK returns the public key for the node key. +// +// Unlike PublicNodeKey, it returns ok=false if there is no node private key +// instead of panicking. +func (p *Persist) PublicNodeKeyOK() (pub key.NodePublic, ok bool) { + if p.PrivateNodeKey.IsZero() { + return + } + return p.PrivateNodeKey.Public(), true +} + // PublicNodeKey returns the public key for the node key. +// +// It panics if there is no node private key. See PublicNodeKeyOK. func (p PersistView) PublicNodeKey() key.NodePublic { return p.ж.PublicNodeKey() } +// PublicNodeKeyOK returns the public key for the node key. +// +// Unlike PublicNodeKey, it returns ok=false if there is no node private key +// instead of panicking. +func (p PersistView) PublicNodeKeyOK() (_ key.NodePublic, ok bool) { + return p.ж.PublicNodeKeyOK() +} + func (p PersistView) Equals(p2 PersistView) bool { return p.ж.Equals(p2.ж) }