Fix shard-direct virtual workspace access for embedded virtual workspaces#241
Merged
kcp-ci-bot merged 5 commits intoJun 8, 2026
Merged
Conversation
The advertised virtual workspace endpoint is the shard's external, SNI-routed URL, which the loopback client config cannot reach. Set the shard-client key/cert unconditionally instead of only when the in-process VW server is disabled.
kcp-ci-bot
added
kind/bug
kcp-ci-bot
added
size/M
mjudeikis
changed the title
kcp-ci-bot
added
release-note
ntnn
reviewed
Jun 8, 2026
| // clients authenticate against directly — the front-proxy AND the shards (whose | ||
| // virtual-workspace endpoint URLs are reached shard-direct) — must validate | ||
| // ServiceAccount tokens issued by any shard, so they all use this variant. | ||
| func ApplyAuthConfigurationWithServiceAccount(deployment *appsv1.Deployment, config *operatorv1alpha1.AuthSpec, rootShard *operatorv1alpha1.RootShard) *appsv1.Deployment { |
Member
There was a problem hiding this comment.
Wouldn't it make more sense to just merge the SA portion in ApplyAuthConfiguration?
Contributor
Author
There was a problem hiding this comment.
I think after recent changed, yes :D
mjudeikis
added
the
tide/merge-method-squash
ntnn
reviewed
Jun 8, 2026
Comment on lines
+218
to
+221
| // Use the ServiceAccount-aware variant: external clients reach this | ||
| // shard's virtual-workspace endpoints shard-direct via its | ||
| // VirtualWorkspaceURL, bypassing the front-proxy, so the shard | ||
| // itself must validate ServiceAccount tokens issued by any shard. |
Member
There was a problem hiding this comment.
Suggested change
| // Use the ServiceAccount-aware variant: external clients reach this | |
| // shard's virtual-workspace endpoints shard-direct via its | |
| // VirtualWorkspaceURL, bypassing the front-proxy, so the shard | |
| // itself must validate ServiceAccount tokens issued by any shard. |
Think the comment is superfluous now
Comment on lines
+207
to
+211
| // Use the ServiceAccount-aware variant: external clients (e.g. the | ||
| // kro kcp-apiexport provider) reach this shard's virtual-workspace | ||
| // endpoints shard-direct via its VirtualWorkspaceURL, bypassing the | ||
| // front-proxy, so the shard itself must validate ServiceAccount | ||
| // tokens issued by any shard. |
Member
There was a problem hiding this comment.
Suggested change
| // Use the ServiceAccount-aware variant: external clients (e.g. the | |
| // kro kcp-apiexport provider) reach this shard's virtual-workspace | |
| // endpoints shard-direct via its VirtualWorkspaceURL, bypassing the | |
| // front-proxy, so the shard itself must validate ServiceAccount | |
| // tokens issued by any shard. |
Think the comment is superfluous now
mjudeikis
force-pushed
the
fix/rootshard-shard-client-cert
branch
from
8936785 to
1f6be42
Compare
Contributor
|
LGTM label has been added. DetailsGit tree hash: db8c8fb2886453a94760b136cbdf628ecb55ae8b |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ntnn The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When virtual workspaces run embedded in the shard (the default,
--run-virtual-workspacesunset), external clients reach a shard's virtual-workspace endpoints at that shard's own external, SNI-routedVirtualWorkspaceURL— directly, bypassing the front-proxy. Two things broke on that shard-direct path:1. Shard client certificate is required on the root shard (not only for external VWs).
The shard forwards virtual-resource requests (e.g. CachedResource replication VWs) to its advertised external URL. Without
--shard-client-cert-file/--shard-client-key-fileit falls back to the loopback client config, whose loopback ServerName the ingress cannot SNI-route (the connection is reset) and whose bearer token is not valid externally. The operator already set these flags unconditionally for regularShards, but for theRootShardonly inside the external-VW branch (kcpVW != nil). They are now set unconditionally on the root shard too. TheClientCertificateis already provisioned and mounted, so only the flags were missing.2. ServiceAccount tokens must be validatable cross-shard on every shard, not only the front-proxy.
ServiceAccount signing keys are per-shard. An external client (e.g. the kro
kcp-apiexportprovider) that authenticates with a ServiceAccount token minted on shard A but connects shard-direct to shard B's virtual-workspace endpoint was rejected with401because shard B only had its own signing key.auth.serviceAccount.enabledloads every shard's signing key — but the operator only wired this into the front-proxy (ApplyFrontProxyAuthConfiguration→applyServiceAccountAuthentication); the shard and rootshard deployments usedApplyAuthConfiguration, which ignores theServiceAccountconfig. The shared helper is renamed toApplyAuthConfigurationWithServiceAccountand is now applied to the shard and rootshard deployments as well, so any shard can validate ServiceAccount tokens issued by any shard when ServiceAccount authentication is enabled.Both were verified end-to-end on a 2-shard (root + theseus) embedded-VW setup: cross-shard
kubectl getof a CachedResource-backed virtual resource resolves, and a provider connecting shard-direct with a ServiceAccount token issued on another shard is no longer rejected with 401.What Type of PR Is This?
/kind bug
Related Issue(s)
Fixes #
Release Notes