diff --git a/cmd/pinniped/cmd/login_oidc_test.go b/cmd/pinniped/cmd/login_oidc_test.go index 6d28d7b6e..42b84f9fa 100644 --- a/cmd/pinniped/cmd/login_oidc_test.go +++ b/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())) }) } } diff --git a/cmd/pinniped/cmd/login_static_test.go b/cmd/pinniped/cmd/login_static_test.go index bf65a4e39..19511edfc 100644 --- a/cmd/pinniped/cmd/login_static_test.go +++ b/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())) }) } } diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index 8759f8437..3e1868f22 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/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 { diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index cecd3e47e..081603236 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/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 { diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index fa0cd8114..7412db764 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/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 { diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index c8dfd0fa9..2756e743b 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/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 diff --git a/internal/testutil/stringutil.go b/internal/testutil/stringutil.go new file mode 100644 index 000000000..35902de60 --- /dev/null +++ b/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") +} diff --git a/internal/testutil/stringutil/stringutil.go b/internal/testutil/stringutil/stringutil.go deleted file mode 100644 index aa8a266f1..000000000 --- a/internal/testutil/stringutil/stringutil.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2024 the Pinniped contributors. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -package stringutil - -import "strings" - -func SplitByNewline(logs string) []string { - if len(logs) == 0 { - return nil - } - - return strings.Split(strings.TrimSpace(logs), "\n") -} diff --git a/test/integration/concierge_webhookauthenticator_status_test.go b/test/integration/concierge_webhookauthenticator_status_test.go index d3840fcad..1b372c39e 100644 --- a/test/integration/concierge_webhookauthenticator_status_test.go +++ b/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", + }, + } }