From 0467e5c1d552e49eefb82cb50eb94cff41b9ac35 Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Mon, 18 Mar 2024 12:05:21 -0400 Subject: [PATCH] Refactor logLines to SplitByNewline, deduplicate --- cmd/pinniped/cmd/login_oidc_test.go | 12 ++--------- cmd/pinniped/cmd/login_static_test.go | 5 +++-- .../jwtcachefiller/jwtcachefiller_test.go | 20 +++++++++---------- .../webhookcachefiller_test.go | 8 ++++---- .../kubecertagent/kubecertagent_test.go | 12 ++--------- .../testutil/conciergetestutil/tlstestutil.go | 2 +- .../loglines.go => stringutil/stringutil.go} | 4 ++-- 7 files changed, 24 insertions(+), 39 deletions(-) rename internal/testutil/{testlogger/loglines.go => stringutil/stringutil.go} (77%) diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index f023e0757..6d28d7b6e 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/cmd/pinniped/cmd/login_oidc_test.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "testing" "time" @@ -22,6 +21,7 @@ import ( "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/testutil/stringutil" "go.pinniped.dev/pkg/conciergeclient" "go.pinniped.dev/pkg/oidcclient" "go.pinniped.dev/pkg/oidcclient/oidctypes" @@ -596,15 +596,7 @@ func TestLoginOIDCCommand(t *testing.T) { require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") require.Len(t, gotOptions, tt.wantOptionsCount) - require.Equal(t, tt.wantLogs, logLines(buf.String())) + require.Equal(t, tt.wantLogs, stringutil.SplitByNewline(buf.String())) }) } } - -func logLines(logs string) []string { - if len(logs) == 0 { - return nil - } - - return strings.Split(strings.TrimSpace(logs), "\n") -} diff --git a/cmd/pinniped/cmd/login_static_test.go b/cmd/pinniped/cmd/login_static_test.go index e2f520bdc..bf65a4e39 100644 --- a/cmd/pinniped/cmd/login_static_test.go +++ b/cmd/pinniped/cmd/login_static_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package cmd @@ -20,6 +20,7 @@ import ( "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/plog" + "go.pinniped.dev/internal/testutil/stringutil" "go.pinniped.dev/pkg/conciergeclient" ) @@ -215,7 +216,7 @@ func TestLoginStaticCommand(t *testing.T) { require.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout") require.Equal(t, tt.wantStderr, stderr.String(), "unexpected stderr") - require.Equal(t, tt.wantLogs, logLines(buf.String())) + require.Equal(t, tt.wantLogs, stringutil.SplitByNewline(buf.String())) }) } } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 0efc42354..cb9f3d851 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -45,7 +45,7 @@ import ( "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/conciergetestutil" "go.pinniped.dev/internal/testutil/conditionstestutil" - "go.pinniped.dev/internal/testutil/testlogger" + "go.pinniped.dev/internal/testutil/stringutil" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -208,12 +208,12 @@ func TestController(t *testing.T) { someJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: goodIssuer, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodOIDCIssuerServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodOIDCIssuerServer.TLS), } someJWTAuthenticatorSpecWithUsernameClaim := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: goodIssuer, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodOIDCIssuerServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodOIDCIssuerServer.TLS), Claims: auth1alpha1.JWTTokenClaims{ Username: "my-custom-username-claim", }, @@ -221,7 +221,7 @@ func TestController(t *testing.T) { someJWTAuthenticatorSpecWithGroupsClaim := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: goodIssuer, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodOIDCIssuerServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodOIDCIssuerServer.TLS), Claims: auth1alpha1.JWTTokenClaims{ Groups: customGroupsClaim, }, @@ -247,12 +247,12 @@ func TestController(t *testing.T) { invalidIssuerJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: "https://.café .com/café/café/café/coffee", Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodOIDCIssuerServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodOIDCIssuerServer.TLS), } invalidIssuerSchemeJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: "http://.café.com/café/café/café/coffee", Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodOIDCIssuerServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodOIDCIssuerServer.TLS), } validIssuerURLButDoesNotExistJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ @@ -262,18 +262,18 @@ func TestController(t *testing.T) { badIssuerJWKSURIJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: badIssuerInvalidJWKSURI, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(badOIDCIssuerServerInvalidJWKSURI.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(badOIDCIssuerServerInvalidJWKSURI.TLS), } badIssuerJWKSURISchemeJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: badIssuerInvalidJWKSURIScheme, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(badOIDCIssuerServerInvalidJWKSURIScheme.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(badOIDCIssuerServerInvalidJWKSURIScheme.TLS), } jwksFetchShouldFailJWTAuthenticatorSpec := &auth1alpha1.JWTAuthenticatorSpec{ Issuer: jwksFetchShouldFailServer.URL, Audience: goodAudience, - TLS: conciergetestutil.TlsSpecFromTLSConfig(jwksFetchShouldFailServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(jwksFetchShouldFailServer.TLS), } happyReadyCondition := func(time metav1.Time, observedGeneration int64) metav1.Condition { @@ -1454,7 +1454,7 @@ func TestController(t *testing.T) { require.NoError(t, err) } - actualLogLines := testlogger.LogLines(log.String()) + actualLogLines := stringutil.SplitByNewline(log.String()) require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct") for logLineNum, logLine := range actualLogLines { diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index e21e7b45c..a8ffc4c67 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -42,7 +42,7 @@ import ( "go.pinniped.dev/internal/testutil" "go.pinniped.dev/internal/testutil/conciergetestutil" "go.pinniped.dev/internal/testutil/conditionstestutil" - "go.pinniped.dev/internal/testutil/testlogger" + "go.pinniped.dev/internal/testutil/stringutil" "go.pinniped.dev/internal/testutil/tlsserver" ) @@ -162,7 +162,7 @@ func TestController(t *testing.T) { goodWebhookAuthenticatorSpecWithCA := auth1alpha1.WebhookAuthenticatorSpec{ Endpoint: goodEndpoint, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodWebhookServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodWebhookServer.TLS), } localhostWebhookAuthenticatorSpecWithCA := auth1alpha1.WebhookAuthenticatorSpec{ Endpoint: localhostEndpointURL, @@ -185,7 +185,7 @@ func TestController(t *testing.T) { } goodWebhookAuthenticatorSpecWith404Endpoint := auth1alpha1.WebhookAuthenticatorSpec{ Endpoint: goodEndpointBut404, - TLS: conciergetestutil.TlsSpecFromTLSConfig(goodWebhookServer.TLS), + TLS: conciergetestutil.TLSSpecFromTLSConfig(goodWebhookServer.TLS), } badWebhookAuthenticatorSpecInvalidTLS := auth1alpha1.WebhookAuthenticatorSpec{ Endpoint: goodEndpoint, @@ -933,7 +933,7 @@ func TestController(t *testing.T) { } else { require.NoError(t, err) } - actualLogLines := testlogger.LogLines(log.String()) + actualLogLines := stringutil.SplitByNewline(log.String()) require.Equal(t, len(actualLogLines), len(tt.wantLogs), "log line count should be correct") for logLineNum, logLine := range actualLogLines { diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index f8fb5482d..c8dfd0fa9 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "fmt" - "strings" "testing" "time" @@ -39,6 +38,7 @@ import ( "go.pinniped.dev/internal/kubeclient" "go.pinniped.dev/internal/plog" "go.pinniped.dev/internal/testutil" + "go.pinniped.dev/internal/testutil/stringutil" "go.pinniped.dev/test/testlib" ) @@ -1085,7 +1085,7 @@ func TestAgentController(t *testing.T) { allAllowedErrors = append(allAllowedErrors, tt.alsoAllowUndesiredDistinctErrors...) assert.Subsetf(t, allAllowedErrors, actualErrors, "actual errors contained additional error(s) which is not expected by the test") - assert.Equal(t, tt.wantDistinctLogs, deduplicate(logLines(buf.String())), "unexpected logs") + assert.Equal(t, tt.wantDistinctLogs, deduplicate(stringutil.SplitByNewline(buf.String())), "unexpected logs") // Assert on all actions that happened to deployments. var actualDeploymentActionVerbs []string @@ -1128,14 +1128,6 @@ func TestAgentController(t *testing.T) { } } -func logLines(logs string) []string { - if len(logs) == 0 { - return nil - } - - return strings.Split(strings.TrimSpace(logs), "\n") -} - func TestMergeLabelsAndAnnotations(t *testing.T) { t.Parallel() diff --git a/internal/testutil/conciergetestutil/tlstestutil.go b/internal/testutil/conciergetestutil/tlstestutil.go index 0dc412e61..f99ba4e86 100644 --- a/internal/testutil/conciergetestutil/tlstestutil.go +++ b/internal/testutil/conciergetestutil/tlstestutil.go @@ -11,7 +11,7 @@ import ( auth1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" ) -func TlsSpecFromTLSConfig(tls *tls.Config) *auth1alpha1.TLSSpec { +func TLSSpecFromTLSConfig(tls *tls.Config) *auth1alpha1.TLSSpec { pemData := make([]byte, 0) for _, certificate := range tls.Certificates { // this is the public part of the certificate, the private is the certificate.PrivateKey diff --git a/internal/testutil/testlogger/loglines.go b/internal/testutil/stringutil/stringutil.go similarity index 77% rename from internal/testutil/testlogger/loglines.go rename to internal/testutil/stringutil/stringutil.go index 8ec15f320..aa8a266f1 100644 --- a/internal/testutil/testlogger/loglines.go +++ b/internal/testutil/stringutil/stringutil.go @@ -1,11 +1,11 @@ // Copyright 2024 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -package testlogger +package stringutil import "strings" -func LogLines(logs string) []string { +func SplitByNewline(logs string) []string { if len(logs) == 0 { return nil }