chore(helm): harden the chart and move the cloud sql proxy to v2 - #99
Merged
Conversation
The client matrix and path filters were duplicated in release.yml and test.yml.
Alpine 3.24 with apk upgrade
WalkthroughThe PR adds a reusable workflow that generates component matrices and JSON change data for test and release workflows. It updates Fleet image bases and security defaults. Deployment, migration, and vulnerability-processing workloads now use configurable security contexts, local CA staging, and Cloud SQL Proxy v2 arguments with graceful shutdown handling. The Cloud SQL Proxy image is updated to version 2.25.0. 🚥 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: 4
🧹 Nitpick comments (2)
charts/fleet/values.yaml (1)
154-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that
fsGroupis required inpodSecurityContext.The templates render this block with
{{- with .Values.fleet.podSecurityContext }}. The CA init containers write into theca-localandca-certsemptyDir volumes as uid 3333, and the vulnerability-processing PVC relies on the samefsGroup. If a user overridesfleet.podSecurityContextwithoutfsGroup, those writes fail at runtime. Add a comment that states this constraint, or keepfsGroupoutside the override surface.🤖 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/fleet/values.yaml` around lines 154 - 158, Document in the podSecurityContext values block that fsGroup must remain configured because CA init-container writes and vulnerability-processing PVC access depend on it, or restructure the values so fsGroup cannot be omitted when users override fleet.podSecurityContext.charts/fleet/templates/job-migration.yaml (1)
184-187: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
SYS_PTRACEis dead configuration in both workloads. The proxy now stops over the admin HTTP endpoint, so no container signals the proxy process. The capability is also ineffective for a non-root uid, and it weakens the hardening goal of this PR.
charts/fleet/templates/job-migration.yaml#L184-L187: remove the conditionaladd: [SYS_PTRACE]block, and dropshareProcessNamespace: trueon line 32 if nothing else needs it.charts/fleet/templates/vulnprocessing/cronjob.yaml#L316-L319: remove the conditionaladd: [SYS_PTRACE]block, and dropshareProcessNamespace: trueon line 77 if nothing else needs it.🤖 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/fleet/templates/job-migration.yaml` around lines 184 - 187, Remove the conditional SYS_PTRACE capability blocks from charts/fleet/templates/job-migration.yaml lines 184-187 and charts/fleet/templates/vulnprocessing/cronjob.yaml lines 316-319. Also remove shareProcessNamespace: true at lines 32 and 77 respectively if no other workload behavior requires it.
🤖 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 @.github/workflows/changes.yaml:
- Around line 19-20: Add pull-requests read permission to the collect job’s
permissions block in changes.yaml, alongside contents: read, so
dorny/paths-filter@v4.0.2 can access pull-request file lists while preserving
the existing permissions.
In `@charts/fleet/templates/job-migration.yaml`:
- Around line 177-188: Update the fleet-migration Job template to add a writable
tmp emptyDir volume and mount it at /tmp, matching the existing deployment.yaml
pattern so the migration command can run with readOnlyRootFilesystem enabled.
Place the volume alongside the Job’s existing volumes and the mount alongside
its container mounts, and preserve the OpenFrame sentinel in values.yaml around
the corresponding fork-specific edit.
In `@charts/fleet/values.yaml`:
- Line 153: Update the five OPENFRAME(hardening) sentinel headers to include the
documentation link “— openframe/docs/helm-chart.md”: charts/fleet/values.yaml
lines 153 and 378, and charts/fleet/templates/deployment.yaml lines 523, 591,
and 606. Preserve each header’s existing change slug and rationale.
- Around line 154-158: Ensure the vulnerability-processing pod security contexts
always provide fsGroup 3333 and never render fsGroupChangePolicy without it. In
charts/fleet/values.yaml lines 154-158, keep fsGroup documented as required or
expose it separately; update charts/fleet/templates/vulnprocessing/bind-job.yaml
lines 36-40 and charts/fleet/templates/vulnprocessing/cronjob.yaml lines 80-86
to default fsGroup while preserving optional overrides and applying the same
behavior in both templates.
---
Nitpick comments:
In `@charts/fleet/templates/job-migration.yaml`:
- Around line 184-187: Remove the conditional SYS_PTRACE capability blocks from
charts/fleet/templates/job-migration.yaml lines 184-187 and
charts/fleet/templates/vulnprocessing/cronjob.yaml lines 316-319. Also remove
shareProcessNamespace: true at lines 32 and 77 respectively if no other workload
behavior requires it.
In `@charts/fleet/values.yaml`:
- Around line 154-158: Document in the podSecurityContext values block that
fsGroup must remain configured because CA init-container writes and
vulnerability-processing PVC access depend on it, or restructure the values so
fsGroup cannot be omitted when users override fleet.podSecurityContext.
🪄 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: 2ea75b78-a988-4bb2-b0d3-3d5801583622
📒 Files selected for processing (11)
.github/workflows/changes.yaml.github/workflows/changes.yml.github/workflows/release.yml.github/workflows/test.ymlDockerfilecharts/fleet/templates/deployment.yamlcharts/fleet/templates/job-migration.yamlcharts/fleet/templates/vulnprocessing/bind-job.yamlcharts/fleet/templates/vulnprocessing/cronjob.yamlcharts/fleet/values.yamltools/ci/helm-values/enable-cloudsql-proxy.yaml
💤 Files with no reviewable changes (1)
- .github/workflows/changes.yml
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 (1)
charts/fleet/templates/vulnprocessing/cronjob.yaml (1)
94-96: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd an
OPENFRAMEsentinel around the shutdown edit.The changed shell block at Line 94 through Line 96 is not enclosed by an
OPENFRAMEstart/end pair. Add YAML-level markers with a change slug, rationale, and documentation link. Keep the markers outside the|shell block.Proposed marker placement
args: + # >>> OPENFRAME(cloudsql-shutdown): preserve vuln_processing exit status while stopping the proxy through HTTP — openframe/docs/helm-chart.md - | /usr/bin/fleet vuln_processing; {{- if .Values.gke.cloudSQL.enableProxy }} rc=$?; wget -q -Y off -T 5 -O- http://127.0.0.1:9091/quitquitquit || true; exit $rc; {{- end }} + # <<< OPENFRAME(cloudsql-shutdown)As per coding guidelines:
**/*.{go,yaml,tpl}edits in a shared upstream file must be wrapped inOPENFRAMEsentinel comments with a change slug, rationale, and documentation link.🤖 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/fleet/templates/vulnprocessing/cronjob.yaml` around lines 94 - 96, Wrap the changed shutdown command block in the cronjob YAML with YAML-level OPENFRAME start and end sentinel comments, keeping both markers outside the pipe-delimited shell block. Include a unique change slug, concise rationale, and documentation link in the markers, and leave the rc capture, quitquitquit request, and exit behavior unchanged.Source: Coding guidelines
🤖 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/fleet/templates/vulnprocessing/cronjob.yaml`:
- Around line 308-312: Update the fleet.securityContext defaults in values.yaml
to include readOnlyRootFilesystem: true, preserving the existing
security-context settings. Render the default Fleet chart and verify the
generated CronJob container includes the read-only root filesystem setting.
---
Outside diff comments:
In `@charts/fleet/templates/vulnprocessing/cronjob.yaml`:
- Around line 94-96: Wrap the changed shutdown command block in the cronjob YAML
with YAML-level OPENFRAME start and end sentinel comments, keeping both markers
outside the pipe-delimited shell block. Include a unique change slug, concise
rationale, and documentation link in the markers, and leave the rc capture,
quitquitquit request, and exit behavior 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: c5abea5f-d852-47b2-a376-ab1d431c9fb7
📒 Files selected for processing (4)
.github/workflows/changes.yamlcharts/fleet/templates/deployment.yamlcharts/fleet/templates/job-migration.yamlcharts/fleet/templates/vulnprocessing/cronjob.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/changes.yaml
- charts/fleet/templates/job-migration.yaml
- charts/fleet/templates/deployment.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/fleet/templates/vulnprocessing/cronjob.yaml`:
- Around line 94-96: Add an OPENFRAME sentinel immediately before the modified
rc/quitquitquit shutdown block, including a unique change slug, rationale, and
documentation link; close the sentinel immediately after exit $rc. Keep the
existing shutdown commands and behavior unchanged.
- Around line 94-96: Update the post-vuln_processing cleanup in the cronjob
command so cloudsql-proxy is guaranteed to terminate even when /quitquitquit
does not respond within the timeout, using a native sidecar lifecycle or a
reliable fallback. Preserve the captured rc and exit with that Fleet status
after cleanup. Add an OPENFRAME sentinel containing the change slug, rationale,
and documentation link as required by the repository convention.
🪄 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: 9faadf82-b619-4f5f-93b3-22b110405ff0
📒 Files selected for processing (4)
charts/fleet/templates/deployment.yamlcharts/fleet/templates/job-migration.yamlcharts/fleet/templates/vulnprocessing/cronjob.yamlcharts/fleet/values.yaml
💤 Files with no reviewable changes (1)
- charts/fleet/templates/deployment.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- charts/fleet/values.yaml
- charts/fleet/templates/job-migration.yaml
ivan-flamingo
approved these changes
Aug 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
securityContext moved into values so fleet runs non-root on a read-only rootfs.
Cloud SQL proxy bumped to v2, so the args changed and it stops over http now
instead of pgrep and kill.
Summary by CodeRabbit
New Features
Improvements