task: add check if install.sh was run with root privileges #3069
Conversation
There was a problem hiding this comment.
I'm not against this. In fact, I would recommend this since regular Docker installation requires higher privileges than regular Linux user. But, what should we do with people that don't have root access and currently are running sentry self-hosted normally? Should we add additional environment variable or a CLI flag that states they're aware about what they're doing?
So, it would be something like:
./install.sh --explicit-no-root-access
# or
EXPLICIT_NO_ROOT_ACCESS=true ./install.sh
|
I'd just print a warning. I don't recall root being required from the old times 😅 If it is required we should also note in the docs too. |
but in the most cases the installation fails if you don't run the script with root I think the idea from @aldy505 with the additional parameter or environment variable is great |
|
Okay, turns out we do say you should use sudo here: https://github.com/getsentry/self-hosted?tab=readme-ov-file#using-linux I'm +1 to the idea where we stop unless specifically overriden by a flag or env variable. The stopping can be a prompt too btw (you should be using sudo, are you sure you wanna continue?). Finally don't forget the CI env variable handling as mentioned earlier. |
the idea with the prompt is great too |
|
I've added a check, if the environment variable |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3069 +/- ##
=======================================
Coverage 99.01% 99.01%
=======================================
Files 3 3
Lines 203 203
=======================================
Hits 201 201
Misses 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
We are running self-hosted without sudo without any problem, also git grep sudo does not show any command which needs sudo, although once it has run install.sh with sudo, it probably needs to get run with sudo next times as well, but sudo is not needed AFAICS.
Okay, turns out we do say you should use sudo here: https://github.com/getsentry/self-hosted?tab=readme-ov-file#using-linux
@BYK This just states that if you are using an environment variable and sudo together, put sudo before environment setting.
For example: There is a new script https://github.com/getsentry/self-hosted/blob/master/install/update-docker-volume-permissions.sh#L9 which is updating permissions, and this won't work without root privileges or Permissions to this Folder. |
|
Root is not required to run the install script.
There should've been a warning printed out during the install script that the docker volume permissions were not updated properly. I believe that is the only part of the install script that may fail if permissions are not granted to the user. Is that what your install was also failing on @LvckyAPI? |
Yeah, on every three instances that we run. Before that we could run the install script relatively easily even without root, but not since then. And since it's documented everywhere that you should use sudo I thought, so that you don't spend ages looking for the problem and opening a ticket again and again, just force sudo and everything will work. |
| echo "You should be using sudo, are you sure you want to continue? [y/n]" | ||
| read -r answer | ||
| if [ "$answer" != "${answer#[Yy]}" ] ;then | ||
| echo "Continuing without sudo. This might cause issues later on." |
There was a problem hiding this comment.
I'm thinking of adding something along the lines of "You should know what you're doing as this might cause issues later on". But that might sound a bit demeaning for some people.
There was a problem hiding this comment.
hmm, into line 16?
There was a problem hiding this comment.
Yeah but I don't think that's the proper wording 😂
There was a problem hiding this comment.
You need to know what you're doing, because this could cause issues later on. ?
@LvckyAPI You should be in docker group user to be able to run docker containers, create docker volumes, and so on. Then you should be able to change docker volume permissions as well.
@aldy505 On the contrary we should not recommend to use root IMO. you should be in docker group to use docker anyway, and you will be fine with that to make all changes in docker containers, volumes, and so on.
@hubertdeng123 I think we should run a container with root user, mounting this volumes, and change permissions with that container, instead of using root user on host. |
Co-authored-by: Reinaldy Rafli <[email protected]>
I'm thinking of having self-hosted as something that would be easier to set up for new people coming in. That's the reason on why I prefer that way. Stating to run
+1 I'd like to see this too. |
|
I'm with @aminvakil here:
Much better solution and avoid the need for sudo, which is always a good thing. |
This does sound like a better approach, I can make a PR for this |
|
#3084 should fix this, so we don't need to run install script with root privileges |
Story
Currently, it's possible to run the
install.shwithout root privileges.This could cause misunderstandings if errors are thrown while running the script without root.
I added a check, if the script was run with root privileges.
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.