[INFRA-372]: Add Plane-EE Helm chart configuration for PostgreSQL read replica support#222
Open
pratapalakshmi wants to merge 4 commits intomasterfrom
Open
[INFRA-372]: Add Plane-EE Helm chart configuration for PostgreSQL read replica support#222pratapalakshmi wants to merge 4 commits intomasterfrom
pratapalakshmi wants to merge 4 commits intomasterfrom
Conversation
…eplica support - Updated `Chart.yaml` to version 2.3.3. - Added `DATABASE_READ_REPLICA_URL` to `app-env.yaml` for conditional inclusion based on values. - Introduced `read_replica` configuration in `values.yaml` for PostgreSQL service.
Contributor
WalkthroughBumps the Plane Enterprise Helm chart to version 2.4.1 and adds PostgreSQL read-replica configuration: new values for ChangesRead-replica and chart version
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
28-30: Consider documenting that read-replica is remote-only.Unlike the
DATABASE_URLblock (Lines 20-26) which derives a URL from the in-cluster service whenlocal_setup=true, the new block has nolocal_setupbranch — read-replica only works when an explicitremote_urlis supplied. That seems intentional (in-cluster single-node Postgres can't serve as its own replica), but it's worth a short comment invalues.yamlnext toread_replica.remote_urlto set operator expectations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml` around lines 28 - 30, Add a short clarifying comment in values.yaml next to the services.postgres.read_replica.remote_url key stating that read-replica is remote-only (the chart/template only sets DATABASE_READ_REPLICA_URL when remote_url is provided and does not derive an in-cluster URL even with local_setup=true); reference the values key services.postgres.read_replica.remote_url and the template output variable DATABASE_READ_REPLICA_URL so operators understand they must supply a remote URL for replicas.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml`:
- Around line 28-30: Guard against enabled=true with an empty remote_url by
updating the template that emits DATABASE_READ_REPLICA_URL: check
.Values.services.postgres.read_replica.enabled and
.Values.services.postgres.read_replica.remote_url together: either only render
DATABASE_READ_REPLICA_URL when remote_url is non-empty, or add a fail-fast check
(e.g. {{- if and .Values.services.postgres.read_replica.enabled (not
.Values.services.postgres.read_replica.remote_url) }}{{ fail
"read_replica.enabled is true but read_replica.remote_url is empty" }}{{- end
}}) to stop chart rendering; ensure this is coordinated with ENABLE_READ_REPLICA
so the ConfigMap and Secret remain consistent (refer to
.Values.services.postgres.read_replica.enabled,
.Values.services.postgres.read_replica.remote_url, DATABASE_READ_REPLICA_URL and
ENABLE_READ_REPLICA).
In `@charts/plane-enterprise/values.yaml`:
- Around line 84-86: The inline comment on the remote_url entry under
read_replica contains trailing whitespace; edit the values for
read_replica.remote_url to remove the extra spaces after "URL ONLY" so the
comment ends immediately after the text (ensure symbol names: read_replica and
remote_url remain unchanged).
---
Nitpick comments:
In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml`:
- Around line 28-30: Add a short clarifying comment in values.yaml next to the
services.postgres.read_replica.remote_url key stating that read-replica is
remote-only (the chart/template only sets DATABASE_READ_REPLICA_URL when
remote_url is provided and does not derive an in-cluster URL even with
local_setup=true); reference the values key
services.postgres.read_replica.remote_url and the template output variable
DATABASE_READ_REPLICA_URL so operators understand they must supply a remote URL
for replicas.
🪄 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: 6e4fe427-4e0c-4586-9d92-0fae4932f121
📒 Files selected for processing (3)
charts/plane-enterprise/Chart.yamlcharts/plane-enterprise/templates/config-secrets/app-env.yamlcharts/plane-enterprise/values.yaml
pratapalakshmi
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
pratapalakshmi
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml`:
- Around line 1-3: The fail-fast validation for postgres read replica should
only run when the chart will generate the Secret (i.e. when
external_secrets.app_env_existingSecret is empty); update the condition around
the existing fail so it is gated by the existing-secret check—either wrap the
current block in the existing "{{- if empty
.Values.external_secrets.app_env_existingSecret }}" guard or add an extra "and
(empty .Values.external_secrets.app_env_existingSecret)" clause to the current
conditional that checks ".Values.services.postgres.read_replica.enabled" and
".Values.services.postgres.read_replica.remote_url", ensuring the
fail("read_replica.enabled is true but read_replica.remote_url is empty") only
triggers when app_env secret will be generated by the chart.
🪄 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: 757058a8-bf71-45cb-b3a1-05291fb4779f
📒 Files selected for processing (3)
charts/plane-enterprise/Chart.yamlcharts/plane-enterprise/templates/config-secrets/app-env.yamlcharts/plane-enterprise/values.yaml
✅ Files skipped from review due to trivial changes (1)
- charts/plane-enterprise/values.yaml
Comment on lines
+1
to
+3
| {{- if and .Values.services.postgres.read_replica.enabled (not .Values.services.postgres.read_replica.remote_url) }} | ||
| {{- fail "read_replica.enabled is true but read_replica.remote_url is empty" }} | ||
| {{- end }} |
Contributor
There was a problem hiding this comment.
Fail-fast validation fires even when an external secret is in use, blocking a valid configuration.
The validation is placed outside the {{- if empty .Values.external_secrets.app_env_existingSecret}} gate that controls Secret generation. A user who supplies external_secrets.app_env_existingSecret (their own pre-existing secret that already contains DATABASE_READ_REPLICA_URL) while setting read_replica.enabled = true and leaving remote_url = "" in values will get a hard render failure, even though their deployment is perfectly valid.
The guard should only apply when the chart is generating the Secret itself:
🛡️ Proposed fix
-{{- if and .Values.services.postgres.read_replica.enabled (not .Values.services.postgres.read_replica.remote_url) }}
-{{- fail "read_replica.enabled is true but read_replica.remote_url is empty" }}
-{{- end }}
+{{- if and (empty .Values.external_secrets.app_env_existingSecret) .Values.services.postgres.read_replica.enabled (not .Values.services.postgres.read_replica.remote_url) }}
+{{- fail "read_replica.enabled is true but read_replica.remote_url is empty" }}
+{{- end }}🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-enterprise/templates/config-secrets/app-env.yaml` around lines 1
- 3, The fail-fast validation for postgres read replica should only run when the
chart will generate the Secret (i.e. when
external_secrets.app_env_existingSecret is empty); update the condition around
the existing fail so it is gated by the existing-secret check—either wrap the
current block in the existing "{{- if empty
.Values.external_secrets.app_env_existingSecret }}" guard or add an extra "and
(empty .Values.external_secrets.app_env_existingSecret)" clause to the current
conditional that checks ".Values.services.postgres.read_replica.enabled" and
".Values.services.postgres.read_replica.remote_url", ensuring the
fail("read_replica.enabled is true but read_replica.remote_url is empty") only
triggers when app_env secret will be generated by the chart.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores