Fix artifact upload permissions in musl aarch64 CI job#616
Fix artifact upload permissions in musl aarch64 CI job#616r1viollet wants to merge 4 commits intomainDataDog/java-profiler:mainfrom r1viollet/fix-musl-aarch64-artifact-permissionsDataDog/java-profiler:r1viollet/fix-musl-aarch64-artifact-permissionsCopy head branch name to clipboard
Conversation
Docker containers create files as root, causing permission denied errors when GitHub Actions tries to read them during artifact zip creation. Add explicit chmod steps after container execution to ensure files are readable by the runner user.
|
CI Test ResultsRun: #28379855368 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-06-29 14:54:15 UTC |
The unwinding report generation creates files in both top-level build/ and subproject build directories. Ensure all are readable by the runner.
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/121631747 |
| echo "musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt | ||
| exit 1 | ||
| fi | ||
| - name: Fix permissions for artifact upload |
There was a problem hiding this comment.
[Sphinx Review — LOW] This fixes the symptom (root-owned files left by the container) rather than the cause. The Docker invocations run as root by default (no --user flag) and bind-mount the workspace, so every file the container writes is owned by root and unreadable/undeletable by the runner user. The chmod-after-the-fact approach must be replicated at every container step (already two copies, with diverging scope) and re-applied for any future container step.
Suggestion: Consider running the containers with --user $(id -u):$(id -g) so emitted files are owned by the runner user, removing the need for repeated post-hoc sudo chmod. If the in-container toolchain requires root, keep the chmod but extract it to a single reused step.
| run: | | ||
| # Files created by Docker container may have restrictive permissions | ||
| # that prevent GitHub Actions from reading them during artifact creation | ||
| sudo chmod -R +r build/ ddprof-test/build/ ddprof-lib/build/ 2>/dev/null || true |
There was a problem hiding this comment.
[Sphinx Review — MEDIUM] The permission-fix commands swallow every failure via 2>/dev/null || true, and the step runs if: always(). If sudo is unavailable, the path is missing, or the chmod is denied, the failure is silently discarded and the step reports success. The downstream consumer prepare_reports.sh also copies from the same root-owned trees with || true on every cp/find. If the chmod silently fails, prepare_reports.sh silently skips the unreadable sources and the resulting test-reports artifact is quietly incomplete — and when the subsequent Upload build artifacts step then fails for the exact permission reason this code was meant to fix, the root cause is invisible.
Suggestion: Drop 2>/dev/null so chmod failures appear in the step log, enabling diagnosis when artifacts are still incomplete or the upload still fails. Keep || true only if a clean exit is genuinely desired, or echo a warning when chmod returns non-zero.
| run: | | ||
| # Files created by Docker container may have restrictive permissions | ||
| # that prevent GitHub Actions from reading them during artifact creation | ||
| sudo chmod -R +r build/ ddprof-test/build/ ddprof-lib/build/ 2>/dev/null || true |
There was a problem hiding this comment.
[Sphinx Review — LOW] sudo chmod -R +r build/ ddprof-test/build/ ddprof-lib/build/ makes every file under the build trees world-readable. This job runs on a self-hosted shared runner group (group: ARM LINUX SHARED), not an ephemeral single-tenant GitHub-hosted runner. On a shared runner the filesystem can be visible to other jobs/tenants, so blanket world-readable build output (which may contain build-injected tokens or signing material) is a non-trivial exposure surface.
Suggestion: Restrict the recursive chmod to the exact report/artifact subtrees that are uploaded (e.g. build/reports, ddprof-test/build/reports) rather than the entire build/ trees, or narrow to o+r only where required.
| \"${{ matrix.config }}\" \"${{ env.JAVA_HOME }}\" \"${{ env.JAVA_TEST_HOME }}\" | ||
| " | ||
| # Fix permissions after Docker container execution (both top-level and subproject builds) | ||
| sudo chmod -R +r build/ ddprof-test/build/ 2>/dev/null || true |
There was a problem hiding this comment.
[Sphinx Review — LOW] The second permission-fix block (appended to Generate Unwinding Report) covers only build/ and ddprof-test/build/, dropping ddprof-lib/build/ which the first block covers. The unwinding-report container runs the build as root in the same way as the test container and can leave root-owned files under ddprof-lib/build/. On the success path prepare_reports.sh reads ddprof-lib/build (find ddprof-lib/build -name 'libjavaProfiler.*' and cp -r ddprof-lib/build/tmp), so files newly created as root there by the unwinding container are not re-chmod'd and may be silently skipped.
Suggestion: Align the second chmod block to cover ddprof-lib/build/ as well, matching the first block, or consolidate both into a single reused step to keep the scope in sync.
- Remove 2>/dev/null to make chmod failures visible in logs - Add ddprof-lib/build/ to second chmod block for consistency - Keep || true for graceful handling of missing paths
|
Thanks for the detailed review! Fixed the actionable issues: Silent failures (MEDIUM): Removed Inconsistent coverage (LOW): Added Re: Re: world-readable on shared runners - These are test builds without secrets, so the exposure is minimal. If this becomes a concern we can narrow to specific report directories. |
- Use 'a+rX' instead of '+r' (umask-independent) - Capital X adds execute only to directories - Consolidates two chmod commands into one - Update comments to explain the approach
|
Great catch on the umask issue! Switched to |
What does this PR do?:
Fixes permission denied errors when uploading build artifacts in the test-linux-musl-aarch64 CI job.
Motivation:
The Alpine Docker container creates files as root, causing GitHub Actions to fail when trying to read them during artifact zip creation:
Additional Notes:
sudo chmod -R a+rXafter Docker execution to ensure files are readable (umask-independent)Xadds execute only to directories, consolidating the previous two-command approachbuild/,ddprof-test/build/,ddprof-lib/build/|| trueonly handles missing paths gracefully)How to test the change?:
Run the test-linux-musl-aarch64 job and verify artifacts upload successfully without permission errors.
For Datadog employees: