STS | Update doc for AWS STS minikube set up #1631
Conversation
WalkthroughThe documentation was reorganized by moving the manual AWS resource creation steps to later in the guide, updating step numbering, and adding a new section on creating a backing store using AWS STS. The content of the manual steps remains unchanged, and no modifications were made to code or public entity declarations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Minikube
participant AWS Console
participant Noobaa CLI
User->>Minikube: Configure OIDC and upload configuration
User->>AWS Console: Create S3 bucket, IAM Identity Provider, IAM Role (manual steps)
User->>Minikube: Continue with test steps (nginx pod, service account token)
User->>AWS CLI: Assume role with web identity (test)
User->>Noobaa CLI: Create backing store using AWS STS role ARN
Noobaa CLI->>AWS Console: Access S3 bucket via STS
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
naveenpaul1
force-pushed
the
sts-minikube-doc
branch
from
33f0468 to
4698a40
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (11)
doc/dev_guide/create_aws_sts_setup_on_minikube.md (11)
87-123: Clean-up obsolete note aboutbase64 -w0The note in line 123 refers to a
base64 -w0flag that no longer appears in any of the preceding commands. This will confuse readers.-Note: in the commands above were piping to `base64 -w0``, since I don't have this flag on MAC I removed it. +Note: older Linux examples sometimes add the `-w0` flag to `base64`. +macOS does not support `-w0`, so the commands above omit it.
154-163: Combine the twoput-objectcalls into oneaws s3 cpexample for brevityYou can simplify the instructions and avoid repeating bucket names:
-aws s3api put-object --bucket ${OPENID_BUCKET_NAME} --key keys.json --body ./keys.json -aws s3api put-object --bucket ${OPENID_BUCKET_NAME} --key '.well-known/openid-configuration' --body ./openid-configuration +aws s3 cp ./keys.json s3://${OPENID_BUCKET_NAME}/keys.json +aws s3 cp ./openid-configuration s3://${OPENID_BUCKET_NAME}/.well-known/openid-configuration
174-177: Grammar fix – singular noun-11. You can verify that the configuration are public available +11. You can verify that the configuration is publicly available
178-178: Remove trailing colon in headingMarkdown-lint MD026 flags trailing punctuation in headings.
-### Creating AWS resources manually - Using the Amazon web console: +### Creating AWS resources manually - Using the Amazon web console
179-180: Minor wording – capitalise “S3” and clarify duplicationMaybe clarify that the bucket already exists:
-12. Create s3 bucket (you already did it in the step Initial Setup - Create S3 Bucket). +12. (Already done) Ensure the S3 bucket created in “Initial Setup – Create S3 Bucket” exists.
181-182: Formatting glitch & double back-tickThe back-tick after `Get thumbprint`` breaks Markdown emphasis.
-… click on `Get thumbprint`` → Audience: api … +… click on **Get thumbprint** → Audience: `api` …
183-184: ScopeAmazonS3FullAccess– least-privilege adviceGranting
AmazonS3FullAccessto the role is convenient but overly broad. Consider recommending a minimal IAM policy limited to the target bucket (e.g.s3:ListBucket,s3:GetObject,s3:PutObject).
207-207: Article usage – “an nginx pod”-16. Create a nginx pod +16. Create an nginx pod
233-233: Typo – “Projected service account token”For consistency with Kubernetes docs, consider lowercase “projected”:
-17. Fetch the Projected service account token +17. Fetch the projected ServiceAccount token
239-239: Wording – add article-18. Use assume-role-with-web-identity +18. Use the `assume-role-with-web-identity` command
267-274: Spelling, commas, and fenced-code language-Noobaa WEB_IDENTITY_TOKEN Audience value is `openshift` because of that You shouls update the Audience in Identity Provider to opeshift. +Noobaa’s `WEB_IDENTITY_TOKEN` audience is `openshift`; therefore, update the Identity Provider’s **Audience** value to `openshift`.-``` +```bash nb backingstore create aws-sts-s3 {backing-store} --target-bucket ${TARGET_BUCKET} --aws-sts-arn ${OIDC_ROLE_ARN}Also replace
{backing-store}with a clearer placeholder like<backing_store_name>.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/dev_guide/create_aws_sts_setup_on_minikube.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/dev_guide/create_aws_sts_setup_on_minikube.md
[grammar] ~174-~174: Do you mean “configurations”?
Context: ...c-read ``` 11. You can verify that the configuration are public available by using your brow...
(DT_NN_ARE_AME)
[misspelling] ~205-~205: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...le ARN>' ``` ### Test: We would create an nginx pod and fetch the Service Account...
(EN_A_VS_AN)
[typographical] ~267-~267: It appears that a comma is missing.
Context: ...udience value is openshift because of that You shouls update the Audience in Ident...
(BECAUSE_OF_YOU_COMMA)
🪛 markdownlint-cli2 (0.17.2)
doc/dev_guide/create_aws_sts_setup_on_minikube.md
178-178: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
204-204: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
273-273: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: run-hac-test
- GitHub Check: run-cli-tests
- GitHub Check: run-kms-dev-test
- GitHub Check: run-admission-test
- GitHub Check: run-kms-kmip-test
- GitHub Check: run-azure-vault-test
- GitHub Check: golangci-lint
- GitHub Check: run-kms-key-rotate-test
- GitHub Check: run-operator-tests
- GitHub Check: run-kms-tls-token-test
- GitHub Check: run-kms-tls-sa-test
- GitHub Check: cnpg-deployment-test
🔇 Additional comments (3)
doc/dev_guide/create_aws_sts_setup_on_minikube.md (3)
125-149: Step 8 JSON snippet looks goodThe
openid-configurationtemplate is accurate and the${OPENID_BUCKET_URL}placeholders are clear.
No action needed.
164-173: Verify bucket-level ACL prerequisitesNew S3 buckets created with “ACLs disabled” (the default as of April 2023) will reject
put-object-aclcalls.
Readers following the guide will hit a 400InvalidRequesterror unless they first enable ACLs at bucket creation or use a bucket policy instead.Consider adding a short note:
> If your bucket was created with **ACLs disabled** (the default), either > enable ACLs or use a bucket policy that grants `GetObject` on the two objects.
198-203: Variable definition looks goodThe placeholder makes it clear how to capture the role ARN.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
doc/dev_guide/create_aws_sts_setup_on_minikube.md (4)
174-176: Grammar polish & claritySubject/verb agreement and adverb usage are off.
-11. You can verify that the configuration are public available by using your browser +11. You can verify that the configuration is publicly available by using your browser
178-178: Remove the trailing colon from headingMarkdown-lint rule MD026 flags trailing punctuation in headings; dropping the “:” keeps the style consistent with the rest of the doc.
-### Creating AWS resources manually - Using the Amazon web console: +### Creating AWS resources manually - Using the Amazon web console
265-269: Typos and wording-Noobaa WEB_IDENTITY_TOKEN Audience value is `openshift` because of that You shouls update the Audience in Identity Provider to opeshift. +Because Noobaa’s `WEB_IDENTITY_TOKEN` audience is `openshift`, update the Identity Provider audience to **openshift**.Fixes: “shouls” → should, “opeshift” → openshift, sentence structure.
273-275: Missing language spec in fenced blockMD040 warns about code blocks without a language identifier; add
bashfor syntax highlighting:-``` -nb backingstore create aws-sts-s3 {backing-store} --target-bucket ${TARGET_BUCKET} --aws-sts-arn ${OIDC_ROLE_ARN} -``` +```bash +nb backingstore create aws-sts-s3 {backing-store} \ + --target-bucket "${TARGET_BUCKET}" \ + --aws-sts-arn "${OIDC_ROLE_ARN}" +```
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/dev_guide/create_aws_sts_setup_on_minikube.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/dev_guide/create_aws_sts_setup_on_minikube.md
[grammar] ~174-~174: Do you mean “configurations”?
Context: ...c-read ``` 11. You can verify that the configuration are public available by using your brow...
(DT_NN_ARE_AME)
[uncategorized] ~174-~174: This verb does not appear to agree with the subject. Consider using a different form.
Context: .... You can verify that the configuration are public available by using your browser ...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~183-~183: A period might be missing here.
Context: ...provider that we added (with structure: https://<oidc_bucket_name>.s3.<aws_region>.amazonaws.com ) → Add the permission: `AmazonS3F...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[uncategorized] ~198-~198: A comma might be missing here.
Context: ...aud": "api" } ``` 15. In later steps you will need to provide the ARN of the...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~198-~198: You might be missing the article “the” here.
Context: ...f the role (you can easily copy it from AWS console, it looks like `arn:aws:iam::<i...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[misspelling] ~205-~205: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...le ARN>' ``` ### Test: We would create an nginx pod and fetch the Service Account...
(EN_A_VS_AN)
[typographical] ~267-~267: It appears that a comma is missing.
Context: ...udience value is openshift because of that You shouls update the Audience in Ident...
(BECAUSE_OF_YOU_COMMA)
🪛 markdownlint-cli2 (0.17.2)
doc/dev_guide/create_aws_sts_setup_on_minikube.md
178-178: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
204-204: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
273-273: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: cnpg-deployment-test
- GitHub Check: run-operator-tests
- GitHub Check: run-cli-tests
- GitHub Check: run-core-config-map-tests
- GitHub Check: run-kms-key-rotate-test
- GitHub Check: golangci-lint
- GitHub Check: run-kms-dev-test
- GitHub Check: run-admission-test
- GitHub Check: run-kms-kmip-test
- GitHub Check: run-kms-tls-sa-test
- GitHub Check: run-hac-test
- GitHub Check: run-azure-vault-test
- GitHub Check: run-kms-tls-token-test
| 13. Create Identity Provider: IAM → Identity providers → Add provider → Provider type: choose OpenID Connect → Provider URL: paste the value of OPENID_BUCKET_URL → click on `Get thumbprint`` → Audience: api (type api in the field) → Click on add provider. | ||
|
|
||
| 14. Create role: IAM → Roles → Create Role → Trusted entity type: Web Identity → Identity Provider should be the name of the provider that we added (with structure: https://<oidc_bucket_name>.s3.<aws_region>.amazonaws.com ) → Add the permission: `AmazonS3FullAccess`. | ||
|
|
||
| When you finish, check in the Trusted entities that you see: | ||
|
|
||
| ```json | ||
| "Principal": { | ||
| "Federated": "arn:aws:iam::<account-id>:oidc-provider/<oidc-bucket-name>.s3.<region>.amazonaws.com" | ||
| }, | ||
| "Action": "sts:AssumeRoleWithWebIdentity", | ||
| "Condition": { | ||
| "StringEquals": { | ||
| "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "api" | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Audience value inconsistency – api vs openshift
Here you instruct users to set the audience to api (and the JSON trust policy below uses "aud": "api").
Later (line 267) you state the audience must be openshift for Noobaa. This contradiction will break STS token validation.
Proposed fix—pick one audience (Noobaa requires openshift) and align both the console instructions and the sample policy:
-… Audience: api (type api in the field) …
+… Audience: openshift (type openshift in the field) …
- "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "api"
+ "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "openshift"Please adjust any subsequent commands (audience field in the pod spec, etc.) to the same value.
📝 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.
| 13. Create Identity Provider: IAM → Identity providers → Add provider → Provider type: choose OpenID Connect → Provider URL: paste the value of OPENID_BUCKET_URL → click on `Get thumbprint`` → Audience: api (type api in the field) → Click on add provider. | |
| 14. Create role: IAM → Roles → Create Role → Trusted entity type: Web Identity → Identity Provider should be the name of the provider that we added (with structure: https://<oidc_bucket_name>.s3.<aws_region>.amazonaws.com ) → Add the permission: `AmazonS3FullAccess`. | |
| When you finish, check in the Trusted entities that you see: | |
| ```json | |
| "Principal": { | |
| "Federated": "arn:aws:iam::<account-id>:oidc-provider/<oidc-bucket-name>.s3.<region>.amazonaws.com" | |
| }, | |
| "Action": "sts:AssumeRoleWithWebIdentity", | |
| "Condition": { | |
| "StringEquals": { | |
| "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "api" | |
| } | |
| ``` | |
| 13. Create Identity Provider: IAM → Identity providers → Add provider → Provider type: choose OpenID Connect → Provider URL: paste the value of OPENID_BUCKET_URL → click on `Get thumbprint` → Audience: openshift (type openshift in the field) → Click on add provider. | |
| 14. Create role: IAM → Roles → Create Role → Trusted entity type: Web Identity → Identity Provider should be the name of the provider that we added (with structure: https://<oidc_bucket_name>.s3.<aws_region>.amazonaws.com ) → Add the permission: `AmazonS3FullAccess`. | |
| When you finish, check in the Trusted entities that you see: | |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~183-~183: A period might be missing here.
Context: ...provider that we added (with structure: https://<oidc_bucket_name>.s3.<aws_region>.amazonaws.com ) → Add the permission: `AmazonS3F...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
🤖 Prompt for AI Agents
In doc/dev_guide/create_aws_sts_setup_on_minikube.md between lines 181 and 196,
the audience value is inconsistently set to "api" in the instructions and JSON
trust policy, while later it is required to be "openshift" for Noobaa. To fix
this, change all occurrences of the audience value from "api" to "openshift" in
the console setup steps, the JSON trust policy snippet, and ensure any related
commands or pod spec fields later in the document also use "openshift" for
consistency and correct STS token validation.
Signed-off-by: naveenpaul1 <napaul@redhat.com> Signed-off-by: Naveen Paul <napaul@redhat.com>
naveenpaul1
force-pushed
the
sts-minikube-doc
branch
from
4698a40 to
a57e6e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
doc/dev_guide/create_aws_sts_setup_on_minikube.md (1)
181-196: Inconsistent audience value (api↔openshift) – breaks STS token validationThis was raised in the previous review but the contradiction is still present:
- Step 13 UI instructions & trusted-entities JSON use
api- The pod spec (line 229) and the “Audience” field in the sample output (line 261) also use
api- The new “Create backing store using STS” section explicitly says the audience must be
openshiftIf Noobaa indeed requires
openshift, all earlier occurrences must be changed to match.- … Audience: api (type api in the field) … + … Audience: openshift (type openshift in the field) … - "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "api" + "<oidc-bucket-name>.s3.<region>.amazonaws.com:aud": "openshift" - audience: api + audience: openshift - "Audience": "api" + "Audience": "openshift"Please update every related snippet/command and retest the flow.
Also applies to: 229-230, 261-261
🧹 Nitpick comments (4)
doc/dev_guide/create_aws_sts_setup_on_minikube.md (4)
174-176: Grammar – singular/plural agreement“the configuration are public available” → “the configuration is publicly available”.
-11. You can verify that the configuration are public available by … +11. You can verify that the configuration is publicly available by …
178-180: Duplicate step – S3 bucket creation repeatedStep 12 tells the reader to “Create s3 bucket”, but the bucket was already created in Step 1. Remove or rephrase to avoid confusion.
267-268: Typos and claritySeveral misspellings reduce professionalism and may confuse readers.
-Noobaa WEB_IDENTITY_TOKEN Audience value is `openshift` because of that You shouls update the Audience in Identity Provider to opeshift. +The Noobaa `WEB_IDENTITY_TOKEN` audience value is `openshift`; therefore, update the Identity Provider audience to `openshift`.
273-275: Missing language hint for fenced code blockMarkdownlint flags this; specify the shell language:
-``` +```bash nb backingstore create aws-sts-s3 {backing-store} --target-bucket ${TARGET_BUCKET} --aws-sts-arn ${OIDC_ROLE_ARN}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
doc/dev_guide/create_aws_sts_setup_on_minikube.md(7 hunks)
🧰 Additional context used
🪛 LanguageTool
doc/dev_guide/create_aws_sts_setup_on_minikube.md
[grammar] ~174-~174: Do you mean “configurations”?
Context: ...c-read ``` 11. You can verify that the configuration are public available by using your brow...
(DT_NN_ARE_AME)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...alue is openshift because of that You shouls update the Audience in Identity Provide...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~267-~267: Ensure spelling is correct
Context: ...te the Audience in Identity Provider to opeshift. ```bash TARGET_BUCKET='<target bucket ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.17.2)
doc/dev_guide/create_aws_sts_setup_on_minikube.md
178-178: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
204-204: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
273-273: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: run-operator-tests
- GitHub Check: cnpg-deployment-test
- GitHub Check: run-kms-key-rotate-test
- GitHub Check: golangci-lint
- GitHub Check: run-kms-tls-sa-test
- GitHub Check: run-kms-tls-token-test
- GitHub Check: run-cli-tests
- GitHub Check: run-kms-kmip-test
- GitHub Check: run-azure-vault-test
- GitHub Check: run-hac-test
- GitHub Check: run-core-config-map-tests
- GitHub Check: run-admission-test
- GitHub Check: run-kms-dev-test
Explain the changes
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit