Skip to content

Commit

Permalink
more log.Error -> fmt.Errorf cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
kradalby authored and juanfont committed Apr 15, 2024
1 parent 1704977 commit 60f0cf9
Show file tree
Hide file tree
Showing 10 changed files with 25 additions and 145 deletions.
2 changes: 0 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ require (
github.com/opencontainers/image-spec v1.1.0-rc6 // indirect
github.com/opencontainers/runc v1.1.12 // indirect
github.com/pelletier/go-toml/v2 v2.1.1 // indirect
github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 // indirect
github.com/pierrec/lz4/v4 v4.1.21 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
Expand All @@ -162,7 +161,6 @@ require (
github.com/safchain/ethtool v0.3.0 // indirect
github.com/sagikazarmark/locafero v0.4.0 // indirect
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/sasha-s/go-deadlock v0.3.1 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/sourcegraph/conc v0.3.0 // indirect
github.com/spf13/afero v1.11.0 // indirect
Expand Down
4 changes: 0 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -336,8 +336,6 @@ github.com/patrickmn/go-cache v2.1.0+incompatible h1:HRMgzkcYKYpi3C8ajMPV8OFXaaR
github.com/patrickmn/go-cache v2.1.0+incompatible/go.mod h1:3Qf8kWWT7OJRJbdiICTKqZju1ZixQ/KpMGzzAfe6+WQ=
github.com/pelletier/go-toml/v2 v2.1.1 h1:LWAJwfNvjQZCFIDKWYQaM62NcYeYViCmWIwmOStowAI=
github.com/pelletier/go-toml/v2 v2.1.1/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5 h1:q2e307iGHPdTGp0hoxKjt1H5pDo6utceo3dQVK3I5XQ=
github.com/petermattis/goid v0.0.0-20180202154549-b0b1615b78e5/go.mod h1:jvVRKCrJTQWu0XVbaOlby/2lO20uSCHEMzzplHXte1o=
github.com/philip-bui/grpc-zerolog v1.0.1 h1:EMacvLRUd2O1K0eWod27ZP5CY1iTNkhBDLSN+Q4JEvA=
github.com/philip-bui/grpc-zerolog v1.0.1/go.mod h1:qXbiq/2X4ZUMMshsqlWyTHOcw7ns+GZmlqZZN05ZHcQ=
github.com/pierrec/lz4/v4 v4.1.14/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4=
Expand Down Expand Up @@ -394,8 +392,6 @@ github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6g
github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWRIczQRv+GVI1AkeQ=
github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA=
github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
github.com/sasha-s/go-deadlock v0.3.1 h1:sqv7fDNShgjcaxkO0JNcOAlr8B9+cV5Ey/OB71efZx0=
github.com/sasha-s/go-deadlock v0.3.1/go.mod h1:F73l+cr82YSh10GxyRI6qZiCgK64VaZjwesgfQ1/iLM=
github.com/sergi/go-diff v1.2.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM=
github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8=
github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I=
Expand Down
32 changes: 5 additions & 27 deletions hscontrol/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import (
"github.com/prometheus/client_golang/prometheus/promhttp"
zl "github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"github.com/sasha-s/go-deadlock"
"golang.org/x/crypto/acme"
"golang.org/x/crypto/acme/autocert"
"golang.org/x/oauth2"
Expand Down Expand Up @@ -79,10 +78,10 @@ const (
registerCacheCleanup = time.Minute * 20
)

func init() {
deadlock.Opts.DeadlockTimeout = 15 * time.Second
deadlock.Opts.PrintAllCurrentGoroutines = true
}
// func init() {
// deadlock.Opts.DeadlockTimeout = 15 * time.Second
// deadlock.Opts.PrintAllCurrentGoroutines = true
// }

// Headscale represents the base app of the service.
type Headscale struct {
Expand All @@ -107,7 +106,7 @@ type Headscale struct {
pollNetMapStreamWG sync.WaitGroup

mapSessions map[types.NodeID]*mapSession
mapSessionMu deadlock.Mutex
mapSessionMu sync.Mutex
}

var (
Expand Down Expand Up @@ -323,11 +322,6 @@ func (h *Headscale) grpcAuthenticationInterceptor(ctx context.Context,

meta, ok := metadata.FromIncomingContext(ctx)
if !ok {
log.Error().
Caller().
Str("client_address", client.Addr.String()).
Msg("Retrieving metadata is failed")

return ctx, status.Errorf(
codes.InvalidArgument,
"Retrieving metadata is failed",
Expand All @@ -336,11 +330,6 @@ func (h *Headscale) grpcAuthenticationInterceptor(ctx context.Context,

authHeader, ok := meta["authorization"]
if !ok {
log.Error().
Caller().
Str("client_address", client.Addr.String()).
Msg("Authorization token is not supplied")

return ctx, status.Errorf(
codes.Unauthenticated,
"Authorization token is not supplied",
Expand All @@ -350,11 +339,6 @@ func (h *Headscale) grpcAuthenticationInterceptor(ctx context.Context,
token := authHeader[0]

if !strings.HasPrefix(token, AuthPrefix) {
log.Error().
Caller().
Str("client_address", client.Addr.String()).
Msg(`missing "Bearer " prefix in "Authorization" header`)

return ctx, status.Error(
codes.Unauthenticated,
`missing "Bearer " prefix in "Authorization" header`,
Expand All @@ -363,12 +347,6 @@ func (h *Headscale) grpcAuthenticationInterceptor(ctx context.Context,

valid, err := h.db.ValidateAPIKey(strings.TrimPrefix(token, AuthPrefix))
if err != nil {
log.Error().
Caller().
Err(err).
Str("client_address", client.Addr.String()).
Msg("failed to validate token")

return ctx, status.Error(codes.Internal, "failed to validate token")
}

Expand Down
39 changes: 3 additions & 36 deletions hscontrol/db/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,7 @@ func RenameNode(tx *gorm.DB,
newName,
)
if err != nil {
log.Error().
Caller().
Str("func", "RenameNode").
Uint64("nodeID", nodeID).
Str("newName", newName).
Err(err).
Msg("failed to rename node")

return err
return fmt.Errorf("renaming node: %w", err)
}

if err := tx.Model(&types.Node{}).Where("id = ?", nodeID).Update("given_name", newName).Error; err != nil {
Expand Down Expand Up @@ -451,13 +443,7 @@ func GetAdvertisedRoutes(tx *gorm.DB, node *types.Node) ([]netip.Prefix, error)
Preload("Node").
Where("node_id = ? AND advertised = ?", node.ID, true).Find(&routes).Error
if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
log.Error().
Caller().
Err(err).
Str("node", node.Hostname).
Msg("Could not get advertised routes for node")

return nil, err
return nil, fmt.Errorf("getting advertised routes for node(%d): %w", node.ID, err)
}

prefixes := []netip.Prefix{}
Expand All @@ -483,13 +469,7 @@ func GetEnabledRoutes(tx *gorm.DB, node *types.Node) ([]netip.Prefix, error) {
Where("node_id = ? AND advertised = ? AND enabled = ?", node.ID, true, true).
Find(&routes).Error
if err != nil && !errors.Is(err, gorm.ErrRecordNotFound) {
log.Error().
Caller().
Err(err).
Str("node", node.Hostname).
Msg("Could not get enabled routes for node")

return nil, err
return nil, fmt.Errorf("getting enabled routes for node(%d): %w", node.ID, err)
}

prefixes := []netip.Prefix{}
Expand All @@ -508,8 +488,6 @@ func IsRoutesEnabled(tx *gorm.DB, node *types.Node, routeStr string) bool {

enabledRoutes, err := GetEnabledRoutes(tx, node)
if err != nil {
log.Error().Err(err).Msg("Could not get enabled routes")

return false
}

Expand Down Expand Up @@ -681,8 +659,6 @@ func DeleteExpiredEphemeralNodes(tx *gorm.DB,
) ([]types.NodeID, []types.NodeID) {
users, err := ListUsers(tx)
if err != nil {
log.Error().Err(err).Msg("Error listing users")

return nil, nil
}

Expand All @@ -691,11 +667,6 @@ func DeleteExpiredEphemeralNodes(tx *gorm.DB,
for _, user := range users {
nodes, err := ListNodesByUser(tx, user.Name)
if err != nil {
log.Error().
Err(err).
Str("user", user.Name).
Msg("Error listing nodes in user")

return nil, nil
}

Expand Down Expand Up @@ -740,10 +711,6 @@ func ExpireExpiredNodes(tx *gorm.DB,

nodes, err := ListNodes(tx)
if err != nil {
log.Error().
Err(err).
Msg("Error listing nodes to find expired nodes")

return time.Unix(0, 0), types.StateUpdate{}, false
}
for _, node := range nodes {
Expand Down
2 changes: 0 additions & 2 deletions hscontrol/db/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,12 +417,10 @@ func FailoverNodeRoutesIfNeccessary(
return nil, nil
}

log.Trace().Msgf("NODE ROUTES: %d", len(nodeRoutes))
changedNodes := make(set.Set[types.NodeID])

nodeRouteLoop:
for _, nodeRoute := range nodeRoutes {
log.Trace().Msgf("NODE ROUTE: %d", nodeRoute.ID)
routes, err := getRoutesByPrefix(tx, netip.Prefix(nodeRoute.Prefix))
if err != nil {
return nil, fmt.Errorf("getting routes by prefix: %w", err)
Expand Down
9 changes: 2 additions & 7 deletions hscontrol/db/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ package db

import (
"errors"
"fmt"

"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/rs/zerolog/log"
"gorm.io/gorm"
)

Expand Down Expand Up @@ -34,12 +34,7 @@ func CreateUser(tx *gorm.DB, name string) (*types.User, error) {
}
user.Name = name
if err := tx.Create(&user).Error; err != nil {
log.Error().
Str("func", "CreateUser").
Err(err).
Msg("Could not create row")

return nil, err
return nil, fmt.Errorf("creating user: %w", err)
}

return &user, nil
Expand Down
10 changes: 2 additions & 8 deletions hscontrol/mapper/mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,7 @@ func (m *Mapper) marshalMapResponse(

jsonBody, err := json.Marshal(resp)
if err != nil {
log.Error().
Caller().
Err(err).
Msg("Cannot marshal map response")
return nil, fmt.Errorf("marshalling map response: %w", err)
}

if debugDumpMapResponsePath != "" {
Expand All @@ -402,10 +399,7 @@ func (m *Mapper) marshalMapResponse(

body, err := json.MarshalIndent(data, "", " ")
if err != nil {
log.Error().
Caller().
Err(err).
Msg("Cannot marshal map response")
return nil, fmt.Errorf("marshalling map response: %w", err)
}

perms := fs.FileMode(debugMapResponsePerm)
Expand Down
36 changes: 5 additions & 31 deletions hscontrol/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ func (h *Headscale) initOIDC() error {
h.oidcProvider, err = oidc.NewProvider(context.Background(), h.cfg.OIDC.Issuer)

if err != nil {
log.Error().
Err(err).
Caller().
Msgf("Could not retrieve OIDC Config: %s", err.Error())

return err
return fmt.Errorf("creating OIDC provider from issuer config: %w", err)
}

h.oauth2Config = &oauth2.Config{
Expand Down Expand Up @@ -514,20 +509,14 @@ func (h *Headscale) validateNodeForOIDCCallback(
User: claims.Email,
Verb: "Reauthenticated",
}); err != nil {
log.Error().
Str("func", "OIDCCallback").
Str("type", "reauthenticate").
Err(err).
Msg("Could not render OIDC callback template")

writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
writer.WriteHeader(http.StatusInternalServerError)
_, werr := writer.Write([]byte("Could not render OIDC callback template"))
if werr != nil {
util.LogErr(err, "Failed to write response")
}

return nil, true, err
return nil, true, fmt.Errorf("rendering OIDC callback template: %w", err)
}

writer.Header().Set("Content-Type", "text/html; charset=utf-8")
Expand Down Expand Up @@ -579,33 +568,24 @@ func (h *Headscale) findOrCreateNewUserForOIDCCallback(
if errors.Is(err, db.ErrUserNotFound) {
user, err = h.db.CreateUser(userName)
if err != nil {
log.Error().
Err(err).
Caller().
Msgf("could not create new user '%s'", userName)
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
writer.WriteHeader(http.StatusInternalServerError)
_, werr := writer.Write([]byte("could not create user"))
if werr != nil {
util.LogErr(err, "Failed to write response")
}

return nil, err
return nil, fmt.Errorf("creating new user: %w", err)
}
} else if err != nil {
log.Error().
Caller().
Err(err).
Str("user", userName).
Msg("could not find or create user")
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
writer.WriteHeader(http.StatusInternalServerError)
_, werr := writer.Write([]byte("could not find or create user"))
if werr != nil {
util.LogErr(err, "Failed to write response")
}

return nil, err
return nil, fmt.Errorf("find or create user: %w", err)
}

return user, nil
Expand Down Expand Up @@ -661,20 +641,14 @@ func renderOIDCCallbackTemplate(
User: claims.Email,
Verb: "Authenticated",
}); err != nil {
log.Error().
Str("func", "OIDCCallback").
Str("type", "authenticate").
Err(err).
Msg("Could not render OIDC callback template")

writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
writer.WriteHeader(http.StatusInternalServerError)
_, werr := writer.Write([]byte("Could not render OIDC callback template"))
if werr != nil {
util.LogErr(err, "Failed to write response")
}

return nil, err
return nil, fmt.Errorf("rendering OIDC callback template: %w", err)
}

return &content, nil
Expand Down
Loading

0 comments on commit 60f0cf9

Please sign in to comment.