From 1c8ab72f4f965aefcc53715df31e2b9a6c543991 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Sun, 5 Mar 2023 21:45:13 -0600 Subject: [PATCH] Update test asserts for Golang 1.19 and 1.20 TLS error messages --- .../oidc_upstream_watcher_test.go | 9 ++--- internal/plog/config_test.go | 35 +++++++++++-------- internal/testutil/assertions.go | 5 +-- internal/testutil/tlsassertions/assertions.go | 10 ++++++ .../tlsassertions/assertions_before_go1.20.go | 10 ++++++ .../testutil/tlsassertions/assertions_test.go | 22 ++++++++++++ internal/upstreamldap/upstreamldap_test.go | 3 +- 7 files changed, 73 insertions(+), 21 deletions(-) create mode 100644 internal/testutil/tlsassertions/assertions.go create mode 100644 internal/testutil/tlsassertions/assertions_before_go1.20.go create mode 100644 internal/testutil/tlsassertions/assertions_test.go diff --git a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go index aad7b1d4..0efb6822 100644 --- a/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go +++ b/internal/controller/supervisorconfig/oidcupstreamwatcher/oidc_upstream_watcher_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/testutil/tlsassertions" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -595,11 +596,11 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { }}, wantErr: controllerlib.ErrSyntheticRequeue.Error(), wantLogs: []string{ - `oidc-upstream-observer "msg"="failed to perform OIDC discovery" "error"="Get \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": x509: certificate signed by unknown authority" "issuer"="` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" "name"="test-name" "namespace"="test-namespace"`, + `oidc-upstream-observer "msg"="failed to perform OIDC discovery" "error"="Get \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": ` + tlsassertions.GetTlsErrorPrefix() + `x509: certificate signed by unknown authority" "issuer"="` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" "name"="test-name" "namespace"="test-namespace"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`, - `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": x509: certificate signed by unknown authority" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": ` + tlsassertions.GetTlsErrorPrefix() + `x509: certificate signed by unknown authority" "reason"="Unreachable" "status"="False" "type"="OIDCDiscoverySucceeded"`, `oidc-upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="additionalAuthorizeParameters parameter names are allowed" "reason"="Success" "status"="True" "type"="AdditionalAuthorizeParametersValid"`, - `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": x509: certificate signed by unknown authority" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, + `oidc-upstream-observer "msg"="found failing condition" "error"="OIDCIdentityProvider has a failing condition" "message"="failed to perform OIDC discovery against \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee\":\nGet \"` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration\": ` + tlsassertions.GetTlsErrorPrefix() + `x509: certificate signed by unknown authority" "name"="test-name" "namespace"="test-namespace" "reason"="Unreachable" "type"="OIDCDiscoverySucceeded"`, }, wantResultingCache: []*oidctestutil.TestUpstreamOIDCIdentityProvider{}, wantResultingUpstreams: []v1alpha1.OIDCIdentityProvider{{ @@ -621,7 +622,7 @@ func TestOIDCUpstreamWatcherControllerSync(t *testing.T) { LastTransitionTime: now, Reason: "Unreachable", Message: `failed to perform OIDC discovery against "` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee": -Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration": x509: certificate signed by unknown authority`, +Get "` + testIssuerURL + `/valid-url-that-is-really-really-long-nanananananananannanananan-batman-nanananananananananananananana-batman-lalalalalalalalalal-batman-weeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee/.well-known/openid-configuration": ` + tlsassertions.GetTlsErrorPrefix() + `x509: certificate signed by unknown authority`, }, }, }, diff --git a/internal/plog/config_test.go b/internal/plog/config_test.go index 25cafadc..a3e7425e 100644 --- a/internal/plog/config_test.go +++ b/internal/plog/config_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package plog @@ -11,6 +11,7 @@ import ( "os" "runtime" "strconv" + "strings" "testing" "time" @@ -43,7 +44,7 @@ func TestFormat(t *testing.T) { wd, err := os.Getwd() require.NoError(t, err) - const startLogLine = 46 // make this match the current line number + const startLogLine = 47 // make this match the current line number Info("hello", "happy", "day", "duration", time.Hour+time.Minute) require.True(t, scanner.Scan()) @@ -122,6 +123,12 @@ func TestFormat(t *testing.T) { WithName("stacky").WithName("does").Info("has a stack trace!") require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) + + line := "1576" + if strings.Contains(runtime.Version(), "1.19") { + line = "1446" + } + require.JSONEq(t, fmt.Sprintf(` { "level": "info", @@ -136,8 +143,8 @@ func TestFormat(t *testing.T) { `go.pinniped.dev/internal/plog.TestFormat %s/config_test.go:%d testing.tRunner - %s/src/testing/testing.go:1446`, - wd, startLogLine+2+13+14+11+12+24, runtime.GOROOT(), + %s/src/testing/testing.go:%s`, + wd, startLogLine+2+13+14+11+12+24, runtime.GOROOT(), line, ), ), ), scanner.Text()) @@ -151,13 +158,13 @@ testing.tRunner require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(nowStr+` plog/config_test.go:%d something happened {"error": "invalid log format, valid choices are the empty string, json and text", "an": "item"}`, - startLogLine+2+13+14+11+12+24+28), scanner.Text()) + startLogLine+2+13+14+11+12+24+28+6), scanner.Text()) Logr().WithName("burrito").Error(errInvalidLogLevel, "wee", "a", "b", "slightly less than a year", 363*24*time.Hour, "slightly more than 2 years", 2*367*24*time.Hour) require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(nowStr+` burrito plog/config_test.go:%d wee {"a": "b", "slightly less than a year": "363d", "slightly more than 2 years": "2y4d", "error": "invalid log level, valid choices are the empty string, info, debug, trace and all"}`, - startLogLine+2+13+14+11+12+24+28+6), scanner.Text()) + startLogLine+2+13+14+11+12+24+28+6+6), scanner.Text()) origTimeNow := textlogger.TimeNow t.Cleanup(func() { @@ -183,19 +190,19 @@ testing.tRunner require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "what is happening" does klog="work?"`, - pid, startLogLine+2+13+14+11+12+24+28+6+26), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6), scanner.Text()) Logr().WithName("panda").V(KlogLevelDebug).Info("are the best", "yes?", "yes.") require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "panda: are the best" yes?="yes."`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6), scanner.Text()) New().WithName("hi").WithName("there").WithValues("a", 1, "b", 2).Always("do it") require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "hi/there: do it" a=1 b=2`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+6), scanner.Text()) l := WithValues("x", 33, "z", 22) l.Debug("what to do") @@ -203,17 +210,17 @@ testing.tRunner require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "what to do" x=33 z=22`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+6), scanner.Text()) require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "and why" x=33 z=22`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+6), scanner.Text()) old.Always("should be klog text format", "for", "sure") require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "created before mode change: should be klog text format" is="old" for="sure"`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10+6), scanner.Text()) // make sure child loggers do not share state old1 := old.WithValues("i am", "old1") @@ -223,11 +230,11 @@ testing.tRunner require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "created before mode change: warn" is="old" i am="old1" warning=true`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10+9), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10+9+6), scanner.Text()) require.True(t, scanner.Scan()) require.NoError(t, scanner.Err()) require.Equal(t, fmt.Sprintf(`I1121 23:37:26.953313%8d config_test.go:%d] "created before mode change/old2: info" is="old"`, - pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10+9+1), scanner.Text()) + pid, startLogLine+2+13+14+11+12+24+28+6+26+6+6+7+1+10+9+1+6), scanner.Text()) Trace("should not be logged", "for", "sure") require.Empty(t, buf.String()) diff --git a/internal/testutil/assertions.go b/internal/testutil/assertions.go index 5d1909a2..8db7ec2e 100644 --- a/internal/testutil/assertions.go +++ b/internal/testutil/assertions.go @@ -12,6 +12,7 @@ import ( "time" "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/testutil/tlsassertions" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" @@ -179,8 +180,8 @@ func WantX509UntrustedCertErrorString(expectedErrorFormatSpecifier string, expec // This is the normal Go x509 library error string. standardErr := `x509: certificate signed by unknown authority` allowedErrorStrings := []string{ - fmt.Sprintf(expectedErrorFormatSpecifier, macOSErr), - fmt.Sprintf(expectedErrorFormatSpecifier, standardErr), + fmt.Sprintf(expectedErrorFormatSpecifier, tlsassertions.GetTlsErrorPrefix()+macOSErr), + fmt.Sprintf(expectedErrorFormatSpecifier, tlsassertions.GetTlsErrorPrefix()+standardErr), } // Allow either. require.Contains(t, allowedErrorStrings, actualErrorStr) diff --git a/internal/testutil/tlsassertions/assertions.go b/internal/testutil/tlsassertions/assertions.go new file mode 100644 index 00000000..80b2ddb5 --- /dev/null +++ b/internal/testutil/tlsassertions/assertions.go @@ -0,0 +1,10 @@ +// Copyright 2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build go1.20 + +package tlsassertions + +func GetTlsErrorPrefix() string { + return "tls: failed to verify certificate: " +} diff --git a/internal/testutil/tlsassertions/assertions_before_go1.20.go b/internal/testutil/tlsassertions/assertions_before_go1.20.go new file mode 100644 index 00000000..f710dba8 --- /dev/null +++ b/internal/testutil/tlsassertions/assertions_before_go1.20.go @@ -0,0 +1,10 @@ +// Copyright 2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +//go:build !go1.20 + +package tlsassertions + +func GetTlsErrorPrefix() string { + return "" +} diff --git a/internal/testutil/tlsassertions/assertions_test.go b/internal/testutil/tlsassertions/assertions_test.go new file mode 100644 index 00000000..17b57787 --- /dev/null +++ b/internal/testutil/tlsassertions/assertions_test.go @@ -0,0 +1,22 @@ +// Copyright 2023 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package tlsassertions + +import ( + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetTlsErrorPrefix(t *testing.T) { + expected := "tls: failed to verify certificate: " + + if strings.Contains(runtime.Version(), "1.19") { + expected = "" + } + + require.Equal(t, expected, GetTlsErrorPrefix()) +} diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index 8c0a5fbf..39f7e4cc 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -19,6 +19,7 @@ import ( "github.com/go-ldap/ldap/v3" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "go.pinniped.dev/internal/testutil/tlsassertions" "k8s.io/apiserver/pkg/authentication/user" "go.pinniped.dev/internal/authenticators" @@ -2025,7 +2026,7 @@ func TestRealTLSDialing(t *testing.T) { caBundle: caForTestServerWithBadCertName.Bundle(), connProto: TLS, context: context.Background(), - wantError: testutil.WantExactErrorString(`LDAP Result Code 200 "Network Error": x509: certificate is valid for 10.2.3.4, not 127.0.0.1`), + wantError: testutil.WantExactErrorString(fmt.Sprintf(`LDAP Result Code 200 "Network Error": %sx509: certificate is valid for 10.2.3.4, not 127.0.0.1`, tlsassertions.GetTlsErrorPrefix())), }, { name: "invalid CA bundle with TLS",