Skip to content

fix(helm): enable readOnlyRootFilesystem + /tmp emptyDir for executor#59

Open
bart-agent[bot] wants to merge 1 commit intomainfrom
bart/fix--remote-executor-readonly-fs-cb860c92
Open

fix(helm): enable readOnlyRootFilesystem + /tmp emptyDir for executor#59
bart-agent[bot] wants to merge 1 commit intomainfrom
bart/fix--remote-executor-readonly-fs-cb860c92

Conversation

@bart-agent
Copy link
Copy Markdown
Contributor

@bart-agent bart-agent bot commented Apr 12, 2026

Summary

  • Enables readOnlyRootFilesystem: true + runAsNonRoot: true + runAsUser: 1000 in the default securityContext (previously commented-out defaults)
  • Adds tmpDir feature flag (enabled: true by default) that mounts a writable emptyDir at /tmp — required so uv's cache dir and other tooling that writes to /tmp continue to work under the enforced read-only root FS
  • Both changes are backwards-compatible: existing deployments that already set securityContext or override tmpDir.enabled: false are unaffected

Relates to CUS-8308, ING-2257.

Companion PR: The application-level fix (injecting UV_CACHE_DIR + HOME env vars into ingestion subprocesses) is in acryldata/datahub-fork#8991.

Test plan

  • Deploy to a test customer environment with readOnlyRootFilesystem: true and verify executor starts and runs ingestion without filesystem errors
  • Verify /tmp is writable inside the container (uv cache writes succeed)
  • Verify existing customers with securityContext: {} override are unaffected

🤖 Generated with Claude Code

Enables readOnlyRootFilesystem: true in the default securityContext and adds
a tmpDir emptyDir volume (enabled by default) mounted at /tmp, so uv and
other tools that write to /tmp work correctly in hardened pod environments.

Relates to CUS-8308, ING-2257.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@skrydal skrydal left a comment

Choose a reason for hiding this comment

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

This change will effectively break deployments for users who do not use code having the change from other PR: https://github.com/acryldata/datahub-fork/pull/8991/changes
As that one sets the HOME/UV_CACHE_DIR to /tmp/....
This is too high of a risk and moreover we shouldn't be mixing up 2 layers of responsibility (config and execution). Please set default values for HOME and UV_CACHE_DIR in values of the helm chart. The change in the datahub-executor will be rendered not needed then.

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.

1 participant