[MOSIP-44117][MOSIP-44158]updated docs for helmsman destroy and observ-infra + updated clamav image #130
Conversation
Signed-off-by: bhumi46 <111699703+bhumi46@users.noreply.github.com>
…ated clamav image Signed-off-by: bhumi46 <thisisbn46@gmail.com>
WalkthroughReplaces commented ClamAV image overrides with explicit values in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🪛 markdownlint-cli2 (0.18.1)README.md566-566: Emphasis used instead of a heading (MD036, no-emphasis-as-heading) 588-588: Blank line inside blockquote (MD028, no-blanks-blockquote) 🔇 Additional comments (3)
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 |
…ated clamav image Signed-off-by: bhumi46 <thisisbn46@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/HELMSMAN_DESTROY_GUIDE.md (1)
25-48: Consider adding language identifiers to fenced code blocks.Several fenced code blocks lack language specifiers, which can improve rendering and accessibility. Consider:
- ASCII diagrams (lines 25-48): Add
textor leave as-is (acceptable for diagrams)- Plain text examples (lines 124-126, 138-140, 184-191, 346-352): Add
textidentifier🔎 Example fixes
For plain text blocks:
-``` +```text destroyFor ASCII art diagrams, either add `text` or leave unmarked (both acceptable): ```diff -``` +```text ┌─────────────────────────────────────────────────────────────────────────┐ │ Reusable Workflow (Core Engine) │ ...Based on static analysis hints.
Also applies to: 124-126, 138-140, 184-191, 346-352
README.md (1)
566-566: Use proper heading syntax instead of bold emphasis.Line 566 uses bold text
**Post-Deployment: Rancher UI Initial Setup**which should be a proper markdown heading for better document structure and navigation.🔎 Proposed fix
-**Post-Deployment: Rancher UI Initial Setup** +### Post-Deployment: Rancher UI Initial SetupBased on static analysis hints.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Helmsman/dsf/external-dsf.yamlREADME.mdRancher-keycloak-integration/README.mddocs/HELMSMAN_DESTROY_GUIDE.mddocs/README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
72-72: Link fragments should be valid
(MD051, link-fragments)
566-566: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
588-588: Blank line inside blockquote
(MD028, no-blanks-blockquote)
docs/HELMSMAN_DESTROY_GUIDE.md
25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
184-184: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
346-346: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (5)
docs/README.md (1)
63-63: LGTM!The documentation entries for the Helmsman Destroy Guide are properly formatted and provide clear descriptions. The links correctly reference the new guide added in this PR.
Also applies to: 71-71
Rancher-keycloak-integration/README.md (1)
1-335: LGTM!The restructuring effectively emphasizes the CI/CD workflow approach while maintaining comprehensive guidance. Key improvements include:
- Clear Quick Start section with workflow-centric steps
- Well-organized secrets configuration guidance
- Proper code block formatting with language identifiers
- Manual execution options preserved in collapsible section
The changes enhance usability while maintaining completeness.
README.md (3)
475-595: LGTM!The new observ-infra section is comprehensive and well-structured, providing:
- Complete tfvars configuration example with detailed explanations
- Clear workflow execution steps
- Post-deployment Rancher UI setup guidance
- Important notes about optional deployment and node sizing recommendations
The content effectively guides users through the optional observability infrastructure deployment.
596-601: LGTM!The Keycloak-Rancher integration section provides clear guidance with:
- Proper timing context (after observ-infra, before MOSIP infra)
- Reference to detailed integration guide
- Clear explanation of the integration's purpose
The cross-reference to the detailed guide is appropriate and helpful.
1257-1260: LGTM!The updated environment destruction section now clearly differentiates between:
- Infrastructure destruction (Terraform-based, full cleanup)
- Helmsman services destruction (Kubernetes-level, preserves infrastructure)
This distinction helps users choose the appropriate cleanup approach for their needs.
…ated clamav image Signed-off-by: bhumi46 <thisisbn46@gmail.com>
…ated clamav image Signed-off-by: bhumi46 <thisisbn46@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
72-73: Fix broken internal link with correct anchor format.The link at line 72 references
#after-observ-infra-run-keycloak--rancher-integration-ci, which does not match the heading at line 596:#### Step 3cb: Keycloak ⇄ Rancher integration (CI) — If usingobserv-infra``. GitHub generates anchors by lowercasing, removing special characters and backticks, replacing spaces with hyphens, and collapsing consecutive hyphens to single hyphens. The correct anchor is#step-3cb-keycloak-rancher-integration-ci-if-using-observ-infra.-See [After observ-infra: Run Keycloak ⇄ Rancher integration (CI)](#after-observ-infra-run-keycloak--rancher-integration-ci) section below +See [After observ-infra: Run Keycloak ⇄ Rancher integration (CI)](#step-3cb-keycloak-rancher-integration-ci-if-using-observ-infra) section below
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdterraform/implementations/aws/observ-infra/aws.tfvars
✅ Files skipped from review due to trivial changes (1)
- terraform/implementations/aws/observ-infra/aws.tfvars
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
72-72: Link fragments should be valid
(MD051, link-fragments)
566-566: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
588-588: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🔇 Additional comments (3)
README.md (3)
475-600: New observ-infra section is well-structured and comprehensive.The introduction of Step 3ca (observ-infra optional deployment) and Step 3cb (Keycloak–Rancher integration) is well-organized with clear configuration examples, prerequisites, and post-deployment guidance. References to external documentation (
Rancher-keycloak-integration/README.md) are appropriate and contextually placed.
1256-1259: Helmsman destruction guide properly integrated into cleanup workflow.The addition of references to both
ENVIRONMENT_DESTRUCTION_GUIDE.md(Terraform-based) andHELMSMAN_DESTROY_GUIDE.md(Helmsman-based) provides clear guidance on the two distinct destruction paths, reducing user confusion about cleanup procedures.
230-241: WireGuard VPN requirement notes clarified for Keycloak–Rancher integration.The updated note at lines 240–241 explicitly calls out that
TF_WG_CONFIGis required for the keycloak-rancher-integration workflow, improving clarity around VPN configuration sequencing and dependencies.
| - ✅ Green checkmark when complete | ||
| - ✅ Observation infrastructure created in AWS with Rancher and Keycloak | ||
|
|
||
| **Post-Deployment: Rancher UI Initial Setup** |
There was a problem hiding this comment.
Replace bold emphasis with a proper markdown heading.
Line 566 uses **Post-Deployment: Rancher UI Initial Setup** (emphasis) instead of a heading. This violates markdown best practices and fails linting (MD036). Convert to a proper heading level (e.g., ##### Post-Deployment: Rancher UI Initial Setup).
🔎 Proposed fix
-**Post-Deployment: Rancher UI Initial Setup**
+##### Post-Deployment: Rancher UI Initial Setup📝 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.
| **Post-Deployment: Rancher UI Initial Setup** | |
| ##### Post-Deployment: Rancher UI Initial Setup |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
566-566: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In README.md around line 566, the text uses bold emphasis instead of a markdown
heading; replace the bold line "**Post-Deployment: Rancher UI Initial Setup**"
with a proper heading (e.g., "##### Post-Deployment: Rancher UI Initial Setup")
to satisfy markdown linting (MD036) and maintain semantic structure.
| > **Important:** Save this new password securely. This password is used for **local user login** to Rancher UI (the `admin` account). After Keycloak-Rancher SAML integration is configured, operators can also login via Keycloak authentication. | ||
| > **Important Notes:** |
There was a problem hiding this comment.
Remove blank line inside blockquote at line 588.
A blank line exists within the blockquote structure that violates MD028 (no-blanks-blockquote). Blockquotes should not contain blank lines; if breaking content is needed, continue the blockquote on the next line without blank spacing.
🔎 Proposed fix
The blockquote at lines 587–589 contains a blank line. Restructure to eliminate it:
> **Important:** Save this new password securely. This password is used for **local user login** to Rancher UI (the `admin` account). After Keycloak-Rancher SAML integration is configured, operators can also login via Keycloak authentication.
-
> **Important Notes:**
>
> - `observ-infra` is optional and intended for production deployments requiring separate management/monitoring infrastructure📝 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.
| > **Important:** Save this new password securely. This password is used for **local user login** to Rancher UI (the `admin` account). After Keycloak-Rancher SAML integration is configured, operators can also login via Keycloak authentication. | |
| > **Important Notes:** | |
| > **Important:** Save this new password securely. This password is used for **local user login** to Rancher UI (the `admin` account). After Keycloak-Rancher SAML integration is configured, operators can also login via Keycloak authentication. | |
| > **Important Notes:** |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
588-588: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 Prompt for AI Agents
In README.md around lines 587 to 589, there is a blank line inside a blockquote
(MD028 violation) — remove the empty line at line 588 and merge the adjacent
lines so the blockquote text is continuous (keep both sentences as part of the
same blockquote without an intervening blank line).
1 participant
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.