improve docker build time + arbitrary non root uid

[alpetric]

• removes the recursive chmod (2.3 mins time gain)
• improves permissions to allow arbitrary non root uid

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	f22fb3c
Status:	✅  Deploy successful!
Preview URL:	https://66cc7215.windmill.pages.dev
Branch Preview URL:	https://alp-docker-build-time.windmill.pages.dev

View logs

[claude]

Claude finished @alpetric's task in 1m 31s —— View job

---

Review of PR #7440: Improve Docker Build Time + Arbitrary Non-root UID

• Analyze the Dockerfile changes
• Review for code quality and best practices
• Check for potential security implications
• Provide feedback

---

Summary

This PR makes two key improvements to the Dockerfile:

1. Build time optimization (~2.3 min saved): Replaces recursive chmod -R 777 with a targeted find ... -type d -exec chmod 777 {} + approach
2. Arbitrary non-root UID support: Restructures cache directory permissions to allow containers to run with any UID (not just 1000)

---

Review Feedback

✅ Positive Changes

1. Build time improvement is well-implemented: Using find -type d -exec chmod 777 only changes directory permissions, avoiding the expensive recursive permission change on thousands of files. This is a significant optimization.

2. Good cache architecture: The approach of building caches in a temp location (/tmp/build_cache/) and then copying with proper permissions is clean and maintainable.

3. Clear comments: The added comments explain the rationale well (e.g., "chmod a+rw adds read+write WITHOUT removing execute bits").

4. ENV HOME=/tmp: Good addition for arbitrary UID support - this ensures processes have a writable home directory regardless of which user runs the container.

⚠️ Suggestions/Observations

1. Line 266: USER root is redundant - The Dockerfile runs as root by default, and there's no prior USER directive switching to a non-root user. This line has no effect. Consider either:

   • Removing it entirely, OR
   • Changing to USER windmill if the intent was to run as the non-root user by default (more secure)

2. Removed uv venv call (was line 197-199 in original): The RUN uv venv command was removed. Verify this was intentional and doesn't affect runtime behavior.

3. Removed /root/.cache copy to /home/windmill/.cache: The original Dockerfile copied cache from root to the windmill user. This is now handled differently via /tmp/.cache with 777 permissions. Ensure this doesn't break any tooling that expects cache in /home/windmill/.cache.

4. Minor typo fix: Good catch fixing the Cr,.eate typo to Create on line 256.

🔒 Security Considerations

• The 777 permissions on directories in /tmp/windmill and ${APP} are acceptable for a container environment where isolation is at the container level rather than user level.
• Setting HOME=/tmp is a common pattern for arbitrary UID support in OpenShift/Kubernetes environments.

---

Verdict

Looks good to merge! The changes are well-reasoned and achieve the stated goals. The only actionable item would be to address the redundant USER root line - either remove it or change to USER windmill if non-root execution is desired by default.

---

Testing Instructions

These changes affect Docker image building and runtime permissions. To verify:

Build the Docker image and confirm it completes faster than before (especially the final permission-setting steps). Then run the container with an arbitrary UID using docker run --user 12345:12345 <image> and verify that Windmill starts correctly and can write to its cache directories (/tmp/windmill/cache/, /tmp/windmill/logs/, etc.). Also test with the default user (UID 1000) to ensure backward compatibility.