STS Server documentation and troubleshooting#9543
STS Server documentation and troubleshooting#9543romayalon wants to merge 1 commit intonoobaa:masterfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 53 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded comprehensive documentation for NooBaa's Security Token Service (STS), covering AWS-like role assumption workflows, configuration semantics stored in account entities, server topology on port 7443, operational procedures, error codes, troubleshooting guidance, and an end-to-end demo walkthrough with Q&A sections. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/dev_guide/STS Server.md (2)
165-167: Add fenced code block languages and polish minor doc typos.Please add explicit languages to bare fences (for example
text,bash) and fix wording typos like “principle”→“principal” (Lines 277/279) and “andn Use” (Line 412). This is small but improves lint compliance and clarity.Also applies to: 266-268, 958-971
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/dev_guide/STS` Server.md around lines 165 - 167, Update the Markdown by adding explicit fenced-block languages for code fences (e.g., change ``` to ```text or ```bash) for the ARN and other example blocks (including the blocks at the ranges noted: around the ARN example and the sections covering 266-268 and 958-971), and correct the typos: replace “principle” with “principal” (occurrences near the cited lines) and fix “andn Use” to the intended text (e.g., “and Use”) so linting and clarity are improved.
30-30: Consolidate duplicate headings to improve structure and readability.There are repeated section titles (
Overview,Technical Details,Troubleshooting,Role in NooBaa DB,Demo Important Points, and repeated STS subsection headers). This makes navigation noisy and triggers markdownlint MD024.Also applies to: 62-62, 99-99, 115-115, 141-141, 225-225, 236-236, 325-325, 368-368, 860-860, 950-950
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/dev_guide/STS` Server.md at line 30, Multiple duplicate top-level headings like "Overview", "Technical Details", "Troubleshooting", "Role in NooBaa DB", "Demo Important Points" and repeated "STS" subsection headers are present and trigger markdownlint MD024; go through the document and consolidate each duplicated heading by keeping the primary/first occurrence of the heading (e.g., the first "Overview", "Technical Details", etc.), remove or merge the subsequent duplicates, and ensure subsection content is moved under the correct unique heading (update section order and any cross-references if necessary) so each heading string appears only once in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/dev_guide/STS` Server.md:
- Line 12: The TOC/Q&A links use incorrect fragment anchors
(`#feature-technical-details`, `#enable-debug-logging`, `#3-troubleshooting`); update
those three links to match the actual generated Markdown anchors by deriving
them from the target headings (convert heading text to lowercase, strip
punctuation, remove any leading numerals, and replace spaces with hyphens).
Specifically, locate the occurrences of the fragments
'#feature-technical-details', '#enable-debug-logging', and '#3-troubleshooting'
and replace them with the correct anchors computed from their corresponding
heading texts (the headings for "Feature Technical Details", "Enable Debug
Logging" under “Endpoint Pod Logs Search”, and the "Troubleshooting" section),
ensuring the links resolve to the real generated anchors.
- Around line 117-137: The documented assume_role_policy schema is inaccurate:
update the docs so assume_role_policy (as implemented in src/api/common_api.js)
uses effect enum values matching the API ("Allow" and "Deny" with
capitalization) and allow action and principal to be either a string or an array
(not array-only), referencing the same email definition for principal; mirror
these same corrections in the other occurrences mentioned so the doc schema
aligns exactly with the API contract for assume_role_policy.
---
Nitpick comments:
In `@docs/dev_guide/STS` Server.md:
- Around line 165-167: Update the Markdown by adding explicit fenced-block
languages for code fences (e.g., change ``` to ```text or ```bash) for the ARN
and other example blocks (including the blocks at the ranges noted: around the
ARN example and the sections covering 266-268 and 958-971), and correct the
typos: replace “principle” with “principal” (occurrences near the cited lines)
and fix “andn Use” to the intended text (e.g., “and Use”) so linting and clarity
are improved.
- Line 30: Multiple duplicate top-level headings like "Overview", "Technical
Details", "Troubleshooting", "Role in NooBaa DB", "Demo Important Points" and
repeated "STS" subsection headers are present and trigger markdownlint MD024; go
through the document and consolidate each duplicated heading by keeping the
primary/first occurrence of the heading (e.g., the first "Overview", "Technical
Details", etc.), remove or merge the subsequent duplicates, and ensure
subsection content is moved under the correct unique heading (update section
order and any cross-references if necessary) so each heading string appears only
once in the file.
🪄 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: Pro
Run ID: e6e6cbc9-26f3-43cf-989a-1779becb0fe7
📒 Files selected for processing (1)
docs/dev_guide/STS Server.md
| **`assume_role_policy` definition** (`src/api/common_api.js`): | ||
| ```javascript | ||
| assume_role_policy: { | ||
| type: 'object', | ||
| required: ['statement'], | ||
| properties: { | ||
| version: { type: 'string' }, | ||
| statement: { | ||
| type: 'array', | ||
| items: { | ||
| type: 'object', | ||
| required: ['effect', 'action', 'principal'], | ||
| properties: { | ||
| effect: { enum: ['allow', 'deny'], type: 'string' }, | ||
| action: { type: 'array', items: { type: 'string' } }, | ||
| principal: { type: 'array', items: { $ref: '#/definitions/email' } } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
assume_role_policy schema in docs does not match implemented API contract.
Line 130 documents effect as lowercase (allow/deny) and Lines 131-132 constrain action/principal to arrays only, but the API schema allows Allow|Deny and supports either string or array. This can mislead operators and produce invalid examples.
Proposed doc fix
assume_role_policy: {
type: 'object',
required: ['statement'],
properties: {
version: { type: 'string' },
statement: {
type: 'array',
items: {
type: 'object',
required: ['effect', 'action', 'principal'],
properties: {
- effect: { enum: ['allow', 'deny'], type: 'string' },
- action: { type: 'array', items: { type: 'string' } },
- principal: { type: 'array', items: { $ref: '#/definitions/email' } }
+ effect: { enum: ['Allow', 'Deny'], type: 'string' },
+ action: {
+ oneOf: [
+ { type: 'string', minLength: 1 },
+ { type: 'array', items: { type: 'string', minLength: 1 }, minItems: 1 }
+ ]
+ },
+ principal: {
+ oneOf: [
+ { type: 'string', minLength: 1 },
+ { type: 'array', items: { type: 'string', minLength: 1 }, minItems: 1 }
+ ]
+ }
}
}
}
}
}Also applies to: 143-157, 190-190, 624-624
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/dev_guide/STS` Server.md around lines 117 - 137, The documented
assume_role_policy schema is inaccurate: update the docs so assume_role_policy
(as implemented in src/api/common_api.js) uses effect enum values matching the
API ("Allow" and "Deny" with capitalization) and allow action and principal to
be either a string or an array (not array-only), referencing the same email
definition for principal; mirror these same corrections in the other occurrences
mentioned so the doc schema aligns exactly with the API contract for
assume_role_policy.
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
bb6a74e to
7f7596b
Compare
Explain the Changes
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit