From b5f1ae3d90b95794b78affd51c3bf27095c1e5b7 Mon Sep 17 00:00:00 2001 From: Juan Font Date: Fri, 25 Nov 2022 15:29:45 +0000 Subject: [PATCH] A big bunch of golang-lint fixes --- db.go | 4 ++++ machine.go | 5 +++++ machine_test.go | 4 +++- protocol_common_poll.go | 10 +++++++++- routes.go | 34 +++++++++++++++++++--------------- 5 files changed, 40 insertions(+), 17 deletions(-) diff --git a/db.go b/db.go index 72386ca8..41949525 100644 --- a/db.go +++ b/db.go @@ -106,6 +106,7 @@ func (h *Headscale) initDB() error { Err(err). Str("enabled_route", prefix.String()). Msg("Error parsing enabled_route") + continue } @@ -114,6 +115,7 @@ func (h *Headscale) initDB() error { log.Info(). Str("enabled_route", prefix.String()). Msg("Route already migrated to new table, skipping") + continue } @@ -335,6 +337,7 @@ func (i *IPPrefix) Scan(destination interface{}) error { return err } *i = IPPrefix(prefix) + return nil default: return fmt.Errorf("%w: unexpected data type %T", ErrCannotParsePrefix, destination) @@ -344,6 +347,7 @@ func (i *IPPrefix) Scan(destination interface{}) error { // Value return json value, implement driver.Valuer interface. func (i IPPrefix) Value() (driver.Value, error) { prefixStr := netip.Prefix(i).String() + return prefixStr, nil } diff --git a/machine.go b/machine.go index 0ce8671b..7b83e44c 100644 --- a/machine.go +++ b/machine.go @@ -937,6 +937,7 @@ func (h *Headscale) GetAdvertisedRoutes(machine *Machine) ([]netip.Prefix, error Err(err). Str("machine", machine.Hostname). Msg("Could not get advertised routes for machine") + return nil, err } @@ -962,6 +963,7 @@ func (h *Headscale) GetEnabledRoutes(machine *Machine) ([]netip.Prefix, error) { Err(err). Str("machine", machine.Hostname). Msg("Could not get enabled routes for machine") + return nil, err } @@ -982,6 +984,7 @@ func (h *Headscale) IsRoutesEnabled(machine *Machine, routeStr string) bool { enabledRoutes, err := h.GetEnabledRoutes(machine) if err != nil { log.Error().Err(err).Msg("Could not get enabled routes") + return false } @@ -1122,12 +1125,14 @@ func (h *Headscale) RoutesToProto(machine *Machine) *v1.Routes { availableRoutes, err := h.GetAdvertisedRoutes(machine) if err != nil { log.Error().Err(err).Msg("Could not get advertised routes") + return nil } enabledRoutes, err := h.GetEnabledRoutes(machine) if err != nil { log.Error().Err(err).Msg("Could not get enabled routes") + return nil } diff --git a/machine_test.go b/machine_test.go index c5073233..6cbf2821 100644 --- a/machine_test.go +++ b/machine_test.go @@ -1159,7 +1159,9 @@ func (s *Suite) TestAutoApproveRoutes(c *check.C) { machine0ByID, err := app.GetMachineByID(0) c.Assert(err, check.IsNil) - app.EnableAutoApprovedRoutes(machine0ByID) + err = app.EnableAutoApprovedRoutes(machine0ByID) + c.Assert(err, check.IsNil) + enabledRoutes, err := app.GetEnabledRoutes(machine0ByID) c.Assert(err, check.IsNil) c.Assert(enabledRoutes, check.HasLen, 3) diff --git a/protocol_common_poll.go b/protocol_common_poll.go index 7df01c86..8732c707 100644 --- a/protocol_common_poll.go +++ b/protocol_common_poll.go @@ -53,7 +53,15 @@ func (h *Headscale) handlePollCommon( } // update routes with peer information - h.EnableAutoApprovedRoutes(machine) + err = h.EnableAutoApprovedRoutes(machine) + if err != nil { + log.Error(). + Caller(). + Bool("noise", isNoise). + Str("machine", machine.Hostname). + Err(err). + Msg("Error running auto approved routes") + } } // From Tailscale client: diff --git a/routes.go b/routes.go index 221db60f..7de728de 100644 --- a/routes.go +++ b/routes.go @@ -1,6 +1,7 @@ package headscale import ( + "errors" "fmt" "net/netip" @@ -44,10 +45,11 @@ func (rs Routes) toPrefixes() []netip.Prefix { for i, r := range rs { prefixes[i] = netip.Prefix(r.Prefix) } + return prefixes } -// isUniquePrefix returns if there is another machine providing the same route already +// isUniquePrefix returns if there is another machine providing the same route already. func (h *Headscale) isUniquePrefix(route Route) bool { var count int64 h.db. @@ -56,6 +58,7 @@ func (h *Headscale) isUniquePrefix(route Route) bool { route.Prefix, route.MachineID, true, true).Count(&count) + return count == 0 } @@ -65,11 +68,11 @@ func (h *Headscale) getPrimaryRoute(prefix netip.Prefix) (*Route, error) { Preload("Machine"). Where("prefix = ? AND advertised = ? AND enabled = ? AND is_primary = ?", IPPrefix(prefix), true, true, true). First(&route).Error - if err != nil && err != gorm.ErrRecordNotFound { + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { return nil, err } - if err == gorm.ErrRecordNotFound { + if errors.Is(err, gorm.ErrRecordNotFound) { return nil, gorm.ErrRecordNotFound } @@ -77,7 +80,7 @@ func (h *Headscale) getPrimaryRoute(prefix netip.Prefix) (*Route, error) { } // getMachinePrimaryRoutes returns the routes that are enabled and marked as primary (for subnet failover) -// Exit nodes are not considered for this, as they are never marked as Primary +// Exit nodes are not considered for this, as they are never marked as Primary. func (h *Headscale) getMachinePrimaryRoutes(m *Machine) ([]Route, error) { var routes []Route err := h.db. @@ -113,14 +116,12 @@ func (h *Headscale) processMachineRoutes(machine *Machine) error { } } advertisedRoutes[netip.Prefix(route.Prefix)] = true - } else { - if route.Advertised { - route.Advertised = false - route.Enabled = false - err := h.db.Save(&route).Error - if err != nil { - return err - } + } else if route.Advertised { + route.Advertised = false + route.Enabled = false + err := h.db.Save(&route).Error + if err != nil { + return err } } } @@ -150,7 +151,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { Preload("Machine"). Where("advertised = ? AND enabled = ?", true, true). Find(&routes).Error - if err != nil && err != gorm.ErrRecordNotFound { + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { log.Error().Err(err).Msg("error getting routes") } @@ -161,7 +162,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { if !route.IsPrimary { _, err := h.getPrimaryRoute(netip.Prefix(route.Prefix)) - if h.isUniquePrefix(route) || err == gorm.ErrRecordNotFound { + if h.isUniquePrefix(route) || errors.Is(err, gorm.ErrRecordNotFound) { route.IsPrimary = true err := h.db.Save(&route).Error if err != nil { @@ -169,6 +170,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { return err } + continue } } @@ -193,7 +195,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { route.MachineID, true, true). Find(&newPrimaryRoutes).Error - if err != nil && err != gorm.ErrRecordNotFound { + if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) { log.Error().Err(err).Msg("error finding new primary route") return err @@ -203,6 +205,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { for _, r := range newPrimaryRoutes { if r.Machine.isOnline() { newPrimaryRoute = &r + break } } @@ -212,6 +215,7 @@ func (h *Headscale) handlePrimarySubnetFailover() error { Str("machine", route.Machine.Hostname). Str("prefix", netip.Prefix(route.Prefix).String()). Msgf("no alternative primary route found") + continue }