Repository navigation
[platform] Introduce expose-services, expose-ingress and expose-external-ips options - #197
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Rate limit exceededNick Volynkin (@NickVolynkin) has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 22 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (6)
WalkthroughThe documentation was updated to change how services are exposed in Cozystack. Instructions were modified to use the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cozystack ConfigMap
participant Cozystack System
User->>Cozystack ConfigMap: Patch ConfigMap (set expose-services / expose-external-ips)
Cozystack ConfigMap-->>Cozystack System: Updated configuration detected
Cozystack System-->>User: Exposes requested services (dashboard, uploadproxy, etc.) externally
Poem
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
CodeRabbit Configuration File (
|
6b23f3c to
efe15c5
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: 2
🧹 Nitpick comments (5)
content/en/docs/operations/faq.md (1)
74-78: Use consistent patch flag style across docs.Other guides use
--type merge(with a space) rather than--type=merge. Consider standardizing on one format for clarity and to avoid confusion.-kubectl patch -n cozy-system configmap cozystack --type=merge -p '{ +kubectl patch -n cozy-system configmap cozystack --type merge -p '{ "data": { "expose-external-ips": "192.168.100.11,192.168.100.12,192.168.100.13" } }'content/en/docs/operations/virtualization/virtual-machines.md (1)
51-51: Clarify phrasing and resource naming.Remove the extra comma and capitalize
ConfigMapfor consistency. For example:@@ -51 +51 -1. Modify your cozystack config map, to enable cdi-uploadproxy along with the dashboard: +1. Modify your Cozystack ConfigMap to enable `cdi-uploadproxy` along with the `dashboard`:content/en/docs/operations/bundles/paas-hosted.md (1)
27-27: Illustrate all new exposure parameters in the example.Consider expanding the example ConfigMap to include
expose-ingressandexpose-external-ipsso users see all related options in context:@@ -27 +27 - expose-services: "api,dashboard" + expose-services: "api,dashboard" + expose-ingress: "tenant-root" + expose-external-ips: "<external-ip1>,<external-ip2>"content/en/docs/operations/bundles/paas-full.md (2)
30-30: Include all new parameters in the example for consistency
The example ConfigMap showsexpose-services, but omitsexpose-ingressandexpose-external-ips. It’s helpful to illustrate all three new options with their defaults so users can copy/paste directly.You might apply a diff like:
expose-services: "api,dashboard,cdi-uploadproxy,vm-exportproxy" + expose-ingress: "tenant-root" + expose-external-ips: ""
48-51: Add missing comma for clarity in the table description
In theexpose-external-ipsrow, add a comma after “If not specified” to improve readability:-| `expose-external-ips` | Comma-separated list of external IPs used for specified ingress controller. If not specified it will use LoadBalancer service by default | +| `expose-external-ips` | Comma-separated list of external IPs used for specified ingress controller. If not specified, it will use LoadBalancer service by default |🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: Possible missing comma found.
Context: ...or specified ingress controller. If not specified it will use LoadBalancer service by def...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (6)
content/en/docs/getting-started/first-deployment.md(1 hunks)content/en/docs/operations/bundles/paas-full.md(2 hunks)content/en/docs/operations/bundles/paas-hosted.md(2 hunks)content/en/docs/operations/faq.md(1 hunks)content/en/docs/operations/oidc/enable_oidc.md(1 hunks)content/en/docs/operations/virtualization/virtual-machines.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
content/en/docs/operations/bundles/paas-full.md
[uncategorized] ~50-~50: Possible missing comma found.
Context: ...or specified ingress controller. If not specified it will use LoadBalancer service by def...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (2)
content/en/docs/operations/bundles/paas-hosted.md (1)
43-45: Document new bundle options correctly.The addition of
expose-services,expose-ingress, andexpose-external-ipsin the parameters table is accurate and aligns with the new functionality.content/en/docs/operations/bundles/paas-full.md (1)
52-52: Verify FAQ link and anchor
Ensure that the/docs/guides/faq/#bundlesanchor exists and points to the correct section on generic bundle options.
| Edit your Cozystack ConfigMap to enable OIDC, this also will expose keycloak service automatically: | ||
|
|
||
| ```bash | ||
| kubectl patch -n cozy-system configmap cozystack --type=merge -p '{"data":{"oidc-enabled": "true"}}' | ||
| kubectl patch -n cozy-system configmap cozystack --type=merge -p '{ | ||
| "data": { | ||
| "oidc-enabled": "true", | ||
| } | ||
| }' |
There was a problem hiding this comment.
Fix invalid JSON and improve clarity.
The multi-line JSON passed to kubectl patch has a trailing comma after "oidc-enabled": "true", which makes it invalid. Also, a small grammar tweak can improve readability.
Apply this diff:
@@ -43 +43
-Edit your Cozystack ConfigMap to enable OIDC, this also will expose keycloak service automatically:
+Edit your Cozystack ConfigMap to enable OIDC; this will also expose the Keycloak service automatically:
@@ -46,5 +46,5 @@
-kubectl patch -n cozy-system configmap cozystack --type=merge -p '{
- "data": {
- "oidc-enabled": "true",
- }
-}'
+kubectl patch -n cozy-system configmap cozystack --type=merge -p '{
+ "data": {
+ "oidc-enabled": "true"
+ }
+}'📝 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.
| Edit your Cozystack ConfigMap to enable OIDC, this also will expose keycloak service automatically: | |
| ```bash | |
| kubectl patch -n cozy-system configmap cozystack --type=merge -p '{"data":{"oidc-enabled": "true"}}' | |
| kubectl patch -n cozy-system configmap cozystack --type=merge -p '{ | |
| "data": { | |
| "oidc-enabled": "true", | |
| } | |
| }' | |
| Edit your Cozystack ConfigMap to enable OIDC; this will also expose the Keycloak service automatically: | |
🤖 Prompt for AI Agents
In content/en/docs/operations/oidc/enable_oidc.md around lines 43 to 50, fix the
invalid JSON in the kubectl patch command by removing the trailing comma after
"oidc-enabled": "true". Also, revise the sentence for better clarity and
grammar, ensuring it clearly states that enabling OIDC will automatically expose
the Keycloak service.
…nal-ips options (#929) docs update: cozystack/website#197 Signed-off-by: Andrei Kvapil <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Added automated migration script to transition configuration from HelmRelease to ConfigMap for service exposure and external IPs. - Introduced new ingress templates for API, CDI upload proxy, and VM export proxy services, enabling dynamic exposure based on centralized configuration. - **Bug Fixes** - Updated NGINX Ingress Controller Helm chart version to 1.6.0. - **Refactor** - Centralized ingress configuration using a ConfigMap, simplifying and unifying service exposure and ingress class management. - Removed legacy parameters and templates for dashboard, CDI upload proxy, and VM export proxy from values and schema files. - Simplified ingress templates for dashboard and Keycloak to rely on centralized ConfigMap data and exposure lists. - Adjusted ingress controller service to conditionally use external IPs based on centralized configuration. - **Documentation** - Updated documentation to reflect the removal of deprecated parameters and clarify current configuration options. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…nal-ips options (#929) docs update: cozystack/website#197 Signed-off-by: Andrei Kvapil <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Added automated migration script to transition configuration from HelmRelease to ConfigMap for service exposure and external IPs. - Introduced new ingress templates for API, CDI upload proxy, and VM export proxy services, enabling dynamic exposure based on centralized configuration. - **Bug Fixes** - Updated NGINX Ingress Controller Helm chart version to 1.6.0. - **Refactor** - Centralized ingress configuration using a ConfigMap, simplifying and unifying service exposure and ingress class management. - Removed legacy parameters and templates for dashboard, CDI upload proxy, and VM export proxy from values and schema files. - Simplified ingress templates for dashboard and Keycloak to rely on centralized ConfigMap data and exposure lists. - Adjusted ingress controller service to conditionally use external IPs based on centralized configuration. - **Documentation** - Updated documentation to reflect the removal of deprecated parameters and clarify current configuration options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit f8210cf) Signed-off-by: Timofei Larkin <[email protected]>
…ips options Signed-off-by: Andrei Kvapil <[email protected]>
efe15c5 to
e5e31dc
Compare
The talm template example invoked the shorthand -n IP, which is no longer registered as of cozystack/talm v0.28.0 (PR cozystack#197). Without this fix the documented bootstrap golden path errors out at parse time with 'flag -n not defined'. Long form --nodes IP is the permanent replacement. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
The talm template example invoked the shorthand -n IP, which is no longer registered as of cozystack/talm v0.28.0 (PR cozystack#197). Without this fix the documented bootstrap golden path errors out at parse time with 'flag -n not defined'. Long form --nodes IP is the permanent replacement. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Wait until cozystack/cozystack#929 released
Summary by CodeRabbit
expose-services,expose-ingress, andexpose-external-ipsfor service exposure and ingress setup.