Skip to content

Do not validate ServiceAccount tokens at the front-proxy layer#74

Merged
kcp-ci-bot merged 2 commits intokcp-dev:mainfrom
embik:drop-sa-ca-flag
Jul 17, 2025
Merged

Do not validate ServiceAccount tokens at the front-proxy layer#74
kcp-ci-bot merged 2 commits intokcp-dev:mainfrom
embik:drop-sa-ca-flag

Conversation

@embik
Copy link
Member

@embik embik commented Jul 17, 2025

Summary

Looks like setting this flag (--service-account-key-file) on the front-proxy is a mistake because it will validate ServiceAccount tokens only against the PKI infrastructure of the root shard for ServiceAccounts, but not against other shards.

Disabling ServiceAccount validation on the front-proxy will let it pass through the request to the individual shard where it is successfully validated.

I've tested this on the kind setup and it seems to now work (i.e. ServiceAccount tokens generated for one workspace worked against the other), but I'm not 100% confident either. This should unblock using ServiceAccounts in a multi-shard environment, but further investigation might be needed (e.g. what is with cross-workspace ServiceAccounts? Afaik we support that now, but it might not be covered by the changes in this PR).

What Type of PR Is This?

/kind bug
/kind cleanup

Related Issue(s)

Fixes #

Release Notes

Do not set `--service-account-key-file` on front-proxy to allow individual shards to authenticate tokens

On-behalf-of: SAP <marvin.beckers@sap.com>
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@embik embik requested a review from mjudeikis July 17, 2025 13:08
@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 17, 2025
@embik
Copy link
Member Author

embik commented Jul 17, 2025

/hold

Just want to try 1-2 more things before merging this.

@kcp-ci-bot kcp-ci-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 17, 2025
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@embik
Copy link
Member Author

embik commented Jul 17, 2025

/hold cancel

@kcp-ci-bot kcp-ci-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 17, 2025
@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2025
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

DetailsGit tree hash: 3e56c69cc254a2e2903b90b7bb4b99ee49739bee

@embik
Copy link
Member Author

embik commented Jul 17, 2025

/retest

2 similar comments
@mjudeikis
Copy link
Contributor

/retest

@mjudeikis
Copy link
Contributor

/retest

@mjudeikis
Copy link
Contributor

/lgtm
/approve

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2025
@kcp-ci-bot kcp-ci-bot merged commit 8599665 into kcp-dev:main Jul 17, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/bug Categorizes issue or PR as related to a bug. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants