Conversation
📝 WalkthroughWalkthroughAdds a new optional file-server component (ConfigMap, Deployment, Service) to the api7 chart and bumps chart/app versions and default image tags from 3.9.7 → 3.9.8 in both api7 and gateway charts and their documentation/values. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- Bump api7 chart version: 0.17.48 -> 0.17.49 - Bump gateway chart version: 0.2.58 -> 0.2.59 - Update appVersion to 3.9.8 for both charts - Update all image tags to v3.9.8 - Add file-server component (disabled by default) - Add vault role_type config option to dashboard, dp_manager and developer_portal
There was a problem hiding this comment.
🧹 Nitpick comments (1)
charts/api7/values.yaml (1)
90-93: Avoid defaultingfile_serverto a mutabledevtag.
tag: "dev"on Line 93 bypasses the deployment fallback to chart appVersion (Line 41 incharts/api7/templates/file-server-deploy.yaml) and makes rollouts non-reproducible when this component is enabled.♻️ Proposed change
file_server: enabled: false replicaCount: 1 image: repository: api7/api7-ee-file-server - pullPolicy: IfNotPresent - tag: "dev" + pullPolicy: Always + tag: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/api7/values.yaml` around lines 90 - 93, The values.yaml currently hardcodes image.tag as "dev" which prevents the file-server deployment from falling back to Chart.AppVersion; change the file_server image tag (image.tag) to be empty/null or remove the hardcoded "dev" so the template (file-server-deploy.yaml) can use the chart appVersion fallback (Chart.AppVersion); ensure the values key referenced by the template (image.tag for the file_server block) is not a non-empty literal "dev".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@charts/api7/values.yaml`:
- Around line 90-93: The values.yaml currently hardcodes image.tag as "dev"
which prevents the file-server deployment from falling back to Chart.AppVersion;
change the file_server image tag (image.tag) to be empty/null or remove the
hardcoded "dev" so the template (file-server-deploy.yaml) can use the chart
appVersion fallback (Chart.AppVersion); ensure the values key referenced by the
template (image.tag for the file_server block) is not a non-empty literal "dev".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0636452-a81a-4789-bd6a-1db508ba7de3
📒 Files selected for processing (9)
charts/api7/Chart.yamlcharts/api7/README.mdcharts/api7/templates/file-server-configmap.yamlcharts/api7/templates/file-server-deploy.yamlcharts/api7/templates/file-server-service.yamlcharts/api7/values.yamlcharts/gateway/Chart.yamlcharts/gateway/README.mdcharts/gateway/values.yaml
✅ Files skipped from review due to trivial changes (4)
- charts/gateway/README.md
- charts/gateway/values.yaml
- charts/gateway/Chart.yaml
- charts/api7/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/api7/README.md
Changes
3.9.8for both chartsfile_server.enabled: false)role_typeconfig (dynamic/static) to dashboard, dp_manager, and developer_portal database vault sectionsTest Plan
helm templaterenders correctly with default values (file-server disabled)helm template --set file_server.enabled=truerenders file-server configmap, deployment, and service correctlySummary by CodeRabbit