From 31225ac7ae61960ff997250c54abcbd72f9ae23e Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Thu, 15 Oct 2020 09:09:49 -0400 Subject: [PATCH] test/integration: reuse CreateTestOIDCProvider helper Signed-off-by: Andrew Keesler --- internal/controller/supervisorconfig/jwks.go | 5 ++++ test/integration/supervisor_discovery_test.go | 25 +++++++++++-------- test/integration/supervisor_keys_test.go | 2 +- test/library/client.go | 19 +++++++++++--- 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/internal/controller/supervisorconfig/jwks.go b/internal/controller/supervisorconfig/jwks.go index 0ba44b72..05f74491 100644 --- a/internal/controller/supervisorconfig/jwks.go +++ b/internal/controller/supervisorconfig/jwks.go @@ -142,6 +142,11 @@ func (c *jwksController) Sync(ctx controllerlib.Context) error { } if !secretNeedsUpdate { // Secret is up to date - we are good to go. + klog.InfoS( + "secret is up to date", + "oidcproviderconfig", + klog.KRef(ctx.Key.Namespace, ctx.Key.Name), + ) return nil } diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 77dcaebf..ce362d63 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -63,14 +63,14 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { badIssuer := fmt.Sprintf("http://%s/badIssuer?cannot-use=queries", env.SupervisorAddress) // When OIDCProviderConfig are created in sequence they each cause a discovery endpoint to appear only for as long as the OIDCProviderConfig exists. - config1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer1, "from-integration-test1") + config1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer1, client) requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config1, client, ns, issuer1) - config2 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer2, "from-integration-test2") + config2 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer2, client) requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config2, client, ns, issuer2) // When multiple OIDCProviderConfigs exist at the same time they each serve a unique discovery endpoint. - config3 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer3, "from-integration-test3") - config4 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer4, "from-integration-test4") + config3 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer3, client) + config4 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer4, client) requireWellKnownEndpointIsWorking(t, issuer3) // discovery for issuer3 is still working after issuer4 started working // When they are deleted they stop serving discovery endpoints. @@ -78,8 +78,8 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config4, client, ns, issuer4) // When the same issuer is added twice, both issuers are marked as duplicates, and neither provider is serving. - config5Duplicate1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer5, "from-integration-test5") - config5Duplicate2 := createOIDCProviderConfig(t, "from-integration-test5-duplicate", client, ns, issuer5) + config5Duplicate1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(ctx, t, issuer5, client) + config5Duplicate2 := library.CreateTestOIDCProvider(ctx, t, issuer5) requireStatus(t, client, ns, config5Duplicate1.Name, v1alpha1.DuplicateOIDCProviderStatus) requireStatus(t, client, ns, config5Duplicate2.Name, v1alpha1.DuplicateOIDCProviderStatus) requireDiscoveryEndpointIsNotFound(t, issuer5) @@ -93,7 +93,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, config5Duplicate2, client, ns, issuer5) // When we create a provider with an invalid issuer, the status is set to invalid. - badConfig := createOIDCProviderConfig(t, "from-integration-test6", client, ns, badIssuer) + badConfig := library.CreateTestOIDCProvider(ctx, t, badIssuer) requireStatus(t, client, ns, badConfig.Name, v1alpha1.InvalidOIDCProviderStatus) requireDiscoveryEndpointIsNotFound(t, badIssuer) } @@ -122,11 +122,16 @@ func requireDiscoveryEndpointIsNotFound(t *testing.T, issuerName string) { require.NoError(t, err) } -func requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t *testing.T, client pinnipedclientset.Interface, ns string, issuerName string, oidcProviderConfigName string) *v1alpha1.OIDCProviderConfig { +func requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear( + ctx context.Context, + t *testing.T, + issuerName string, + client pinnipedclientset.Interface, +) *v1alpha1.OIDCProviderConfig { t.Helper() - newOIDCProviderConfig := createOIDCProviderConfig(t, oidcProviderConfigName, client, ns, issuerName) + newOIDCProviderConfig := library.CreateTestOIDCProvider(ctx, t, issuerName) requireWellKnownEndpointIsWorking(t, issuerName) - requireStatus(t, client, ns, oidcProviderConfigName, v1alpha1.SuccessOIDCProviderStatus) + requireStatus(t, client, newOIDCProviderConfig.Namespace, newOIDCProviderConfig.Name, v1alpha1.SuccessOIDCProviderStatus) return newOIDCProviderConfig } diff --git a/test/integration/supervisor_keys_test.go b/test/integration/supervisor_keys_test.go index 09c720ec..feb7f50c 100644 --- a/test/integration/supervisor_keys_test.go +++ b/test/integration/supervisor_keys_test.go @@ -28,7 +28,7 @@ func TestSupervisorOIDCKeys(t *testing.T) { // Create our OPC under test. // TODO: maybe use this in other supervisor test? - opc := library.CreateTestOIDCProvider(ctx, t) + opc := library.CreateTestOIDCProvider(ctx, t, "") // Ensure a secret is created with the OPC's JWKS. var updatedOPC *configv1alpha1.OIDCProviderConfig diff --git a/test/library/client.go b/test/library/client.go index 7460526a..f12219da 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -161,15 +162,21 @@ func CreateTestWebhookIDP(ctx context.Context, t *testing.T) corev1.TypedLocalOb // CreateTestOIDCProvider creates and returns a test OIDCProviderConfig in // $PINNIPED_TEST_SUPERVISOR_NAMESPACE, which will be automatically deleted at the end of the // current test's lifetime. It generates a random, valid, issuer for the OIDCProviderConfig. -func CreateTestOIDCProvider(ctx context.Context, t *testing.T) *configv1alpha1.OIDCProviderConfig { +// +// If the provided issuer is not the empty string, then it will be used for the +// OIDCProviderConfig.Spec.Issuer field. Else, a random issuer will be generated. +func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer string) *configv1alpha1.OIDCProviderConfig { t.Helper() testEnv := IntegrationEnv(t) createContext, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - issuer, err := randomIssuer() - require.NoError(t, err) + if issuer == "" { + var err error + issuer, err = randomIssuer() + require.NoError(t, err) + } opcs := NewPinnipedClientset(t).ConfigV1alpha1().OIDCProviderConfigs(testEnv.SupervisorNamespace) opc, err := opcs.Create(createContext, &configv1alpha1.OIDCProviderConfig{ @@ -191,7 +198,11 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T) *configv1alpha1.O deleteCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() err := opcs.Delete(deleteCtx, opc.Name, metav1.DeleteOptions{}) - require.NoErrorf(t, err, "could not cleanup test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + notFound := k8serrors.IsNotFound(err) + // It's okay if it is not found, because it might have been deleted by another part of this test. + if !notFound { + require.NoErrorf(t, err, "could not cleanup test OIDCProviderConfig %s/%s", opc.Namespace, opc.Name) + } }) return opc