[SYCL][Devops] Fix DockerFile linting issues discovered by trivy
#16290
Conversation
See https://avd.aquasec.com/misconfig/ds017 Docker best practices says that running `update` and `install` commands separately may lead to situations where Docker skips `update` step and re-uses cache leading to outdated versions of packages being installed.
See https://avd.aquasec.com/misconfig/ds002 Made it so that the last `USER` command in `base` and `build` is not `root`.
See https://avd.aquasec.com/misconfig/ds002 Made it so our docker files have at least one `USER` command which is not `root`.
|
Note for reviewers: I have very little idea about how to validate this (though I see that some CI containers job have been launched), so any suggestions here are appreciated. |
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
CI failed in making the docker image, but for running them yeah I don't think we can do that here |
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
AlexeySachkov
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Apparently some containers re-use other containers, that issue was fixed. Failure on AMD seems to be an infrastructure one (runner got disconnected). Moreover, we don't use images built as part of the PR in our pre-commit, so it shouldn't really be related. @sarnex, @intel/dpcpp-devops-reviewers, @intel/llvm-gatekeepers, we need to agree on a strategy here. Considering that this change has a potential to disrupt all pre-commits, I suggest that we merge it ASAP and then monitor all pre-commits to quickly revert if we spot any issues. |
|
@AlexeySachkov That's fine with me, can you watch the precommit CI or should I? If you, let me know when you're ready for merge. |
I can watch it for a few more hours, let's hit merge |
AlexeySachkov
deleted the
private/asachkov/apply-docker-best-practices
branch
|
cool, if something crazy happens feel free to ping me |
|
https://github.com/intel/llvm/actions/runs/12239030332 - post-commit job to re-build containers has finished successfully. If I understand correctly, starting from this point all new pre-commits should use new containers. |
|
yeah i think we need to wait for a new precommit/postcommit run. you can run them manually in a fake pr if you want or just wait and check |
|
@AlexeySachkov Looks like the nightly failed, can you take a look? https://github.com/intel/llvm/actions/runs/12248716063/job/34169121051#step:4:194 |
Thanks for letting me know, @sarnex, we seem to have one more case of one docker image re-using another which is not covered by pre-commit. I will address |
We already have `sycl` user created by a base image, no need to repeat that. This is a follow-up to intel#16290 to address an issue which was not caught by pre-commit.
#16324 should hopefully fix that |
…16411) This is a re-submit of #16290 with fixes from #16324 and some more extra changes. Issues addressed: - AVD-DS-0017 (HIGH): The instruction 'RUN <package-manager> update' should always be followed by '<package-manager> install' in the same RUN statement. See https://avd.aquasec.com/misconfig/ds017 - AVD-DS-0002 (HIGH): Specify at least 1 USER command in Dockerfile with non-root user as argument See https://avd.aquasec.com/misconfig/ds002 - AVD-DS-0002 (HIGH): Last USER command in Dockerfile should not be 'root' Issues remaining: - AVD-DS-0026 (LOW): Add HEALTHCHECK instruction in your Dockerfile See https://avd.aquasec.com/misconfig/ds026 I didn't add `HEALTHCHECK` command to our containers, because I don't know if that makes sense and which command to launch. I.e. our containers they only provide some pre-installed tools, but they don't launch any services which we could check. User creation was outlined into a separate helper script. Our containers only come with `sycl_ci` user now which requires a password to use `sudo`. However, it is still possible to get the original `sycl` user for those who uses that container locally and needs `sudo` access.
Linter/checker used: trivy v0.58.0
Issues addressed:
See https://avd.aquasec.com/misconfig/ds017
See https://avd.aquasec.com/misconfig/ds002
Issues remaining:
See https://avd.aquasec.com/misconfig/ds026
I didn't add
HEALTHCHECKcommand to our containers, because I don't know if that makes sense and which command to launch. I.e. our containers they only provide some pre-installed tools, but they don't launch any services which we could check.