AB-437: Make files in /code owned by the acousticbrainz user #372
Conversation
Dockerfile
Outdated
| COPY package.json /code | ||
| COPY --chown=acousticbrainz:acousticbrainz package.json /code | ||
|
|
||
| USER acousticbrainz |
There was a problem hiding this comment.
Thinking out loud about this - if we don't want to create the user in the -base target, we could skip the installation of npm packages here. Instead, only install in -prod. During dev we get users to run npm install again anyway to create node_modules in their acousticbrainz-server which is bind-mounted into the container.
This allows us to keep running as root in development (and fixes the concern in the PR body).
Still a pending question about when we should chown /code (and requirements.txt) in the -prod build
There was a problem hiding this comment.
I would prefer not to add an extra step to the build instructions. In ListenBrainz, we use a different container to build npm assets automatically into the correct place, which is ideally what I'd want for AB too.
|
This change causes jenkins to fail too because it tries to write results to |
There was a problem hiding this comment.
FWIW, MusicBrainz Server is installed under /home/musicbrainz/musicbrainz-server path and ownership of musicbrainz user in containers produced from musicbrainz-server repository (used in production, beta, test, and ci), see https://github.com/metabrainz/musicbrainz-server/search?q=chown_mb&unscoped_q=chown_mb
Development setup is available from musicbrainz-docker repository (mbvm-38-dev branch) but it does not share the same codebase for now, it just installs MBS under /musicbrainz-server path and ownership of root user. It should use the same standard as the main musicbrainz-server repository uses and ultimately be merged into it.
Ideally, /opt/musicbrainz-server would be a cleaner installation path but we are not there yet.
There was a problem hiding this comment.
I think we should just put the profile file in some other dir in production to fix the bug. The cons about making the dev environment a bit more inconvenient seem worse than the benefit we get out of running as a non-root user in dev, imo.
Also, creating a user with a uid that doesn't exist in the host on dev sounds non-ideal.
|
we also decided that we should add the test script from jenkins into the repo, so that jenkins isn't the only location that it's stored. |
We load code as a volume so not needed while building the image Add code in a specific step for tests, because they don't use a volume Run production entrypoint as root
|
Tests fixed. In the end I decided to leave dev/test running as root. The jenkins script was already in the repo here, so I just updated the project configuration to run the script from the repo instead of having it copied into the execute shell configuration |
This was a little bit more involved than I had hoped, because npm installs things in the working directory... Because of this I had to make the
acousticbrainzuser earlier, and thenCOPY --chowneverywhere, and useUSERto switch between users. Now, everything in /code is owned by acousticbrainz.Some upsides: now, everywhere that you run this, it's running as a non-privileged user!
Some downsides
pip installin a shell if you temporarily want to install something while developing. Should we revert back to running as root for this?sudo chownmagic@paramsingh any feedback on what uid we should be running as in dev?