From 50f9b434e77dab01958023b5ea4fd8699e082d2f Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Tue, 15 Dec 2020 11:00:44 -0500 Subject: [PATCH 1/2] SameIssuerHostMustUseSameSecret is a valid OIDCProvider status I saw this message in our CI logs, which led me to this fix. could not update status: OIDCProvider.config.supervisor.pinniped.dev "acceptance-provider" is invalid: status.status: Unsupported value: "SameIssuerHostMustUseSameSecret": supported values: "Success", "Duplicate", "Invalid" Also - correct an integration test error message that was misleading. Signed-off-by: Andrew Keesler --- apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl | 2 +- .../config.supervisor.pinniped.dev_oidcproviders.yaml | 1 + .../apis/supervisor/config/v1alpha1/types_oidcprovider.go | 2 +- .../crds/config.supervisor.pinniped.dev_oidcproviders.yaml | 1 + .../apis/supervisor/config/v1alpha1/types_oidcprovider.go | 2 +- .../crds/config.supervisor.pinniped.dev_oidcproviders.yaml | 1 + .../apis/supervisor/config/v1alpha1/types_oidcprovider.go | 2 +- .../crds/config.supervisor.pinniped.dev_oidcproviders.yaml | 1 + test/library/client.go | 6 ++++-- 9 files changed, 12 insertions(+), 6 deletions(-) diff --git a/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl b/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl index 908470f0..dee102c9 100644 --- a/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl +++ b/apis/supervisor/config/v1alpha1/types_oidcprovider.go.tmpl @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// +kubebuilder:validation:Enum=Success;Duplicate;Invalid +// +kubebuilder:validation:Enum=Success;Duplicate;Invalid;SameIssuerHostMustUseSameSecret type OIDCProviderStatusCondition string const ( diff --git a/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml b/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b..e25a9f81 100644 --- a/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/deploy/supervisor/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -108,6 +108,7 @@ spec: - Success - Duplicate - Invalid + - SameIssuerHostMustUseSameSecret type: string type: object required: diff --git a/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f0..dee102c9 100644 --- a/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.17/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// +kubebuilder:validation:Enum=Success;Duplicate;Invalid +// +kubebuilder:validation:Enum=Success;Duplicate;Invalid;SameIssuerHostMustUseSameSecret type OIDCProviderStatusCondition string const ( diff --git a/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b..e25a9f81 100644 --- a/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.17/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -108,6 +108,7 @@ spec: - Success - Duplicate - Invalid + - SameIssuerHostMustUseSameSecret type: string type: object required: diff --git a/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f0..dee102c9 100644 --- a/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.18/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// +kubebuilder:validation:Enum=Success;Duplicate;Invalid +// +kubebuilder:validation:Enum=Success;Duplicate;Invalid;SameIssuerHostMustUseSameSecret type OIDCProviderStatusCondition string const ( diff --git a/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b..e25a9f81 100644 --- a/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.18/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -108,6 +108,7 @@ spec: - Success - Duplicate - Invalid + - SameIssuerHostMustUseSameSecret type: string type: object required: diff --git a/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go b/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go index 908470f0..dee102c9 100644 --- a/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go +++ b/generated/1.19/apis/supervisor/config/v1alpha1/types_oidcprovider.go @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// +kubebuilder:validation:Enum=Success;Duplicate;Invalid +// +kubebuilder:validation:Enum=Success;Duplicate;Invalid;SameIssuerHostMustUseSameSecret type OIDCProviderStatusCondition string const ( diff --git a/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml b/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml index 6ff3a42b..e25a9f81 100644 --- a/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml +++ b/generated/1.19/crds/config.supervisor.pinniped.dev_oidcproviders.yaml @@ -108,6 +108,7 @@ spec: - Success - Duplicate - Invalid + - SameIssuerHostMustUseSameSecret type: string type: object required: diff --git a/test/library/client.go b/test/library/client.go index d11869d2..3593b92e 100644 --- a/test/library/client.go +++ b/test/library/client.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -265,12 +266,13 @@ func CreateTestOIDCProvider(ctx context.Context, t *testing.T, issuer string, ce // Wait for the OIDCProvider to enter the expected phase (or time out). var result *configv1alpha1.OIDCProvider - require.Eventuallyf(t, func() bool { + assert.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) + }, 60*time.Second, 1*time.Second, "expected the OIDCProvider to have status %q", expectStatus) + require.Equal(t, expectStatus, result.Status.Status) return opc } From be4e34d0c06cc7c8a27014c76e62e147e723eaf5 Mon Sep 17 00:00:00 2001 From: Andrew Keesler Date: Tue, 15 Dec 2020 11:30:06 -0500 Subject: [PATCH 2/2] Retry a couple of times if we fail to get a token from the Supervisor I hope this will make TestSupervisorLogin less flaky. There are some instances where the front half of the OIDC login flow happens so fast that the JWKS controller doesn't have time to properly generate an asymmetric key. Signed-off-by: Andrew Keesler --- test/integration/supervisor_login_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index ed4ca1eb..6fa7875f 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -165,8 +165,12 @@ func TestSupervisorLogin(t *testing.T) { authcode := callback.URL.Query().Get("code") require.NotEmpty(t, authcode) - // Call the token endpoint to get tokens. - tokenResponse, err := downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier()) + // Call the token endpoint to get tokens. Give the Supervisor a couple of seconds to wire up its signing key. + var tokenResponse *oauth2.Token + assert.Eventually(t, func() bool { + tokenResponse, err = downstreamOAuth2Config.Exchange(oidcHTTPClientContext, authcode, pkceParam.Verifier()) + return err == nil + }, time.Second*5, time.Second*1) require.NoError(t, err) expectedIDTokenClaims := []string{"iss", "exp", "sub", "aud", "auth_time", "iat", "jti", "nonce", "rat"}