search

EFForg / badger-sett

Automated training for Privacy Badger. Badger Sett automates browsers to visit websites to produce fresh Privacy Badger tracker data.
https://www.eff.org/badger-pretraining
MIT License
119 stars 13 forks source link

Replace hardcoded chown user with USER variable #48

Closed copperwall closed 4 years ago

copperwall commented 4 years ago

I tried running this on a machine of mine and I got an error because there was no bennett user on my host machine. Changing this to use $USER made the docker build step work without erroring out and applied the correct permissions to the privacybadger directory so that the application could write to it in the docker container and I could view it on the host machine.

ablanathtanalba commented 4 years ago

I definitely think replacing this hard coded username with something dynamic is a good idea, but this solution won't work with all builds. Maybe replacing with this but then adding some description to the documentation that the user may have to modify that variable for it to work for them? I get around this problem on my local machine by hard coding my own username in. Curious to hear what @bcyphers thinks.

ghostwords commented 4 years ago

this solution won't work with all builds

Could you clarify when using $USER in the Dockerfile wouldn't work?

ablanathtanalba commented 4 years ago

My apologies for not getting back to this sooner! I'm no Docker expert, but I believe this wouldn't always be the case for getting around this issue because it assumes the Docker user id and the group id are both set to the output of $USER.

copperwall commented 4 years ago

Hey @ablanathtanalba, thanks for the feedback. Would changing the chown arguments to use the $UID and $GID variables supplied as build arguments? It looks like the --chown arg to COPY supports those as well.

ablanathtanalba commented 4 years ago

@copperwall does that solution work in your environment? It doesn't work on mine, Ubuntu version 18 and Docker 18.09

It looks like this is an issue that's common in other places: here is an issue in a different repository that looks like they faced the same problem, and came up with a complicated workaround. There is also this tool that looks like it was designed especially to mitigate this problem.

The quickest workaround would probably be adding a section to the README in the setup portion that asks the user to hardcode their own uid and gid into that line. The more complicated (but dynamic) solution would be something like I posted above... what do you think?

copperwall commented 4 years ago

I'm up to look into dynamic solutions! Also I've been testing stuff up on Fedora with Docker 19.03, so I can try out my changes on a Ubuntu/Docker 18 environment too

copperwall commented 4 years ago

Hey @ablanathtanalba, I tried separating the COPY --chown=$USER:$USER privacybadger $PBPATH

into separate COPY privacybadger $PBPATH and RUN chown -R $USER:$USER $PBPATH commands, and that ran successfully for me on Fedora 31 with Docker 19.03 and Ubuntu 18.04 with Docker 18.09.

Does this seem like a good solution to you?

ablanathtanalba commented 4 years ago

Hey @copperwall -- LGTM! This works for me as well in Ubuntu. Nice work! @bcyphers -- what do you think?