Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert auth rpc services to use slog #50513

Merged
merged 1 commit into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {

// Add in a login hook for generating state during user login.
as.ulsGenerator, err = userloginstate.NewGenerator(userloginstate.GeneratorConfig{
Log: log,
Log: as.logger,
AccessLists: &as,
Access: &as,
UsageEvents: &as,
Expand Down
2 changes: 2 additions & 0 deletions lib/auth/grpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -5119,6 +5119,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
Emitter: cfg.Emitter,
Reporter: cfg.AuthServer.Services.UsageReporter,
Clock: cfg.AuthServer.GetClock(),
Logger: cfg.AuthServer.logger.With(teleport.ComponentKey, "users.service"),
})
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -5133,6 +5134,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
Emitter: cfg.Emitter,
Reporter: cfg.AuthServer.Services.UsageReporter,
Clock: cfg.AuthServer.GetClock(),
Logger: cfg.AuthServer.logger.With(teleport.ComponentKey, "presence.service"),
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down
39 changes: 30 additions & 9 deletions lib/auth/presence/presencev1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ package presencev1

import (
"context"
"log/slog"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/emptypb"

"github.com/gravitational/teleport"
Expand All @@ -34,6 +34,7 @@ import (
"github.com/gravitational/teleport/lib/services"
usagereporter "github.com/gravitational/teleport/lib/usagereporter/teleport"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

// Backend is the subset of the backend resources that the Service modifies.
Expand Down Expand Up @@ -65,7 +66,7 @@ type ServiceConfig struct {
AuthServer AuthServer
Backend Backend
Cache Cache
Logger logrus.FieldLogger
Logger *slog.Logger
Emitter apievents.Emitter
Reporter usagereporter.UsageReporter
Clock clockwork.Clock
Expand All @@ -79,7 +80,7 @@ type Service struct {
authServer AuthServer
backend Backend
cache Cache
logger logrus.FieldLogger
logger *slog.Logger
emitter apievents.Emitter
reporter usagereporter.UsageReporter
clock clockwork.Clock
Expand All @@ -103,7 +104,7 @@ func NewService(cfg ServiceConfig) (*Service, error) {
}

if cfg.Logger == nil {
cfg.Logger = logrus.WithField(teleport.ComponentKey, "presence.service")
cfg.Logger = slog.With(teleport.ComponentKey, "presence.service")
}
if cfg.Clock == nil {
cfg.Clock = clockwork.NewRealClock()
Expand Down Expand Up @@ -149,7 +150,11 @@ func (s *Service) GetRemoteCluster(

v3, ok := rc.(*types.RemoteClusterV3)
if !ok {
s.logger.Warnf("expected type RemoteClusterV3, got %T for %q", rc, rc.GetName())
s.logger.WarnContext(ctx, "unexpected remote cluster type",
"got_type", logutils.TypeAttr(rc),
"expected_type", "RemoteClusterV3",
"remote_cluster", rc.GetName(),
)
return nil, trace.BadParameter("encountered unexpected remote cluster type")
}

Expand Down Expand Up @@ -180,7 +185,11 @@ func (s *Service) ListRemoteClusters(
for _, rc := range page {
v3, ok := rc.(*types.RemoteClusterV3)
if !ok {
s.logger.Warnf("expected type RemoteClusterV3, got %T for %q", rc, rc.GetName())
s.logger.WarnContext(ctx, "unexpected remote cluster type",
"got_type", logutils.TypeAttr(rc),
"expected_type", "RemoteClusterV3",
"remote_cluster", rc.GetName(),
)
continue
}
concretePage = append(concretePage, v3)
Expand Down Expand Up @@ -234,7 +243,11 @@ func (s *Service) UpdateRemoteCluster(
}
v3, ok := rc.(*types.RemoteClusterV3)
if !ok {
s.logger.Warnf("expected type RemoteClusterV3, got %T for user %q", rc, rc.GetName())
s.logger.WarnContext(ctx, "unexpected remote cluster type",
"got_type", logutils.TypeAttr(rc),
"expected_type", "RemoteClusterV3",
"remote_cluster", rc.GetName(),
)
return nil, trace.BadParameter("encountered unexpected remote cluster type")
}
return v3, nil
Expand Down Expand Up @@ -271,7 +284,11 @@ func (s *Service) UpdateRemoteCluster(
}
v3, ok := rc.(*types.RemoteClusterV3)
if !ok {
s.logger.Warnf("expected type RemoteClusterV3, got %T for user %q", rc, rc.GetName())
s.logger.WarnContext(ctx, "unexpected remote cluster type",
"got_type", logutils.TypeAttr(rc),
"expected_type", "RemoteClusterV3",
"remote_cluster", rc.GetName(),
)
return nil, trace.BadParameter("encountered unexpected remote cluster type")
}

Expand Down Expand Up @@ -330,7 +347,11 @@ func (s *Service) ListReverseTunnels(
for _, rc := range page {
v3, ok := rc.(*types.ReverseTunnelV2)
if !ok {
s.logger.Warnf("expected type ReverseTunnelV2, got %T for %q", rc, rc.GetName())
s.logger.WarnContext(ctx, "unexpected reverse tunnel type",
"got_type", logutils.TypeAttr(rc),
"expected_type", "ReverseTunnelV2",
"reverse_tunnel", rc.GetName(),
)
continue
}
concretePage = append(concretePage, v3)
Expand Down
7 changes: 0 additions & 7 deletions lib/auth/trust/trustv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (
"time"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/emptypb"

"github.com/gravitational/teleport"
trustpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/trust/v1"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/authz"
Expand All @@ -52,7 +50,6 @@ type ServiceConfig struct {
Authorizer authz.Authorizer
Cache services.AuthorityGetter
Backend services.TrustInternal
Logger *logrus.Entry
AuthServer authServer
}

Expand All @@ -63,7 +60,6 @@ type Service struct {
cache services.AuthorityGetter
backend services.TrustInternal
authServer authServer
logger *logrus.Entry
}

// NewService returns a new trust gRPC service.
Expand All @@ -77,12 +73,9 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
return nil, trace.BadParameter("authorizer is required")
case cfg.AuthServer == nil:
return nil, trace.BadParameter("authServer is required")
case cfg.Logger == nil:
cfg.Logger = logrus.WithField(teleport.ComponentKey, "trust.service")
}

return &Service{
logger: cfg.Logger,
authorizer: cfg.Authorizer,
cache: cfg.Cache,
backend: cfg.Backend,
Expand Down
14 changes: 7 additions & 7 deletions lib/auth/userloginstate/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ package userloginstate
import (
"context"
"fmt"
"log/slog"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"

"github.com/gravitational/teleport/api/client/proto"
usageeventsv1 "github.com/gravitational/teleport/api/gen/proto/go/usageevents/v1"
Expand All @@ -49,7 +49,7 @@ type AccessListsAndLockGetter interface {
// GeneratorConfig is the configuration for the user login state generator.
type GeneratorConfig struct {
// Log is a logger to use for the generator.
Log *logrus.Entry
Log *slog.Logger

// AccessLists is a service for retrieving access lists and locks from the backend.
AccessLists AccessListsAndLockGetter
Expand Down Expand Up @@ -107,7 +107,7 @@ func (g *GeneratorConfig) CheckAndSetDefaults() error {

// Generator will generate a user login state from a user.
type Generator struct {
log *logrus.Entry
log *slog.Logger
accessLists AccessListsAndLockGetter
access services.Access
usageEvents UsageEventsClient
Expand Down Expand Up @@ -191,7 +191,7 @@ func (g *Generator) Generate(ctx context.Context, user types.User, ulsService se
if g.usageEvents != nil {
// Emit the usage event metadata.
if err := g.emitUsageEvent(ctx, user, uls, inheritedRoles, inheritedTraits); err != nil {
g.log.WithError(err).Debug("Error emitting usage event during user login state generation, skipping")
g.log.DebugContext(ctx, "Error emitting usage event during user login state generation, skipping", "error", err)
}
}

Expand Down Expand Up @@ -245,7 +245,7 @@ func (g *Generator) handleAccessListMembership(ctx context.Context, user types.U
if err != nil || membershipKind == accesslists.MembershipOrOwnershipTypeNone {
// Log any error.
if err != nil {
g.log.WithError(err).Warn("checking access list membership")
g.log.WarnContext(ctx, "checking access list membership", "error", err)
}
return inheritedRoles, inheritedTraits, nil
}
Expand Down Expand Up @@ -290,7 +290,7 @@ func (g *Generator) handleAccessListOwnership(ctx context.Context, user types.Us
if err != nil || ownershipType == accesslists.MembershipOrOwnershipTypeNone {
// Log any error.
if err != nil {
g.log.WithError(err).Warn("checking access list ownership")
g.log.WarnContext(ctx, "checking access list ownership", "error", err)
}
return inheritedRoles, inheritedTraits, nil
}
Expand Down Expand Up @@ -497,6 +497,6 @@ func (g *Generator) emitSkippedAccessListEvent(ctx context.Context, accessListNa
UserMessage: "access list skipped because it references non-existent role(s)",
},
}); err != nil {
g.log.WithError(err).Warn("Failed to emit access list skipped warning audit event.")
g.log.WarnContext(ctx, "Failed to emit access list skipped warning audit event", "error", err)
}
}
5 changes: 2 additions & 3 deletions lib/auth/userloginstate/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/client/proto"
Expand All @@ -42,6 +41,7 @@ import (
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local"
"github.com/gravitational/teleport/lib/utils"
)

const ownerUser = "owner"
Expand Down Expand Up @@ -729,7 +729,6 @@ func initGeneratorSvc(t *testing.T) (*Generator, *svc) {
ulsService, err := local.NewUserLoginStateService(mem)
require.NoError(t, err)

log := logrus.WithField("test", "logger")
svc := &svc{
AccessLists: accessListsSvc,
Access: accessSvc,
Expand All @@ -739,7 +738,7 @@ func initGeneratorSvc(t *testing.T) (*Generator, *svc) {
emitter := &eventstest.MockRecorderEmitter{}

generator, err := NewGenerator(GeneratorConfig{
Log: log,
Log: utils.NewSlogLoggerForTests(),
AccessLists: svc,
Access: svc,
UsageEvents: svc,
Expand Down
10 changes: 0 additions & 10 deletions lib/auth/userloginstate/userloginstatev1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ import (

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/emptypb"

"github.com/gravitational/teleport"
userloginstatev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/userloginstate/v1"
"github.com/gravitational/teleport/api/types"
conv "github.com/gravitational/teleport/api/types/userloginstate/convert/v1"
Expand All @@ -36,8 +34,6 @@ import (

// ServiceConfig is the service config for the Access Lists gRPC service.
type ServiceConfig struct {
// Logger is the logger to use.
Logger logrus.FieldLogger

// Authorizer is the authorizer to use.
Authorizer authz.Authorizer
Expand All @@ -58,10 +54,6 @@ func (c *ServiceConfig) checkAndSetDefaults() error {
return trace.BadParameter("user login states service is missing")
}

if c.Logger == nil {
c.Logger = logrus.WithField(teleport.ComponentKey, "user_login_state_crud_service")
}

if c.Clock == nil {
c.Clock = clockwork.NewRealClock()
}
Expand All @@ -72,7 +64,6 @@ func (c *ServiceConfig) checkAndSetDefaults() error {
type Service struct {
userloginstatev1.UnimplementedUserLoginStateServiceServer

log logrus.FieldLogger
authorizer authz.Authorizer
userLoginStates services.UserLoginStates
clock clockwork.Clock
Expand All @@ -85,7 +76,6 @@ func NewService(cfg ServiceConfig) (*Service, error) {
}

return &Service{
log: cfg.Logger,
authorizer: cfg.Authorizer,
userLoginStates: cfg.UserLoginStates,
clock: cfg.Clock,
Expand Down
7 changes: 0 additions & 7 deletions lib/auth/userpreferences/userpreferencesv1/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,8 @@ import (
"context"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/emptypb"

"github.com/gravitational/teleport"
userpreferences "github.com/gravitational/teleport/api/gen/proto/go/userpreferences/v1"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/services"
Expand All @@ -35,7 +33,6 @@ import (
type ServiceConfig struct {
Backend services.UserPreferences
Authorizer authz.Authorizer
Logger *logrus.Entry
}

// Service implements the teleport.userpreferences.v1.UserPreferencesService RPC service.
Expand All @@ -44,7 +41,6 @@ type Service struct {

backend services.UserPreferences
authorizer authz.Authorizer
log *logrus.Entry
}

// NewService returns a new user preferences gRPC service.
Expand All @@ -54,14 +50,11 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
return nil, trace.BadParameter("backend is required")
case cfg.Authorizer == nil:
return nil, trace.BadParameter("authorizer is required")
case cfg.Logger == nil:
cfg.Logger = logrus.WithField(teleport.ComponentKey, "userpreferences.service")
}

return &Service{
backend: cfg.Backend,
authorizer: cfg.Authorizer,
log: cfg.Logger,
}, nil
}

Expand Down
Loading
Loading