newtmitch / docker-sonar-scanner

Quick sonar scanner docker image
MIT License
129 stars 88 forks source link

Allow non-root user to execute sonar-scanner #10

Closed kassovix closed 5 years ago

kassovix commented 6 years ago

There may be a better way, but this is working for a use in Jenkins pipelines. To reproduce the issue, just run with -u option : $ docker run --rm -ti -u 1000:1000 -v $(pwd):/root/src newtmitch/sonar-scanner:3.2 sonar-scanner -Dsonar.projectBaseDir=./src/dev /bin/sh: 0: Can't open /root/sonar-scanner/bin/sonar-scanner

newtmitch commented 5 years ago

My guess is that the better approach is to create a non-root user, install the binaries in a place that user can access directly, and bypass using root at all. That's likely the better way to solve this issue. I don't know much about using this image in a project that uses a Jenkins pipeline, however, so not sure why Jenkins would be causing a problem based on which user inside the docker image itself was running binaries, and if using a user other than root would actually solve that problem entirely.

katanacrimson commented 5 years ago

Installing in a directory other than root would be far better. chmod'ing the /root directory is perpendicular to *nix security designs; it should be owned exclusively by root and not interactable by any other user.

Recommend looking into /usr/local/bin or /opt/ for an install location instead.

newtmitch commented 5 years ago

Installing in a directory other than root would be far better. chmod'ing the /root directory is perpendicular to *nix security designs; it should be owned exclusively by root and not interactable by any other user.

Recommend looking into /usr/local/bin or /opt/ for an install location instead.

Good call, @damianb. I'll look into doing that.

kassovix commented 5 years ago

I made changes to do as @damianb suggested. Let me know if this is better, and if you want me to change other Dockerfiles as well ;)

theoutsider24 commented 5 years ago

Is there any idea when this may be part of the latest image on Docker Hub? I use this image in our Bamboo environment but the .scannerwork file ends up belonging to root and can't be cleaned down after scans

channias commented 5 years ago

+1 If this PR can be merged, it'll be very usefull !

@newtmitch any idea when you have time to merge this ?

newtmitch commented 5 years ago

Sorry everyone, this stuff got away from me. Let me take a look at this in the next couple of days. Sorry for the wait.

newtmitch commented 5 years ago

I made changes to do as @damianb suggested. Let me know if this is better, and if you want me to change other Dockerfiles as well ;)

Hey @kassovix - my testing so far says this looks pretty good. Can you make changes to the other Dockerfiles to bring them all into alignment? Also please update the README.md as it references /root/src everywhere as well, and it would be good for those to be updated with this same set of changes for quick reference.

Thanks for the assist!

newtmitch commented 5 years ago

@kassovix once you've updated those other files I can merge this in.

newtmitch commented 5 years ago

@kassovix I'm going to try to replicate what you've done here with the other Dockerfiles and the README itself, given the time this has been open, and in case you're not watching. 😄 If you do decide to continue working on this, please let me know and I'll hold off.

One more request, though, before you do a PR with updated work, is to squash your commits down to a single commit before opening the PR (or down to a reasonable set of commits that need to be kept post-merge for some reason). Looks like you had a few commits as you were working through the details but those could be squashed to a single commit to keep the commit as a bubble and easy to track. Let me know if you need more details here, as I clearly don't have PR rules for this repo (I didn't expect it to actually get used this much, honestly).

newtmitch commented 5 years ago

I'm closing this PR for now, I've addressed in #26 and updated the remaining Dockerfiles, and the README. I've also used the more standard /usr/src for the location of the source code in the container, as I've seen that used in other projects as well for source.

@kassovix thanks much for the work here, I used your changes as the basis for the rest, even though I didn't merge your PR as-is. 😃

kassovix commented 5 years ago

Just see your updates here, indeed I was not watching very closely :) Thank you for merging this in #27 , I can use back your image now ;)