From 273ac62ec2660a78fb30ae2dd15b916d804863fc Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Wed, 2 Dec 2020 15:32:54 -0600 Subject: [PATCH] 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 --- test/integration/supervisor_discovery_test.go | 14 +- test/integration/supervisor_keys_test.go | 2 +- test/integration/supervisor_login_test.go | 4 +- test/integration/supervisor_upstream_test.go | 80 +----------- test/library/client.go | 121 +++++++++++++++--- 5 files changed, 114 insertions(+), 107 deletions(-) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 32c4c004..b029cc17 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/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 diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go index 17e6a580..d59c713e 100644 --- a/test/integration/supervisor_keys_test.go +++ b/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 diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 0ce937fb..c3ff63ec 100644 --- a/test/integration/supervisor_login_test.go +++ b/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) diff --git a/test/integration/supervisor_upstream_test.go b/test/integration/supervisor_upstream_test.go index e38f2b17..dd3fa528 100644 --- a/test/integration/supervisor_upstream_test.go +++ b/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 -} diff --git a/test/library/client.go b/test/library/client.go index f5aba3a6..d95f0426 100644 --- a/test/library/client.go +++ b/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,12 +141,8 @@ func CreateTestWebhookAuthenticator(ctx context.Context, t *testing.T) corev1.Ty defer cancel() webhook, err := webhooks.Create(createContext, &auth1alpha1.WebhookAuthenticator{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-webhook-", - Labels: map[string]string{"pinniped.dev/test": ""}, - Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, - }, - Spec: testEnv.TestWebhook, + ObjectMeta: testObjectMeta(t, "webhook"), + Spec: testEnv.TestWebhook, }, metav1.CreateOptions{}) require.NoError(t, err, "could not create test WebhookAuthenticator") t.Logf("created test WebhookAuthenticator %s/%s", webhook.Namespace, webhook.Name) @@ -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, err = randomIssuer() - require.NoError(t, err) + issuer = randomIssuer(t) } opcs := NewSupervisorClientset(t).ConfigV1alpha1().OIDCProviders(testEnv.SupervisorNamespace) opc, err := opcs.Create(createContext, &configv1alpha1.OIDCProvider{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: "test-oidc-provider-", - Labels: map[string]string{"pinniped.dev/test": ""}, - Annotations: map[string]string{"pinniped.dev/testName": t.Name()}, - }, + ObjectMeta: testObjectMeta(t, "oidc-provider"), 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 + } + + // 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() (string, error) { +func randomIssuer(t *testing.T) string { var buf [8]byte - if _, err := io.ReadFull(rand.Reader, buf[:]); err != nil { - return "", fmt.Errorf("could not generate random state: %w", err) - } - return fmt.Sprintf("http://test-issuer-%s.pinniped.dev", hex.EncodeToString(buf[:])), nil + _, err := io.ReadFull(rand.Reader, buf[:]) + 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()}, + } }