Add guide for enabling Shoot trust in Garden clusters with OIDC#943
Conversation
✅ Deploy Preview for gardener-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
gardener-prow
Bot
added
the
size/L
📝 WalkthroughWalkthroughA new operator-facing documentation guide was added describing how to establish OIDC-based trust between a Gardener “shoot” and a “garden” cluster, detailing required shoot annotations, the authentication flow, OpenID Connect discovery/JWKS, configurator behavior, verification steps, and an RBAC example. (48 words) Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/documentation/guides/administer-shoots/garden-shoot-trust.md`:
- Line 201: Typo in the markdown header "# Shoot cluser" should be corrected to
"# Shoot cluster": update the header text in the document (look for the string
"Shoot cluser") and change it to "Shoot cluster" so the heading is spelled
correctly.
- Line 131: Replace the misspelled word "essense" in the sentence "In essense,
this will create the following OIDC resource in the Garden cluster:" with the
correct spelling "essence" (edit the markdown text where that sentence appears).
- Around line 192-194: The fenced code block containing the claim example
"ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:<sub-claim>" should
include a language specifier (use "text") so update the block fence from ``` to
```text to enable proper syntax highlighting and satisfy the markdown lint rule.
- Line 29: Update the incomplete sentence "Additionally, RBAC rules in the
Garden cluster that grant permissions to the shoot's service account identity
which will perform requests to the Garden cluster." to a complete sentence by
adding the modal verb "must" (e.g., "Additionally, RBAC rules in the Garden
cluster must grant permissions to the shoot's service account identity which
will perform requests to the Garden cluster.") so the phrase referencing RBAC
rules and the shoot's service account identity is grammatically correct and
clear.
- Around line 138-152: The placeholders in the YAML are inconsistent; replace
all occurrences of <shoot-id> and <project-ns> with the standardized
placeholders <shoot-uid> and <project-namespace> respectively so they match
earlier usage; specifically update the name field and the groupsPrefix and
usernamePrefix values, and ensure issuerURL still uses <shoot-uid> and any other
occurrences in the block (e.g., groupsClaim, jwks, supportedSigningAlgs
sections) reflect the unified <project-namespace> and <shoot-uid> naming.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca01fc62-02a6-465a-9c2a-49b8e758bdcb
⛔ Files ignored due to path filters (1)
website/documentation/guides/administer-shoots/images/garden-shoot-trust.pngis excluded by!**/*.png
📒 Files selected for processing (1)
website/documentation/guides/administer-shoots/garden-shoot-trust.md
| authentication.gardener.cloud/trusted: 'true' | ||
| ``` | ||
|
|
||
| Additionally, RBAC rules in the Garden cluster that grant permissions to the shoot's service account identity which will perform requests to the Garden cluster. |
There was a problem hiding this comment.
Fix incomplete sentence.
The sentence is grammatically incomplete. It should read: "Additionally, RBAC rules in the Garden cluster must grant permissions to the shoot's service account identity which will perform requests to the Garden cluster."
📝 Proposed fix
-Additionally, RBAC rules in the Garden cluster that grant permissions to the shoot's service account identity which will perform requests to the Garden cluster.
+Additionally, RBAC rules in the Garden cluster must grant permissions to the shoot's service account identity which will perform requests to the Garden cluster.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Additionally, RBAC rules in the Garden cluster that grant permissions to the shoot's service account identity which will perform requests to the Garden cluster. | |
| Additionally, RBAC rules in the Garden cluster must grant permissions to the shoot's service account identity which will perform requests to the Garden cluster. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/documentation/guides/administer-shoots/garden-shoot-trust.md` at line
29, Update the incomplete sentence "Additionally, RBAC rules in the Garden
cluster that grant permissions to the shoot's service account identity which
will perform requests to the Garden cluster." to a complete sentence by adding
the modal verb "must" (e.g., "Additionally, RBAC rules in the Garden cluster
must grant permissions to the shoot's service account identity which will
perform requests to the Garden cluster.") so the phrase referencing RBAC rules
and the shoot's service account identity is grammatically correct and clear.
| name: <project-ns>--<shoot-name>--<shoot-id> | ||
| spec: | ||
| audiences: | ||
| - garden | ||
| groupsClaim: groups | ||
| groupsPrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:' | ||
| issuerURL: "https://discovery.<service-account-issuer-hostname>/projects/<project-name>/shoots/<shoot-uid>/issuer" | ||
| jwks: | ||
| distributedClaims: true | ||
| maxTokenExpirationSeconds: 7200 | ||
| supportedSigningAlgs: | ||
| - RS256 | ||
| usernameClaim: sub | ||
| usernamePrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:' | ||
| ``` |
There was a problem hiding this comment.
Inconsistent placeholder naming.
The placeholders use both <shoot-id> and <shoot-uid> interchangeably, as well as <project-ns> and <project-namespace>. This inconsistency may confuse readers.
Consider standardizing to either <shoot-uid> and <project-namespace> throughout the document (since these match the earlier usage in line 70 where the status shows <shoot-uid>).
🔧 Proposed fix to standardize placeholders
- name: <project-ns>--<shoot-name>--<shoot-id>
+ name: <project-namespace>--<shoot-name>--<shoot-uid>
spec:
audiences:
- garden
groupsClaim: groups
- groupsPrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:'
+ groupsPrefix: 'ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:'
issuerURL: "https://discovery.<service-account-issuer-hostname>/projects/<project-name>/shoots/<shoot-uid>/issuer"
jwks:
distributedClaims: true
maxTokenExpirationSeconds: 7200
supportedSigningAlgs:
- RS256
usernameClaim: sub
- usernamePrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:'
+ usernamePrefix: 'ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: <project-ns>--<shoot-name>--<shoot-id> | |
| spec: | |
| audiences: | |
| - garden | |
| groupsClaim: groups | |
| groupsPrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:' | |
| issuerURL: "https://discovery.<service-account-issuer-hostname>/projects/<project-name>/shoots/<shoot-uid>/issuer" | |
| jwks: | |
| distributedClaims: true | |
| maxTokenExpirationSeconds: 7200 | |
| supportedSigningAlgs: | |
| - RS256 | |
| usernameClaim: sub | |
| usernamePrefix: 'ns:<project-ns>:shoot:<shoot-name>:<shoot-id>:' | |
| ``` | |
| name: <project-namespace>--<shoot-name>--<shoot-uid> | |
| spec: | |
| audiences: | |
| - garden | |
| groupsClaim: groups | |
| groupsPrefix: 'ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:' | |
| issuerURL: "https://discovery.<service-account-issuer-hostname>/projects/<project-name>/shoots/<shoot-uid>/issuer" | |
| jwks: | |
| distributedClaims: true | |
| maxTokenExpirationSeconds: 7200 | |
| supportedSigningAlgs: | |
| - RS256 | |
| usernameClaim: sub | |
| usernamePrefix: 'ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@website/documentation/guides/administer-shoots/garden-shoot-trust.md` around
lines 138 - 152, The placeholders in the YAML are inconsistent; replace all
occurrences of <shoot-id> and <project-ns> with the standardized placeholders
<shoot-uid> and <project-namespace> respectively so they match earlier usage;
specifically update the name field and the groupsPrefix and usernamePrefix
values, and ensure issuerURL still uses <shoot-uid> and any other occurrences in
the block (e.g., groupsClaim, jwks, supportedSigningAlgs sections) reflect the
unified <project-namespace> and <shoot-uid> naming.
theoddora
changed the title
n-boshnakov
left a comment
n-boshnakov
left a comment
There was a problem hiding this comment.
Great documentation, just some minor notes!
|
|
||
| ## Overview | ||
|
|
||
| This guide explains how to enable trust between the Garden cluster and a shoot cluster so that workloads running in the shoot can authenticate directly with the Garden API server using Kubernetes service account tokens (JWTs), removing the need for static credentials. Authorization decisions are then handled by RBAC in the Garden cluster. |
There was a problem hiding this comment.
When using Gardener specific terminology, it's best to stick to the Gardener glossary. As an example:
Garden API server -> should be Gardener API server
Garden cluster -> should be garden cluster
Could you please update the terms used throughout the topic?
|
|
||
| ### Step 2: Annotate Shoot as Trusted | ||
|
|
||
| To have the Garden cluster trust this shoot's issuer, add the trusted annotation: |
There was a problem hiding this comment.
| To have the Garden cluster trust this shoot's issuer, add the trusted annotation: | |
| To have the Garden cluster trust this shoot's issuer, add the `trusted` annotation: |
| Additionally, RBAC rules in the Garden cluster should grant permissions to the shoot's service account identity which will perform requests to the Garden cluster. | ||
|
|
||
| ### Authentication | ||
| The authentication flow, when a request is made is: |
There was a problem hiding this comment.
| The authentication flow, when a request is made is: | |
| When a request is made, the authentication flow is: |
n-boshnakov
left a comment
n-boshnakov
left a comment
There was a problem hiding this comment.
Thank you for the changes!
Are you waiting for another reviewer or can the PR be merged?
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: n-boshnakov 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 |
gardener-prow
Bot
added
lgtm
|
LGTM label has been added. DetailsGit tree hash: f3d1d255d1e488732bb3c6ba362aeb0644d2bfaa |
|
Thank you for the review and suggestions @n-boshnakov ! If @dimityrmirchev or @vpnachev can also take a look? Please let me know if you find anything incorrect, I can address it in a follow-up PR. |
|
@n-boshnakov the approve function in github acts as an /approve label for prow. That combined with the /lgtm label will directly merge the PR. If you do not to directly merge it, you can use a "comment" review together with the /lgtm label (see gardener/garden-shoot-trust-configurator#111 (review) as an example). I will take a look at this PR, however I am occupied with other work and will take some time until I have the capacity to review. |
Thank you, I'll use the comment option from now on. |
vpnachev
left a comment
vpnachev
left a comment
There was a problem hiding this comment.
Overall looks very nice, I have just some minor comments.
|
|
||
| ## Overview | ||
|
|
||
| This guide explains how to enable trust between the garden cluster and a shoot cluster so that workloads running in the shoot can authenticate directly with the Gardener API server using Kubernetes service account tokens (JWTs), removing the need for static credentials. Authorization decisions are then handled by RBAC in the garden cluster. |
There was a problem hiding this comment.
Authorization decisions are then handled by RBAC in the garden cluster.
Actually, it is not necessary the authorization to be handled by RBAC, it could be a webhook or some other plugin. I would change this sentence to be more inclusive/open towards other authorizers or remove it.
| ```yaml | ||
| annotations: | ||
| authentication.gardener.cloud/issuer: managed | ||
| authentication.gardener.cloud/trusted: 'true' |
There was a problem hiding this comment.
authentication.gardener.cloud/trusted requires the gardener installation to have oidc extension and the trust-configurator installed, right?
There was a problem hiding this comment.
Correct. I have briefly mentioned below that
The following components help to enable this feature in the garden cluster:
oidc-webhook-authenticator
garden-shoot-trust-configurator
Not sure if I should explicitly outline a sample gardener setup with garden extensions shoot-oidc-service and garden-shoot-trust-configurator and also the discovery server. What do you think?
There was a problem hiding this comment.
Gardener operator deploys the discovery server unless it is disabled, i.e. when Garden.Spec.VirtualCluster.Gardener.DiscoveryServer=nil. I think you can add a Prerequisite subsection between Overview and How It Works, it will need a clarification that the prerequisites are fulfilled by the Garden admins, end-users are not expected to control enablement of the discovery server, oidc extension and shoot-trust configurator, so - they will have to check with their admin.
| authentication.gardener.cloud/trusted: 'true' | ||
| ``` | ||
|
|
||
| Additionally, RBAC rules in the garden cluster should grant permissions to the shoot's service account identity which will perform requests to the garden cluster. |
There was a problem hiding this comment.
| Additionally, RBAC rules in the garden cluster should grant permissions to the shoot's service account identity which will perform requests to the garden cluster. | |
| Additionally, authorization in the garden cluster should be configured to grant permissions to the shoot's service account identity which will perform requests to the garden cluster. |
| ### Authentication | ||
| When a request is made, the authentication flow is: | ||
|
|
||
|  |
There was a problem hiding this comment.
If you can, remove the authorization part from the diagram.
|
|
||
| ### Step 1: Enable Managed Issuer | ||
|
|
||
| By enabling a managed service account issuer, Gardener configures the shoot API server to sign service account tokens with a managed key pair. The Gardener Discovery Server then exposes the corresponding OIDC discovery documents and JWKS at a publicly accessible URL, allowing the garden cluster to verify the token signatures. |
There was a problem hiding this comment.
configures the shoot API server to sign service account tokens with a managed key pair
I think this is always the case, not only when the managed issuer is enabled, so maybe it can be removed (cc @dimityrmirchev for opinion)
| We can find the publicly available discovery documents via the OIDC endpoint `{issuer-url}/.well-known/openid-configuration`: | ||
|
|
||
| ```bash | ||
| curl https://discovery.<domain>/projects/<project-name>/shoots/<shoot-uid>/issuer/.well-known/openid-configuration | jq . |
There was a problem hiding this comment.
If I am not mistaken, usually commands are prefixed with some prompt symbol to distinguish them from the command output.
| curl https://discovery.<domain>/projects/<project-name>/shoots/<shoot-uid>/issuer/.well-known/openid-configuration | jq . | |
| $ curl https://discovery.<domain>/projects/<project-name>/shoots/<shoot-uid>/issuer/.well-known/openid-configuration | jq . |
| "aud": [ | ||
| "garden" | ||
| ], | ||
| "iss": "https://discovery.<service-account-issuer-hostname>/projects/<project-name>/shoots/<shoot-uid>/issuer", |
There was a problem hiding this comment.
discovery.<domain> vs discovery.<service-account-issuer-hostname>, this will need to be unified accrross the document.
|
|
||
| ### Step 3: Configure RBAC | ||
|
|
||
| Authentication (token verification) is now handled automatically, but the authenticated identity has no permissions by default. You must create RBAC rules in the garden cluster to authorize the requested actions. |
There was a problem hiding this comment.
| Authentication (token verification) is now handled automatically, but the authenticated identity has no permissions by default. You must create RBAC rules in the garden cluster to authorize the requested actions. | |
| Authentication (token verification) is now handled automatically, but the authenticated identity has very restricted permissions by default, i.e. those available via the `system:authenticated` group. You must create RBAC rules, or configure any other authorizer that is in place, in the garden cluster to authorize the requested actions. |
| > The `garden-shoot-trust-configurator` configures a prefix to the token's `sub` and `groups` claim when deriving the name. This ensures identities from different shoots are always unique, even if they share the same ServiceAccount name. The resulting username has the form: | ||
| > | ||
| > ```yaml | ||
| > ns:<project-namespace>:shoot:<shoot-name>:<shoot-uid>:<sub-claim> |
There was a problem hiding this comment.
For completeness, add the group prefix (even though it is available above).
| # Target shoot cluster | ||
| TOKEN=$(kubectl create token health-reporter -n health-reporter --audience garden) | ||
|
|
||
| curl -k -H "Authorization: Bearer $TOKEN" \ |
There was a problem hiding this comment.
| curl -k -H "Authorization: Bearer $TOKEN" \ | |
| curl -H "Authorization: Bearer $TOKEN" \ |
|
Thank you @vpnachev and @dimityrmirchev for reviewing and providing suggestions - I have addressed the comments in a follow-up PR: #944 |
4 participants
How to categorize this PR?
/kind enhancement
What this PR does / why we need it:
Add a guide for enabling trust between a shoot and the garden clusters with OIDC.
Which issue(s) this PR fixes:
Part of gardener/garden-shoot-trust-configurator#23
Special notes for your reviewer:
N/A
CC: @dimityrmirchev
Summary by CodeRabbit
Release Notes