Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 9 commits into from
Closed

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

wants to merge 9 commits into from

Conversation

kassovix
Copy link
Contributor

@kassovix kassovix commented Aug 10, 2018

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

There may be a better way, but this is working.
@newtmitch
Copy link
Owner

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
Copy link

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
Copy link
Owner

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

kassovix commented Feb 9, 2019

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
Copy link

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
Copy link

channias commented May 7, 2019

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

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

@newtmitch
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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

@newtmitch
Copy link
Owner

@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 newtmitch mentioned this pull request May 13, 2019
2 tasks
@newtmitch
Copy link
Owner

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. 😃

@newtmitch newtmitch closed this May 13, 2019
@kassovix
Copy link
Contributor Author

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 ;)

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.

None yet

5 participants