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
WalkthroughAdds a complete Helm chart for the plane-mcp-server: chart metadata, values/questions, README, .helmignore, template helpers, and templates for secrets, service account, Deployment, Service, optional Redis StatefulSet, Ingress, and cert-manager issuers/certificates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant K8sAPI
participant Ingress
participant AppPod
participant Redis
participant CertManager
User->>Helm: helm install/upgrade plane-mcp-server (values)
Helm->>K8sAPI: create Secrets, ServiceAccount, Services, Deployment/StatefulSet, Ingress, Issuer/Certificate
K8sAPI->>AppPod: schedule & start pod (env from Secret, serviceAccount)
alt services.redis.local_setup == true
K8sAPI->>Redis: provision StatefulSet + PVC
Redis-->>AppPod: accept connections on REDIS_URL
else
AppPod-->>Redis: connect to external REDIS_URL
end
Ingress->>CertManager: ACME challenge (if TLS enabled)
CertManager->>K8sAPI: create/issue Certificate
Ingress->>AppPod: route external requests to service:8211
User->>Ingress: access application via hostname
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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
| - variable: services.redis.external_redis_url | ||
| label: "Remote Redis URL" | ||
| type: string | ||
| default: "redis://" | ||
| show_if: "services.redis.local_setup=false" |
There was a problem hiding this comment.
Incomplete default for external Redis URL.
The default value "redis://" is an incomplete URL that will cause connection failures. Consider using an empty string or a complete example URL with a clear placeholder.
Proposed fix
- variable: services.redis.external_redis_url
label: "Remote Redis URL"
type: string
- default: "redis://"
+ default: ""
show_if: "services.redis.local_setup=false"📝 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.
| - variable: services.redis.external_redis_url | |
| label: "Remote Redis URL" | |
| type: string | |
| default: "redis://" | |
| show_if: "services.redis.local_setup=false" | |
| - variable: services.redis.external_redis_url | |
| label: "Remote Redis URL" | |
| type: string | |
| default: "" | |
| show_if: "services.redis.local_setup=false" |
🤖 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 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.
| | dockerRegistry.enabled | true | | Enable Docker registry authentication for pulling images | | ||
| | dockerRegistry.loginid | planeengineering | | Docker registry login ID/username | | ||
| | dockerRegistry.password | | | Docker registry password or token | |
There was a problem hiding this comment.
Default loginid value mismatch with values.yaml.
The README states the default dockerRegistry.loginid is planeengineering, but values.yaml sets it to makeplane. Documentation should match the actual defaults.
📝 Proposed fix
-| dockerRegistry.loginid | planeengineering | | Docker registry login ID/username |
+| dockerRegistry.loginid | makeplane | | Docker registry login ID/username |📝 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.
| | dockerRegistry.enabled | true | | Enable Docker registry authentication for pulling images | | |
| | dockerRegistry.loginid | planeengineering | | Docker registry login ID/username | | |
| | dockerRegistry.password | | | Docker registry password or token | | |
| | dockerRegistry.enabled | true | | Enable Docker registry authentication for pulling images | | |
| | dockerRegistry.loginid | makeplane | | Docker registry login ID/username | | |
| | dockerRegistry.password | | | Docker registry password or token | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/README.md` around lines 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.
| | mcp.example.com | / | <http://<release-name>-api:8000> | Yes | | ||
|
|
||
| ## Verify | ||
|
|
||
| - After install, the MCP Server listens on Service `<release-name>-api` port 8000 |
There was a problem hiding this comment.
Documentation port mismatch.
The README references port 8000 in the Custom Ingress Routes table (line 98) and Verify section (line 102), but the Ingress template (ingress.yaml) configures the backend service on port 8211. This inconsistency will confuse users attempting to set up custom ingress routes.
📝 Proposed fix to correct port references
| Host | Path | Service | Required |
| ----------------------- | :-----------: | ------------------------------------------ | :------: |
-| mcp.example.com | / | <http://<release-name>-api:8000> | Yes |
+| mcp.example.com | / | <http://<release-name>-api:8211> | Yes |
## Verify
-- After install, the MCP Server listens on Service `<release-name>-api` port 8000
+- After install, the MCP Server listens on Service `<release-name>-api` port 8211📝 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.
| | mcp.example.com | / | <http://<release-name>-api:8000> | Yes | | |
| ## Verify | |
| - After install, the MCP Server listens on Service `<release-name>-api` port 8000 | |
| | mcp.example.com | / | <http://<release-name>-api:8211> | Yes | | |
| ## Verify | |
| - After install, the MCP Server listens on Service `<release-name>-api` port 8211 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/README.md` around lines 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.
| REDIS_HOST: {{ .Release.Name }}-redis | ||
| REDIS_PORT: "6379" | ||
| {{- if .Values.services.redis.local_setup }} | ||
| REDIS_URL: "redis://{{ .Release.Name }}-redis.{{ .Release.Namespace }}.svc.cluster.local:6379/" | ||
| {{- else }} | ||
| REDIS_URL: {{ .Values.services.redis.external_redis_url | default "" | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other charts in this repo handle Redis host/port conditionally
rg -n "REDIS_HOST|REDIS_PORT" --glob "charts/*/templates/**/*.yaml" -A2 -B2Repository: makeplane/helm-charts
Length of output: 814
🏁 Script executed:
# Find and examine the values.yaml file for redis configuration
fd values.yaml charts/plane-mcp-server | head -1 | xargs catRepository: makeplane/helm-charts
Length of output: 1023
🏁 Script executed:
# Examine the full app-env.yaml file
cat -n charts/plane-mcp-server/templates/config-secrets/app-env.yamlRepository: makeplane/helm-charts
Length of output: 1117
🏁 Script executed:
# Look for redis-statefulset.yaml or similar service definitions
fd -e yaml -e yml charts/plane-mcp-server/templates | grep -i redisRepository: makeplane/helm-charts
Length of output: 502
🏁 Script executed:
# Check if there are any other references to REDIS_HOST or REDIS_PORT in the entire chart
rg -n "REDIS_HOST|REDIS_PORT" charts/plane-mcp-server --type yaml -B5 -A5Repository: makeplane/helm-charts
Length of output: 1599
🏁 Script executed:
# Look for Redis service or statefulset definition
fd . charts/plane-mcp-server/templates -type f -name "*.yaml" | xargs grep -l "redis\|Redis" | head -10Repository: makeplane/helm-charts
Length of output: 235
🏁 Script executed:
# Check for any README or documentation about the application
fd -e md -e txt charts/plane-mcp-serverRepository: makeplane/helm-charts
Length of output: 462
🏁 Script executed:
# Search for how other charts handle external service configuration
rg -A5 "local_setup|external" charts/plane-mcp-server/templates --type yamlRepository: makeplane/helm-charts
Length of output: 1233
🏁 Script executed:
# Look at the complete templates directory structure
ls -la charts/plane-mcp-server/templates/Repository: makeplane/helm-charts
Length of output: 480
🏁 Script executed:
# Examine the redis-statefulset.yaml to see the service definition
cat -n charts/plane-mcp-server/templates/workloads/redis-statefulset.yaml | head -30Repository: makeplane/helm-charts
Length of output: 872
🏁 Script executed:
# Search for how the app actually uses REDIS_HOST and REDIS_PORT
# First, find the main application code in the repository
fd . --type f -name "*.py" -o -name "*.js" -o -name "*.ts" | grep -v node_modules | grep -E "(app|server|main)" | head -20Repository: makeplane/helm-charts
Length of output: 295
🏁 Script executed:
# Check if there's any reference to REDIS_HOST usage in code
rg "REDIS_HOST|REDIS_PORT" --type py --type js | head -20Repository: makeplane/helm-charts
Length of output: 213
Conditionally set REDIS_HOST and REDIS_PORT based on local_setup.
When services.redis.local_setup is false, REDIS_HOST still points to {{ .Release.Name }}-redis, which won't exist since the Redis service is only created when local_setup: true (as shown in redis-statefulset.yaml). This inconsistency can cause confusion. REDIS_URL is correctly conditionalized, but REDIS_HOST and REDIS_PORT (lines 13-14) should follow the same pattern.
Suggested fix
{{- if .Values.services.redis.local_setup }}
REDIS_HOST: {{ .Release.Name }}-redis
REDIS_PORT: "6379"
{{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/templates/config-secrets/app-env.yaml` around lines
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.
| - backend: | ||
| service: | ||
| port: | ||
| number: 8211 | ||
| name: {{ .Release.Name }}-api |
There was a problem hiding this comment.
Incorrect Ingress backend service structure.
The name field is incorrectly nested under port. According to the Kubernetes Ingress API (networking.k8s.io/v1), name and port should be siblings under service.
🐛 Proposed fix for correct backend structure
- backend:
service:
+ name: {{ .Release.Name }}-api
port:
number: 8211
- name: {{ .Release.Name }}-api
path: /
pathType: Prefix📝 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.
| - backend: | |
| service: | |
| port: | |
| number: 8211 | |
| name: {{ .Release.Name }}-api | |
| - backend: | |
| service: | |
| name: {{ .Release.Name }}-api | |
| port: | |
| number: 8211 | |
| path: / | |
| pathType: Prefix |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/templates/ingress/ingress.yaml` around lines 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.
| tls: | ||
| - hosts: | ||
| - {{ .Values.ingress.host | quote }} | ||
| secretName: {{ .Release.Name }}-ssl-cert |
There was a problem hiding this comment.
TLS hosts list indentation issue.
The host entry under hosts: needs proper indentation to be recognized as a list item under the hosts key.
🐛 Proposed fix for TLS indentation
{{- if .Values.ingress.ssl.enabled }}
tls:
- hosts:
- - {{ .Values.ingress.host | quote }}
+ - {{ .Values.ingress.host | quote }}
secretName: {{ .Release.Name }}-ssl-cert
{{- 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.
| tls: | |
| - hosts: | |
| - {{ .Values.ingress.host | quote }} | |
| secretName: {{ .Release.Name }}-ssl-cert | |
| tls: | |
| - hosts: | |
| - {{ .Values.ingress.host | quote }} | |
| secretName: {{ .Release.Name }}-ssl-cert |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/templates/ingress/ingress.yaml` around lines 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.
| name: {{ .Release.Name }}-issuer-api-token-secret | ||
| type: Opaque | ||
| stringData: | ||
| api-token: {{ .Values.ingress.ssl.token | default "default-api-token" | quote }} |
There was a problem hiding this comment.
Avoid deploying a placeholder API token.
The default value "default-api-token" is a security risk. If a user forgets to set .Values.ingress.ssl.token, this placeholder will be deployed and the ACME DNS challenge will fail silently or produce confusing errors. Consider failing the template if the token is empty when using DNS01 issuers.
Proposed fix to fail fast on missing token
- api-token: {{ .Values.ingress.ssl.token | default "default-api-token" | quote }}
+ api-token: {{ required "ingress.ssl.token is required for DNS01 issuers (cloudflare/digitalocean)" .Values.ingress.ssl.token | quote }}📝 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.
| api-token: {{ .Values.ingress.ssl.token | default "default-api-token" | quote }} | |
| api-token: {{ required "ingress.ssl.token is required for DNS01 issuers (cloudflare/digitalocean)" .Values.ingress.ssl.token | quote }} |
🤖 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 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.
| apiVersion: v1 | ||
| automountServiceAccountToken: true | ||
| kind: ServiceAccount | ||
| metadata: | ||
| namespace: {{ .Release.Namespace }} | ||
| name: {{ .Release.Name }}-srv-account | ||
| {{- if .Values.dockerRegistry.enabled }} | ||
| imagePullSecrets: | ||
| - name: {{ .Release.Name }}-docker-registry-credentials | ||
| {{- end}} No newline at end of file |
There was a problem hiding this comment.
Invalid ServiceAccount manifest structure.
The automountServiceAccountToken field is placed before kind, which is invalid for a Kubernetes resource manifest. This will cause the resource to fail validation.
🐛 Proposed fix for correct ServiceAccount structure
apiVersion: v1
-automountServiceAccountToken: true
kind: ServiceAccount
metadata:
namespace: {{ .Release.Namespace }}
name: {{ .Release.Name }}-srv-account
+automountServiceAccountToken: true
{{- if .Values.dockerRegistry.enabled }}
imagePullSecrets:
- name: {{ .Release.Name }}-docker-registry-credentials
{{- end}}🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 6-6: syntax error: expected , but found ''
(syntax)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/templates/service-account.yaml` around lines 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.
| enabled: true | ||
| host: 'mcp.example.com' | ||
| ingressClass: 'nginx' | ||
| ingressAnnotations: { nginx.ingress.kubernetes.io/proxy-body-size: "10m" } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if ingressAnnotations is used anywhere in templates
rg -n "ingressAnnotations" charts/plane-mcp-server/Repository: makeplane/helm-charts
Length of output: 498
🏁 Script executed:
find charts/plane-mcp-server -name "*ingress*" -type fRepository: makeplane/helm-charts
Length of output: 119
🏁 Script executed:
cat -n charts/plane-mcp-server/templates/ingress/ingress.yamlRepository: makeplane/helm-charts
Length of output: 1095
🏁 Script executed:
find charts/plane-mcp-server/templates -name "*.yaml" | xargs rg -l "ingressAnnotations"Repository: makeplane/helm-charts
Length of output: 49
ingressAnnotations value is unused by the Ingress template.
The ingressAnnotations map is defined in values.yaml but the ingress.yaml template does not reference or apply it. The template only hardcodes a single annotation (nginx.ingress.kubernetes.io/proxy-body-size) using the clientMaxBodySize value. Either remove this unused value or update the template to apply custom annotations from ingressAnnotations.
🤖 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 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.
| plane_oauth: | ||
| enabled: false | ||
| client_id: '' | ||
| client_secret: '' | ||
| provider_base_url: '' |
There was a problem hiding this comment.
OAuth field naming inconsistency with README.
The README (line 67) documents services.api.plane_oauth.base_url, but values.yaml defines provider_base_url. Similarly, README mentions redirect_uri but it's not present here. Align documentation with actual values or vice versa.
📝 Suggested alignment (if README is the source of truth)
plane_oauth:
enabled: false
client_id: ''
client_secret: ''
- provider_base_url: ''
+ base_url: ''
+ redirect_uri: ''📝 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.
| plane_oauth: | |
| enabled: false | |
| client_id: '' | |
| client_secret: '' | |
| provider_base_url: '' | |
| plane_oauth: | |
| enabled: false | |
| client_id: '' | |
| client_secret: '' | |
| base_url: '' | |
| redirect_uri: '' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/plane-mcp-server/values.yaml` around lines 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.
* 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
| - id: build-plane-mcp-server | ||
| if: ${{ env.BUILD_PLANE_MCP_SERVER == 'true' }} | ||
| name: Build Plane-MCP-Server | ||
| working-directory: code | ||
| env: | ||
| EXPORT_DIR: ${{env.PREVIEW_BUILD_FOLDER}} | ||
| CHART_REPO: plane-mcp-server | ||
| CR_KEY: ${{ env.GPG_KEY_NAME }} | ||
| CR_PASSPHRASE_FILE: ${{env.GNUPGHOME}}/gpg-passphrase | ||
| CR_KEYRING: ${{env.GNUPGHOME}}/secring.gpg | ||
| run: | | ||
| flatBranchName=$(echo "${{ github.ref_name}}" | sed 's/\//\-/g') | ||
| sed -i "s/name: ${{env.CHART_REPO}}/name: ${{ env.CHART_PREFIX }}-${{env.CHART_REPO}}/" charts/${{env.CHART_REPO}}/Chart.yaml | ||
| sed -i "s/description: .*/description: ${flatBranchName}/g" charts/${{env.CHART_REPO}}/Chart.yaml | ||
| # sed -i "s/version: \(.*\)/version: \1-${flatBranchName}/" charts/${{env.CHART_REPO}}/Chart.yaml | ||
|
|
||
| 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 | ||
| cp charts/${{env.CHART_REPO}}/README.md ${{ env.EXPORT_DIR }}/${{env.CHART_REPO}}/${{env.CHART_REPO}}.md | ||
| helm repo index ${{ env.EXPORT_DIR }}/${{env.CHART_REPO}} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/chart-preview.yml | sed -n '120,150p'Repository: makeplane/helm-charts
Length of output: 2178
🏁 Script executed:
grep -n "helm package" .github/workflows/chart-preview.ymlRepository: makeplane/helm-charts
Length of output: 622
🏁 Script executed:
cat -n .github/workflows/chart-preview.yml | sed -n '95,125p'Repository: makeplane/helm-charts
Length of output: 2367
🏁 Script executed:
cat -n .github/workflows/chart-preview.yml | sed -n '75,105p'Repository: makeplane/helm-charts
Length of output: 2053
🏁 Script executed:
cat -n .github/workflows/chart-preview.yml | head -90 | tail -20Repository: makeplane/helm-charts
Length of output: 1196
🏁 Script executed:
cd .github/workflows && ls -la && head -30 chart-preview.ymlRepository: makeplane/helm-charts
Length of output: 1166
🏁 Script executed:
grep -n "shellcheck\|shfmt" .github/workflows/chart-preview.ymlRepository: makeplane/helm-charts
Length of output: 47
🏁 Script executed:
fd -type f "(actionlint|shellcheck|pre-commit)" -o "\.pre-commit.*" | head -20Repository: makeplane/helm-charts
Length of output: 235
🏁 Script executed:
find . -maxdepth 2 -name ".pre-commit-config.yml" -o -name ".pre-commit-config.yaml" -o -name "actionlint.yaml" 2>/dev/null | head -10Repository: makeplane/helm-charts
Length of output: 47
🏁 Script executed:
find . -name "*.yml" -o -name "*.yaml" | xargs grep -l "actionlint\|shellcheck" 2>/dev/null | head -10Repository: makeplane/helm-charts
Length of output: 47
Quote unquoted variables in helm package commands to prevent word-splitting issues.
Unquoted expansions in the helm package commands cause SC2086 violations. This pattern appears in all three build steps (lines 102, 122, 142), not just the MCP step. The variables $CR_KEYRING, charts/$CHART_REPO, and ${{ env.EXPORT_DIR }}/${{env.CHART_REPO}}/charts need quoting.
🔧 Suggested patch for line 142
- 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
+ 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"Apply the same quoting pattern to lines 102 and 122 for consistency.
🧰 Tools
🪛 actionlint (1.7.11)
[error] 136-136: shellcheck reported issue in this script: SC2086:info:6:106: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 136-136: shellcheck reported issue in this script: SC2086:info:6:47: Double quote to prevent globbing and word splitting
(shellcheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/chart-preview.yml around lines 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.
1 participant
Description
Type of Change
Summary by CodeRabbit
New Features
Documentation
Chores
Tests