release:Plane-MCP-Server:v1.0.0 - #216
Conversation
* Bump application version to 2.5.0 in Chart.yaml * Update default Plane version to 2.5.0 in questions.yml, README.md, and values.yaml * Ensure documentation reflects the new version and configuration options
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded a Helm chart for the Plane MCP server. The chart includes configurable workloads, optional local Redis, ingress and certificate resources, installation documentation, interactive configuration, local rendering support, and chart preview and release workflow integration. ChangesPlane MCP Server chart
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant Helm
participant Kubernetes
participant MCPServer
participant Ingress
participant Redis
Operator->>Helm: Install plane-mcp-server chart
Helm->>Kubernetes: Create Secret, ServiceAccount, Service, and Deployment
alt Local Redis enabled
Helm->>Kubernetes: Create Redis StatefulSet and PVC
MCPServer->>Redis: Connect through release Redis Service
else External Redis configured
MCPServer->>Redis: Connect using external Redis URL
end
alt Ingress enabled
Helm->>Ingress: Create NGINX Ingress or Traefik IngressRoute
Ingress->>MCPServer: Route host traffic to port 8211
end
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
charts/plane-mcp-server/templates/_helpers.tpl (1)
1-3: Template name collision and hardcoded registry URL.Two concerns:
Template name collision: The
imagePullSecrettemplate name is also defined inplane-ceandplane-enterprisecharts. If these charts share a parent umbrella chart or are rendered together, the last-loaded definition wins silently. Consider namespacing the template name (e.g.,plane-mcp-server.imagePullSecret).Hardcoded registry: Unlike
plane-ce(uses.Values.dockerRegistry.host) andplane-enterprise(uses.Values.dockerRegistry.registry), this chart hardcodesindex.docker.io/v1/. This limits flexibility for users with private registries.♻️ Proposed fix to namespace template and make registry configurable
-{{- define "imagePullSecret" }} -{{- printf "{\"auths\":{\"index.docker.io/v1/\":{\"username\":\"%s\",\"password\":\"%s\"}}}" .Values.dockerRegistry.loginid .Values.dockerRegistry.password | b64enc }} +{{- define "plane-mcp-server.imagePullSecret" }} +{{- printf "{\"auths\":{\"%s\":{\"username\":\"%s\",\"password\":\"%s\"}}}" (.Values.dockerRegistry.registry | default "https://index.docker.io/v1/") .Values.dockerRegistry.loginid .Values.dockerRegistry.password | b64enc }} {{- end }}Then update
docker-registry.yamlto use{{ include "plane-mcp-server.imagePullSecret" . }}and adddockerRegistry.registrytovalues.yaml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/_helpers.tpl` around lines 1 - 3, The template "imagePullSecret" collides with other charts and hardcodes Docker Hub; rename/namespace the helper (e.g., define "plane-mcp-server.imagePullSecret" instead of "imagePullSecret") and change its hardcoded registry string to use a configurable value (e.g., .Values.dockerRegistry.registry or .Values.dockerRegistry.host) so external registries work; then update callers such as docker-registry.yaml to call the new helper via {{ include "plane-mcp-server.imagePullSecret" . }} and add dockerRegistry.registry to values.yaml with a sensible default.charts/plane-mcp-server/values.yaml (1)
40-40: Missing trailing newline.POSIX convention recommends files end with a newline character.
📝 Add trailing newline
external_redis_url: '' # INCASE OF REMOTE REDIS ONLY +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/values.yaml` at line 40, The file ends without a trailing newline which violates POSIX conventions; update the values.yaml so the final line containing the external_redis_url key (external_redis_url: '') is followed by a newline character at EOF (ensure the file ends with a single newline).charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml (2)
53-56: RemovecreationTimestamp: nullfrom PVC metadata.This field is typically auto-generated by Kubernetes and should not be specified in templates. It appears to be a remnant from
kubectl get -o yamloutput.Proposed fix
metadata: - creationTimestamp: null namespace: {{ .Release.Namespace }} name: pvc-{{ .Release.Name }}-redis-vol🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml` around lines 53 - 56, The PVC metadata contains a hard-coded creationTimestamp which should be removed; edit the resource block defining metadata for the PersistentVolumeClaim named pvc-{{ .Release.Name }}-redis-vol and delete the line "creationTimestamp: null" so the metadata section only includes namespace and name (allowing Kubernetes to auto-populate creationTimestamp).
37-49: Add security context and remove deprecated field.Trivy correctly flags that the container runs with default (root) privileges. For Redis/Valkey, consider adding a security context. Also,
serviceAccount(line 48) is deprecated—onlyserviceAccountNameis needed.Proposed fix to add security context and remove deprecated field
spec: + securityContext: + runAsNonRoot: true + runAsUser: 999 + fsGroup: 999 containers: - image: {{ .Values.services.redis.image }} imagePullPolicy: Always name: {{ .Release.Name }}-redis stdin: true tty: true + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: false volumeMounts: - mountPath: /data name: pvc-{{ .Release.Name }}-redis-vol subPath: '' - serviceAccount: {{ .Release.Name }}-srv-account serviceAccountName: {{ .Release.Name }}-srv-accountNote: The Valkey alpine image runs as user 999 by default, so
runAsUser: 999aligns with that.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml` around lines 37 - 49, Add a pod/container securityContext to run the Redis container as non-root and harden filesystem/capabilities and remove the deprecated serviceAccount field; specifically, inside the container spec for the container named "{{ .Release.Name }}-redis" add a securityContext with runAsUser: 999, runAsNonRoot: true, readOnlyRootFilesystem: true (or false if Redis needs writable /data — prefer making only /data writable via volume), and drop all capabilities (e.g., capabilities.drop: ["ALL"]) or at least NET_RAW/NET_ADMIN, and then remove the top-level serviceAccount field and keep only serviceAccountName: {{ .Release.Name }}-srv-account. Ensure changes target the same container block and keep imagePullPolicy/name/volumeMounts intact.charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml (3)
32-35: Removenamespacefrom pod template metadata.The
namespacefield in the pod template (line 33) is unnecessary and ignored by Kubernetes—pods inherit the namespace from the Deployment.Proposed fix
template: metadata: - namespace: {{ .Release.Namespace }} labels: app.name: {{ .Release.Namespace }}-{{ .Release.Name }}-api🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml` around lines 32 - 35, Remove the unnecessary namespace field from the pod template's metadata in the Deployment template: delete the line "namespace: {{ .Release.Namespace }}" under the pod template metadata block so pods inherit the Deployment's namespace; ensure surrounding indentation and the metadata.labels (app.name) remain intact in the template (look for the metadata block that contains app.name label in plane-mcp-server.deployment.yaml).
36-37: Timestamp annotation triggers pod recreation on everyhelm upgrade.The
timestamp: {{ now | quote }}annotation will have a new value on everyhelm upgrade, forcing pod recreation even when nothing else changes. If this is intentional (e.g., to pick up new ConfigMap/Secret changes), consider documenting it. Otherwise, remove it to enable stable deployments.If intentional, add a comment explaining the purpose. If not needed:
Proposed fix to remove forced redeployment
annotations: - timestamp: {{ now | quote }} + # Add specific annotations as needed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml` around lines 36 - 37, The deployment template currently sets an ever-changing annotation "timestamp: {{ now | quote }}" under the annotations block, which forces pod recreation on every helm upgrade; remove that line from the annotations (or replace it with a static/versioned annotation) to avoid unnecessary redeploys, or if the behavior is intentional, add an inline comment next to the annotations block explaining that the timestamp is used to deliberately trigger pod restarts to pick up ConfigMap/Secret changes.
38-57: Add security context and remove deprecatedserviceAccountfield.Trivy flags that the container runs with default (root) privileges. Consider adding a security context for defense in depth. Also,
serviceAccount(line 56) is deprecated—onlyserviceAccountNameis needed.Proposed fix
spec: + securityContext: + runAsNonRoot: true + runAsUser: 1000 + fsGroup: 1000 containers: - name: {{ .Release.Name }}-api imagePullPolicy: Always image: {{ .Values.services.api.image | default "makeplane/plane-mcp-server" }}:{{ .Values.dockerRegistry.default_tag | default "latest" }} stdin: true tty: true + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true resources: requests: memory: {{ .Values.services.api.memoryRequest | default "50Mi" | quote }} cpu: {{ .Values.services.api.cpuRequest | default "50m" | quote }} limits: memory: {{ .Values.services.api.memoryLimit | default "1000Mi" | quote }} cpu: {{ .Values.services.api.cpuLimit | default "500m" | quote }} envFrom: - secretRef: name: {{ .Release.Name }}-app-secrets optional: false - serviceAccount: {{ .Release.Name }}-srv-account serviceAccountName: {{ .Release.Name }}-srv-accountNote: Verify that the MCP server container image supports running as non-root before applying
runAsNonRoot: true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml` around lines 38 - 57, The deployment currently uses the deprecated serviceAccount field and has no container security context, so add a securityContext on the container spec for the {{ .Release.Name }}-api container (e.g., setting runAsNonRoot: true, runAsUser: <non-root-uid>, and readOnlyRootFilesystem: true / dropCapabilities as appropriate) and remove the deprecated top-level serviceAccount field while keeping serviceAccountName: {{ .Release.Name }}-srv-account; ensure you confirm the plane-mcp-server image supports non-root before enabling runAsNonRoot and choose a suitable non-root UID for runAsUser.charts/plane-mcp-server/questions.yml (1)
130-133: Consider using a placeholder email address.The default SSL email
"engineering@plane.so"should be changed to a generic placeholder like"admin@example.com"to avoid accidental use of the Plane team's email for certificate notifications.Proposed fix
- variable: ingress.ssl.email label: "SSL Email" type: string - default: "engineering@plane.so" + default: ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/questions.yml` around lines 130 - 133, Change the default value for the Helm chart question variable ingress.ssl.email from "engineering@plane.so" to a neutral placeholder such as "admin@example.com"; update the variable named ingress.ssl.email in questions.yml so the default no longer points to the Plane team email and uses the generic placeholder instead.charts/plane-mcp-server/templates/ingress/issuers-certs.yaml (1)
46-46: Remove trailing whitespace after document separator.The trailing space after
---on line 46 may cause issues with some YAML parsers and linters.Proposed fix
---- +---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml` at line 46, Remove the trailing space after the YAML document separator '---' in the ingress/issuers-certs.yaml template: locate the line that contains only '---' and delete any trailing whitespace characters so the separator is exactly '---' with no extra spaces, then re-run YAML linting to confirm the file parses cleanly.
🤖 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-mcp-server/questions.yml`:
- Around line 92-96: The default value for the variable
services.redis.external_redis_url is an invalid/incomplete URI ("redis://");
update the default to a safe placeholder such as an empty string "" (or a full
example like "redis://user:pass@hostname:6379/0") so consumers don’t attempt to
connect to an invalid URL; locate the variable named
services.redis.external_redis_url in questions.yml and replace the default
accordingly and, if present, adjust the label/help text to indicate it should be
a full Redis URL when provided.
In `@charts/plane-mcp-server/README.md`:
- Around line 44-46: The README's table documents dockerRegistry.loginid default
as "planeengineering" but the chart's values.yaml sets dockerRegistry.loginid to
"makeplane"; update the README entry for dockerRegistry.loginid to match
values.yaml (set the default to "makeplane") or vice versa if you intend to
change the actual default—ensure dockerRegistry.loginid in the README and
values.yaml are identical and reflect the intended default value.
- Around line 98-102: The README's Custom Ingress Routes table and Verify
section reference port 8000 but the ingress backend is configured to use port
8211; update all occurrences in README (the table row for mcp.example.com and
the Verify bullet that mentions Service `<release-name>-api` port 8000) to use
port 8211 so the docs match the ingress.yaml backend configuration.
In `@charts/plane-mcp-server/templates/config-secrets/app-env.yaml`:
- Around line 13-19: The REDIS_HOST and REDIS_PORT entries are unconditional but
should be tied to services.redis.local_setup like REDIS_URL; update the template
so REDIS_HOST and REDIS_PORT are emitted only when
.Values.services.redis.local_setup is true (same condition block used for
REDIS_URL) and leave them out when using an external redis URL (i.e., when
.Values.services.redis.local_setup is false); adjust the template around the
REDIS_HOST / REDIS_PORT keys to match the existing conditional logic for
REDIS_URL.
In `@charts/plane-mcp-server/templates/ingress/ingress.yaml`:
- Around line 24-27: The TLS hosts list is mis-indented so the host entry isn't
recognized as an item of the hosts array; adjust the indentation so the dash and
host template are indented under the hosts key (e.g., change the block under tls
to have hosts: on its line and the line with - {{ .Values.ingress.host | quote
}} indented as a list item under hosts), keeping secretName at the same
indentation level as the tls list item; update the tls block that references
.Values.ingress.host and {{ .Release.Name }}-ssl-cert accordingly.
- Around line 15-19: The Ingress backend currently nests the service name under
port (backend -> service -> port -> name); update the structure so that under
backend -> service you have two sibling fields: name: {{ .Release.Name }}-api
and port: with number: 8211 (i.e. move the name field out of port to be a direct
child of service) so the backend follows the networking.k8s.io/v1 schema.
In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml`:
- Line 11: The template currently supplies a risky placeholder for api-token via
.Values.ingress.ssl.token; remove the default and make Helm render fail fast
when the token is missing by using Helm's required validation for
.Values.ingress.ssl.token (so the template errors with a clear message instead
of deploying "default-api-token"), updating the api-token entry in the
ingress/issuers-certs.yaml template and associated references to rely on the
required value.
In `@charts/plane-mcp-server/templates/service-account.yaml`:
- Around line 1-10: The manifest has automountServiceAccountToken placed before
kind which invalidates the Kubernetes resource; reorder the keys so the
top-level order is apiVersion, kind, metadata, then
automountServiceAccountToken, and keep the conditional imagePullSecrets ({{- if
.Values.dockerRegistry.enabled }}) after those entries; update the template
around the ServiceAccount block (look for the ServiceAccount resource, the
automountServiceAccountToken field, metadata, imagePullSecrets and the
.Release.Name/.Values.dockerRegistry.enabled references) to follow that valid
structure.
In `@charts/plane-mcp-server/values.yaml`:
- Around line 30-34: The values.yaml currently defines plane_oauth with keys
provider_base_url, client_id, and client_secret but README documents
services.api.plane_oauth.base_url and redirect_uri; update the chart to match
the README (or update the README to match the chart). Specifically, either
rename provider_base_url to base_url and add a redirect_uri key under
plane_oauth in values.yaml (alongside client_id and client_secret), or change
the README reference services.api.plane_oauth.base_url to
services.api.plane_oauth.provider_base_url and remove/add redirect_uri guidance
as appropriate so the key names (plane_oauth.provider_base_url,
plane_oauth.client_id, plane_oauth.client_secret, plane_oauth.redirect_uri) are
consistent between docs and values.yaml.
- Line 11: The values.yaml defines ingressAnnotations but the Ingress template
(ingress.yaml) ignores it and instead hardcodes
nginx.ingress.kubernetes.io/proxy-body-size using clientMaxBodySize; update the
ingress.yaml template to merge and apply the ingressAnnotations map (while
preserving or overriding the existing
nginx.ingress.kubernetes.io/proxy-body-size derived from
.Values.clientMaxBodySize) so custom annotations from .Values.ingressAnnotations
are rendered on the Ingress, or remove the ingressAnnotations entry from
values.yaml if you choose not to support custom annotations.
---
Nitpick comments:
In `@charts/plane-mcp-server/questions.yml`:
- Around line 130-133: Change the default value for the Helm chart question
variable ingress.ssl.email from "engineering@plane.so" to a neutral placeholder
such as "admin@example.com"; update the variable named ingress.ssl.email in
questions.yml so the default no longer points to the Plane team email and uses
the generic placeholder instead.
In `@charts/plane-mcp-server/templates/_helpers.tpl`:
- Around line 1-3: The template "imagePullSecret" collides with other charts and
hardcodes Docker Hub; rename/namespace the helper (e.g., define
"plane-mcp-server.imagePullSecret" instead of "imagePullSecret") and change its
hardcoded registry string to use a configurable value (e.g.,
.Values.dockerRegistry.registry or .Values.dockerRegistry.host) so external
registries work; then update callers such as docker-registry.yaml to call the
new helper via {{ include "plane-mcp-server.imagePullSecret" . }} and add
dockerRegistry.registry to values.yaml with a sensible default.
In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml`:
- Line 46: Remove the trailing space after the YAML document separator '---' in
the ingress/issuers-certs.yaml template: locate the line that contains only
'---' and delete any trailing whitespace characters so the separator is exactly
'---' with no extra spaces, then re-run YAML linting to confirm the file parses
cleanly.
In
`@charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml`:
- Around line 32-35: Remove the unnecessary namespace field from the pod
template's metadata in the Deployment template: delete the line "namespace: {{
.Release.Namespace }}" under the pod template metadata block so pods inherit the
Deployment's namespace; ensure surrounding indentation and the metadata.labels
(app.name) remain intact in the template (look for the metadata block that
contains app.name label in plane-mcp-server.deployment.yaml).
- Around line 36-37: The deployment template currently sets an ever-changing
annotation "timestamp: {{ now | quote }}" under the annotations block, which
forces pod recreation on every helm upgrade; remove that line from the
annotations (or replace it with a static/versioned annotation) to avoid
unnecessary redeploys, or if the behavior is intentional, add an inline comment
next to the annotations block explaining that the timestamp is used to
deliberately trigger pod restarts to pick up ConfigMap/Secret changes.
- Around line 38-57: The deployment currently uses the deprecated serviceAccount
field and has no container security context, so add a securityContext on the
container spec for the {{ .Release.Name }}-api container (e.g., setting
runAsNonRoot: true, runAsUser: <non-root-uid>, and readOnlyRootFilesystem: true
/ dropCapabilities as appropriate) and remove the deprecated top-level
serviceAccount field while keeping serviceAccountName: {{ .Release.Name
}}-srv-account; ensure you confirm the plane-mcp-server image supports non-root
before enabling runAsNonRoot and choose a suitable non-root UID for runAsUser.
In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml`:
- Around line 53-56: The PVC metadata contains a hard-coded creationTimestamp
which should be removed; edit the resource block defining metadata for the
PersistentVolumeClaim named pvc-{{ .Release.Name }}-redis-vol and delete the
line "creationTimestamp: null" so the metadata section only includes namespace
and name (allowing Kubernetes to auto-populate creationTimestamp).
- Around line 37-49: Add a pod/container securityContext to run the Redis
container as non-root and harden filesystem/capabilities and remove the
deprecated serviceAccount field; specifically, inside the container spec for the
container named "{{ .Release.Name }}-redis" add a securityContext with
runAsUser: 999, runAsNonRoot: true, readOnlyRootFilesystem: true (or false if
Redis needs writable /data — prefer making only /data writable via volume), and
drop all capabilities (e.g., capabilities.drop: ["ALL"]) or at least
NET_RAW/NET_ADMIN, and then remove the top-level serviceAccount field and keep
only serviceAccountName: {{ .Release.Name }}-srv-account. Ensure changes target
the same container block and keep imagePullPolicy/name/volumeMounts intact.
In `@charts/plane-mcp-server/values.yaml`:
- Line 40: The file ends without a trailing newline which violates POSIX
conventions; update the values.yaml so the final line containing the
external_redis_url key (external_redis_url: '') is followed by a newline
character at EOF (ensure the file ends with a single newline).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b9ad3705-9634-44e3-9077-7bc86aef3650
📒 Files selected for processing (13)
charts/plane-mcp-server/.helmignorecharts/plane-mcp-server/Chart.yamlcharts/plane-mcp-server/README.mdcharts/plane-mcp-server/questions.ymlcharts/plane-mcp-server/templates/_helpers.tplcharts/plane-mcp-server/templates/config-secrets/app-env.yamlcharts/plane-mcp-server/templates/config-secrets/docker-registry.yamlcharts/plane-mcp-server/templates/ingress/ingress.yamlcharts/plane-mcp-server/templates/ingress/issuers-certs.yamlcharts/plane-mcp-server/templates/service-account.yamlcharts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yamlcharts/plane-mcp-server/templates/workloads/redis-statefulset.yamlcharts/plane-mcp-server/values.yaml
* Update test.sh to include option for Plane-MCP-Server in Helm chart selection. * Modify chart-preview.yml to add build step for Plane-MCP-Server and update environment variables accordingly. * Enhance chart-releaser.yml to support building and publishing Plane-MCP-Server, including README handling and conditional logic for setup.
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 @.github/workflows/chart-preview.yml:
- Around line 126-145: The helm package invocation in the Build Plane-MCP-Server
step uses unquoted shell expansions which can cause word-splitting (SC2086);
update the helm package command (the line invoking "helm package --sign --key
\"$CR_KEY\" --keyring $CR_KEYRING --passphrase-file \"$CR_PASSPHRASE_FILE\"
charts/$CHART_REPO -u -d ${{ env.EXPORT_DIR }}/${{env.CHART_REPO}}/charts") to
quote the $CR_KEYRING, the charts path (charts/$CHART_REPO) and the destination
path (${{ env.EXPORT_DIR }}/${{env.CHART_REPO}}/charts), and apply the same
quoting pattern to the other two helm package lines in the file to prevent
word-splitting.
🪄 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: 6c40683f-6a83-4516-b551-1d26358fbbc7
📒 Files selected for processing (3)
.github/workflows/chart-preview.yml.github/workflows/chart-releaser.ymltest.sh
|
Howdy, just wanted to bump this PR as:
Thanks! |
- Fix invalid ServiceAccount YAML (automountServiceAccountToken before kind)
- Fix Ingress backend service structure (name/port sibling order)
- Fix TLS hosts list indentation
- Apply ingressAnnotations from values in ingress template
- Use required helper for DNS01 issuer api-token (drop "default-api-token" placeholder)
- Namespace imagePullSecret helper to plane-mcp-server.imagePullSecret
- Fix questions.yml external_redis_url default ("redis://" → "")
- Fix README: loginid default (planeengineering → makeplane), port 8000 → 8211, OAuth field names
- Bump chart version to 1.0.1
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…h other charts Other charts (plane-ce, plane-enterprise) only set REDIS_URL conditionally based on local_setup — they do not set REDIS_HOST or REDIS_PORT. Align plane-mcp-server to the same pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… to match other charts" This reverts commit a2c82d0.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
charts/plane-mcp-server/templates/ingress/issuers-certs.yaml (1)
16-59: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse
access-tokenfor the DigitalOak DNS01 Secret key.The DigitalOcean solver expects the token Secret key to store the API token for TXT-record validation, but this Certificate references
api-token. The Issuer solver configuration otherwise matches the Ingress TLS Secret:name: {{ .Release.Name }}-ssl-certand the host from{{ .Values.ingress.host }}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml` around lines 16 - 59, The DigitalOcean branch of the Issuer solver currently references the wrong Secret key. Update the tokenSecretRef under the digitalocean solver to use the access-token key, while leaving the Cloudflare api-token reference and Certificate configuration unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/plane-mcp-server/templates/ingress/ingress.yaml`:
- Around line 9-10: Update the ingress template around ingressAnnotations so
nginx.ingress.kubernetes.io/proxy-body-size is rendered only once. Remove the
duplicate default path or merge defaults with user-provided annotations before
the toYaml/nindent output, preserving the configured 10m default and allowing
user overrides.
In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml`:
- Line 11: Validate the issuer value against the supported allowlist of
cloudflare, digitalocean, and http before rendering any resources. Add this
validation to the shared ingress certificate flow so unsupported values are
rejected before the Secret, Issuer, or Certificate templates render, while
preserving the existing token requirement for DNS issuers.
---
Outside diff comments:
In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml`:
- Around line 16-59: The DigitalOcean branch of the Issuer solver currently
references the wrong Secret key. Update the tokenSecretRef under the
digitalocean solver to use the access-token key, while leaving the Cloudflare
api-token reference and Certificate configuration unchanged.
🪄 Autofix
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 Plus
Run ID: 9deffd2b-4205-4978-8b2e-3b63f4a3c90f
📒 Files selected for processing (8)
charts/plane-mcp-server/Chart.yamlcharts/plane-mcp-server/README.mdcharts/plane-mcp-server/questions.ymlcharts/plane-mcp-server/templates/_helpers.tplcharts/plane-mcp-server/templates/config-secrets/docker-registry.yamlcharts/plane-mcp-server/templates/ingress/ingress.yamlcharts/plane-mcp-server/templates/ingress/issuers-certs.yamlcharts/plane-mcp-server/templates/service-account.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- charts/plane-mcp-server/templates/_helpers.tpl
- charts/plane-mcp-server/Chart.yaml
- charts/plane-mcp-server/templates/service-account.yaml
- charts/plane-mcp-server/README.md
- charts/plane-mcp-server/questions.yml
- charts/plane-mcp-server/templates/config-secrets/docker-registry.yaml
| name: {{ .Release.Name }}-issuer-api-token-secret | ||
| type: Opaque | ||
| stringData: | ||
| api-token: {{ required "ingress.ssl.token is required for DNS01 issuers (cloudflare/digitalocean)" .Values.ingress.ssl.token | quote }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 '\.Values\.ingress\.ssl\.issuer|cloudflare|digitalocean|http' \
charts/plane-mcp-server/templates/ingress/issuers-certs.yaml \
charts/plane-mcp-server/values.yaml \
charts/plane-mcp-server/README.mdRepository: makeplane/helm-charts
Length of output: 11786
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- chart files ---'
git ls-files 'charts/plane-mcp-server/templates/ingress/*' 'charts/plane-mcp-server/templates/*'
printf '\n%s\n' '--- issuer/cert template ---'
cat -n charts/plane-mcp-server/templates/ingress/issuers-certs.yaml
printf '\n%s\n' '--- ingress-related templates ---'
for f in $(git ls-files 'charts/plane-mcp-server/templates' | grep -E '(ingress|issuer|cert|yaml|yml)'); do
printf '\n## %s\n' "$f"
rg -n -C 3 'cert-manager|Issuer|Certificate|Ingress|ingress\.ssl|api-token-token|dns01|http01|cloudflare|digitalocean' "$f" || true
done
printf '\n%s\n' '--- all issuer references ---'
rg -n '\.Values\.ingress\.ssl\.issuer' charts/plane-mcp-server/templates || true
printf '\n%s\n' '--- all issuer allowlist/typo mentions (case-insensitive) ---'
rg -n -i 'issuer|invalid|require|allowed|cloudflare|digitalocean' charts/plane-mcp-server/templates charts/plane-mcp-server/values.yaml charts/plane-mcp-server/README.md || true
printf '\n%s\n' '--- Helm tool availability ---'
command -v helm || trueRepository: makeplane/helm-charts
Length of output: 11588
Validate unsupported issuer values before rendering DNS secrets.
issuers-certs.yaml requires ingress.ssl.token for every issuer except http, while the Issuer solvers only handle cloudflare, digitalocean, and http. A typo like cloudfare would fail the Secret required check but still render an Issuer with no DNS solver. Add explicit allowlist validation and apply it before the Secret, Issuer, and Certificate resources.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-mcp-server/templates/ingress/issuers-certs.yaml` at line 11,
Validate the issuer value against the supported allowlist of cloudflare,
digitalocean, and http before rendering any resources. Add this validation to
the shared ingress certificate flow so unsupported values are rejected before
the Secret, Issuer, or Certificate templates render, while preserving the
existing token requirement for DNS issuers.
… and update versioning
…ce ingress support for Traefik, and update README
…ADME.md, questions.yml, deployment template, and values.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/plane-mcp-server/questions.yml (2)
84-88: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftWire external Redis to
REDIS_HOSTandREDIS_PORT.When
services.redis.local_setup=false, the template emits onlyREDIS_URL. The app usesREDIS_HOSTandREDIS_PORTfor Redis storage; without them, it falls back toMemoryStore, so OAuth tokens are lost on restart and are not shared across replicas. Parse.Values.services.redis.external_redis_urlinto supported variables, or add explicitREDIS_URLsupport in the app. Fail rendering when the external URL is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/plane-mcp-server/questions.yml` around lines 84 - 88, Update the Redis configuration template controlled by services.redis.local_setup and external_redis_url so external Redis deployments provide the REDIS_HOST and REDIS_PORT values consumed by the app, parsing them from the configured URL or adding equivalent application support for REDIS_URL. Ensure rendering fails when local_setup is false and external_redis_url is empty, while preserving the local Redis configuration path.
40-43: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winAvoid defaulting to empty OAuth credentials in disabled OAuth mode.
values.yamlleaves.enabled,.client_id,.client_secret, and.provider_base_urlblank when OAuth is disabled, whileapp-env.yamlrenders those fields to the running secret as empty strings. Formakeplane/plane-mcp-server:v0.2.11, HTTP mode initializesPlaneOAuthProvideron startup and rejects missing OAuth credentials. Add a non-OAuth default for disabled OAuth or require valid OAuth credentials when it is enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/plane-mcp-server/questions.yml` around lines 40 - 43, Update the OAuth configuration defaults used by app-env.yaml and values.yaml so disabled OAuth does not render empty credentials that trigger PlaneOAuthProvider initialization failure. Provide a valid non-OAuth default for disabled mode, while ensuring enabled mode requires non-empty enabled, client_id, client_secret, and provider_base_url values.
🧹 Nitpick comments (1)
charts/plane-mcp-server/values.yaml (1)
24-24: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPin the default MCP image.
services.api.tag: latestis mutable. The Deployment usesimagePullPolicy: Alwaysand a changingtimestamp, so a Helm upgrade can roll out an unreviewed image. Pin the default to a tested release tag or an immutable digest. Keep the same value inquestions.yml.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@charts/plane-mcp-server/values.yaml` at line 24, Update the default services.api.tag in values.yaml from latest to a tested, immutable release tag or image digest, and set the matching value in questions.yml. Preserve the existing image configuration and ensure both defaults remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/plane-mcp-server/questions.yml`:
- Line 108: Update the show_if condition for the maxRequestBodyBytes question to
match the Traefik template behavior by supporting ingressClass values that start
with “traefik”; alternatively, change the Traefik resource templates to require
the exact “traefik” value. Keep the UI and template conditions consistent for
values such as traefik-internal.
---
Outside diff comments:
In `@charts/plane-mcp-server/questions.yml`:
- Around line 84-88: Update the Redis configuration template controlled by
services.redis.local_setup and external_redis_url so external Redis deployments
provide the REDIS_HOST and REDIS_PORT values consumed by the app, parsing them
from the configured URL or adding equivalent application support for REDIS_URL.
Ensure rendering fails when local_setup is false and external_redis_url is
empty, while preserving the local Redis configuration path.
- Around line 40-43: Update the OAuth configuration defaults used by
app-env.yaml and values.yaml so disabled OAuth does not render empty credentials
that trigger PlaneOAuthProvider initialization failure. Provide a valid
non-OAuth default for disabled mode, while ensuring enabled mode requires
non-empty enabled, client_id, client_secret, and provider_base_url values.
---
Nitpick comments:
In `@charts/plane-mcp-server/values.yaml`:
- Line 24: Update the default services.api.tag in values.yaml from latest to a
tested, immutable release tag or image digest, and set the matching value in
questions.yml. Preserve the existing image configuration and ensure both
defaults remain identical.
🪄 Autofix
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 Plus
Run ID: bcd2c798-6308-42a0-a0e2-1014b0fb6049
📒 Files selected for processing (15)
.github/workflows/chart-preview.ymlcharts/plane-mcp-server/Chart.yamlcharts/plane-mcp-server/README.mdcharts/plane-mcp-server/questions.ymlcharts/plane-mcp-server/templates/_helpers.tplcharts/plane-mcp-server/templates/config-secrets/app-env.yamlcharts/plane-mcp-server/templates/config-secrets/docker-registry.yamlcharts/plane-mcp-server/templates/ingress/ingress.yamlcharts/plane-mcp-server/templates/ingress/issuers-certs.yamlcharts/plane-mcp-server/templates/ingress/nginx-ingress.yamlcharts/plane-mcp-server/templates/ingress/traefik-ingress.yamlcharts/plane-mcp-server/templates/ingress/traefik-middleware.yamlcharts/plane-mcp-server/templates/service-account.yamlcharts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yamlcharts/plane-mcp-server/values.yaml
💤 Files with no reviewable changes (1)
- charts/plane-mcp-server/templates/service-account.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/plane-mcp-server/templates/ingress/issuers-certs.yaml
- charts/plane-mcp-server/templates/config-secrets/app-env.yaml
- .github/workflows/chart-preview.yml
- charts/plane-mcp-server/Chart.yaml
- charts/plane-mcp-server/README.md
…ntroller and enhance documentation
| provider_base_url: '' | ||
|
|
||
| redis: | ||
| local_setup: true |
| email: engineering@plane.so | ||
|
|
||
| services: | ||
| storage_class: '' |
There was a problem hiding this comment.
move storage_class to redis block
| traefik: | ||
| maxRequestBodyBytes: 10485760 # in bytes (default: 10 MiB) | ||
| # nginx-specific options (only used when ingressClass does not start with "traefik") | ||
| ingressAnnotations: { nginx.ingress.kubernetes.io/proxy-body-size: "10m" } |
There was a problem hiding this comment.
change the way
ingressController: 'traefik`
traefix:
ingressClassName: 'traefik'
maxRequestBodyBytes: 10485760
annotations: {}
nginx:
ingressClassName: 'nginx'
annotations: { nginx.ingress.kubernetes.io/proxy-body-size: "10m" }
| PLANE_INTERNAL_BASE_URL: {{ .Values.services.api.plane_internal_base_url | quote }} | ||
| {{- if .Values.services.redis.local_setup }} | ||
| REDIS_HOST: {{ .Release.Name }}-redis | ||
| REDIS_PORT: "6379" |
| @@ -0,0 +1 @@ | |||
| {{- /* Ingress rendering is split into nginx-ingress.yaml and traefik-ingress.yaml */ -}} | |||
| @@ -0,0 +1,24 @@ | |||
| {{- if and .Values.ingress.enabled .Values.ingress.host (eq .Values.ingress.ingressController "traefik") }} | |||
|
|
|||
There was a problem hiding this comment.
move the middleware code in the same file
| @@ -0,0 +1,12 @@ | |||
| {{- if and .Values.ingress.enabled (eq .Values.ingress.ingressController "traefik") }} | |||
There was a problem hiding this comment.
move this code to traefik-ingress.yaml
- Restructure ingress block with separate traefik/nginx sub-blocks - Move storage_class into redis block - Add redis password support with --requirepass and password-embedded REDIS_URL - Merge traefik middleware into traefik-ingress.yaml - Delete stub ingress.yaml and traefik-middleware.yaml - Bump chart version to 1.0.1, appVersion to v0.2.11 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/plane-mcp-server/README.md`:
- Around line 79-80: Update the ingress configuration table to document the
nested ingress.nginx.ingressClassName key consumed by the NGINX template,
replacing the misleading ingress.ingressClass reference. Keep the Traefik
cert-manager HTTP01 solver setting documented separately.
In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml`:
- Around line 44-48: Update the Redis configuration around the StatefulSet args
so services.redis.password is stored in a Kubernetes Secret rather than rendered
directly in the PodSpec. Expose the Secret value through a Secret-backed
environment variable and reference that variable in the --requirepass argument,
preserving conditional password configuration.
- Line 68: Update the Redis StatefulSet template around storageClassName so the
field is rendered only when .Values.services.redis.storage_class is non-empty;
preserve the quoted configured value when provided, and omit the entire
storageClassName key otherwise so Kubernetes can select its default
StorageClass.
In `@charts/plane-mcp-server/values.yaml`:
- Line 41: Update the Redis URL construction in the app-env template to
percent-encode .Values.services.redis.password before placing it in URI
userinfo, preserving correct parsing for reserved characters while retaining the
existing empty-password behavior.
🪄 Autofix
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 Plus
Run ID: 03687882-59e4-4a8a-b5bc-71d69307d534
📒 Files selected for processing (10)
charts/plane-mcp-server/Chart.yamlcharts/plane-mcp-server/README.mdcharts/plane-mcp-server/questions.ymlcharts/plane-mcp-server/templates/config-secrets/app-env.yamlcharts/plane-mcp-server/templates/ingress/issuers-certs.yamlcharts/plane-mcp-server/templates/ingress/nginx-ingress.yamlcharts/plane-mcp-server/templates/ingress/traefik-ingress.yamlcharts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yamlcharts/plane-mcp-server/templates/workloads/redis-statefulset.yamlcharts/plane-mcp-server/values.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- charts/plane-mcp-server/Chart.yaml
- charts/plane-mcp-server/templates/ingress/issuers-certs.yaml
- charts/plane-mcp-server/templates/config-secrets/app-env.yaml
- charts/plane-mcp-server/templates/workloads/plane-mcp-server.deployment.yaml
- charts/plane-mcp-server/questions.yml
| | ingress.ingressController | traefik | | Ingress controller type. Allowed: `nginx`, `traefik`. When set to `traefik`, a native Traefik `IngressRoute` CRD is rendered. Any other value renders a standard `networking.k8s.io/v1 Ingress`. | | ||
| | ingress.ingressClass | traefik | Yes | The `ingressClassName` set on the Kubernetes `Ingress` resource (nginx only). For Traefik, also used by cert-manager http01 challenge solvers. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document the NGINX ingress class key used by the template.
The NGINX template reads .Values.ingress.nginx.ingressClassName, but this table tells users to set ingress.ingressClass. The documented setting will not control the rendered spec.ingressClassName. Document the nested key here, or change the template to consume ingress.ingressClass. Keep any cert-manager-specific setting documented separately.
📝 Proposed documentation fix
-| ingress.ingressClass | traefik | Yes | The `ingressClassName` set on the Kubernetes `Ingress` resource (nginx only). For Traefik, also used by cert-manager http01 challenge solvers. |
+| ingress.nginx.ingressClassName | nginx | Yes | The `ingressClassName` rendered on the NGINX `Ingress`. Document `ingress.ingressClass` separately if cert-manager uses it. |🧰 Tools
🪛 LanguageTool
[grammar] ~79-~79: Ensure spelling is correct
Context: ...ress controller type. Allowed: nginx, traefik. When set to traefik, a native Traefi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-mcp-server/README.md` around lines 79 - 80, Update the ingress
configuration table to document the nested ingress.nginx.ingressClassName key
consumed by the NGINX template, replacing the misleading ingress.ingressClass
reference. Keep the Traefik cert-manager HTTP01 solver setting documented
separately.
| {{- if .Values.services.redis.password }} | ||
| args: | ||
| - --requirepass | ||
| - {{ .Values.services.redis.password | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find redis template and values =="
fd -a 'redis-statefulset\.yaml|values.*\.ya?ml' . | sed 's#^\./##' | sort
echo
echo "== redis-statefulset.yaml outline/size =="
wc -l charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml
cat -n charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml | sed -n '1,130p'
echo
echo "== Search for services.redis.password references =="
rg -n "services\.redis\.(password|storage_class|storage|storageClass|storageClassName)|redis-statefulset|requirespass|requirepass" charts -S || true
echo
echo "== Helm dependency and chart version context if present =="
fd -a 'Chart.yaml|requirements.yaml' charts -x sh -c 'echo "--- $1"; cat "$1"' sh {}Repository: makeplane/helm-charts
Length of output: 5362
🌐 Web query:
Kubernetes apps/v1 StatefulSet pod template spec container command args cannot include secrets environment variables documentation
💡 Result:
There is no restriction preventing Kubernetes StatefulSets (or other Pod-based workloads) from using Secrets in container command or args fields [1][2]. While you cannot reference a Secret directly within the command or args fields, you can achieve this by first exposing the Secret as an environment variable and then referencing that variable within the command or args string [1][3]. To use a Secret value in your command or arguments: 1. Define an environment variable in your container specification that pulls the value from a Secret using secretKeyRef [4][3]. 2. Reference that environment variable in your command or args field using the Kubernetes substitution syntax
Citations:
- 1: https://kubernetes.io/docs/tasks/inject-data-application/define-command-argument-container/
- 2: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/?fireglass_rsn=true
- 3: https://stackoverflow.com/questions/50248525/is-there-a-way-to-put-kubernetes-secret-value-in-args-field-of-yaml-file
- 4: https://kubernetes.io/docs/concepts/configuration/secret/
- 5: https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/
- 6: https://registry.terraform.io/providers/hashicorp/kubernetes/latest/docs/resources/stateful_set
- 7: https://kubernetes.website.cncfstack.com/docs/tasks/inject-data-application/define-command-argument-container/
Keep the Redis password out of the StatefulSet manifest.
When services.redis.password is set, line 47 renders the credential directly in spec.template.spec.containers[].args. Store the Redis password in a Kubernetes Secret and pass it through a Secret-based configuration or a Secret-backed environment variable plus $(VAR_NAME) argument. The StatefulSet PodSpec is exposed in the Kubernetes object specification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml` around
lines 44 - 48, Update the Redis configuration around the StatefulSet args so
services.redis.password is stored in a Kubernetes Secret rather than rendered
directly in the PodSpec. Expose the Secret value through a Secret-backed
environment variable and reference that variable in the --requirepass argument,
preserving conditional password configuration.
| resources: | ||
| requests: | ||
| storage: {{ .Values.services.redis.volume_size | default "1Gi" | quote }} | ||
| storageClassName: {{ .Values.services.redis.storage_class | quote }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
helm template plane-mcp-server charts/plane-mcp-server \
--set ingress.enabled=false |
rg -n 'storageClassName: ""'Repository: makeplane/helm-charts
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git files around relevant chart =="
git ls-files | rg '(^|/)charts/plane-mcp-server/templates/workloads/redis-statefulset\.yaml$|(^|/)charts/plane-mcp-server/values\.yaml$|(^|/)charts/plane-mcp-server/templates/.*redis.*\.yaml$'
echo
echo "== redis-statefulset excerpt =="
cat -n charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml | sed -n '1,120p'
echo
echo "== values storage class occurrences =="
rg -n 'storage_class|storageClassName|local_setup|redis' charts/plane-mcp-server/values.yaml charts/plane-mcp-server/templates -g '*.yaml' -g '*.yml'Repository: makeplane/helm-charts
Length of output: 6144
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== list Helm charts containing Redis StatefulSet templates =="
git ls-files | rg 'redis-statefulset\.yaml$|values\.yaml$' | sed -n '1,120p'
echo
echo "== static Helm expression evidence =="
python3 - <<'PY'
from pathlib import Path
p = Path("charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml")
s = p.read_text()
checks = {
"contains_template_storageClassName": "storageClassName: {{ .Values.services.redis.storage_class | quote }}" in s,
"default_storage_class_in_values": any("storage_class: ''" in line or "storage_class: \"\"" in line for line in Path("charts/plane-mcp-server/values.yaml").read_text().splitlines()),
}
for k, v in checks.items():
print(f"{k}={v}")
print("\nTemplate line:")
for i, line in enumerate(s.splitlines(), 1):
if "storageClassName" in line:
print(f"{i}: {line}")
PYRepository: makeplane/helm-charts
Length of output: 599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)charts/plane-mcp-server/templates/workloads/redis-statefulset\.yaml$|(^|/)charts/plane-mcp-server/values\.yaml$|(^|/)charts/plane-mcp-server/templates/.*redis.*\.yaml$' || true
echo
echo "== relevant manifests and values snippets =="
for f in charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml charts/plane-mcp-server/values.yaml; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n "storageClassName|storage_class|redis|local_setup|serviceName|persistentVolume|persistence" "$f" -C 2 || true
fi
done
echo
echo "== template expression evidence =="
python3 - <<'PY'
from pathlib import Path
for path in ["charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml", "charts/plane-mcp-server/values.yaml"]:
p=Path(path)
if p.exists():
print(f"\n[{path}]")
lines=p.read_text().splitlines()
for i,l in enumerate(lines,1):
if "storageClassName" in l or "storage_class" in l:
print(f"{i}: {l}")
PYRepository: makeplane/helm-charts
Length of output: 2860
🌐 Web query:
Kubernetes PersistentVolumeClaim storageClassName empty string default StorageClass
💡 Result:
In Kubernetes, the storageClassName field of a PersistentVolumeClaim (PVC) behaves differently depending on whether it is unset (null) or explicitly set to an empty string (""). 1. If storageClassName is unset (null): If your cluster has a default StorageClass defined (marked with the storageclass.kubernetes.io/is-default-class: "true" annotation), the cluster's control plane will automatically assign that default StorageClass to your PVC [1][2][3]. Thanks to the "Retroactive Default StorageClass" feature (GA as of Kubernetes v1.28), this assignment can happen even after the PVC has been created [4][5]. If no default StorageClass exists, the PVC remains unbound until a matching PersistentVolume (PV) becomes available or a default is later configured [1][6]. 2. If storageClassName is explicitly set to an empty string (""): This instructs Kubernetes that you do not want to use any StorageClass (specifically, it disables dynamic provisioning for that claim) [2][7][6]. A PVC with storageClassName: "" will only bind to a PersistentVolume that also has no storageClassName [2][6]. Crucially, if you set this value to "", the control plane will not overwrite it with the cluster's default StorageClass, even if one is present [1][2][6]. In summary, setting storageClassName: "" is the standard way to opt out of default StorageClass assignment and explicitly request a volume that has no class [2][6]. An unset field defaults to "give me the default class if one exists" [4].
Citations:
- 1: https://kubernetes.io/docs/concepts/storage/storage-classes/
- 2: https://kubernetes.io/docs/concepts/storage/persistent-volumes/
- 3: https://kubernetes.io/docs/concepts/storage/dynamic-provisioning/
- 4: https://kubernetes.io/blog/2023/01/05/retroactive-default-storage-class/
- 5: https://kubernetes.io/blog/2023/08/18/retroactive-default-storage-class-ga/
- 6: https://github.com/kubernetes/website/blob/master/content/en/docs/concepts/storage/persistent-volumes.md
- 7: https://kubernetes.io/blog/2017/03/dynamic-provisioning-and-storage-classes-kubernetes/
Omit storageClassName when storage_class is empty.
services.redis.storage_class defaults to '', so the StatefulSet template renders storageClassName: "". That disables default StorageClass use for the Redis PVC; only omitting the field allows Kubernetes to select the default StorageClass.
Proposed rendering fix
- storageClassName: {{ .Values.services.redis.storage_class | quote }}
+ {{ if .Values.services.redis.storage_class }}
+ storageClassName: {{ .Values.services.redis.storage_class | quote }}
+ {{ end }}📝 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.
| storageClassName: {{ .Values.services.redis.storage_class | quote }} | |
| {{ if .Values.services.redis.storage_class }} | |
| storageClassName: {{ .Values.services.redis.storage_class | quote }} | |
| {{ end }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml` at line
68, Update the Redis StatefulSet template around storageClassName so the field
is rendered only when .Values.services.redis.storage_class is non-empty;
preserve the quoted configured value when provided, and omit the entire
storageClassName key otherwise so Kubernetes can select its default
StorageClass.
| image: valkey/valkey:7.2.11-alpine | ||
| storage_class: '' | ||
| volume_size: 500Mi | ||
| password: '' # sets --requirepass on local Redis and embeds password in REDIS_URL (redis://:password@host:6379/) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
helm template plane-mcp-server charts/plane-mcp-server \
--set ingress.enabled=false \
--set-string 'services.redis.password=p@ss/#?%word' |
rg -n -A1 'REDIS_URL:'Repository: makeplane/helm-charts
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked chart files:"
git ls-files charts/plane-mcp-server | sed -n '1,120p'
echo
echo "Relevant values.yaml section:"
sed -n '35,48p' charts/plane-mcp-server/values.yaml
echo
echo "REDIS_URL usages:"
rg -n "REDIS_URL|services\.redis|redis://|password" charts/plane-mcp-server -g '*.yaml'Repository: makeplane/helm-charts
Length of output: 2959
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from urllib.parse import uses_authority
# Mirror URI segment encoding semantics for the relevant password characters.
reserved_delims = ("@", "/", "#", "?", "%")
raw = "p@ss/#?%word"
encoded = ""
for ch in raw:
encoded += "%" + format(ord(ch), "2x") if ch in reserved_delims else ch
print("raw_password:", raw)
print("encoded_password:", encoded)
for scheme in ("redis", "rediss"):
url = f"{scheme}://:{raw}`@host`:6379/"
print("\nurl:", url)
# Python's parser cannot produce a valid userinfo/URL for these inputs, so show that split/parsing is fragile.
# We only inspect the string to demonstrate URL structure without executing repository code.
if "@" in url:
userinfo, hostpart = url.split("//", maxsplit=1)[1].split("@", maxsplit=1)
print("userinfo:", userinfo)
print("hostpart:", hostpart)
PYRepository: makeplane/helm-charts
Length of output: 340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "app-env.yaml relevant section:"
sed -n '1,30p' charts/plane-mcp-server/templates/config-secrets/app-env.yaml
echo
echo "redis-statefulset.yaml relevant section:"
sed -n '35,55p' charts/plane-mcp-server/templates/workloads/redis-statefulset.yamlRepository: makeplane/helm-charts
Length of output: 2075
🌐 Web query:
RFC3986 URI authority component password contains query delimiter @ userinfo percent-encoding
💡 Result:
According to RFC 3986, the userinfo component, which may include a password, is terminated by the first commercial at-sign ("@") character [1][2]. Because the "@" character acts as a reserved delimiter that separates the userinfo from the host, using a literal "@" within a password is not allowed if that password is part of the userinfo field [1][3]. If a password must contain an "@" character, it must be percent-encoded as "%40" [4][5]. The ABNF syntax for userinfo is defined as: userinfo = *( unreserved / pct-encoded / sub-delims / ":") [1][2] In this definition, "pct-encoded" allows for the representation of reserved characters—including "@"—that would otherwise be interpreted as delimiters [1][4]. It is important to note that the use of the "user:password" format within the userinfo field is explicitly deprecated by RFC 3986 due to the security risks associated with passing authentication information in clear text [1][6]. Furthermore, modern web standards, such as the WHATWG URL Standard, enforce specific percent-encoding rules for the userinfo component to ensure consistent parsing and to prevent ambiguity caused by reserved characters like "@" [7].
Citations:
- 1: https://www.rfc-editor.org/rfc/rfc3986.html
- 2: https://greenbytes.de/tech/webdav/rfc3986.html
- 3: https://greenbytes.de/tech/specs/rfc3986.html
- 4: https://www.ietf.org/rfc/rfc3986
- 5: https://stackoverflow.com/questions/19509028/can-i-use-an-at-symbol-inside-urls
- 6: https://greenbytes.de/tech/webdav/rfc3986.pdf
- 7: https://url.spec.whatwg.org/
Encode the Redis password before embedding it in REDIS_URL.
charts/plane-mcp-server/templates/config-secrets/app-env.yaml writes .Values.services.redis.password directly into the Redis URI userinfo. If services.redis.password contains URI delimiters such as @, /, #, ?, or %, Redis clients can parse the URL incorrectly; encode the password component or use a separate supported Redis auth setting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-mcp-server/values.yaml` at line 41, Update the Redis URL
construction in the app-env template to percent-encode
.Values.services.redis.password before placing it in URI userinfo, preserving
correct parsing for reserved characters while retaining the existing
empty-password behavior.
3 participants
Description
Type of Change
Summary by CodeRabbit