Skip to content

Conversation

AlexeySachkov
Copy link
Contributor

We already have sycl user created by a base image, no need to repeat that.
This is a follow-up to #16290 to address an issue which was not caught by pre-commit.

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.
@AlexeySachkov
Copy link
Contributor Author

#7 [stage-0 2/8] RUN apt update && apt install -yqq wget
#7 0.247 
#7 0.247 WARNING: apt does not have a stable CLI interface. Use with caution in scripts.
#7 0.247 
#7 0.283 Reading package lists...
#7 1.007 E: Could not open lock file /var/lib/apt/lists/lock - open (13: Permission denied)
#7 1.007 E: Unable to lock directory /var/lib/apt/lists/
#7 ERROR: process "/bin/sh -c apt update && apt install -yqq wget" did not complete successfully: exit code: 100

Any ideas how to fix that? Doubt that it is caused by the change

@sarnex
Copy link
Contributor

sarnex commented Dec 10, 2024

did we change anything about permissions or groups? sorry i forget

@AlexeySachkov
Copy link
Contributor Author

did we change anything about permissions or groups? sorry i forget

Well, technically we do switch into sycl user in a base image, which is generally earlier than it used to be. But if that's an issue, then it would have failed like this even before this PR.

I'm not entirely sure which exact user is doing apt-get here

@sarnex
Copy link
Contributor

sarnex commented Dec 10, 2024

i think it used be running as root but now its running as sycl. sycl should have sudo nopass, can you try just adding a sudo to the failing line?

@sarnex
Copy link
Contributor

sarnex commented Dec 11, 2024

🙏

@sarnex
Copy link
Contributor

sarnex commented Dec 11, 2024

 #7 0.324 sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper
#7 0.325 sudo: a password is required

uuh

@AlexeySachkov
Copy link
Contributor Author

 #7 0.324 sudo: a terminal is required to read the password; either use the -S option to read from standard input or configure an askpass helper
#7 0.325 sudo: a password is required

uuh

Perhaps I need to move USER sycl to the beginning. I'm not entirely sure how those base docker images work, i.e. it could be that we invoke those instructions as some "default" user which is not in sudoers

@sarnex
Copy link
Contributor

sarnex commented Dec 11, 2024

@AlexeySachkov I think we need this in the _build image too.

I tried to do the same thing before but it kinda got push back

but if it was running as root before i would argue its actually slightly better with the above change

@AlexeySachkov
Copy link
Contributor Author

@AlexeySachkov I think we need this in the _build image too.

I tried to do the same thing before but it kinda got push back

It seems to me that the main point of push back was --privileged? Let me try add sycl to sudoers

@sarnex
Copy link
Contributor

sarnex commented Dec 11, 2024

oh yeah my bad

@AlexeySachkov AlexeySachkov marked this pull request as draft December 11, 2024 16:49
@AlexeySachkov
Copy link
Contributor Author

Considering that we have reverted the original commit, I will close this for now in favor of a new PR which will be a complete re-submit of the whole refactoring.

sarnex pushed a commit that referenced this pull request Dec 19, 2024
…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.
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.

2 participants