From 4f241c04af7a86970aeab0a3e0198df807a04b86 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 9 Dec 2024 12:56:03 +0100 Subject: [PATCH] client-go auth: contextual logging No API changes are needed. In one case, a context is passed in via the http.Request. In others there is simply no need to change the calls. --- .../client-go/plugin/pkg/client/auth/azure/azure_stub.go | 1 + .../k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go | 3 ++- .../k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go | 6 +++++- .../k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_stub.go | 1 + .../k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go | 6 ++++-- 5 files changed, 13 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/azure/azure_stub.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/azure/azure_stub.go index 22d3c6b3fe4..d82b6766cc0 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/azure/azure_stub.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/azure/azure_stub.go @@ -25,6 +25,7 @@ import ( func init() { if err := rest.RegisterAuthProviderPlugin("azure", newAzureAuthProvider); err != nil { + //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register azure auth plugin: %v", err) } } diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go index 1af2afdb912..f21ca89bb16 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/exec.go @@ -363,6 +363,7 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { return r.base.RoundTrip(req) } + ctx := req.Context() creds, err := r.a.getCreds() if err != nil { return nil, fmt.Errorf("getting credentials: %v", err) @@ -377,7 +378,7 @@ func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { } if res.StatusCode == http.StatusUnauthorized { if err := r.a.maybeRefreshCreds(creds); err != nil { - klog.Errorf("refreshing credentials: %v", err) + klog.FromContext(ctx).Error(err, "Refreshing credentials failed") } } return res, nil diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go index fb300ae12a4..3cdeeced620 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/exec/metrics.go @@ -112,7 +112,11 @@ func incrementCallsMetric(err error) { metrics.ExecPluginCalls.Increment(failureExitCode, pluginNotFoundError) default: // We don't know about this error type. - klog.V(2).InfoS("unexpected exec plugin return error type", "type", reflect.TypeOf(err).String(), "err", err) + // TODO (?): this code gets called through + // https://github.com/kubernetes/kubernetes/blob/8a5cf7b66f4bf8c76c4a78e249ee7c21f7d7403e/staging/src/k8s.io/client-go/transport/config.go#L152-L154 + // which would have to be changed to accept a context. + // Probably not worth it? + klog.TODO().V(2).Info("unexpected exec plugin return error type", "type", reflect.TypeOf(err).String(), "err", err) metrics.ExecPluginCalls.Increment(failureExitCode, clientInternalError) } } diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_stub.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_stub.go index 99585f93917..2aa21f00391 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_stub.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/gcp/gcp_stub.go @@ -25,6 +25,7 @@ import ( func init() { if err := rest.RegisterAuthProviderPlugin("gcp", newGCPAuthProvider); err != nil { + //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register gcp auth plugin: %v", err) } } diff --git a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go index 70e8b57b14f..70f15c3a34b 100644 --- a/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go +++ b/staging/src/k8s.io/client-go/plugin/pkg/client/auth/oidc/oidc.go @@ -49,6 +49,7 @@ const ( func init() { if err := restclient.RegisterAuthProviderPlugin("oidc", newOIDCAuthProvider); err != nil { + //nolint:logcheck // Should not happen. klog.Fatalf("Failed to register oidc auth plugin: %v", err) } } @@ -125,8 +126,9 @@ func newOIDCAuthProvider(clusterAddress string, cfg map[string]string, persister } if len(cfg[cfgExtraScopes]) > 0 { - klog.V(2).Infof("%s auth provider field depricated, refresh request don't send scopes", - cfgExtraScopes) + // TODO (?): the auth provider factory API would have to be changed + // to support contextual logging here. Not worth it? + klog.TODO().V(2).Info("Ignoring deprecated extra scopes, refresh request don't send scopes", "ignoredConfigField", cfgExtraScopes) } var certAuthData []byte