fix(MAJORLEA-006-2): 2 review findings across 2 files - #88
Conversation
| @@ -42,5 +42,5 @@ kind: ConfigMap | |||
| metadata: | |||
There was a problem hiding this comment.
🦩 🔴 Frontend ConfigMap uses inline shell-style default syntax not supported by envsubst
Changed NODE_ENV: "${NODE_ENV:-production}" and API_TIMEOUT: "${API_TIMEOUT:-30000}" to NODE_ENV: "${NODE_ENV}" and API_TIMEOUT: "${API_TIMEOUT}" in the frontend-config ConfigMap's data section, replacing unsupported bash-style default-value syntax with plain envsubst-compatible placeholders, exactly per the suggested fix.
🤖 Prompt for AI agents
In kubernetes/base/config.yaml around line 42, review and complete this code-review fix: Frontend ConfigMap uses inline shell-style default syntax not supported by envsubst.
What the draft fix changed: Changed `NODE_ENV: "${NODE_ENV:-production}"` and `API_TIMEOUT: "${API_TIMEOUT:-30000}"` to `NODE_ENV: "${NODE_ENV}"` and `API_TIMEOUT: "${API_TIMEOUT}"` in the `frontend-config` ConfigMap's `data` section, replacing unsupported bash-style default-value syntax with plain envsubst-compatible placeholders, exactly per the suggested fix.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer
| - name: SPRING_REDIS_HOST | ||
| value: "${REDIS_HOST}" | ||
| - name: SPRING_REDIS_PORT | ||
| value: "6379" | ||
| value: "${REDIS_PORT}" | ||
| - name: JAVA_MIN_HEAP | ||
| value: "${JAVA_MIN_HEAP}" | ||
| - name: JAVA_MAX_HEAP |
There was a problem hiding this comment.
🦩 🔴 backend-service.yaml hardcodes Redis port instead of using an envsubst placeholder
Changed the hardcoded value: "6379" to value: "${REDIS_PORT}" for the SPRING_REDIS_PORT env var in the Deployment container spec (kubernetes/base/backend-service.yaml, line 35), matching the envsubst placeholder pattern used for SPRING_REDIS_HOST and all other environment-specific values in the file. This is a mechanical templating change; the only unverified part is ensuring the deployment pipeline's envsubst variable list/defaults file is updated to include REDIS_PORT (defaulting to 6379) outside this file's scope.
🤖 Prompt for AI agents
In kubernetes/base/backend-service.yaml around line 35, review and complete this code-review fix: backend-service.yaml hardcodes Redis port instead of using an envsubst placeholder.
What the draft fix changed: Changed the hardcoded `value: "6379"` to `value: "${REDIS_PORT}"` for the `SPRING_REDIS_PORT` env var in the Deployment container spec (`kubernetes/base/backend-service.yaml`, line 35), matching the envsubst placeholder pattern used for `SPRING_REDIS_HOST` and all other environment-specific values in the file. This is a mechanical templating change; the only unverified part is ensuring the deployment pipeline's envsubst variable list/defaults file is updated to include `REDIS_PORT` (defaulting to 6379) outside this file's scope.
Verify the change is correct and complete; do not refactor unrelated code.
fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer
Closes 2 review findings across 2 files.
Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.
kubernetes/base/config.yaml:42kubernetes/base/backend-service.yaml:35What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.
Run: https://product-hub.flamingo.so/admin/code-review
Run id:
89424001-8db6-4ce6-8840-ab853de69d56Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.