From 52cc3bc8eb8e50c2d4bd4e420a54d845e8872dcc Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 30 May 2022 15:31:06 +0200 Subject: [PATCH 1/4] Check all errors for db.Save --- api_key.go | 5 ++++- machine.go | 32 ++++++++++++++++++++++++-------- preauth_keys.go | 14 +++++++++++--- routes.go | 7 ++++++- 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/api_key.go b/api_key.go index a968b260..41c83857 100644 --- a/api_key.go +++ b/api_key.go @@ -57,7 +57,10 @@ func (h *Headscale) CreateAPIKey( Hash: hash, Expiration: expiration, } - h.db.Save(&key) + + if err := h.db.Save(&key).Error; err != nil { + return "", nil, fmt.Errorf("failed to save API key to database: %w", err) + } return keyStr, &key, nil } diff --git a/machine.go b/machine.go index 845b6491..ef4b86a3 100644 --- a/machine.go +++ b/machine.go @@ -367,23 +367,30 @@ func (h *Headscale) SetTags(machine *Machine, tags []string) error { return err } h.setLastStateChangeToNow(machine.Namespace.Name) - h.db.Save(machine) + + if err := h.db.Save(machine).Error; err != nil { + return fmt.Errorf("failed to update tags for machine in the database: %w", err) + } return nil } // ExpireMachine takes a Machine struct and sets the expire field to now. -func (h *Headscale) ExpireMachine(machine *Machine) { +func (h *Headscale) ExpireMachine(machine *Machine) error { now := time.Now() machine.Expiry = &now h.setLastStateChangeToNow(machine.Namespace.Name) - h.db.Save(machine) + if err := h.db.Save(machine).Error; err != nil { + return fmt.Errorf("failed to expire machine in the database: %w", err) + } + + return nil } -// RefreshMachine takes a Machine struct and sets the expire field to now. -func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) { +// RefreshMachine takes a Machine struct and sets the expire field. +func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) error { now := time.Now() machine.LastSuccessfulUpdate = &now @@ -391,7 +398,11 @@ func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) { h.setLastStateChangeToNow(machine.Namespace.Name) - h.db.Save(machine) + if err := h.db.Save(machine).Error; err != nil { + return fmt.Errorf("failed to refresh machine (update expiration) in the database: %w", err) + } + + return nil } // DeleteMachine softs deletes a Machine from the database. @@ -761,7 +772,9 @@ func (h *Headscale) RegisterMachine(machine Machine, machine.IPAddresses = ips - h.db.Save(&machine) + if err := h.db.Save(machine).Error; err != nil { + return nil, fmt.Errorf("failed register(save) machine in the database: %w", err) + } log.Trace(). Caller(). @@ -821,7 +834,10 @@ func (h *Headscale) EnableRoutes(machine *Machine, routeStrs ...string) error { } machine.EnabledRoutes = newRoutes - h.db.Save(&machine) + + if err := h.db.Save(machine).Error; err != nil { + return fmt.Errorf("failed enable routes for machine in the database: %w", err) + } return nil } diff --git a/preauth_keys.go b/preauth_keys.go index 55f62226..b32ff636 100644 --- a/preauth_keys.go +++ b/preauth_keys.go @@ -4,6 +4,7 @@ import ( "crypto/rand" "encoding/hex" "errors" + "fmt" "strconv" "time" @@ -60,7 +61,10 @@ func (h *Headscale) CreatePreAuthKey( CreatedAt: &now, Expiration: expiration, } - h.db.Save(&key) + + if err := h.db.Save(&key).Error; err != nil { + return nil, fmt.Errorf("failed to create key in the database: %w", err) + } return &key, nil } @@ -114,9 +118,13 @@ func (h *Headscale) ExpirePreAuthKey(k *PreAuthKey) error { } // UsePreAuthKey marks a PreAuthKey as used. -func (h *Headscale) UsePreAuthKey(k *PreAuthKey) { +func (h *Headscale) UsePreAuthKey(k *PreAuthKey) error { k.Used = true - h.db.Save(k) + if err := h.db.Save(k).Error; err != nil { + return fmt.Errorf("failed to update key used status in the database: %w", err) + } + + return nil } // checkKeyValidity does the heavy lifting for validation of the PreAuthKey coming from a node diff --git a/routes.go b/routes.go index e8de299b..d062f827 100644 --- a/routes.go +++ b/routes.go @@ -1,6 +1,8 @@ package headscale import ( + "fmt" + "inet.af/netaddr" ) @@ -108,7 +110,10 @@ func (h *Headscale) EnableNodeRoute( } machine.EnabledRoutes = enabledRoutes - h.db.Save(&machine) + + if err := h.db.Save(&machine).Error; err != nil { + return fmt.Errorf("failed to update node routes in the database: %w", err) + } return nil } From a09633e859300fbf00c079f7ec7e284d37081b7e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Mon, 30 May 2022 15:39:24 +0200 Subject: [PATCH 2/4] Check errors of more database calls --- api.go | 13 +++++++++++-- db.go | 4 +++- poll.go | 15 ++++++++++++++- 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/api.go b/api.go index 461492a7..f68b225c 100644 --- a/api.go +++ b/api.go @@ -475,7 +475,16 @@ func (h *Headscale) handleMachineRefreshKey( Str("machine", machine.Name). Msg("We have the OldNodeKey in the database. This is a key refresh") machine.NodeKey = NodePublicKeyStripPrefix(registerRequest.NodeKey) - h.db.Save(&machine) + + if err := h.db.Save(&machine).Error; err != nil { + log.Error(). + Caller(). + Err(err). + Msg("Failed to update machine key in the database") + ctx.String(http.StatusInternalServerError, "Internal server error") + + return + } resp.AuthURL = "" resp.User = *machine.Namespace.toUser() @@ -485,7 +494,7 @@ func (h *Headscale) handleMachineRefreshKey( Caller(). Err(err). Msg("Cannot encode message") - ctx.String(http.StatusInternalServerError, "Extremely sad!") + ctx.String(http.StatusInternalServerError, "Internal server error") return } diff --git a/db.go b/db.go index 9130d90e..0c1bfec6 100644 --- a/db.go +++ b/db.go @@ -175,7 +175,9 @@ func (h *Headscale) setValue(key string, value string) error { return nil } - h.db.Create(keyValue) + if err := h.db.Create(keyValue).Error; err != nil { + return fmt.Errorf("failed to create key value pair in the database: %w", err) + } return nil } diff --git a/poll.go b/poll.go index f6dc1387..4ea10a28 100644 --- a/poll.go +++ b/poll.go @@ -126,7 +126,20 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { machine.Endpoints = req.Endpoints machine.LastSeen = &now } - h.db.Updates(machine) + + if err := h.db.Updates(machine).Error; err != nil { + if err != nil { + log.Error(). + Str("handler", "PollNetMap"). + Str("id", ctx.Param("id")). + Str("machine", machine.Name). + Err(err). + Msg("Failed to persist/update machine in the database") + ctx.String(http.StatusInternalServerError, ":(") + + return + } + } data, err := h.getMapResponse(machineKey, req, machine) if err != nil { From 5ecfbbaf5d423921b6d9a2935cb337734fa06d85 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 31 May 2022 10:05:00 +0200 Subject: [PATCH 3/4] Fix pointer in machine save call --- integration_cli_test.go | 2 +- machine.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration_cli_test.go b/integration_cli_test.go index 9ce31318..1b0f2a52 100644 --- a/integration_cli_test.go +++ b/integration_cli_test.go @@ -60,7 +60,7 @@ func (s *IntegrationCLITestSuite) SetupTest() { } headscaleOptions := &dockertest.RunOptions{ - Name: "headscale", + Name: "headscale-cli", Mounts: []string{ fmt.Sprintf("%s/integration_test/etc:/etc/headscale", currentPath), }, diff --git a/machine.go b/machine.go index ef4b86a3..4f112f67 100644 --- a/machine.go +++ b/machine.go @@ -772,7 +772,7 @@ func (h *Headscale) RegisterMachine(machine Machine, machine.IPAddresses = ips - if err := h.db.Save(machine).Error; err != nil { + if err := h.db.Save(&machine).Error; err != nil { return nil, fmt.Errorf("failed register(save) machine in the database: %w", err) } From a19af0458286a4f18bcd0d33d797d19e4c0a6853 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 31 May 2022 11:03:08 +0200 Subject: [PATCH 4/4] Fix errors introduced by merge --- machine.go | 9 ++++++--- poll.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/machine.go b/machine.go index de3ac731..c2276459 100644 --- a/machine.go +++ b/machine.go @@ -420,7 +420,7 @@ func (h *Headscale) RenameMachine(machine *Machine, newName string) error { h.setLastStateChangeToNow(machine.Namespace.Name) - if err := h.db.Save(machine).Error; err != nil { + if err := h.db.Save(machine).Error; err != nil { return fmt.Errorf("failed to rename machine in the database: %w", err) } @@ -428,7 +428,7 @@ func (h *Headscale) RenameMachine(machine *Machine, newName string) error { } // RefreshMachine takes a Machine struct and sets the expire field to now. -func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) { +func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) error { now := time.Now() machine.LastSuccessfulUpdate = &now @@ -437,7 +437,10 @@ func (h *Headscale) RefreshMachine(machine *Machine, expiry time.Time) { h.setLastStateChangeToNow(machine.Namespace.Name) if err := h.db.Save(machine).Error; err != nil { - return fmt.Errorf("failed to refresh machine (update expiration) in the database: %w", err) + return fmt.Errorf( + "failed to refresh machine (update expiration) in the database: %w", + err, + ) } return nil diff --git a/poll.go b/poll.go index f85f9265..239f260b 100644 --- a/poll.go +++ b/poll.go @@ -121,7 +121,7 @@ func (h *Headscale) PollNetMapHandler(ctx *gin.Context) { log.Error(). Str("handler", "PollNetMap"). Str("id", ctx.Param("id")). - Str("machine", machine.Name). + Str("machine", machine.Hostname). Err(err). Msg("Failed to persist/update machine in the database") ctx.String(http.StatusInternalServerError, ":(")