Fix some utils, spacing, func naming, test inputs, etc.

cmd/pinniped/cmd/login_oidc_test.go

@@ -21,7 +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/internal/testutil"
 	"go.pinniped.dev/pkg/conciergeclient"
 	"go.pinniped.dev/pkg/oidcclient"
 	"go.pinniped.dev/pkg/oidcclient/oidctypes"
@@ -596,7 +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, stringutil.SplitByNewline(buf.String()))
+			require.Equal(t, tt.wantLogs, testutil.SplitByNewline(buf.String()))
 		})
 	}
 }

cmd/pinniped/cmd/login_static_test.go

@@ -20,7 +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/internal/testutil"
 	"go.pinniped.dev/pkg/conciergeclient"
 )
 
@@ -216,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, stringutil.SplitByNewline(buf.String()))
+			require.Equal(t, tt.wantLogs, testutil.SplitByNewline(buf.String()))
 		})
 	}
 }

internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go

@@ -45,7 +45,6 @@ import (
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/conciergetestutil"
 	"go.pinniped.dev/internal/testutil/conditionstestutil"
-	"go.pinniped.dev/internal/testutil/stringutil"
 	"go.pinniped.dev/internal/testutil/tlsserver"
 )
 
@@ -654,21 +653,22 @@ func TestController(t *testing.T) {
 			jwtAuthenticators: []runtime.Object{
 				&auth1alpha1.JWTAuthenticator{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-name",
+						Name:       "test-name",
+						Generation: 1234,
 					},
 					Spec: *someJWTAuthenticatorSpec,
 					Status: auth1alpha1.JWTAuthenticatorStatus{
 						Conditions: conditionstestutil.Replace(
-							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 1233),
 							[]metav1.Condition{
 								// sad and unknwn will update with new statuses and timestamps
-								sadReadyCondition(frozenTimeInThePast, 0),
-								sadDiscoveryURLValidx509(goodIssuer, frozenTimeInThePast, 0),
-								unknownAuthenticatorValid(frozenTimeInThePast, 0),
-								unknownJWKSURLValid(frozenTimeInThePast, 0),
-								unknownJWKSFetch(frozenTimeInThePast, 0),
+								sadReadyCondition(frozenTimeInThePast, 1232),
+								sadDiscoveryURLValidx509(goodIssuer, frozenTimeInThePast, 1231),
+								unknownAuthenticatorValid(frozenTimeInThePast, 1232),
+								unknownJWKSURLValid(frozenTimeInThePast, 1111),
+								unknownJWKSFetch(frozenTimeInThePast, 1122),
 								// this one will remain unchanged as it was good to begin with
-								happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 0),
+								happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 4321),
 							},
 						),
 						Phase: "Error",
@@ -688,15 +688,16 @@ func TestController(t *testing.T) {
 			wantActions: func() []coretesting.Action {
 				updateStatusAction := coretesting.NewUpdateAction(jwtAuthenticatorsGVR, "", &auth1alpha1.JWTAuthenticator{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-name",
+						Name:       "test-name",
+						Generation: 1234,
 					},
 					Spec: *someJWTAuthenticatorSpec,
 					Status: auth1alpha1.JWTAuthenticatorStatus{
 						Conditions: conditionstestutil.Replace(
-							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 0),
+							allHappyConditionsSuccess(goodIssuer, frozenMetav1Now, 1234),
 							[]metav1.Condition{
 								// this timestamp should not have updated, it didn't change.
-								happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 0),
+								happyTLSConfigurationValidCAParsed(frozenTimeInThePast, 1234),
 							},
 						),
 						Phase: "Ready",
@@ -1703,7 +1704,7 @@ func TestController(t *testing.T) {
 				require.NoError(t, err)
 			}
 
-			actualLogLines := stringutil.SplitByNewline(log.String())
+			actualLogLines := testutil.SplitByNewline(log.String())
 			require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct")
 
 			for logLineNum, logLine := range actualLogLines {

internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go

@@ -42,7 +42,6 @@ import (
 	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/conciergetestutil"
 	"go.pinniped.dev/internal/testutil/conditionstestutil"
-	"go.pinniped.dev/internal/testutil/stringutil"
 	"go.pinniped.dev/internal/testutil/tlsserver"
 )
 
@@ -450,15 +449,16 @@ func TestController(t *testing.T) {
 			webhooks: []runtime.Object{
 				&auth1alpha1.WebhookAuthenticator{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-name",
+						Name:       "test-name",
+						Generation: 1234,
 					},
 					Spec: goodWebhookAuthenticatorSpecWithCA,
 					Status: auth1alpha1.WebhookAuthenticatorStatus{
 						Conditions: conditionstestutil.Replace(
-							allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 666),
+							allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 1233),
 							[]metav1.Condition{
-								sadReadyCondition(frozenTimeInThePast, 777),
-								happyEndpointURLValid(frozenTimeInThePast, 888),
+								sadReadyCondition(frozenTimeInThePast, 1232),
+								happyEndpointURLValid(frozenTimeInThePast, 1231),
 							},
 						),
 						Phase: "Ready",
@@ -480,14 +480,15 @@ func TestController(t *testing.T) {
 			wantActions: func() []coretesting.Action {
 				updateStatusAction := coretesting.NewUpdateAction(webhookAuthenticatorGVR, "", &auth1alpha1.WebhookAuthenticator{
 					ObjectMeta: metav1.ObjectMeta{
-						Name: "test-name",
+						Name:       "test-name",
+						Generation: 1234,
 					},
 					Spec: goodWebhookAuthenticatorSpecWithCA,
 					Status: auth1alpha1.WebhookAuthenticatorStatus{
 						Conditions: conditionstestutil.Replace(
-							allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 0),
+							allHappyConditionsSuccess(goodWebhookDefaultServingCertEndpoint, frozenMetav1Now, 1234),
 							[]metav1.Condition{
-								happyEndpointURLValid(frozenTimeInThePast, 0),
+								happyEndpointURLValid(frozenTimeInThePast, 1234),
 							},
 						),
 						Phase: "Ready",
@@ -1333,7 +1334,7 @@ func TestController(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 			}
-			actualLogLines := stringutil.SplitByNewline(log.String())
+			actualLogLines := testutil.SplitByNewline(log.String())
 			require.Equal(t, len(tt.wantLogs), len(actualLogLines), "log line count should be correct")
 
 			for logLineNum, logLine := range actualLogLines {

internal/controller/conditionsutil/conditions_util.go

@@ -66,7 +66,7 @@ func mergeIDPCondition(existing *[]metav1.Condition, new *metav1.Condition) bool
 	return false
 }
 
-// MergeConfigConditions merges conditions into conditionsToUpdate. If returns true if it merged any error conditions.
+// MergeConfigConditions merges conditions into conditionsToUpdate. It returns true if it merged any error conditions.
 func MergeConfigConditions(conditions []*metav1.Condition, observedGeneration int64, conditionsToUpdate *[]metav1.Condition, log plog.MinLogger, now metav1.Time) bool {
 	hadErrorCondition := false
 	for i := range conditions {

internal/controller/kubecertagent/kubecertagent_test.go

@@ -38,7 +38,6 @@ 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 +1084,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(stringutil.SplitByNewline(buf.String())), "unexpected logs")
+			assert.Equal(t, tt.wantDistinctLogs, deduplicate(testutil.SplitByNewline(buf.String())), "unexpected logs")
 
 			// Assert on all actions that happened to deployments.
 			var actualDeploymentActionVerbs []string

internal/testutil/stringutil.go

@@ -0,0 +1,14 @@
+// Copyright 2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package testutil
+
+import "strings"
+
+func SplitByNewline(lineToSplit string) []string {
+	if len(lineToSplit) == 0 {
+		return nil
+	}
+
+	return strings.Split(strings.TrimSpace(lineToSplit), "\n")
+}

test/integration/concierge_webhookauthenticator_status_test.go

@@ -37,7 +37,8 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) {
 			},
 			initialPhase:    v1alpha1.WebhookAuthenticatorPhaseReady,
 			finalConditions: allSuccessfulWebhookAuthenticatorConditions(),
-		}, {
+		},
+		{
 			name: "valid spec with invalid CA in TLS config will result in a WebhookAuthenticator that is not ready",
 			spec: func() *v1alpha1.WebhookAuthenticatorSpec {
 				caBundleString := "invalid base64-encoded data"
@@ -74,7 +75,8 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) {
 					},
 				},
 			),
-		}, {
+		},
+		{
 			name: "valid spec with valid CA in TLS config but does not match issuer server will result in a WebhookAuthenticator that is not ready",
 			spec: func() *v1alpha1.WebhookAuthenticatorSpec {
 				webhookSpec := testEnv.TestWebhook.DeepCopy()
@@ -105,7 +107,8 @@ func TestConciergeWebhookAuthenticatorStatus_Parallel(t *testing.T) {
 					},
 				},
 			),
-		}, {
+		},
+		{
 			name: "invalid with unresponsive endpoint will result in a WebhookAuthenticator that is not ready",
 			spec: func() *v1alpha1.WebhookAuthenticatorSpec {
 				webhookSpec := testEnv.TestWebhook.DeepCopy()
@@ -223,7 +226,8 @@ func TestConciergeWebhookAuthenticatorCRDValidations_Parallel(t *testing.T) {
 					},
 				},
 			},
-		}, {
+		},
+		{
 			// since the CRD validations do not assess fitness of the value provided
 			name: "valid authenticator can have TLS CertificateAuthorityData string that is an invalid certificate",
 			webhookAuthenticator: &v1alpha1.WebhookAuthenticator{
@@ -260,31 +264,37 @@ func TestConciergeWebhookAuthenticatorCRDValidations_Parallel(t *testing.T) {
 		})
 	}
 }
+
 func allSuccessfulWebhookAuthenticatorConditions() []metav1.Condition {
 	return []metav1.Condition{{
 		Type:    "AuthenticatorValid",
 		Status:  "True",
 		Reason:  "Success",
 		Message: "authenticator initialized",
-	}, {
-		Type:    "EndpointURLValid",
-		Status:  "True",
-		Reason:  "Success",
-		Message: "endpoint is a valid URL",
-	}, {
-		Type:    "Ready",
-		Status:  "True",
-		Reason:  "Success",
-		Message: "the WebhookAuthenticator is ready",
-	}, {
-		Type:    "TLSConfigurationValid",
-		Status:  "True",
-		Reason:  "Success",
-		Message: "successfully parsed specified CA bundle",
-	}, {
-		Type:    "WebhookConnectionValid",
-		Status:  "True",
-		Reason:  "Success",
-		Message: "tls verified",
-	}}
+	},
+		{
+			Type:    "EndpointURLValid",
+			Status:  "True",
+			Reason:  "Success",
+			Message: "endpoint is a valid URL",
+		},
+		{
+			Type:    "Ready",
+			Status:  "True",
+			Reason:  "Success",
+			Message: "the WebhookAuthenticator is ready",
+		},
+		{
+			Type:    "TLSConfigurationValid",
+			Status:  "True",
+			Reason:  "Success",
+			Message: "successfully parsed specified CA bundle",
+		},
+		{
+			Type:    "WebhookConnectionValid",
+			Status:  "True",
+			Reason:  "Success",
+			Message: "tls verified",
+		},
+	}
 }