plog.TestLogger returns a buffer instead of taking one in

2026-02-14 18:10:17 +00:00 · 2024-11-07 16:21:41 -06:00
parent dc86c9305c
commit 0c131f11f8
17 changed files with 52 additions and 86 deletions
--- a/internal/controller/apicerts/certs_expirer_test.go
+++ b/internal/controller/apicerts/certs_expirer_test.go
@@ -4,14 +4,12 @@
 package apicerts

 import (
-	"bytes"
 	"context"
 	"crypto/ecdsa"
 	"crypto/elliptic"
 	"crypto/rand"
 	"crypto/x509"
 	"errors"
-	"io"
 	"strings"
 	"testing"
 	"time"
@@ -96,6 +94,8 @@ func TestExpirerControllerFilters(t *testing.T) {
 				0,
 			).Core().V1().Secrets()
 			withInformer := testutil.NewObservableWithInformerOption()
+
+			logger, _ := plog.TestLogger(t)
 			_ = NewCertsExpirerController(
 				test.namespace,
 				certsSecretResourceName,
@@ -104,7 +104,7 @@ func TestExpirerControllerFilters(t *testing.T) {
 				withInformer.WithInformer,
 				0,  // renewBefore, not needed
 				"", // not needed
-				plog.TestLogger(t, io.Discard),
+				logger,
 			)

 			unrelated := corev1.Secret{}
@@ -256,7 +256,7 @@ func TestExpirerControllerSync(t *testing.T) {
 				0,
 			)

-			var log bytes.Buffer
+			logger, log := plog.TestLogger(t)

 			c := NewCertsExpirerController(
 				namespace,
@@ -266,7 +266,7 @@ func TestExpirerControllerSync(t *testing.T) {
 				controllerlib.WithInformer,
 				test.renewBefore,
 				fakeTestKey,
-				plog.TestLogger(t, &log),
+				logger,
 			)

 			// Must start informers before calling TestRunSynchronously().
--- a/internal/controller/authenticator/cachecleaner/cachecleaner_test.go
+++ b/internal/controller/authenticator/cachecleaner/cachecleaner_test.go
@@ -4,7 +4,6 @@
 package cachecleaner

 import (
-	"bytes"
 	"context"
 	"testing"

@@ -146,8 +145,7 @@ func TestController(t *testing.T) {
 			}
 			webhooks := informers.Authentication().V1alpha1().WebhookAuthenticators()
 			jwtAuthenticators := informers.Authentication().V1alpha1().JWTAuthenticators()
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			controller := New(cache, webhooks, jwtAuthenticators, logger)

--- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go
+++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go
@@ -4,7 +4,6 @@
 package jwtcachefiller

 import (
-	"bytes"
 	"context"
 	"crypto/ecdsa"
 	"crypto/elliptic"
@@ -2414,8 +2413,7 @@ func TestController(t *testing.T) {
 			kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0)
 			cache := authncache.New()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			if tt.cache != nil {
 				tt.cache(t, cache, tt.wantClose)
@@ -2892,8 +2890,7 @@ func TestControllerFilterSecret(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)
 			frozenClock := clocktesting.NewFakeClock(nowDoesntMatter)
@@ -2953,8 +2950,7 @@ func TestControllerFilterConfigMap(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)
 			frozenClock := clocktesting.NewFakeClock(nowDoesntMatter)
--- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go
+++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go
@@ -4,7 +4,6 @@
 package webhookcachefiller

 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"encoding/base64"
@@ -2128,8 +2127,7 @@ func TestController(t *testing.T) {
 			kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0)
 			cache := authncache.New()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			if tt.cache != nil {
 				tt.cache(t, cache)
@@ -2375,8 +2373,7 @@ func TestControllerFilterSecret(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)
 			frozenClock := clocktesting.NewFakeClock(nowDoesntMatter)
@@ -2439,8 +2436,7 @@ func TestControllerFilterConfigMap(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)
 			frozenClock := clocktesting.NewFakeClock(nowDoesntMatter)
--- a/internal/controller/conditionsutil/conditions_util_test.go
+++ b/internal/controller/conditionsutil/conditions_util_test.go
@@ -4,7 +4,6 @@
 package conditionsutil

 import (
-	"bytes"
 	"strings"
 	"testing"
 	"time"
@@ -167,8 +166,7 @@ func TestMergeIDPConditions(t *testing.T) {

 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			result := MergeConditions(
 				test.newConditions,
--- a/internal/controller/impersonatorconfig/impersonator_config_test.go
+++ b/internal/controller/impersonatorconfig/impersonator_config_test.go
@@ -4,7 +4,6 @@
 package impersonatorconfig

 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
@@ -75,8 +74,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
 			servicesInformer := sharedInformerFactory.Core().V1().Services()
 			secretsInformer := sharedInformerFactory.Core().V1().Secrets()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			_ = NewImpersonatorConfigController(
 				installedInNamespace,
@@ -569,8 +567,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 		// Defer starting the informers until the last possible moment so that the
 		// nested Before's can keep adding things to the informer caches.
 		var startInformersAndController = func() {
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			// Set this at the last second to allow for injection of server override.
 			subject = NewImpersonatorConfigController(
--- a/internal/controller/kubecertagent/kubecertagent_test.go
+++ b/internal/controller/kubecertagent/kubecertagent_test.go
@@ -4,7 +4,6 @@
 package kubecertagent

 import (
-	"bytes"
 	"context"
 	"fmt"
 	"slices"
@@ -1156,8 +1155,7 @@ func TestAgentController(t *testing.T) {

 			kubeInformers := informers.NewSharedInformerFactory(kubeClientset, 0)

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
--- a/internal/controller/kubecertagent/legacypodcleaner_test.go
+++ b/internal/controller/kubecertagent/legacypodcleaner_test.go
@@ -4,7 +4,6 @@
 package kubecertagent

 import (
-	"bytes"
 	"context"
 	"fmt"
 	"testing"
@@ -149,8 +148,7 @@ func TestLegacyPodCleanerController(t *testing.T) {
 			}

 			kubeInformers := informers.NewSharedInformerFactory(kubeClientset, 0)
-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)
 			controller := NewLegacyPodCleanerController(
 				AgentConfig{
 					Namespace: "concierge",
--- a/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go
+++ b/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go
@@ -4,7 +4,6 @@
 package serviceaccounttokencleanup

 import (
-	"bytes"
 	"context"
 	"errors"
 	"testing"
@@ -29,14 +28,14 @@ func TestNewServiceAccountTokenCleanupController(t *testing.T) {
 	observableWithInformerOption := testutil.NewObservableWithInformerOption()
 	secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()

-	var log bytes.Buffer
+	logger, _ := plog.TestLogger(t)
 	_ = NewLegacyServiceAccountTokenCleanupController(
 		namespace,
 		legacySecretName,
 		nil, // not needed for this test
 		secretsInformer,
 		observableWithInformerOption.WithInformer,
-		plog.TestLogger(t, &log),
+		logger,
 	)

 	secretsInformerFilter := observableWithInformerOption.GetFilterForInformer(secretsInformer)
@@ -138,14 +137,14 @@ func TestSync(t *testing.T) {
 				tt.addReactors(kubeAPIClient)
 			}

-			var log bytes.Buffer
+			logger, _ := plog.TestLogger(t)
 			controller := NewLegacyServiceAccountTokenCleanupController(
 				tt.namespace,
 				tt.secretNameToDelete,
 				kubeAPIClient,
 				kubeInformers.Core().V1().Secrets(),
 				controllerlib.WithInformer,
-				plog.TestLogger(t, &log),
+				logger,
 			)

 			ctx, cancel := context.WithCancel(context.Background())
--- a/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/githubupstreamwatcher/github_upstream_watcher_test.go
@@ -4,7 +4,6 @@
 package githubupstreamwatcher

 import (
-	"bytes"
 	"context"
 	"crypto/x509"
 	"encoding/base64"
@@ -2451,8 +2450,7 @@ func TestController(t *testing.T) {
 				),
 			})

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			gitHubIdentityProviderInformer := supervisorInformers.IDP().V1alpha1().GitHubIdentityProviders()

@@ -2846,8 +2844,7 @@ func TestController_OnlyWantActions(t *testing.T) {

 			kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(tt.secrets...), 0)

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			controller := New(
 				namespace,
@@ -2967,8 +2964,7 @@ func TestGitHubUpstreamWatcherControllerFilterSecret(t *testing.T) {

 			kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0)

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			observableInformers := testutil.NewObservableWithInformerOption()
 			secretInformer := kubeInformers.Core().V1().Secrets()
@@ -3024,8 +3020,7 @@ func TestGitHubUpstreamWatcherControllerFilterConfigMaps(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			observableInformers := testutil.NewObservableWithInformerOption()
 			configMapInformer := k8sinformers.NewSharedInformerFactoryWithOptions(kubernetesfake.NewSimpleClientset(), 0).Core().V1().ConfigMaps()
@@ -3081,8 +3076,7 @@ func TestGitHubUpstreamWatcherControllerFilterGitHubIDP(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			observableInformers := testutil.NewObservableWithInformerOption()
 			gitHubIdentityProviderInformer := supervisorinformers.NewSharedInformerFactory(supervisorfake.NewSimpleClientset(), 0).IDP().V1alpha1().GitHubIdentityProviders()
--- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
+++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go
@@ -4,7 +4,6 @@
 package oidcupstreamwatcher

 import (
-	"bytes"
 	"context"
 	"crypto/x509"
 	"encoding/base64"
@@ -113,8 +112,7 @@ func TestOIDCUpstreamWatcherControllerFilterSecret(t *testing.T) {
 			configMapInformer := kubeInformers.Core().V1().ConfigMaps()
 			withInformer := testutil.NewObservableWithInformerOption()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			New(
 				cache,
@@ -173,8 +171,7 @@ func TestOIDCUpstreamWatcherControllerFilterConfigMaps(t *testing.T) {
 			configMapInformer := kubeInformers.Core().V1().ConfigMaps()
 			withInformer := testutil.NewObservableWithInformerOption()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			New(
 				cache,
@@ -1626,8 +1623,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) {
 				&upstreamoidc.ProviderConfig{Name: "initial-entry"},
 			})

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, log := plog.TestLogger(t)

 			validatorCache := expiringcache.NewExpiring()
 			if tt.inputValidatorCache != nil {
--- a/internal/crypto/ptls/dialer_test.go
+++ b/internal/crypto/ptls/dialer_test.go
@@ -5,7 +5,6 @@
 package ptls_test

 import (
-	"bytes"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
@@ -63,8 +62,7 @@ func TestDialer(t *testing.T) {
 			t.Parallel()
 			dialer := ptls.NewDialer()

-			var log bytes.Buffer
-			logger := plog.TestLogger(t, &log)
+			logger, _ := plog.TestLogger(t)

 			err := dialer.IsReachableAndTLSValidationSucceeds(
 				context.Background(),
@@ -135,8 +133,7 @@ func TestDialer_AppliesTimeouts(t *testing.T) {
 			maxTimeForTest := time.After(test.maxTestDuration)
 			testPassed := make(chan bool)
 			go func() {
-				var log bytes.Buffer
-				logger := plog.TestLogger(t, &log)
+				logger, _ := plog.TestLogger(t)

 				dialer := ptls.NewDialer()
 				err := dialer.IsReachableAndTLSValidationSucceeds(
--- a/internal/crypto/ptls/log_profiles_test.go
+++ b/internal/crypto/ptls/log_profiles_test.go
@@ -4,7 +4,6 @@
 package ptls

 import (
-	"bytes"
 	"strings"
 	"testing"

@@ -14,8 +13,7 @@ import (
 )

 func TestLogAllProfiles(t *testing.T) {
-	var log bytes.Buffer
-	logger := plog.TestLogger(t, &log)
+	logger, log := plog.TestLogger(t)

 	LogAllProfiles(logger)

--- a/internal/plog/plog_test.go
+++ b/internal/plog/plog_test.go
@@ -4,7 +4,6 @@
 package plog

 import (
-	"bytes"
 	"fmt"
 	"runtime"
 	"strings"
@@ -360,8 +359,8 @@ func TestPlog(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()

-			var log bytes.Buffer
-			tt.run(TestLogger(t, &log))
+			subjectLogger, log := TestLogger(t)
+			tt.run(subjectLogger)

 			require.Equal(t, strings.TrimSpace(tt.want), strings.TrimSpace(log.String()))
 		})
--- a/internal/plog/testing.go
+++ b/internal/plog/testing.go
@@ -4,6 +4,7 @@
 package plog

 import (
+	"bytes"
 	"context"
 	"io"
 	"math"
@@ -59,12 +60,15 @@ func AddZapOverridesToContext(
 	return context.WithValue(ctx, testOverridesContextKey, overrides)
 }

-func TestLogger(t *testing.T, w io.Writer) Logger {
+func TestLogger(t *testing.T) (Logger, *bytes.Buffer) {
 	t.Helper()

+	var log bytes.Buffer
+
 	return New().withLogrMod(func(l logr.Logger) logr.Logger {
-		return l.WithSink(testZapr(t, w, "json").GetSink())
-	})
+			return l.WithSink(testZapr(t, &log, "json").GetSink())
+		}),
+		&log
 }

 func TestConsoleLogger(t *testing.T, w io.Writer) Logger {
--- a/internal/tokenclient/tokenclient_test.go
+++ b/internal/tokenclient/tokenclient_test.go
@@ -4,7 +4,6 @@
 package tokenclient

 import (
-	"bytes"
 	"context"
 	"errors"
 	"sync"
@@ -34,8 +33,7 @@ func TestNew(t *testing.T) {
 	mockClient := fake.NewSimpleClientset().CoreV1().ServiceAccounts("")
 	mockTime := time.Now()
 	mockClock := clocktesting.NewFakeClock(mockTime)
-	var log bytes.Buffer
-	testLogger := plog.TestLogger(t, &log)
+	logger, _ := plog.TestLogger(t)

 	type args struct {
 		serviceAccountName   string
@@ -56,7 +54,7 @@ func TestNew(t *testing.T) {
 				serviceAccountName:   "serviceAccountName",
 				serviceAccountClient: mockClient,
 				whatToDoWithToken:    mockWhatToDoWithTokenFunc,
-				logger:               testLogger,
+				logger:               logger,
 			},
 			expected: &TokenClient{
 				serviceAccountName:   "serviceAccountName",
@@ -64,7 +62,7 @@ func TestNew(t *testing.T) {
 				whatToDoWithToken:    mockWhatToDoWithTokenFunc,
 				expirationSeconds:    600,
 				clock:                clock.RealClock{},
-				logger:               testLogger,
+				logger:               logger,
 			},
 		},
 		{
@@ -73,7 +71,7 @@ func TestNew(t *testing.T) {
 				serviceAccountName:   "custom-serviceAccountName",
 				serviceAccountClient: mockClient,
 				whatToDoWithToken:    mockWhatToDoWithTokenFunc,
-				logger:               testLogger,
+				logger:               logger,
 				opts: []Opt{
 					WithExpirationSeconds(777),
 					withClock(mockClock),
@@ -85,7 +83,7 @@ func TestNew(t *testing.T) {
 				whatToDoWithToken:    mockWhatToDoWithTokenFunc,
 				expirationSeconds:    777,
 				clock:                mockClock,
-				logger:               testLogger,
+				logger:               logger,
 			},
 		},
 	}
@@ -169,7 +167,7 @@ func TestFetchToken(t *testing.T) {
 			t.Parallel()

 			mockClock := clocktesting.NewFakeClock(mockTime.Time)
-			var log bytes.Buffer
+			logger, _ := plog.TestLogger(t)

 			require.NotEmpty(t, tt.serviceAccountName)

@@ -178,7 +176,7 @@ func TestFetchToken(t *testing.T) {
 				tt.serviceAccountName,
 				mockClient.CoreV1().ServiceAccounts("any-namespace-works"),
 				nil,
-				plog.TestLogger(t, &log),
+				logger,
 				WithExpirationSeconds(tt.expirationSeconds),
 			)
 			tokenClient.clock = mockClock
@@ -325,7 +323,7 @@ func TestStart(t *testing.T) {
 			t.Parallel()

 			mockClient := fake.NewSimpleClientset()
-			var logs bytes.Buffer
+			logger, _ := plog.TestLogger(t)

 			var mutex sync.Mutex
 			// These variables are accessed by the reactor and by the callback function in the goroutine which is
@@ -344,7 +342,7 @@ func TestStart(t *testing.T) {
 					t.Logf("received token %q with ttl %q", token, ttl)
 					receivedTokens = append(receivedTokens, receivedToken{token: token, ttl: ttl})
 				},
-				plog.TestLogger(t, &logs),
+				logger,
 			)

 			mockClient.PrependReactor(verb, resource, func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
--- a/pkg/oidcclient/login_test.go
+++ b/pkg/oidcclient/login_test.go
@@ -3881,10 +3881,10 @@ func TestLoggers(t *testing.T) {
 	}), nil)

 	t.Run("with new logger, outputs logs", func(t *testing.T) {
-		var log bytes.Buffer
+		logger, log := plog.TestLogger(t)

 		token, err := Login(issuer.URL, "clientID",
-			WithLoginLogger(plog.TestLogger(t, &log)),
+			WithLoginLogger(logger),
 		)
 		// This error is expected, we're testing logs not discovery
 		require.EqualError(t, err, `could not perform OIDC discovery for "`+issuer.URL+`": Get "`+issuer.URL+`/.well-known/openid-configuration": tls: failed to verify certificate: x509: certificate signed by unknown authority`)