Skip to content

Commit

Permalink
Convert lib/kube to use slog (#50437)
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored Dec 20, 2024
1 parent 6061a88 commit b1b6bb2
Show file tree
Hide file tree
Showing 35 changed files with 384 additions and 332 deletions.
8 changes: 4 additions & 4 deletions lib/kube/grpc/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ package kubev1
import (
"context"
"errors"
"log/slog"
"slices"

"github.com/gravitational/trace"
"github.com/gravitational/trace/trail"
"github.com/sirupsen/logrus"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"

Expand Down Expand Up @@ -85,7 +85,7 @@ type Config struct {
// Authz authenticates user.
Authz authz.Authorizer
// Log is the logger function.
Log logrus.FieldLogger
Log *slog.Logger
// Emitter is used to emit audit events.
Emitter apievents.Emitter
// Component name to include in log output.
Expand Down Expand Up @@ -139,9 +139,9 @@ func (c *Config) CheckAndSetDefaults() error {
c.Component = "kube.grpc"
}
if c.Log == nil {
c.Log = logrus.New()
c.Log = slog.Default()
}
c.Log = c.Log.WithFields(logrus.Fields{teleport.ComponentKey: c.Component})
c.Log = c.Log.With(teleport.ComponentKey, c.Component)
return nil
}

Expand Down
11 changes: 5 additions & 6 deletions lib/kube/kubeconfig/kubeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ package kubeconfig

import (
"bytes"
"context"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/clientcmd"
Expand All @@ -36,11 +36,10 @@ import (
"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

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

const (
// teleportKubeClusterNameExtension is the name of the extension that
Expand Down Expand Up @@ -268,7 +267,7 @@ func UpdateConfig(path string, v Values, storeAllCAs bool, fs ConfigFS) error {
} else if !trace.IsBadParameter(err) {
return trace.Wrap(err)
}
log.WithError(err).Warn("Kubernetes integration is not supported when logging in with a hardware private key.")
log.WarnContext(context.Background(), "Kubernetes integration is not supported when logging in with a hardware private key", "error", err)
}

return SaveConfig(path, *config, fs)
Expand Down Expand Up @@ -493,7 +492,7 @@ func PathFromEnv() string {
var configPath string
if len(parts) > 0 {
configPath = parts[0]
log.Debugf("Using kubeconfig from environment: %q.", configPath)
log.DebugContext(context.Background(), "Using kubeconfig from environment", "config_path", configPath)
}

return configPath
Expand Down
45 changes: 28 additions & 17 deletions lib/kube/proxy/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ import (
"context"
"crypto/tls"
"fmt"
"log/slog"
"net"
"net/http"
"net/url"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
authzapi "k8s.io/api/authorization/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilnet "k8s.io/apimachinery/pkg/util/net"
Expand Down Expand Up @@ -77,11 +77,11 @@ func (f *Forwarder) getKubeDetails(ctx context.Context) error {
kubeClusterName := f.cfg.KubeClusterName
tpClusterName := f.cfg.ClusterName

f.log.
WithField("kubeconfigPath", kubeconfigPath).
WithField("kubeClusterName", kubeClusterName).
WithField("serviceType", serviceType).
Debug("Reading Kubernetes details.")
f.log.DebugContext(ctx, "Reading Kubernetes details",
"kubeconfig_path", kubeconfigPath,
"kube_cluster_name", kubeClusterName,
"service_type", serviceType,
)

// Proxy service should never have creds, forwards to kube service
if serviceType == ProxyService {
Expand All @@ -100,7 +100,7 @@ func (f *Forwarder) getKubeDetails(ctx context.Context) error {
case KubeService:
return trace.BadParameter("no Kubernetes credentials found; Kubernetes_service requires either a valid kubeconfig_file or to run inside of a Kubernetes pod")
case LegacyProxyService:
f.log.Debugf("Could not load Kubernetes credentials. This proxy will still handle Kubernetes requests for trusted teleport clusters or Kubernetes nodes in this teleport cluster")
f.log.DebugContext(ctx, "Could not load Kubernetes credentials. This proxy will still handle Kubernetes requests for trusted teleport clusters or Kubernetes nodes in this teleport cluster")
}
return nil
}
Expand All @@ -124,39 +124,48 @@ func (f *Forwarder) getKubeDetails(ctx context.Context) error {
for cluster, clientCfg := range cfg.Contexts {
clusterCreds, err := extractKubeCreds(ctx, serviceType, cluster, clientCfg, f.log, f.cfg.CheckImpersonationPermissions)
if err != nil {
f.log.WithError(err).Warnf("failed to load credentials for cluster %q.", cluster)
f.log.WarnContext(ctx, "failed to load credentials for cluster",
"cluster", cluster,
"error", err,
)
continue
}
kubeCluster, err := types.NewKubernetesClusterV3(types.Metadata{
Name: cluster,
}, types.KubernetesClusterSpecV3{})
if err != nil {
f.log.WithError(err).Warnf("failed to create KubernetesClusterV3 from credentials for cluster %q.", cluster)
f.log.WarnContext(ctx, "failed to create KubernetesClusterV3 from credentials for cluster",
"cluster", cluster,
"error", err,
)
continue
}

details, err := newClusterDetails(ctx,
clusterDetailsConfig{
cluster: kubeCluster,
kubeCreds: clusterCreds,
log: f.log.WithField("cluster", kubeCluster.GetName()),
log: f.log.With("cluster", kubeCluster.GetName()),
checker: f.cfg.CheckImpersonationPermissions,
component: serviceType,
clock: f.cfg.Clock,
})
if err != nil {
f.log.WithError(err).Warnf("Failed to create cluster details for cluster %q.", cluster)
f.log.WarnContext(ctx, "Failed to create cluster details for cluster",
"cluster", cluster,
"error", err,
)
return trace.Wrap(err)
}
f.clusterDetails[cluster] = details
}
return nil
}

func extractKubeCreds(ctx context.Context, component string, cluster string, clientCfg *rest.Config, log logrus.FieldLogger, checkPermissions servicecfg.ImpersonationPermissionsChecker) (*staticKubeCreds, error) {
log = log.WithField("cluster", cluster)
func extractKubeCreds(ctx context.Context, component string, cluster string, clientCfg *rest.Config, log *slog.Logger, checkPermissions servicecfg.ImpersonationPermissionsChecker) (*staticKubeCreds, error) {
log = log.With("cluster", cluster)

log.Debug("Checking Kubernetes impersonation permissions.")
log.DebugContext(ctx, "Checking Kubernetes impersonation permissions")
client, err := kubernetes.NewForConfig(clientCfg)
if err != nil {
return nil, trace.Wrap(err, "failed to generate Kubernetes client for cluster %q", cluster)
Expand All @@ -165,9 +174,11 @@ func extractKubeCreds(ctx context.Context, component string, cluster string, cli
// For each loaded cluster, check impersonation permissions. This
// check only logs when permissions are not configured, but does not fail startup.
if err := checkPermissions(ctx, cluster, client.AuthorizationV1().SelfSubjectAccessReviews()); err != nil {
log.WithError(err).Warning("Failed to test the necessary Kubernetes permissions. The target Kubernetes cluster may be down or have misconfigured RBAC. This teleport instance will still handle Kubernetes requests towards this Kubernetes cluster.")
log.WarnContext(ctx, "Failed to test the necessary Kubernetes permissions. The target Kubernetes cluster may be down or have misconfigured RBAC. This teleport instance will still handle Kubernetes requests towards this Kubernetes cluster.",
"error", err,
)
} else {
log.Debug("Have all necessary Kubernetes impersonation permissions.")
log.DebugContext(ctx, "Have all necessary Kubernetes impersonation permissions")
}

targetAddr, err := parseKubeHost(clientCfg.Host)
Expand All @@ -192,7 +203,7 @@ func extractKubeCreds(ctx context.Context, component string, cluster string, cli
return nil, trace.Wrap(err, "failed to generate transport from kubeconfig: %v", err)
}

log.Debug("Initialized Kubernetes credentials")
log.DebugContext(ctx, "Initialized Kubernetes credentials")
return &staticKubeCreds{
tlsConfig: tlsConfig,
transportConfig: transportConfig,
Expand Down
3 changes: 1 addition & 2 deletions lib/kube/proxy/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ func TestGetKubeCreds(t *testing.T) {
rbacSupportedTypes[allowedResourcesKey{apiGroup: "resources.teleport.dev", resourceKind: "teleportroles"}] = utils.KubeCustomResource
rbacSupportedTypes[allowedResourcesKey{apiGroup: "resources.teleport.dev", resourceKind: "teleportroles/status"}] = utils.KubeCustomResource

logger := utils.NewLoggerForTests()
ctx := context.TODO()
const teleClusterName = "teleport-cluster"
dir := t.TempDir()
Expand Down Expand Up @@ -351,7 +350,7 @@ current-context: foo
CheckImpersonationPermissions: tt.impersonationCheck,
Clock: clockwork.NewFakeClock(),
},
log: logger,
log: utils.NewSlogLoggerForTests(),
}
err := fwd.getKubeDetails(ctx)
tt.assertErr(t, err)
Expand Down
14 changes: 7 additions & 7 deletions lib/kube/proxy/cluster_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package proxy
import (
"context"
"encoding/base64"
"log/slog"
"strings"
"sync"
"time"
Expand All @@ -29,7 +30,6 @@ import (
"github.com/aws/aws-sdk-go/service/eks"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/version"
Expand Down Expand Up @@ -91,7 +91,7 @@ type clusterDetailsConfig struct {
// cluster is the cluster to create a proxied cluster for.
cluster types.KubeCluster
// log is the logger to use.
log *logrus.Entry
log *slog.Logger
// checker is the permissions checker to use.
checker servicecfg.ImpersonationPermissionsChecker
// resourceMatchers is the list of resource matchers to match the cluster against
Expand Down Expand Up @@ -135,7 +135,7 @@ func newClusterDetails(ctx context.Context, cfg clusterDetailsConfig) (_ *kubeDe
// Create the codec factory and the list of supported types for RBAC.
codecFactory, rbacSupportedTypes, gvkSupportedRes, err := newClusterSchemaBuilder(cfg.log, creds.getKubeClient())
if err != nil {
cfg.log.WithError(err).Warn("Failed to create cluster schema. Possibly the cluster is offline.")
cfg.log.WarnContext(ctx, "Failed to create cluster schema, the cluster may be offline", "error", err)
// If the cluster is offline, we will not be able to create the codec factory
// and the list of supported types for RBAC.
// We mark the cluster as offline and continue to create the kubeDetails but
Expand All @@ -145,7 +145,7 @@ func newClusterDetails(ctx context.Context, cfg clusterDetailsConfig) (_ *kubeDe

kubeVersion, err := creds.getKubeClient().Discovery().ServerVersion()
if err != nil {
cfg.log.WithError(err).Warn("Failed to get Kubernetes cluster version. Possibly the cluster is offline.")
cfg.log.WarnContext(ctx, "Failed to get Kubernetes cluster version, the cluster may be offline", "error", err)
}

ctx, cancel := context.WithCancel(ctx)
Expand Down Expand Up @@ -198,13 +198,13 @@ func newClusterDetails(ctx context.Context, cfg clusterDetailsConfig) (_ *kubeDe
} else {
refreshDelay.Inc()
}
cfg.log.WithError(err).Error("Failed to update cluster schema")
cfg.log.ErrorContext(ctx, "Failed to update cluster schema", "error", err)
continue
}

kubeVersion, err := creds.getKubeClient().Discovery().ServerVersion()
if err != nil {
cfg.log.WithError(err).Warn("Failed to get Kubernetes cluster version. Possibly the cluster is offline.")
cfg.log.WarnContext(ctx, "Failed to get Kubernetes cluster version, the cluster may be offline", "error", err)
}

// Restore details refresh delay to the default value, in case previously cluster was offline.
Expand Down Expand Up @@ -389,7 +389,7 @@ func getAWSClientRestConfig(cloudClients cloud.Clients, clock clockwork.Clock, r

// getStaticCredentialsFromKubeconfig loads a kubeconfig from the cluster and returns the access credentials for the cluster.
// If the config defines multiple contexts, it will pick one (the order is not guaranteed).
func getStaticCredentialsFromKubeconfig(ctx context.Context, component KubeServiceType, cluster types.KubeCluster, log *logrus.Entry, checker servicecfg.ImpersonationPermissionsChecker) (*staticKubeCreds, error) {
func getStaticCredentialsFromKubeconfig(ctx context.Context, component KubeServiceType, cluster types.KubeCluster, log *slog.Logger, checker servicecfg.ImpersonationPermissionsChecker) (*staticKubeCreds, error) {
config, err := clientcmd.Load(cluster.GetKubeconfig())
if err != nil {
return nil, trace.WrapWithMessage(err, "unable to parse kubeconfig for cluster %q", cluster.GetName())
Expand Down
5 changes: 2 additions & 3 deletions lib/kube/proxy/cluster_details_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,19 @@ import (
"time"

"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/version"
"k8s.io/client-go/discovery"
"k8s.io/client-go/kubernetes"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/utils"
)

func TestNewClusterDetails(t *testing.T) {
t.Parallel()
ctx := context.Background()
log := logrus.New().WithContext(ctx)

getClusterDetailsConfig := func(c clockwork.FakeClock) (clusterDetailsConfig, *clusterDetailsClientSet) {
client := &clusterDetailsClientSet{}
Expand All @@ -48,7 +47,7 @@ func TestNewClusterDetails(t *testing.T) {
kubeClient: client,
},
cluster: &types.KubernetesClusterV3{},
log: log,
log: utils.NewSlogLoggerForTests(),
clock: c,
}, client
}
Expand Down
4 changes: 2 additions & 2 deletions lib/kube/proxy/ephemeral_containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (f *Forwarder) ephemeralContainers(authCtx *authContext, w http.ResponseWri
if err != nil {
// This error goes to kubernetes client and is not visible in the logs
// of the teleport server if not logged here.
f.log.Errorf("Failed to create cluster session: %v.", err)
f.log.ErrorContext(req.Context(), "Failed to create cluster session", "error", err)
return nil, trace.Wrap(err)
}
// sess.Close cancels the connection monitor context to release it sooner.
Expand All @@ -101,7 +101,7 @@ func (f *Forwarder) ephemeralContainers(authCtx *authContext, w http.ResponseWri
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
// This error goes to kubernetes client and is not visible in the logs
// of the teleport server if not logged here.
f.log.Errorf("Failed to set up forwarding headers: %v.", err)
f.log.ErrorContext(req.Context(), "Failed to set up forwarding headers", "error", err)
return nil, trace.Wrap(err)
}
if !sess.isLocalKubernetesCluster {
Expand Down
Loading

0 comments on commit b1b6bb2

Please sign in to comment.