Extend the test client helpers in ./test/library/client.go.

This adds a few new "create test object" helpers and extends `CreateTestOIDCProvider()` to optionally wait for the created OIDCProvider to enter some expected status condition.

Signed-off-by: Matt Moyer <moyerm@vmware.com>

test/integration/supervisor_discovery_test.go

@@ -111,7 +111,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 
 		// When the same issuer is added twice, both issuers are marked as duplicates, and neither provider is serving.
 		config6Duplicate1, _ := requireCreatingOIDCProviderCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer6, client)
-		config6Duplicate2 := library.CreateTestOIDCProvider(ctx, t, issuer6, "")
+		config6Duplicate2 := library.CreateTestOIDCProvider(ctx, t, issuer6, "", "")
 		requireStatus(t, client, ns, config6Duplicate1.Name, v1alpha1.DuplicateOIDCProviderStatusCondition)
 		requireStatus(t, client, ns, config6Duplicate2.Name, v1alpha1.DuplicateOIDCProviderStatusCondition)
 		requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer6)
@@ -136,7 +136,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 		}
 
 		// When we create a provider with an invalid issuer, the status is set to invalid.
-		badConfig := library.CreateTestOIDCProvider(ctx, t, badIssuer, "")
+		badConfig := library.CreateTestOIDCProvider(ctx, t, badIssuer, "", "")
 		requireStatus(t, client, ns, badConfig.Name, v1alpha1.InvalidOIDCProviderStatusCondition)
 		requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, badIssuer)
 		requireDeletingOIDCProviderCausesDiscoveryEndpointsToDisappear(t, badConfig, client, ns, scheme, addr, caBundle, badIssuer)
@@ -162,7 +162,7 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) {
 	certSecretName1 := "integration-test-cert-1"
 
 	// Create an OIDCProvider with a spec.tls.secretName.
-	oidcProvider1 := library.CreateTestOIDCProvider(ctx, t, issuer1, certSecretName1)
+	oidcProvider1 := library.CreateTestOIDCProvider(ctx, t, issuer1, certSecretName1, "")
 	requireStatus(t, pinnipedClient, oidcProvider1.Namespace, oidcProvider1.Name, v1alpha1.SuccessOIDCProviderStatusCondition)
 
 	// The spec.tls.secretName Secret does not exist, so the endpoints should fail with TLS errors.
@@ -198,7 +198,7 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) {
 	certSecretName2 := "integration-test-cert-2"
 
 	// Create an OIDCProvider with a spec.tls.secretName.
-	oidcProvider2 := library.CreateTestOIDCProvider(ctx, t, issuer2, certSecretName2)
+	oidcProvider2 := library.CreateTestOIDCProvider(ctx, t, issuer2, certSecretName2, "")
 	requireStatus(t, pinnipedClient, oidcProvider2.Namespace, oidcProvider2.Name, v1alpha1.SuccessOIDCProviderStatusCondition)
 
 	// Create the Secret.
@@ -241,7 +241,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts(t *testing.T) {
 	issuerUsingHostname := fmt.Sprintf("%s://%s/issuer1", scheme, address)
 
 	// Create an OIDCProvider without a spec.tls.secretName.
-	oidcProvider1 := library.CreateTestOIDCProvider(ctx, t, issuerUsingIPAddress, "")
+	oidcProvider1 := library.CreateTestOIDCProvider(ctx, t, issuerUsingIPAddress, "", "")
 	requireStatus(t, pinnipedClient, oidcProvider1.Namespace, oidcProvider1.Name, v1alpha1.SuccessOIDCProviderStatusCondition)
 
 	// There is no default TLS cert and the spec.tls.secretName was not set, so the endpoints should fail with TLS errors.
@@ -255,7 +255,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts(t *testing.T) {
 
 	// Create an OIDCProvider with a spec.tls.secretName.
 	certSecretName := "integration-test-cert-1"
-	oidcProvider2 := library.CreateTestOIDCProvider(ctx, t, issuerUsingHostname, certSecretName)
+	oidcProvider2 := library.CreateTestOIDCProvider(ctx, t, issuerUsingHostname, certSecretName, "")
 	requireStatus(t, pinnipedClient, oidcProvider2.Namespace, oidcProvider2.Name, v1alpha1.SuccessOIDCProviderStatusCondition)
 
 	// Create the Secret.
@@ -428,7 +428,7 @@ func requireCreatingOIDCProviderCausesDiscoveryEndpointsToAppear(
 	client pinnipedclientset.Interface,
 ) (*v1alpha1.OIDCProvider, *ExpectedJWKSResponseFormat) {
 	t.Helper()
-	newOIDCProvider := library.CreateTestOIDCProvider(ctx, t, issuerName, "")
+	newOIDCProvider := library.CreateTestOIDCProvider(ctx, t, issuerName, "", "")
 	jwksResult := requireDiscoveryEndpointsAreWorking(t, supervisorScheme, supervisorAddress, supervisorCABundle, issuerName, nil)
 	requireStatus(t, client, newOIDCProvider.Namespace, newOIDCProvider.Name, v1alpha1.SuccessOIDCProviderStatusCondition)
 	return newOIDCProvider, jwksResult

test/integration/supervisor_keys_test.go

@@ -27,7 +27,7 @@ func TestSupervisorOIDCKeys(t *testing.T) {
 	defer cancel()
 
 	// Create our OPC under test.
-	opc := library.CreateTestOIDCProvider(ctx, t, "", "")
+	opc := library.CreateTestOIDCProvider(ctx, t, "", "", "")
 
 	// Ensure a secret is created with the OPC's JWKS.
 	var updatedOPC *configv1alpha1.OIDCProvider

test/integration/supervisor_login_test.go

@@ -92,10 +92,10 @@ func TestSupervisorLogin(t *testing.T) {
 				CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte(env.SupervisorTestUpstream.CABundle)),
 			},
 			Client: idpv1alpha1.OIDCClient{
-				SecretName: makeTestClientCredsSecret(t, env.SupervisorTestUpstream.ClientID, env.SupervisorTestUpstream.ClientSecret).Name,
+				SecretName: library.CreateClientCredsSecret(t, env.SupervisorTestUpstream.ClientID, env.SupervisorTestUpstream.ClientSecret).Name,
 			},
 		}
-		upstream := makeTestUpstream(t, spec, idpv1alpha1.PhaseReady)
+		upstream := library.CreateTestUpstreamOIDCProvider(t, spec, idpv1alpha1.PhaseReady)
 
 		// Make request to authorize endpoint - should pass, since we now have an upstream.
 		req, err = http.NewRequestWithContext(ctx, http.MethodGet, downstreamAuthURL, nil)

test/integration/supervisor_upstream_test.go

@@ -4,13 +4,10 @@
 package integration
 
 import (
-	"context"
 	"encoding/base64"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"go.pinniped.dev/generated/1.19/apis/supervisor/idp/v1alpha1"
@@ -28,7 +25,7 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) {
 				SecretName: "does-not-exist",
 			},
 		}
-		upstream := makeTestUpstream(t, spec, v1alpha1.PhaseError)
+		upstream := library.CreateTestUpstreamOIDCProvider(t, spec, v1alpha1.PhaseError)
 		expectUpstreamConditions(t, upstream, []v1alpha1.Condition{
 			{
 				Type:    "ClientCredentialsValid",
@@ -56,10 +53,10 @@ func TestSupervisorUpstreamOIDCDiscovery(t *testing.T) {
 				AdditionalScopes: []string{"email", "profile"},
 			},
 			Client: v1alpha1.OIDCClient{
-				SecretName: makeTestClientCredsSecret(t, "test-client-id", "test-client-secret").Name,
+				SecretName: library.CreateClientCredsSecret(t, "test-client-id", "test-client-secret").Name,
 			},
 		}
-		upstream := makeTestUpstream(t, spec, v1alpha1.PhaseReady)
+		upstream := library.CreateTestUpstreamOIDCProvider(t, spec, v1alpha1.PhaseReady)
 		expectUpstreamConditions(t, upstream, []v1alpha1.Condition{
 			{
 				Type:    "ClientCredentialsValid",
@@ -87,74 +84,3 @@ func expectUpstreamConditions(t *testing.T, upstream *v1alpha1.UpstreamOIDCProvi
 	}
 	require.ElementsMatch(t, expected, normalized)
 }
-
-func makeTestClientCredsSecret(t *testing.T, clientID string, clientSecret string) *corev1.Secret {
-	t.Helper()
-	env := library.IntegrationEnv(t)
-	client := library.NewClientset(t)
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-
-	created, err := client.CoreV1().Secrets(env.SupervisorNamespace).Create(ctx, &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace:    env.SupervisorNamespace,
-			GenerateName: "test-client-creds-",
-			Labels:       map[string]string{"pinniped.dev/test": ""},
-			Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
-		},
-		Type: "secrets.pinniped.dev/oidc-client",
-		StringData: map[string]string{
-			"clientID":     clientID,
-			"clientSecret": clientSecret,
-		},
-	}, metav1.CreateOptions{})
-	require.NoError(t, err)
-	t.Cleanup(func() {
-		err := client.CoreV1().Secrets(env.SupervisorNamespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{})
-		require.NoError(t, err)
-	})
-	t.Logf("created test client credentials Secret %s", created.Name)
-	return created
-}
-
-func makeTestUpstream(t *testing.T, spec v1alpha1.UpstreamOIDCProviderSpec, expectedPhase v1alpha1.UpstreamOIDCProviderPhase) *v1alpha1.UpstreamOIDCProvider {
-	t.Helper()
-	env := library.IntegrationEnv(t)
-	client := library.NewSupervisorClientset(t)
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
-	defer cancel()
-
-	// Create the UpstreamOIDCProvider using GenerateName to get a random name.
-	created, err := client.IDPV1alpha1().
-		UpstreamOIDCProviders(env.SupervisorNamespace).
-		Create(ctx, &v1alpha1.UpstreamOIDCProvider{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace:    env.SupervisorNamespace,
-				GenerateName: "test-upstream-",
-				Labels:       map[string]string{"pinniped.dev/test": ""},
-				Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
-			},
-			Spec: spec,
-		}, metav1.CreateOptions{})
-	require.NoError(t, err)
-
-	// Always clean this up after this point.
-	t.Cleanup(func() {
-		err := client.IDPV1alpha1().
-			UpstreamOIDCProviders(env.SupervisorNamespace).
-			Delete(context.Background(), created.Name, metav1.DeleteOptions{})
-		require.NoError(t, err)
-	})
-	t.Logf("created test UpstreamOIDCProvider %s", created.Name)
-
-	// Wait for the UpstreamOIDCProvider to enter the expected phase (or time out).
-	var result *v1alpha1.UpstreamOIDCProvider
-	require.Eventuallyf(t, func() bool {
-		var err error
-		result, err = client.IDPV1alpha1().
-			UpstreamOIDCProviders(created.Namespace).Get(ctx, created.Name, metav1.GetOptions{})
-		require.NoError(t, err)
-		return result.Status.Phase == expectedPhase
-	}, 60*time.Second, 1*time.Second, "expected the UpstreamOIDCProvider to go into phase %s", expectedPhase)
-	return result
-}

test/library/client.go

@@ -25,6 +25,7 @@ import (
 
 	auth1alpha1 "go.pinniped.dev/generated/1.19/apis/concierge/authentication/v1alpha1"
 	configv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/config/v1alpha1"
+	idpv1alpha1 "go.pinniped.dev/generated/1.19/apis/supervisor/idp/v1alpha1"
 	conciergeclientset "go.pinniped.dev/generated/1.19/client/concierge/clientset/versioned"
 	supervisorclientset "go.pinniped.dev/generated/1.19/client/supervisor/clientset/versioned"
 
@@ -140,11 +141,7 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
 	defer cancel()
 
 	webhook, err := webhooks.Create(createContext, &auth1alpha1.WebhookAuthenticator{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: testObjectMeta(t, "webhook"),
-			GenerateName: "test-webhook-",
-			Labels:       map[string]string{"pinniped.dev/test": ""},
-			Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
-		},
 		Spec:       testEnv.TestWebhook,
 	}, metav1.CreateOptions{})
 	require.NoError(t, err, "could not create test WebhookAuthenticator")
@@ -172,7 +169,7 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty
 //
 // If the provided issuer is not the empty string, then it will be used for the
 // OIDCProvider.Spec.Issuer field. Else, a random issuer will be generated.
-func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer, certSecretName string) *configv1alpha1.OIDCProvider {
+func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer string, certSecretName string, expectStatus configv1alpha1.OIDCProviderStatusCondition) *configv1alpha1.OIDCProvider {
 	t.Helper()
 	testEnv := IntegrationEnv(t)
 
@@ -180,18 +177,12 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer, certSecre
 	defer cancel()
 
 	if issuer == "" {
-		var err error
+		issuer = randomIssuer(t)
-		issuer, err = randomIssuer()
-		require.NoError(t, err)
 	}
 
 	opcs := NewSupervisorClientset(t).ConfigV1alpha1().OIDCProviders(testEnv.SupervisorNamespace)
 	opc, err := opcs.Create(createContext, &configv1alpha1.OIDCProvider{
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: testObjectMeta(t, "oidc-provider"),
-			GenerateName: "test-oidc-provider-",
-			Labels:       map[string]string{"pinniped.dev/test": ""},
-			Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
-		},
 		Spec: configv1alpha1.OIDCProviderSpec{
 			Issuer: issuer,
 			TLS:    &configv1alpha1.OIDCProviderTLSSpec{SecretName: certSecretName},
@@ -213,13 +204,103 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer, certSecre
 		}
 	})
 
+	// If we're not expecting any particular status, just return the new OIDCProvider immediately.
+	if expectStatus == "" {
 		return opc
 	}
 
-func randomIssuer() (string, error) {
+	// Wait for the OIDCProvider to enter the expected phase (or time out).
+	var result *configv1alpha1.OIDCProvider
+	require.Eventuallyf(t, func() bool {
+		var err error
+		result, err = opcs.Get(ctx, opc.Name, metav1.GetOptions{})
+		require.NoError(t, err)
+		return result.Status.Status == expectStatus
+	}, 60*time.Second, 1*time.Second, "expected the UpstreamOIDCProvider to go into phase %s", expectStatus)
+
+	return opc
+}
+
+func randomIssuer(t *testing.T) string {
 	var buf [8]byte
-	if _, err := io.ReadFull(rand.Reader, buf[:]); err != nil {
+	_, err := io.ReadFull(rand.Reader, buf[:])
-		return "", fmt.Errorf("could not generate random state: %w", err)
+	require.NoError(t, err)
+	return fmt.Sprintf("http://test-issuer-%s.pinniped.dev", hex.EncodeToString(buf[:]))
+}
+
+func CreateTestSecret(t *testing.T, namespace string, baseName string, secretType string, stringData map[string]string) *corev1.Secret {
+	t.Helper()
+	client := NewClientset(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	created, err := client.CoreV1().Secrets(namespace).Create(ctx, &corev1.Secret{
+		ObjectMeta: testObjectMeta(t, baseName),
+		Type:       corev1.SecretType(secretType),
+		StringData: stringData,
+	}, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	t.Cleanup(func() {
+		err := client.CoreV1().Secrets(namespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{})
+		require.NoError(t, err)
+	})
+	t.Logf("created test Secret %s", created.Name)
+	return created
+}
+
+func CreateClientCredsSecret(t *testing.T, clientID string, clientSecret string) *corev1.Secret {
+	t.Helper()
+	env := IntegrationEnv(t)
+	return CreateTestSecret(t,
+		env.SupervisorNamespace,
+		"test-client-creds-",
+		"secrets.pinniped.dev/oidc-client",
+		map[string]string{
+			"clientID":     clientID,
+			"clientSecret": clientSecret,
+		},
+	)
+}
+
+func CreateTestUpstreamOIDCProvider(t *testing.T, spec idpv1alpha1.UpstreamOIDCProviderSpec, expectedPhase idpv1alpha1.UpstreamOIDCProviderPhase) *idpv1alpha1.UpstreamOIDCProvider {
+	t.Helper()
+	env := IntegrationEnv(t)
+	client := NewSupervisorClientset(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
+	defer cancel()
+
+	// Create the UpstreamOIDCProvider using GenerateName to get a random name.
+	upstreams := client.IDPV1alpha1().UpstreamOIDCProviders(env.SupervisorNamespace)
+
+	created, err := upstreams.Create(ctx, &idpv1alpha1.UpstreamOIDCProvider{
+		ObjectMeta: testObjectMeta(t, "upstream"),
+		Spec:       spec,
+	}, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	// Always clean this up after this point.
+	t.Cleanup(func() {
+		err := upstreams.Delete(context.Background(), created.Name, metav1.DeleteOptions{})
+		require.NoError(t, err)
+	})
+	t.Logf("created test UpstreamOIDCProvider %s", created.Name)
+
+	// Wait for the UpstreamOIDCProvider to enter the expected phase (or time out).
+	var result *idpv1alpha1.UpstreamOIDCProvider
+	require.Eventuallyf(t, func() bool {
+		var err error
+		result, err = upstreams.Get(ctx, created.Name, metav1.GetOptions{})
+		require.NoError(t, err)
+		return result.Status.Phase == expectedPhase
+	}, 60*time.Second, 1*time.Second, "expected the UpstreamOIDCProvider to go into phase %s", expectedPhase)
+	return result
+}
+
+func testObjectMeta(t *testing.T, baseName string) metav1.ObjectMeta {
+	return metav1.ObjectMeta{
+		GenerateName: fmt.Sprintf("test-%s-", baseName),
+		Labels:       map[string]string{"pinniped.dev/test": ""},
+		Annotations:  map[string]string{"pinniped.dev/testName": t.Name()},
 	}
-	return fmt.Sprintf("http://test-issuer-%s.pinniped.dev", hex.EncodeToString(buf[:])), nil
 }