OCPBUGS-86058: warn about Authentication CR key naming requirement for STS provisioning#2911
OCPBUGS-86058: warn about Authentication CR key naming requirement for STS provisioning#2911chdeshpa-hue wants to merge 2 commits into
Conversation
…visioning When creating the manifestsSecretRef for AWS STS/IRSA clusters, the Authentication CR must use the exact key name cluster-authentication-02-config.yaml. The kube-apiserver bootstrap render command reads this manifest from a hardcoded path. If the key name differs, the custom serviceAccountIssuer is silently ignored and the install fails with token issuer mismatch. Add a warning about this requirement, recommend using --from-file to preserve canonical filenames, and add troubleshooting guidance for the common InvalidIdentityToken failure. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThis pull request updates AWS STS provisioning docs: adds an explicit step to create the installer-manifests Secret from ChangesAWS STS Provisioning Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @chdeshpa-hue. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/aws-sts-provisioning.md (1)
90-92: ⚡ Quick winAdd language specifier to code block.
The code block showing the error message should have a language specifier for proper syntax highlighting and to satisfy markdown linting rules.
📝 Proposed fix
-``` +```text error assuming role: InvalidIdentityToken: Token issuer does not match provider</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/aws-sts-provisioning.mdaround lines 90 - 92, The fenced code block
containing the error message "error assuming role: InvalidIdentityToken: Token
issuer does not match provider" should include a language specifier to satisfy
markdown linting; update the triple-backtick fence that surrounds that string to
use "text" (i.e., ```text) so the block is rendered/highlighted correctly.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@docs/aws-sts-provisioning.md:
- Line 109: The JSONPath snippet using "{range .data}{@.key}{"\n"}{end}" fails
because kubectl/oc JSONPath cannot range over maps (".data" is a map), so
replace the jsonpath output with a supported alternative: use a go-template that
ranges over .data keys (replace the "-o jsonpath=..." form in theoc get secret cluster-manifests -n <namespace> -o jsonpathcommand) or output JSON and pipe
to jq to list.data | keys[](replace the sameoc get secret cluster-manifests ... -o jsonpathusage). Ensure the docs show one of these two
alternatives instead of the JSONPath range example.
Nitpick comments:
In@docs/aws-sts-provisioning.md:
- Around line 90-92: The fenced code block containing the error message "error
assuming role: InvalidIdentityToken: Token issuer does not match provider"
should include a language specifier to satisfy markdown linting; update the
triple-backtick fence that surrounds that string to use "text" (i.e., ```text)
so the block is rendered/highlighted correctly.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Enterprise **Run ID**: `d77a2e57-4739-4dc1-a43f-0c26f2562093` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between ab4b2490385a31a5481a0ef69a40a71f88ac2faf and 7b758cdb9b5f577ed1eba4ce2c90d8ab331cede0. </details> <details> <summary>📒 Files selected for processing (1)</summary> * `docs/aws-sts-provisioning.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
chdeshpa-hue
changed the title
openshift-ci-robot
added
jira/severity-important
|
@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-86058, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-86058, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
2uasimojo
left a comment
2uasimojo
left a comment
There was a problem hiding this comment.
This is a nice find and a solid improvement.
Based on the verbosity I'm assuming an agent generated the delta. It's not bad, but let's tighten things up a bit. Specifically, I feel we don't need to go into nearly as much detail in the procedure section. Let's limit that part to emphasizing the --from-file=<dir> recommendation, and fold the remaining explanation into the Troubleshooting item. If we must mention this specific issue in the procedure section, let's keep it brief, along the lines of: "WARNING: Altering original filenames can have <link to troubleshooting section>consequences</link>"
- Condense verbose WARNING block in procedure section to a brief one-liner linking to the Troubleshooting section (per 2uasimojo) - Move detailed technical explanation (hardcoded path, silent fallback, affected/unaffected manifests) into the Troubleshooting section - Replace broken jsonpath command with go-template for listing secret data keys — jsonpath range only works on arrays, not maps (confirmed by 2uasimojo and CodeRabbit) - Add text language specifier to error message code block Co-authored-by: Cursor <cursoragent@cursor.com>
|
@2uasimojo Thanks for the review — good call on tightening the procedure section. Pushed an update:
|
|
@chdeshpa-hue: This pull request references Jira Issue OCPBUGS-86058, which is valid. 3 validation(s) were run on this bug
DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/aws-sts-provisioning.md`:
- Around line 51-53: The fenced code block containing "kubectl create secret
generic cluster-manifests --from-file=_output/manifests/" lacks a language
specifier; update the markdown fenced block to include a language identifier
(e.g., "bash") so the block becomes ```bash ... ``` to satisfy MD040 and enable
syntax highlighting.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9a46db80-986b-487f-853d-77ba37d7a765
📒 Files selected for processing (1)
docs/aws-sts-provisioning.md
| ``` | ||
| kubectl create secret generic cluster-manifests --from-file=_output/manifests/ | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block is missing a language identifier, which prevents proper syntax highlighting.
📝 Proposed fix
- ```
+ ```bash
kubectl create secret generic cluster-manifests --from-file=_output/manifests/</details>
As per coding guidelines, the static analysis tool markdownlint-cli2 flagged this as MD040 (fenced-code-language).
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/aws-sts-provisioning.md` around lines 51 - 53, The fenced code block
containing "kubectl create secret generic cluster-manifests
--from-file=_output/manifests/" lacks a language specifier; update the markdown
fenced block to include a language identifier (e.g., "bash") so the block
becomes ```bash ... ``` to satisfy MD040 and enable syntax highlighting.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 2uasimojo, chdeshpa-hue 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 |
openshift-ci
Bot
added
the
approved
3 participants
Summary
Clarify the manifest secret key naming requirement for the Authentication CR
when provisioning AWS STS/IRSA clusters via Hive.
When Hive extracts
manifestsSecretRefentries into the installer's manifestdirectory, each secret key becomes the filename on disk. The kube-apiserver
bootstrap render step reads the Authentication CR from a hardcoded path:
If the secret key doesn't match this exact filename, the bootstrap kube-apiserver
silently starts with the default
serviceAccountIssuer(
https://kubernetes.default.svc) instead of the custom S3 OIDC issuer. Thiscauses
machine-api-controllersto receive tokens with the wrong issuer, andAWS STS rejects them with
InvalidIdentityToken. Workers never provision andthe install times out.
Other credential manifests (operator Secrets) are not affected — they are
applied by GVK/content, not by filename. Only the Authentication CR has this
filename dependency.
Changes
about the required key name
cluster-authentication-02-config.yaml--from-fileas therecommended approach (preserves canonical filenames automatically)
InvalidIdentityTokenfailure mode with diagnostic steps and remediation
Why this matters
A customer experienced repeated STS install timeouts because their
manifestsSecretRefused a non-canonical key name for the Authentication CR.The fix was simply using the correct key name — no code change required.
This documentation change prevents the same misconfiguration for future users.
Technical detail
The hardcoded filename expectation exists in:
installer/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template:--cluster-auth-file=/assets/manifests/cluster-authentication-02-config.yamlcluster-kube-apiserver-operator/pkg/cmd/render/render.go:--cluster-auth-fileflag with silentos.IsNotExisthandlingMade with Cursor
Summary by CodeRabbit