chore: Add s3 custom CA airgapped support#153
Conversation
Promoting Develop to Master
Promoting Develop to Master
feat: Add airgapped support to Plane-Enterprise configuration (#148)
|
Warning Rate limit exceeded@akshat5302 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 18 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughBumps chart and app versions, updates Plane image defaults and docs, adds air-gapped S3 CA support (new airgapped keys), emits conditional secret/env entries, and adds a startup wrapper + secret volume/mount in the API deployment to install a provided CA before starting the service. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Helm User
participant Helm as Helm (template renderer)
participant K8s as Kubernetes API
participant Pod as API Pod (container)
User->>Helm: helm install/upgrade (values with airgapped?)
Helm->>K8s: Render Deployment, Secret (s3-custom-ca), ConfigMap with envs
K8s-->>User: Resources created/updated
rect rgba(220,240,220,0.6)
note right of Pod: Pod start — when airgapped.enabled & s3SecretName set
Pod->>Pod: Mount `s3-custom-ca` at /s3-custom-ca (readOnly)
Pod->>Pod: Set envs: SSL_CERT_FILE, SSL_CERT_DIR, REQUESTS_CA_BUNDLE, CURL_CA_BUNDLE, (AWS_CA_BUNDLE)
Pod->>Pod: Run startup wrapper:
Pod->>Pod: - if /s3-custom-ca/<key> exists → copy to /usr/local/share/ca-certificates && update-ca-certificates
end
Pod->>Pod: exec original API entrypoint (./bin/docker-entrypoint-api-ee.sh)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
twtaylor
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
54-56: Also set REQUESTS_CA_BUNDLE (and optionally SSL_CERT_FILE) for broader client coverage.Many Python/HTTP clients honor REQUESTS_CA_BUNDLE; setting both avoids surprises beyond boto.
Apply:
{{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} - AWS_CA_BUNDLE: {{ .Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt" | quote }} + AWS_CA_BUNDLE: {{ .Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt" | quote }} + REQUESTS_CA_BUNDLE: {{ .Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt" | quote }} + # Uncomment if you want OpenSSL consumers to use the same bundle: + # SSL_CERT_FILE: {{ .Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt" | quote }} {{- end }}If workers or other pods also reach S3, confirm they source this ConfigMap too; otherwise replicate this block for those workloads.
charts/plane-enterprise/README.md (1)
96-97: Tighten wording and add a minimal Secret creation example.Small grammar fix and actionable example reduces misconfig risk.
Apply:
-| airgapped.s3CrtFileLocation | "" | No | If specified, overrides the CA that is used to communicate with S3. Specifies the location in the container where the new .crt file lies | -| airgapped.s3SecretName | "" | No | Specifies the secret where to copy the overriding .crt file from for use in overriding s3's native CA | +| airgapped.s3CrtFileLocation | "/s3-custom-ca/s3-custom-ca.crt" | No | Path inside the container to the custom CA bundle used for S3; must match the filename provided by the Secret | +| airgapped.s3SecretName | "" | No | Name of the Secret that provides the custom CA certificate for S3 |Add after the table (one-line example):
kubectl -n plane create secret generic s3-ca --from-file=s3-custom-ca.crt=/path/to/your-ca.crtLanguage nit: “subdomain” (not “sudomain”) appears elsewhere in the doc—consider a quick pass.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/README.md(1 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(1 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)charts/plane-enterprise/values.yaml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
charts/plane-enterprise/README.md
[grammar] ~96-~96: There might be a mistake here.
Context: ...container where the new .crt file lies | | airgapped.s3SecretName | "" | No | Spe...
(QB_NEW_EN)
🔇 Additional comments (2)
charts/plane-enterprise/Chart.yaml (1)
8-8: Version bump looks good.Chart version advanced to 1.3.4-rc3, appVersion unchanged. No issues.
charts/plane-enterprise/templates/workloads/api.deployment.yaml (1)
61-66: Verify CA volumeMount with Helm locally Helm isn’t available here; run yourhelm templatecommand in your local environment to confirm the custom CA is mounted at.Values.airgapped.s3CrtFileLocationusingsubPathand that it lines up with your AWS_CA_BUNDLE/REQUESTS_CA_BUNDLE settings.
charts/plane-enterprise/templates/workloads/api.deployment.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
charts/plane-enterprise/README.md(1 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)charts/plane-enterprise/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/plane-enterprise/values.yaml
- charts/plane-enterprise/templates/workloads/api.deployment.yaml
🧰 Additional context used
🪛 LanguageTool
charts/plane-enterprise/README.md
[grammar] ~96-~96: There might be a mistake here.
Context: ...container where the new .crt file lies | | airgapped.s3SecretName | "" | No | Spe...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...ing s3's native CA for the s3 client. | | license.licenseDomain | plane.example....
(QB_NEW_EN)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
charts/plane-enterprise/README.md (1)
96-96: Tighten wording; note AWS_CA_BUNDLE is auto-set.
Clarify that this path is used by boto3/botocore and that the chart sets theAWS_CA_BUNDLEenv var when bothairgapped.enabledandairgapped.s3SecretNameare configured.-| airgapped.s3CrtFileLocation | "/s3-custom-ca/s3-custom-ca.crt" | No | Path inside the container to the CA certificate file used for S3 (Boto). Effective when `airgapped.enabled=true` and `airgapped.s3SecretName` is set. | +| airgapped.s3CrtFileLocation | "/s3-custom-ca/s3-custom-ca.crt" | No | Path inside the container to the CA certificate file used for S3 (boto3/botocore). Effective when `airgapped.enabled=true` and `airgapped.s3SecretName` is set; the chart will set `AWS_CA_BUNDLE` to this path. |
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/plane-enterprise/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
charts/plane-enterprise/README.md
[grammar] ~97-~97: There might be a mistake here.
Context: ...3’s CA when airgapped.enabled=true. | | license.licenseDomain | plane.example....
(QB_NEW_EN)
- Updated Chart.yaml to version 1.3.4-rc3. - Added configuration options for CA bundle in values.yaml. - Modified app-env.yaml to conditionally set AWS_CA_BUNDLE based on airgapped settings. - Updated api.deployment.yaml to include volume and volumeMounts for the CA bundle when airgapped is enabled.
twtaylor
force-pushed
the
airgapped-rc3
branch
from
0f3eb30 to
52d6139
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/plane-enterprise/README.md (1)
97-97: Document namespace requirement and add a copy-paste Secret example.Call out that the Secret must live in the release namespace and be PEM-encoded; users often miss the filename key requirement.
-| airgapped.s3SecretName | "" | No | Name of the Secret that contains the CA certificate (.crt). The Secret must include a data key whose filename matches the basename of `airgapped.s3CrtFileLocation` (default: `s3-custom-ca.crt`). Used to override S3’s CA when `airgapped.enabled=true`. | +| airgapped.s3SecretName | "" | No | Name of the Secret (in the same namespace as the release) that contains the PEM-encoded CA certificate (.crt). The Secret must include a data key whose filename matches the basename of `airgapped.s3CrtFileLocation` (default: `s3-custom-ca.crt`). Used to override S3’s CA when `airgapped.enabled=true`. |Example to create the Secret (adjust namespace and path):
kubectl -n plane create secret generic plane-s3-ca \ --from-file=s3-custom-ca.crt=/path/to/your/ca-bundle.crtRun this to confirm templates and values align with the docs (conditional env + mount, filename, and path):
#!/bin/bash set -euo pipefail # Verify AWS_CA_BUNDLE env is conditionally emitted rg -nC3 -g 'charts/plane-enterprise/**' 'AWS_CA_BUNDLE|airgapped\.s3CrtFileLocation' # Verify gating on both airgapped.enabled and airgapped.s3SecretName rg -nC2 -g 'charts/plane-enterprise/**' -e '{{-?\s*if\s+and\s+.*\.Values\.airgapped\.enabled.*\.Values\.airgapped\.s3SecretName' # Check that a Secret volume + volumeMount is added and mount path matches dirname of default rg -nC3 -g 'charts/plane-enterprise/**' -e 'volume(Mounts)?' -e 's3-custom-ca' \ | sed -n '1,200p'
🧹 Nitpick comments (1)
charts/plane-enterprise/README.md (1)
96-96: Use “boto3” and specify PEM/bundle to avoid ambiguity.Tighten wording so users know a PEM-encoded bundle (with intermediates) is expected and align the library name.
-| airgapped.s3CrtFileLocation | "/s3-custom-ca/s3-custom-ca.crt" | No | Path inside the container to the CA certificate file used for S3 (Boto). Effective when `airgapped.enabled=true` and `airgapped.s3SecretName` is set. | +| airgapped.s3CrtFileLocation | "/s3-custom-ca/s3-custom-ca.crt" | No | Path inside the container to the PEM-encoded CA certificate bundle (.crt) used for S3 (boto3). Effective when `airgapped.enabled=true` and `airgapped.s3SecretName` is set. |
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
charts/plane-enterprise/Chart.yaml(1 hunks)charts/plane-enterprise/README.md(1 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(1 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)charts/plane-enterprise/values.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- charts/plane-enterprise/Chart.yaml
- charts/plane-enterprise/templates/config-secrets/app-env.yaml
- charts/plane-enterprise/values.yaml
- charts/plane-enterprise/templates/workloads/api.deployment.yaml
🧰 Additional context used
🪛 LanguageTool
charts/plane-enterprise/README.md
[grammar] ~97-~97: There might be a mistake here.
Context: ...3’s CA when airgapped.enabled=true. | | license.licenseDomain | plane.example....
(QB_NEW_EN)
- Added init container to install custom CA certificates when airgapped is enabled. - Updated volume and volumeMounts to include shared directory for CA certificates. - Set environment variables for SSL certificate paths in the API container.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (1)
42-50: Secret items pinning looks correct and resolves key-name drift.Using
itemswith the basename ofs3CrtFileLocationguarantees a stable filename and avoids surprises when the Secret has multiple keys.
🧹 Nitpick comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
75-83: Confirm Secret key matches the expected basename.The items mapping assumes the Secret contains a key named
{{ base .Values.airgapped.s3CrtFileLocation }}(defaults3-custom-ca.crt). Verify docs and examples instruct users to name the key exactly that, or add a separate configurables3SecretKeyvalue.
62-62: Trim trailing spaces flagged by yamllint.Whitespace-only diffs add churn and can fail linters. Please remove trailing spaces on the noted lines.
- echo "Installing custom CA certificates..."␠ + echo "Installing custom CA certificates..."Also applies to: 66-66, 71-71, 74-74, 84-84, 87-87, 90-90
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 66-66: trailing spaces
(trailing-spaces)
[error] 71-71: trailing spaces
(trailing-spaces)
[error] 74-74: trailing spaces
(trailing-spaces)
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
92-100: InitContainer mounts are correct.Conditional mount of
s3-custom-caand sharedca-certificatesvolume is clean and minimal.
118-127: Align with AWS_CA_BUNDLE from app-env.You set SSL_CERT_FILE/REQUESTS_CA_BUNDLE/CURL_CA_BUNDLE here;
AWS_CA_BUNDLEis set via envFrom (config). Ensure both point to the same path (/etc/ssl/certs/ca-certificates.crt) to avoid mixed trust sources.
charts/plane-enterprise/templates/workloads/api.deployment.yaml
Outdated
Show resolved
Hide resolved
| volumeMounts: | ||
| - name: ca-certificates | ||
| mountPath: /etc/ssl/certs | ||
| readOnly: true | ||
| {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against empty CA volume replacing system certs.
Mounting /etc/ssl/certs from emptyDir means a failed/broken init step yields no trust store. The refactor above adds a sanity check; keep that to fail fast. Alternatively, only mount the single bundle file instead of the whole directory.
- - name: ca-certificates
- mountPath: /etc/ssl/certs
+ - name: ca-certificates
+ mountPath: /etc/ssl/certs/ca-certificates.crt
+ subPath: ca-certificates.crt
readOnly: true
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (1)
42-50: Secret item pinning looks good (addresses earlier feedback).Mapping the secret
itemsto the basename ofs3CrtFileLocationprevents filename drift. Nice.
🧹 Nitpick comments (6)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (6)
48-49: Quote templated strings for safety.Wrap the rendered basenames in quotes to avoid YAML parsing surprises.
- - key: {{ (.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base }} - path: {{ (.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base }} + - key: {{ ((.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base) | quote }} + path: {{ ((.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base) | quote }}
64-69: Mount only the CA file via subPath.Since the secret is pinned to a single key, mount just that file to reduce surface area.
volumeMounts: - name: s3-custom-ca - mountPath: /s3-custom-ca + mountPath: /s3-custom-ca/{{ ((.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base) }} + subPath: {{ ((.Values.airgapped.s3CrtFileLocation | default "/s3-custom-ca/s3-custom-ca.crt") | base) }} readOnly: true
69-78: Verify CA env strategy and duplication.You set system-wide CA env vars here; app-env.yaml sets AWS_CA_BUNDLE. Confirm there’s no conflict and that setting all four is intended. If AWS_CA_BUNDLE alone suffices for boto, consider trimming to the minimum.
80-105: Harden the CA install path; handle non-root and missing tools.Risk:
update-ca-certificatesmay be absent, or the container may run non-root and lack write perms to/usr/local/share/ca-certificatesor/etc/ssl/certs. Guard and fail gracefully; avoid mutating the root FS when possible.Minimal hardening inline:
- set -e + set -euo pipefail @@ - mkdir -p /usr/local/share/ca-certificates + mkdir -p /usr/local/share/ca-certificates || true @@ - if [ -f "/s3-custom-ca/$S3_CERT_FILE" ]; then + if [ -s "/s3-custom-ca/$S3_CERT_FILE" ]; then echo "Installing S3 custom CA certificate..." - cp "/s3-custom-ca/$S3_CERT_FILE" /usr/local/share/ca-certificates/s3-custom-ca.crt - # Update CA certificates - update-ca-certificates - echo "CA certificates installed successfully" + cp -f "/s3-custom-ca/$S3_CERT_FILE" /usr/local/share/ca-certificates/s3-custom-ca.crt || true + if command -v update-ca-certificates >/dev/null 2>&1; then + if [ "$(id -u)" = "0" ]; then + update-ca-certificates || echo "WARN: update-ca-certificates failed; falling back to bundle append." + else + echo "WARN: not running as root; skipping update-ca-certificates." + fi + else + echo "WARN: update-ca-certificates not found; appending to bundle if writable." + fi + # Fallback: append to SSL_CERT_FILE if it exists and is writable + BUNDLE="${SSL_CERT_FILE:-/etc/ssl/certs/ca-certificates.crt}" + if [ -w "$BUNDLE" ]; then + printf "\n" >> "$BUNDLE" || true + cat "/s3-custom-ca/$S3_CERT_FILE" >> "$BUNDLE" || true + fi + echo "CA certificate handling completed"Preferable pattern (optional): move this logic to an initContainer that builds a single
ca-certificates.crtinto anemptyDir, then mount that file into/etc/ssl/certs/ca-certificates.crtviasubPath. This avoids mutating the main container FS and works for non-root.
84-84: Fix YAML lint: remove trailing spaces.Clean up trailing spaces flagged by yamllint.
- + @@ - + @@ - + @@ - +Also applies to: 87-87, 90-90, 103-103
35-41: Add a rollout trigger on secret changes.If the S3 CA secret changes outside Helm, the pod won’t roll automatically. Consider annotating the pod template with a checksum of the secret using
lookup(Helm 3).Example (outside this hunk):
metadata: annotations: timestamp: {{ now | quote }} {{- $s3 := (lookup "v1" "Secret" .Release.Namespace .Values.airgapped.s3SecretName) -}} {{- if $s3 }} checksum/secret-s3-custom-ca: {{ $s3.data | toJson | sha256sum }} {{- end }}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
charts/plane-enterprise/README.md(1 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/plane-enterprise/README.md
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
- Added new air-gapped settings in README for better clarity. - Updated values.yaml to reflect changes in airgapped secret handling. - Modified app-env.yaml to use the new s3SecretKey for CA bundle configuration. - Adjusted api.deployment.yaml to ensure correct mounting of the custom CA certificate based on the new configuration.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
charts/plane-enterprise/values.yaml (2)
15-21: Align comments with actual values; document mount path and env usage.The comment references a “full path” but
s3CrtFileLocationno longer exists. Clarify that the Secret key is the filename, it mounts to/s3-custom-ca/<key>, and env vars (AWS_CA_BUNDLE/REQUESTS_CA_BUNDLE) should point to that file.- # The boto module used by API does not use the CA bundles in the container, so we need to - # mount the CA bundle into the API pod by passing it in as an environment - # variable. Should be the full path to the CA bundle file, e.g. - # - s3SecretName: "s3-custom-ca" - # (default: "s3-custom-ca.crt"). - s3SecretKey: "s3-custom-ca.crt" + # The boto client ignores container CA bundles unless pointed via env vars. + # Provide the Secret that holds your custom CA and the data key (filename). + # The file is mounted at: /s3-custom-ca/<s3SecretKey> + # Point AWS_CA_BUNDLE/REQUESTS_CA_BUNDLE to that mounted file. + s3SecretName: "s3-custom-ca" + # Key (filename) inside the Secret containing the PEM-encoded CA (default below). + s3SecretKey: "s3-custom-ca.crt"
18-18: Remove trailing spaces (yamllint).Line 18 has trailing whitespace.
- # + #charts/plane-enterprise/templates/workloads/api.deployment.yaml (1)
84-84: Trim trailing spaces (yamllint).Whitespace-only lines flagged by yamllint.
Also applies to: 87-87, 90-90, 103-103
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/plane-enterprise/README.md(3 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(2 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)charts/plane-enterprise/values.yaml(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- charts/plane-enterprise/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/plane-enterprise/templates/config-secrets/app-env.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
charts/plane-enterprise/values.yaml
[error] 18-18: trailing spaces
(trailing-spaces)
🔇 Additional comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
64-78: Double-check AWS_CA_BUNDLE is not set in two places.If
app-env.yamlalso sets AWS_CA_BUNDLE, ensure only one source (envFrom vs directenv) wins, or make them identical to avoid confusion.Also applies to: 106-121
64-78: Point CA env vars to the mounted custom CA file; gate consistently.If
update-ca-certificatesisn’t run (or isn’t present), pointing to/etc/ssl/certs/ca-certificates.crtwon’t pick up the custom CA. Use the mounted file path and includes3SecretKeyin the condition to avoid partial configs.- {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} + {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName .Values.airgapped.s3SecretKey }} volumeMounts: - name: s3-custom-ca mountPath: /s3-custom-ca readOnly: true env: - - name: SSL_CERT_FILE - value: "/etc/ssl/certs/ca-certificates.crt" - - name: SSL_CERT_DIR - value: "/etc/ssl/certs" - - name: REQUESTS_CA_BUNDLE - value: "/etc/ssl/certs/ca-certificates.crt" - - name: CURL_CA_BUNDLE - value: "/etc/ssl/certs/ca-certificates.crt" + - name: REQUESTS_CA_BUNDLE + value: {{ printf "/s3-custom-ca/%s" ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) | quote }} + - name: CURL_CA_BUNDLE + value: {{ printf "/s3-custom-ca/%s" ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) | quote }} + # AWS SDKs (incl. boto) honor AWS_CA_BUNDLE: + - name: AWS_CA_BUNDLE + value: {{ printf "/s3-custom-ca/%s" ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) | quote }}Likely an incorrect or invalid review comment.
charts/plane-enterprise/templates/workloads/api.deployment.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
42-50: Align gating with mounts and sanitize Secret items (use base + quote).
- Volumes are gated on s3SecretKey, but mounts (Line 64) are not — can produce "volumeMounts refer to volume not found".
- Secret items should use basename and quotes to avoid invalid key/path when users pass paths.
- {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName .Values.airgapped.s3SecretKey }} + {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} volumes: - name: s3-custom-ca secret: secretName: {{ .Values.airgapped.s3SecretName }} items: - - key: {{ .Values.airgapped.s3SecretKey | default "s3-custom-ca.crt" }} - path: {{ .Values.airgapped.s3SecretKey | default "s3-custom-ca.crt" }} + - key: {{ ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) | quote }} + path: {{ ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) | quote }} {{- end }}
80-105: Remove runtime CA installation; avoid update-ca-certificates in app container.This often fails in minimal/RO images and is unnecessary if using AWS_CA_BUNDLE or direct bundle paths. Just exec the entrypoint.
command: - /bin/bash - -c - | - set -e - - {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} - echo "Installing custom CA certificates..." - - # Ensure ca-certificates directory exists - mkdir -p /usr/local/share/ca-certificates - - # Install custom S3 CA if available - S3_CERT_FILE="{{ (.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base }}" - if [ -f "/s3-custom-ca/$S3_CERT_FILE" ]; then - echo "Installing S3 custom CA certificate..." - cp "/s3-custom-ca/$S3_CERT_FILE" "/usr/local/share/ca-certificates/$S3_CERT_FILE" - # Update CA certificates - update-ca-certificates - echo "CA certificates installed successfully" - else - echo "No custom S3 CA certificate found, skipping..." - fi - {{- end }} - + set -euo pipefail # Start the API exec ./bin/docker-entrypoint-api-ee.sh
🧹 Nitpick comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
64-68: Optionally mount only the CA file via subPath.Reduces surface and avoids directory semantics when only a single PEM is needed.
volumeMounts: - name: s3-custom-ca - mountPath: /s3-custom-ca + mountPath: /s3-custom-ca/{{ ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) }} + subPath: {{ ((.Values.airgapped.s3SecretKey | default "s3-custom-ca.crt") | base) }} readOnly: true
83-104: Trim trailing spaces flagged by YAMLlint.Lines 84, 87, 90, and 103 have trailing whitespace.
- set -e - + set -e @@ - echo "Installing custom CA certificates..." - + echo "Installing custom CA certificates..." @@ - mkdir -p /usr/local/share/ca-certificates - + mkdir -p /usr/local/share/ca-certificates @@ - +
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
- Introduced new airgapped settings in questions.yml for enabling airgapped mode. - Updated README to reflect changes in airgapped configuration, including default values for s3SecretName and s3SecretKey. - Modified app-env.yaml and api.deployment.yaml to utilize the new s3SecretKey directly for CA bundle and secret handling.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
69-78: Don’t override global trust to system bundle; rely on AWS_CA_BUNDLE or point to the custom PEM.Setting these to the system bundle blunts the intent of a custom CA and can mask misconfigurations.
- env: - - name: SSL_CERT_FILE - value: "/etc/ssl/certs/ca-certificates.crt" - - name: SSL_CERT_DIR - value: "/etc/ssl/certs" - - name: REQUESTS_CA_BUNDLE - value: "/etc/ssl/certs/ca-certificates.crt" - - name: CURL_CA_BUNDLE - value: "/etc/ssl/certs/ca-certificates.crt" + # Tip: Prefer AWS_CA_BUNDLE from app-env for boto/S3. + # Optionally, point tools to the mounted custom PEM instead of the system bundle: + # env: + # - name: REQUESTS_CA_BUNDLE + # value: "/s3-custom-ca/{{ (.Values.airgapped.s3SecretKey | base) }}" + # - name: CURL_CA_BUNDLE + # value: "/s3-custom-ca/{{ (.Values.airgapped.s3SecretKey | base) }}"
83-102: Avoid runtime update-ca-certificates in the main container.Often unavailable and unnecessary; rely on the mounted PEM and AWS_CA_BUNDLE instead.
- {{- if and .Values.airgapped.enabled .Values.airgapped.s3SecretName }} - echo "Installing custom CA certificates..." - - # Ensure ca-certificates directory exists - mkdir -p /usr/local/share/ca-certificates - - # Install custom S3 CA if available - S3_CERT_FILE="{{ .Values.airgapped.s3SecretKey }}" - if [ -f "/s3-custom-ca/$S3_CERT_FILE" ]; then - echo "Installing S3 custom CA certificate..." - cp "/s3-custom-ca/$S3_CERT_FILE" "/usr/local/share/ca-certificates/$S3_CERT_FILE" - # Update CA certificates - update-ca-certificates - echo "CA certificates installed successfully" - else - echo "No custom S3 CA certificate found, skipping..." - fi - {{- end }} + # Using mounted CA at /s3-custom-ca and AWS_CA_BUNDLE; no system store mutation.
🧹 Nitpick comments (3)
charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
35-37: Quote the AWS_CA_BUNDLE path and keep it consistent with Deployment mount.Add | quote to avoid YAML parsing surprises. Ensure the filename segment matches the Secret.items path used in api.deployment.yaml.
- AWS_CA_BUNDLE: "/s3-custom-ca/{{ .Values.airgapped.s3SecretKey }}" + AWS_CA_BUNDLE: {{ printf "/s3-custom-ca/%s" .Values.airgapped.s3SecretKey | quote }}charts/plane-enterprise/templates/workloads/api.deployment.yaml (2)
42-50: Harden Secret items: quote and normalize to basename.Prevents invalid keys/paths if values contain dots or stray spaces; ensures deterministic filename.
- items: - - key: {{ .Values.airgapped.s3SecretKey }} - path: {{ .Values.airgapped.s3SecretKey }} + items: + - key: {{ (.Values.airgapped.s3SecretKey | toString | base) | quote }} + path: {{ (.Values.airgapped.s3SecretKey | toString | base) | quote }}
84-84: Trim trailing spaces flagged by yamllint.- + - + - + - +Also applies to: 87-87, 90-90, 103-103
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
charts/plane-enterprise/README.md(3 hunks)charts/plane-enterprise/questions.yml(2 hunks)charts/plane-enterprise/templates/config-secrets/app-env.yaml(2 hunks)charts/plane-enterprise/templates/workloads/api.deployment.yaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/plane-enterprise/questions.yml
- charts/plane-enterprise/README.md
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/plane-enterprise/templates/workloads/api.deployment.yaml
[error] 84-84: trailing spaces
(trailing-spaces)
[error] 87-87: trailing spaces
(trailing-spaces)
[error] 90-90: trailing spaces
(trailing-spaces)
[error] 103-103: trailing spaces
(trailing-spaces)
🔇 Additional comments (1)
charts/plane-enterprise/templates/config-secrets/app-env.yaml (1)
57-57: LGTM: USE_STORAGE_PROXY toggle is clear and safe defaults.
charts/plane-enterprise/templates/workloads/api.deployment.yaml
Outdated
Show resolved
Hide resolved
…irgapped configuration
…y airgapped configuration
…onfiguration check
4 participants
Description
Used for overriding Boto's native s3 CA handling. Allows the user to add their own .crt file.
Type of Change
Test Scenarios
References
Summary by CodeRabbit
New Features
Documentation
Chores