From 7856dc6729c9212710a3c8d98d74fd983f921f49 Mon Sep 17 00:00:00 2001 From: "Nelo-T. Wallus" Date: Tue, 12 May 2026 17:17:45 +0200 Subject: [PATCH 1/2] Wait for authenticators to be initialized The authenticators do have a healthcheck, however this healthcheck is not exposed and it is not easy to expose without intrusive changes. An alternative would've been to use the update function returned by kubeAuthConfig.New which _does_ wait until the authenticator's healthcheck are ready, however that would create each authenticator twice and just discard the first iteration. Instead the authenticator is validated by continually authenticating a dummy token created with the first issuer until the error is no longer "not initialized". Signed-off-by: Nelo-T. Wallus Signed-off-by: Nelo-T. Wallus --- pkg/authentication/index.go | 51 ++++++++++++++++++++++- test/e2e/authentication/workspace_test.go | 24 +++++------ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/pkg/authentication/index.go b/pkg/authentication/index.go index 335664cd323..693024676b1 100644 --- a/pkg/authentication/index.go +++ b/pkg/authentication/index.go @@ -18,9 +18,14 @@ package authentication import ( "context" + "encoding/base64" "errors" "fmt" + "net/http" + "strings" + "time" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/klog/v2" kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator" @@ -30,6 +35,14 @@ import ( tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1" ) +// authenticatorSetupTimeout is duration to wait for an authenticator to +// be initialized, means it finished the discovery of its issuers and is +// ready to validate requests. +// +// 10s is the time upstream waits in the updateAuthenticationConfig when +// an authenticator is updated. +const authenticatorSetupTimeout = 10 * time.Second + // AuthenticatorIndex implements a mapping from workspace type to authenticator.Request. type AuthenticatorIndex interface { Lookup(wsType logicalcluster.Path) (authenticator.Request, bool) @@ -76,12 +89,11 @@ func buildAuthenticator( } ctx, cancel := context.WithCancelCause(lifecycleCtx) + logger := klog.FromContext(ctx).WithValues("controller", controllerName) authn, _, _, _, err := kubeAuthConfig.New(ctx) if err != nil { - logger := klog.FromContext(ctx).WithValues("controller", controllerName) logger.Error(err, "Failed to start workspace authenticator.") - cancel(fmt.Errorf("authenticator failed to start: %w", err)) return authenticatorState{}, err } @@ -92,9 +104,44 @@ func buildAuthenticator( return authenticatorState{}, errCauseEmpty } + if len(wac.Spec.JWT) > 0 { + // There shouldn't be an authenticator without at least one JWT + // at the very least because the spec validates this but better + // safe than sorry. + + // Wait for the authenticator to be initialized. + // + // The authenticators do have a healthcheck, however this healthcheck is not exposed + // and it is not easy to expose without intrusive changes. + // + // An alternative would've been to use the update function returned by kubeAuthConfig.New + // which _does_ wait until the authenticator's healthcheck are ready, however that would + // create each authenticator twice and just discard the first iteration. + // + // Instead the authenticator is validated by continually authenticating a dummy token + // created with the first issuer until the error is no longer "not initialized". + dummyToken := dummyTokenForIssuer(wac.Spec.JWT[0].Issuer.URL) + if err := wait.PollUntilContextTimeout(ctx, time.Second, authenticatorSetupTimeout, true, func(ctx context.Context) (bool, error) { + dummyReq, _ := http.NewRequestWithContext(ctx, http.MethodGet, "/", http.NoBody) + dummyReq.Header.Set("Authorization", "Bearer "+dummyToken) + _, _, err := authn.AuthenticateRequest(dummyReq) + return err == nil || !strings.Contains(err.Error(), "not initialized"), nil + }); err != nil { + logger.Error(err, "Failed to validate workspace authenticator.") + cancel(fmt.Errorf("authenticator failed to validate within %q: %w", authenticatorSetupTimeout, err)) + return authenticatorState{}, err + } + } + return authenticatorState{cancel: cancel, authenticator: authn}, nil } +func dummyTokenForIssuer(issuer string) string { + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none"}`)) + payload := base64.RawURLEncoding.EncodeToString([]byte(fmt.Sprintf(`{"iss":%q}`, issuer))) + return header + "." + payload + "." +} + // wrapWithSecurityFilters wraps an authenticator with security filters // that prevent workspace-local auth from escalating to system-level access. func wrapWithSecurityFilters(authn authenticator.Request) authenticator.Request { diff --git a/test/e2e/authentication/workspace_test.go b/test/e2e/authentication/workspace_test.go index e77475e3183..350189d6168 100644 --- a/test/e2e/authentication/workspace_test.go +++ b/test/e2e/authentication/workspace_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/xrstf/mockoidc" @@ -35,7 +36,6 @@ import ( tenancyv1alpha1 "github.com/kcp-dev/sdk/apis/tenancy/v1alpha1" kcpclientset "github.com/kcp-dev/sdk/client/clientset/versioned/cluster" kcptesting "github.com/kcp-dev/sdk/testing" - kcptestinghelpers "github.com/kcp-dev/sdk/testing/helpers" "github.com/kcp-dev/kcp/pkg/authorization/bootstrap" "github.com/kcp-dev/kcp/test/e2e/fixtures/authfixtures" @@ -616,24 +616,20 @@ func TestWorkspaceOIDCTokenReview(t *testing.T) { }, } - var response *authenticationv1.TokenReview - kcptestinghelpers.Eventually(t, func() (bool, string) { - var err error - response, err = kubeClusterClient.Cluster(teamPath).AuthenticationV1().TokenReviews().Create(t.Context(), review, metav1.CreateOptions{}) - if err != nil { - return false, err.Error() - } - return response.Status.Authenticated, response.Status.Error - }, wait.ForeverTestTimeout, 500*time.Millisecond) - - require.Contains(t, response.Status.Audiences, kcpDefaultAudience) + // This check cannot happen in an .Eventually - when a valid request + // comes in it must succeed on the first try. + response, err := kubeClusterClient.Cluster(teamPath).AuthenticationV1().TokenReviews().Create(t.Context(), review, metav1.CreateOptions{}) + require.NoError(t, err) + // Ensure the token is valid and has the right audience + assert.True(t, response.Status.Authenticated, "token was not authenticated: %s", response.Status.Error) + assert.Contains(t, response.Status.Audiences, kcpDefaultAudience) // Ensure the correct clusters are mentioned in the TokenReview. - require.Equal(t, + assert.Equal(t, authenticationv1.ExtraValue{teamWs.Spec.Cluster}, response.Status.User.Extra["authentication.kcp.io/cluster-name"], ) - require.Contains(t, + assert.Contains(t, response.Status.User.Extra["authentication.kcp.io/scopes"], fmt.Sprintf("cluster:%s", teamWs.Spec.Cluster), ) From ec8c5d9b0affa784d623a0e6f9322807f5841952 Mon Sep 17 00:00:00 2001 From: Mangirdas Judeikis Date: Wed, 13 May 2026 14:02:18 +0300 Subject: [PATCH 2/2] auth init v2 --- pkg/authentication/index.go | 63 ++++++++++++++++++-------------- pkg/authentication/index_lazy.go | 5 +++ 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/pkg/authentication/index.go b/pkg/authentication/index.go index 693024676b1..b4fa125ce33 100644 --- a/pkg/authentication/index.go +++ b/pkg/authentication/index.go @@ -78,6 +78,13 @@ func getAuthConfigKey(authConfig *tenancyv1alpha1.WorkspaceAuthenticationConfigu } // buildAuthenticator builds a JWT authenticator from the provided WAC. +// +// The returned authenticator is not necessarily initialized yet; the upstream +// JWT authenticator performs OIDC discovery asynchronously. Callers on the +// request hot path (lazy index) should waitForAuthenticatorInit before serving +// requests. Callers driven by informer events (eager index) intentionally do +// not wait, so that a slow discovery does not block the informer event handler +// or silently drop the authenticator on timeout. func buildAuthenticator( lifecycleCtx context.Context, baseAudiences authenticator.Audiences, @@ -104,36 +111,36 @@ func buildAuthenticator( return authenticatorState{}, errCauseEmpty } - if len(wac.Spec.JWT) > 0 { - // There shouldn't be an authenticator without at least one JWT - // at the very least because the spec validates this but better - // safe than sorry. - - // Wait for the authenticator to be initialized. - // - // The authenticators do have a healthcheck, however this healthcheck is not exposed - // and it is not easy to expose without intrusive changes. - // - // An alternative would've been to use the update function returned by kubeAuthConfig.New - // which _does_ wait until the authenticator's healthcheck are ready, however that would - // create each authenticator twice and just discard the first iteration. - // - // Instead the authenticator is validated by continually authenticating a dummy token - // created with the first issuer until the error is no longer "not initialized". - dummyToken := dummyTokenForIssuer(wac.Spec.JWT[0].Issuer.URL) - if err := wait.PollUntilContextTimeout(ctx, time.Second, authenticatorSetupTimeout, true, func(ctx context.Context) (bool, error) { - dummyReq, _ := http.NewRequestWithContext(ctx, http.MethodGet, "/", http.NoBody) - dummyReq.Header.Set("Authorization", "Bearer "+dummyToken) - _, _, err := authn.AuthenticateRequest(dummyReq) - return err == nil || !strings.Contains(err.Error(), "not initialized"), nil - }); err != nil { - logger.Error(err, "Failed to validate workspace authenticator.") - cancel(fmt.Errorf("authenticator failed to validate within %q: %w", authenticatorSetupTimeout, err)) - return authenticatorState{}, err - } + return authenticatorState{cancel: cancel, authenticator: authn}, nil +} + +// waitForAuthenticatorInit blocks until the authenticator's underlying JWT +// verifier has been initialized (i.e. OIDC discovery has completed), or until +// authenticatorSetupTimeout elapses. +// +// The authenticators do have a healthcheck, however this healthcheck is not +// exposed and it is not easy to expose without intrusive changes. +// +// An alternative would've been to use the update function returned by +// kubeAuthConfig.New which _does_ wait until the authenticator's healthcheck +// are ready, however that would create each authenticator twice and just +// discard the first iteration. +// +// Instead the authenticator is validated by continually authenticating a +// dummy token created with the first issuer until the error is no longer +// "not initialized". +func waitForAuthenticatorInit(ctx context.Context, authn authenticator.Request, wac *tenancyv1alpha1.WorkspaceAuthenticationConfiguration) error { + if len(wac.Spec.JWT) == 0 { + return nil } - return authenticatorState{cancel: cancel, authenticator: authn}, nil + dummyToken := dummyTokenForIssuer(wac.Spec.JWT[0].Issuer.URL) + return wait.PollUntilContextTimeout(ctx, time.Second, authenticatorSetupTimeout, true, func(ctx context.Context) (bool, error) { + dummyReq, _ := http.NewRequestWithContext(ctx, http.MethodGet, "/", http.NoBody) + dummyReq.Header.Set("Authorization", "Bearer "+dummyToken) + _, _, err := authn.AuthenticateRequest(dummyReq) + return err == nil || !strings.Contains(err.Error(), "not initialized"), nil + }) } func dummyTokenForIssuer(issuer string) string { diff --git a/pkg/authentication/index_lazy.go b/pkg/authentication/index_lazy.go index eae1d7f1c57..10fe6645e46 100644 --- a/pkg/authentication/index_lazy.go +++ b/pkg/authentication/index_lazy.go @@ -181,6 +181,11 @@ func (idx *lazyIndex) buildUnionAuthenticator(clusterPath logicalcluster.Path, w parentCancel(err) return authenticatorState{}, err } + if err := waitForAuthenticatorInit(parentCtx, state.authenticator, wac); err != nil { + err = fmt.Errorf("authenticator for WorkspaceAuthenticationConfiguration %q from %q failed to validate within %q: %w", ref.Name, clusterName, authenticatorSetupTimeout, err) + parentCancel(err) + return authenticatorState{}, err + } authenticators = append(authenticators, state.authenticator) }