Skip to content

Helm chart update - Unconditional tmp volume mounts - #44070

Merged
BCTBB merged 2 commits into
mainfrom
helm-tmp-mount-fix
Apr 23, 2026
Merged

Helm chart update - Unconditional tmp volume mounts#44070
BCTBB merged 2 commits into
mainfrom
helm-tmp-mount-fix

Conversation

@BCTBB

@BCTBB BCTBB commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Related issue: Resolves #43330

  • Update tmp volume mounts to be unconditional
    • Fixes an issue where fleet.tls.enabled = false, databse.tls.enabled = false, osquery.logging.statusPlugin != "filesystem", osquery.logging.resultPlugin != "filesystem", and fleet.additionalCAs.enabled = false, all at once, would lead to exclusion of the tmp volume mount and affecting software installer uploads.
  • Bump helm chart version from 6.8.10 -> 6.9.0

Summary by CodeRabbit

  • Release
    • Updated Helm chart version to v6.9.0
  • Improvements
    • Enhanced deployment configuration to properly support additional Certificate Authority (CA) handling alongside existing security configurations.

@BCTBB
BCTBB marked this pull request as ready for review April 23, 2026 18:00
@coderabbitai

coderabbitai Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This pull request updates the Fleet Helm chart version from v6.8.10 to v6.9.0 in the Chart.yaml metadata file. Additionally, the deployment.yaml template is modified to expand the conditional logic for volumeMounts and volumes blocks. These blocks now render when fleet TLS, database TLS, osquery filesystem logging, or fleet.additionalCAs.enabled is true, ensuring the ca-certs volume mount and CA volume definitions are properly gated with the additionalCAs condition.

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title mentions 'Unconditional tmp volume mounts' but the actual changes also include making CA volume conditional alongside the tmp volume, which is not reflected in the title. Consider clarifying the title to reflect both the unconditional tmp volume mounts AND the conditional CA volume changes, or provide more specific context about the primary change.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the fix and relates it to issue #43330, but doesn't follow the provided template structure with checkboxes.
Linked Issues check ✅ Passed The PR addresses issue #43330 by making tmp volume mounts unconditional and updating CA volume conditions, ensuring tmp volume is available for software installer caching regardless of TLS/logging settings.
Out of Scope Changes check ✅ Passed The changes are all in-scope: version bump (6.8.10 to 6.9.0) and conditional logic updates for volume mounts directly address issue #43330's requirement for writable tmp directories.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch helm-tmp-mount-fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
charts/fleet/templates/deployment.yaml (1)

544-545: Optional: unify emptyDir style.

Line 545 uses emptyDir: (null) while line 581 uses emptyDir: {}. Both are accepted by Kubernetes, but using the same form helps readers.

Proposed tweak
         - name: tmp
-          emptyDir:
+          emptyDir: {}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@charts/fleet/templates/deployment.yaml` around lines 544 - 545, The file uses
two styles for emptyDir volumes; unify them for readability by changing the tmp
volume definition (volume name "tmp" in the Deployment spec) to use the same
form as the other emptyDir occurrence (e.g., replace emptyDir: with emptyDir:
{}), or alternatively update the other occurrence to use the null form—ensure
both emptyDir blocks (including the one for volume "tmp") use the identical
style throughout the template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@charts/fleet/templates/deployment.yaml`:
- Around line 544-545: The file uses two styles for emptyDir volumes; unify them
for readability by changing the tmp volume definition (volume name "tmp" in the
Deployment spec) to use the same form as the other emptyDir occurrence (e.g.,
replace emptyDir: with emptyDir: {}), or alternatively update the other
occurrence to use the null form—ensure both emptyDir blocks (including the one
for volume "tmp") use the identical style throughout the template.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ae5076c4-631a-4f7d-9384-098563c28b94

📥 Commits

Reviewing files that changed from the base of the PR and between 28908e6 and 37f3d56.

📒 Files selected for processing (2)
  • charts/fleet/Chart.yaml
  • charts/fleet/templates/deployment.yaml

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@BCTBB
BCTBB merged commit 0594b5a into main Apr 23, 2026
7 checks passed
@BCTBB
BCTBB deleted the helm-tmp-mount-fix branch April 23, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read-only file system error on software installation in web UI due to Helm chart deployment.yaml template

2 participants