Skip to content

Commit

Permalink
Convert more lib packages to slog. (#47787)
Browse files Browse the repository at this point in the history 
* Convert `lib/observability` to `slog` from `logrus`

* Inject slog into `lib/observability`

* Convert `lib/release` to slog

* Migrate to `slog` in `lib/cgroup`

* Fix missed variable

* Convert `lib/auth/join` to slog

* Fix missing logutils import

* Ignore sloglint

* Convert `lib/auditd` to Slog

* Switch `lib/player` to use `slog`
  • Loading branch information
@strideynet
strideynet authored Oct 24, 2024
1 parent 4e560fd commit de0d9f4
Show file tree
Hide file tree
Showing 11 changed files with 75 additions and 70 deletions.
11 changes: 6 additions & 5 deletions lib/auditd/auditd_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ package auditd

import (
"bytes"
"context"
"encoding/binary"
"errors"
"log/slog"
"math"
"os"
"strconv"
Expand All @@ -32,7 +34,6 @@ import (
"github.com/gravitational/trace"
"github.com/mdlayher/netlink"
"github.com/mdlayher/netlink/nlenc"
log "github.com/sirupsen/logrus"
)

// featureStatus is a 3 state boolean yes/no/unknown type.
Expand Down Expand Up @@ -92,7 +93,7 @@ func IsLoginUIDSet() bool {
client := NewClient(Message{})
defer func() {
if err := client.Close(); err != nil {
log.WithError(err).Warn("Failed to close auditd client.")
slog.WarnContext(context.TODO(), "Failed to close auditd client", "error", err)
}
}()
// We don't need to acquire the internal client mutex as the connection is
Expand All @@ -108,7 +109,7 @@ func IsLoginUIDSet() bool {

loginuid, err := getSelfLoginUID()
if err != nil {
log.WithError(err).Debug("failed to read login UID")
slog.DebugContext(context.TODO(), "Failed to read login UID", "error", err)
return false
}

Expand Down Expand Up @@ -142,7 +143,7 @@ func SendEvent(event EventType, result ResultType, msg Message) error {
defer func() {
err := client.Close()
if err != nil {
log.WithError(err).Error("failed to close auditd client")
slog.ErrorContext(context.TODO(), "Failed to close auditd client", "error", err)
}
}()

Expand Down Expand Up @@ -206,7 +207,7 @@ func NewClient(msg Message) *Client {

execName, err := os.Executable()
if err != nil {
log.WithError(err).Warn("failed to get executable name")
slog.WarnContext(context.TODO(), "Failed to get executable name", "error", err)
execName = UnknownValue
}

Expand Down
62 changes: 33 additions & 29 deletions lib/auth/join/join.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
log "github.com/sirupsen/logrus"
"go.opentelemetry.io/otel"
"golang.org/x/crypto/ssh"

Expand Down Expand Up @@ -262,13 +261,13 @@ func Register(ctx context.Context, params RegisterParams) (result *RegisterResul
// If an explicit AuthClient has been provided, we want to go straight to
// using that rather than trying both proxy and auth dialing.
if params.AuthClient != nil {
log.Info("Attempting registration with existing auth client.")
slog.InfoContext(ctx, "Attempting registration with existing auth client.")
result, err := registerThroughAuthClient(ctx, token, params, params.AuthClient)
if err != nil {
log.WithError(err).Error("Registration with existing auth client failed.")
slog.ErrorContext(ctx, "Registration with existing auth client failed.", "error", err)
return nil, trace.Wrap(err)
}
log.Info("Successfully registered with existing auth client.")
slog.InfoContext(ctx, "Successfully registered with existing auth client.")
return result, nil
}

Expand All @@ -283,35 +282,35 @@ func Register(ctx context.Context, params RegisterParams) (result *RegisterResul
registerMethods := []registerMethod{registerThroughAuth, registerThroughProxy}

if !params.ProxyServer.IsEmpty() {
log.WithField("proxy-server", params.ProxyServer).Debugf("Registering node to the cluster.")
slog.DebugContext(ctx, "Registering node to the cluster.", "proxy_server", params.ProxyServer)

registerMethods = []registerMethod{registerThroughProxy}

if proxyServerIsAuth(params.ProxyServer) {
log.Debugf("The specified proxy server appears to be an auth server.")
slog.DebugContext(ctx, "The specified proxy server appears to be an auth server.")
}
} else {
log.WithField("auth-servers", params.AuthServers).Debugf("Registering node to the cluster.")
slog.DebugContext(ctx, "Registering node to the cluster.", "auth_servers", params.AuthServers)

if params.GetHostCredentials == nil {
log.Debugf("Missing client, it is not possible to register through proxy.")
slog.DebugContext(ctx, "Missing client, it is not possible to register through proxy.")
registerMethods = []registerMethod{registerThroughAuth}
} else if authServerIsProxy(params.AuthServers) {
log.Debugf("The first specified auth server appears to be a proxy.")
slog.DebugContext(ctx, "The first specified auth server appears to be a proxy.")
registerMethods = []registerMethod{registerThroughProxy, registerThroughAuth}
}
}

var collectedErrs []error
for _, method := range registerMethods {
log.Infof("Attempting registration %s.", method.desc)
slog.InfoContext(ctx, "Attempting registration.", "method", method.desc)
result, err := method.call(ctx, token, params)
if err != nil {
collectedErrs = append(collectedErrs, err)
log.WithError(err).Debugf("Registration %s failed.", method.desc)
slog.DebugContext(ctx, "Registration failed.", "method", method.desc, "error", err)
continue
}
log.Infof("Successfully registered %s.", method.desc)
slog.InfoContext(ctx, "Successfully registered.", "method", method.desc)
return result, nil
}
return nil, trace.NewAggregate(collectedErrs...)
Expand Down Expand Up @@ -413,19 +412,19 @@ func registerThroughAuth(
// depending on the configured values for Insecure, CAPins and CAPath.
switch {
case params.Insecure:
log.Warnf("Insecure mode enabled. Auth Server cert will not be validated and CAPins and CAPath value will be ignored.")
client, err = insecureRegisterClient(params)
slog.WarnContext(ctx, "Insecure mode enabled. Auth Server cert will not be validated and CAPins and CAPath value will be ignored.")
client, err = insecureRegisterClient(ctx, params)
case len(params.CAPins) != 0:
// CAPins takes precedence over CAPath
client, err = pinRegisterClient(ctx, params)
case params.CAPath != "":
client, err = caPathRegisterClient(params)
client, err = caPathRegisterClient(ctx, params)
default:
// We fall back to insecure mode here - this is a little odd but is
// necessary to preserve the behavior of registration. At a later date,
// we may consider making this an error asking the user to provide
// Insecure, CAPins or CAPath.
client, err = insecureRegisterClient(params)
client, err = insecureRegisterClient(ctx, params)
}
if err != nil {
return nil, trace.Wrap(err, "building auth client")
Expand Down Expand Up @@ -489,11 +488,12 @@ func getHostAddresses(params RegisterParams) []string {
// insecureRegisterClient attempts to connects to the Auth Server using the
// CA on disk. If no CA is found on disk, Teleport will not verify the Auth
// Server it is connecting to.
func insecureRegisterClient(params RegisterParams) (*authclient.Client, error) {
log.Warnf("Joining cluster without validating the identity of the Auth " +
"Server. This may open you up to a Man-In-The-Middle (MITM) attack if an " +
"attacker can gain privileged network access. To remedy this, use the CA pin " +
"value provided when join token was generated to validate the identity of " +
func insecureRegisterClient(ctx context.Context, params RegisterParams) (*authclient.Client, error) {
//nolint:sloglint // Conjoined string literals trip up the linter.
slog.WarnContext(ctx, "Joining cluster without validating the identity of the Auth "+
"Server. This may open you up to a Man-In-The-Middle (MITM) attack if an "+
"attacker can gain privileged network access. To remedy this, use the CA pin "+
"value provided when join token was generated to validate the identity of "+
"the Auth Server or point to a valid Certificate via the CA Path option.")

tlsConfig := utils.TLSConfig(params.CipherSuites)
Expand All @@ -506,6 +506,7 @@ func insecureRegisterClient(params RegisterParams) (*authclient.Client, error) {
client.LoadTLS(tlsConfig),
},
CircuitBreakerConfig: params.CircuitBreakerConfig,
Context: ctx,
})
if err != nil {
return nil, trace.Wrap(err, "creating insecure auth client")
Expand Down Expand Up @@ -533,6 +534,7 @@ func pinRegisterClient(
client.LoadTLS(tlsConfig),
},
CircuitBreakerConfig: params.CircuitBreakerConfig,
Context: ctx,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -566,7 +568,7 @@ func pinRegisterClient(
}

}
log.Infof("Joining remote cluster %v with CA pin.", certs[0].Subject.CommonName)
slog.InfoContext(ctx, "Joining remote cluster with CA pin.", "cluster", certs[0].Subject.CommonName)

// Create another client, but this time with the CA provided to validate
// that the Auth Server was issued a certificate by the same CA.
Expand All @@ -584,6 +586,7 @@ func pinRegisterClient(
client.LoadTLS(tlsConfig),
},
CircuitBreakerConfig: params.CircuitBreakerConfig,
Context: ctx,
})
if err != nil {
return nil, trace.Wrap(err)
Expand All @@ -592,7 +595,7 @@ func pinRegisterClient(
return authClient, nil
}

func caPathRegisterClient(params RegisterParams) (*authclient.Client, error) {
func caPathRegisterClient(ctx context.Context, params RegisterParams) (*authclient.Client, error) {
tlsConfig := utils.TLSConfig(params.CipherSuites)
tlsConfig.Time = params.Clock.Now

Expand All @@ -606,22 +609,23 @@ func caPathRegisterClient(params RegisterParams) (*authclient.Client, error) {
// we may wish to consider changing this to return an error - but this is a
// breaking change.
if trace.IsNotFound(err) {
log.Warnf("Falling back to insecurely joining because a missing or empty CA Path was provided.")
return insecureRegisterClient(params)
slog.WarnContext(ctx, "Falling back to insecurely joining because a missing or empty CA Path was provided.")
return insecureRegisterClient(ctx, params)
}

certPool := x509.NewCertPool()
certPool.AddCert(cert)
tlsConfig.RootCAs = certPool

log.Infof("Joining remote cluster %v, validating connection with certificate on disk.", cert.Subject.CommonName)
slog.InfoContext(ctx, "Joining remote cluster, validating connection with certificate on disk.", "cluster", cert.Subject.CommonName)

client, err := authclient.NewClient(client.Config{
Addrs: getHostAddresses(params),
Credentials: []client.Credentials{
client.LoadTLS(tlsConfig),
},
CircuitBreakerConfig: params.CircuitBreakerConfig,
Context: ctx,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -661,7 +665,7 @@ func registerUsingTokenRequestForParams(token string, hostKeys *newHostKeys, par
func registerUsingIAMMethod(
ctx context.Context, joinServiceClient joinServiceClient, token string, hostKeys *newHostKeys, params RegisterParams,
) (*proto.Certs, error) {
log.Infof("Attempting to register %s with IAM method using regional STS endpoint", params.ID.Role)
slog.InfoContext(ctx, "Attempting to register with IAM method using region STS endpoint.", "role", params.ID.Role)
// Call RegisterUsingIAMMethod and pass a callback to respond to the challenge with a signed join request.
certs, err := joinServiceClient.RegisterUsingIAMMethod(ctx, func(challenge string) (*proto.RegisterUsingIAMMethodRequest, error) {
// create the signed sts:GetCallerIdentity request and include the challenge
Expand All @@ -679,11 +683,11 @@ func registerUsingIAMMethod(
}, nil
})
if err != nil {
log.WithError(err).Infof("Failed to register %s using regional STS endpoint", params.ID.Role)
slog.InfoContext(ctx, "Failed to register using regional STS endpoint", "role", params.ID.Role, "error", err)
return nil, trace.Wrap(err, "registering via IAM method streaming RPC")
}

log.Infof("Successfully registered %s with IAM method using regional STS endpoint", params.ID.Role)
slog.InfoContext(ctx, "Successfully registered with IAM method using regional STS endpoint.", "role", params.ID.Role)
return certs, nil
}

Expand Down
17 changes: 8 additions & 9 deletions lib/cgroup/cgroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import "C"
import (
"bufio"
"bytes"
"context"
"encoding/binary"
"os"
"path/filepath"
Expand All @@ -38,17 +39,15 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

var log = logrus.WithFields(logrus.Fields{
teleport.ComponentKey: teleport.ComponentCgroup,
})
var logger = logutils.NewPackageLogger(teleport.ComponentKey, teleport.ComponentCgroup)

// Config holds configuration for the cgroup service.
type Config struct {
Expand Down Expand Up @@ -97,7 +96,7 @@ func New(config *Config) (*Service, error) {
return nil, trace.Wrap(err)
}

log.Debugf("Teleport session hierarchy mounted at: %v.", s.teleportRoot)
logger.DebugContext(context.TODO(), "Teleport session hierarchy mounted.", "hierarchy_root", s.teleportRoot)
return s, nil
}

Expand All @@ -110,7 +109,7 @@ func (s *Service) Close(skipUnmount bool) error {
}

if skipUnmount {
log.Debugf("Cleaned up Teleport session hierarchy at: %v.", s.teleportRoot)
logger.DebugContext(context.TODO(), "Cleaned up Teleport session hierarchy.", "hierarchy_root", s.teleportRoot)
return nil
}

Expand All @@ -119,7 +118,7 @@ func (s *Service) Close(skipUnmount bool) error {
return trace.Wrap(err)
}

log.Debugf("Cleaned up and unmounted Teleport session hierarchy at: %v.", s.teleportRoot)
logger.DebugContext(context.TODO(), "Cleaned up and unmounted Teleport session hierarchy.", "hierarchy_root", s.teleportRoot)
return nil
}

Expand Down Expand Up @@ -154,7 +153,7 @@ func (s *Service) Remove(sessionID string) error {
return trace.Wrap(err)
}

log.Debugf("Removed cgroup for session: %v.", sessionID)
logger.DebugContext(context.TODO(), "Removed cgroup for session.", "session_id", sessionID)
return nil
}

Expand Down Expand Up @@ -320,7 +319,7 @@ func (s *Service) mount() error {
if err != nil {
return trace.Wrap(err)
}
log.Debugf("Mounted cgroup filesystem to %v.", s.MountPath)
logger.DebugContext(context.TODO(), "Mounted cgroup filesystem.", "mount_path", s.MountPath)

// Create cgroup that will hold Teleport sessions.
err = os.MkdirAll(s.teleportRoot, fileMode)
Expand Down
8 changes: 4 additions & 4 deletions lib/observability/tracing/tracing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ package tracing
import (
"context"
"crypto/tls"
"log/slog"
"net"
"net/url"
"strings"
"time"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/propagation"
Expand Down Expand Up @@ -73,7 +73,7 @@ type Config struct {
// DialTimeout is the timeout for dialing the exporter.
DialTimeout time.Duration
// Logger is the logger to use.
Logger logrus.FieldLogger
Logger *slog.Logger
// Client is the client to use to export traces. This takes precedence over creating a
// new client with the ExporterURL. Ownership of the client is transferred to the
// tracing provider. It should **NOT** be closed by the caller.
Expand All @@ -99,7 +99,7 @@ func (c *Config) CheckAndSetDefaults() error {
}

if c.Logger == nil {
c.Logger = logrus.WithField(teleport.ComponentKey, teleport.ComponentTracing)
c.Logger = slog.With(teleport.ComponentKey, teleport.ComponentTracing)
}

if c.Client != nil {
Expand Down Expand Up @@ -218,7 +218,7 @@ func NewTraceProvider(ctx context.Context, cfg Config) (*Provider, error) {
// override the global logging handled with one that uses the
// configured logger instead
otel.SetErrorHandler(otel.ErrorHandlerFunc(func(err error) {
cfg.Logger.WithError(err).Warnf("failed to export traces.")
cfg.Logger.WarnContext(ctx, "Failed to export traces", "error", err)
}))

// set global provider to our provider wrapper to have all tracers use the common TracerOptions
Expand Down
5 changes: 2 additions & 3 deletions lib/observability/tracing/tracing_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -515,8 +514,8 @@ func TestConfig_CheckAndSetDefaults(t *testing.T) {
}
require.Empty(t, cmp.Diff(tt.expectedCfg, tt.cfg,
cmpopts.IgnoreUnexported(Config{}),
cmpopts.IgnoreInterfaces(struct{ logrus.FieldLogger }{})),
)
cmpopts.IgnoreFields(Config{}, "Logger"),
))
require.NotNil(t, tt.cfg.Logger)
})
}
Expand Down
Loading

0 comments on commit de0d9f4

Please sign in to comment.