FOGProject / fogproject

An open source computer cloning & management system
https://fogproject.org
GNU General Public License v3.0
1.09k stars 221 forks source link

Using the -U parameter with installfog.sh breaks the installation #502

Closed Pieshka closed 1 year ago

Pieshka commented 1 year ago

I tried reinstalling FOG on a test VM. For this, I used the -U parameter to not read the configuration from /opt/fog/.fogsettings (I wanted to change some options). However, it turned out that using this parameter breaks the whole installation because it sets the webroot to "/" instead of "/fog/", so PXE doesn't work, and also the installer itself has problems with the installation (as shown in the screenshot).

Screenshot 2022-10-15 130853

Removing /opt/fog/.fogsettings and starting the installer without the -U parameter, does not cause any problems.

My VM: Fedora 36 FOG 1.5.9.193

Sebastian-Roth commented 1 year ago

Thanks for reporting. As far as I know this is not the only installer option which doesn't work as intended. Very few people use the options and so we hardly ever find out. There are some logic quirks in the installer and webroot is definitely one of them.

Hmm, looks like webroot set to a default in installfog.sh line 444 but only if /opt/fog/.fogsettings does not exist.

Digging through the commit history I found when this was initially added and it looks pretty similar although this was in the OS specific config.sh files without a doupdate check. When this part was moved to the installfog.sh script it ended up within the if [ "$doupdate" = "1" ] check but was moved outside the check shortly after and obviously stayed there up until now - 7 years.

I still don't get the logic of this check. Maybe it tries to fix cases where it was set without slashes in earlier versions of FOG but I am not sure. I think we should remove the check and rather impose a simple default for webroot similar to what we have for many other variables in line 372ff. What do you think?

Pieshka commented 1 year ago

I also think it is better to simplify it. When upgrading, just load the webroot from the .fogsettings file, and in any other case leave the default "/fog/". Unless the user used the -W parameter to specify the webroot themselves (however, when upgrading, this parameter should be ignored - we rather not want to change the webroot when upgrading).

lukebarone commented 1 year ago

So I have a fix for this, by making the webroot set to /fog/ unless set to something else.

Out of curiousity, is there anyone here who does not use /fog/ for their webroot?

Sebastian-Roth commented 1 year ago

@lukebarone Thanks for picking up this topic again.

Out of curiousity, is there anyone here who does not use /fog/ for their webroot?

I have a feeling this was added just because "we can change anything" through installer command line parameters. But I doubt there are many people out there using anything different than the default /fog/. On the other hand it wouldn't be nice to break existing installs on updating. Have you tried to setup a FOG server with a different webroot (using 1.5.9 let's say) and then upgrade from that to the very latest dev-branch to see if that breaks anything? From the logic it should not break but I keep finding quirks in the installer code that hit me in the back... That's why I usually do some testing.

lukebarone commented 1 year ago

I'll give it a try with a custom one and report back

lukebarone commented 1 year ago

There are LOTS of issues with having a custom webroot, it turns out. /fog/ is hardcoded in quite a few areas. (Example: config files for Apache, PHP files that reload to /fog/index.php, etc)

This is going to be a bit of a bigger project if we need to support the custom $webroot variables properly. My existing commit appears to work correctly if the user is comfortable with the default. I suggest we work on dropping the custom URL portion until there is time to deal with it correctly - potentially putting a call out to anyone who has a custom webroot assigned? Or am I completely misunderstanding it?

Sebastian-Roth commented 1 year ago

@lukebarone said:

There are LOTS of issues with having a custom webroot, it turns out.

I have not looked into this in a long time and had a feeling this is not working as intended. Thanks heaps for digging this up!

I suggest we work on dropping the custom URL portion until there is time to deal with it correctly - potentially putting a call out to anyone who has a custom webroot assigned? Or am I completely misunderstanding it?

Not at all. I think you are spot on. I am just not sure if you mean removing that part from the installer completely or just leave things as they are for now. While it would make sense to remove this altogether until it's properly implemented I wonder if it's worth the time (right now) and on the other hand potentially causes issues when removed - not saying it does, just can't overlook the consequences it might have for existing installs and FOG autoinstall setups.

Please open a pull request based on your commit for us to merge in if you are fine with leaving the parameter as is.

Sebastian-Roth commented 1 year ago

Closing this as fixed, thanks to @lukebarone.

Opened a new issue to address the still open question on custom webroot: https://github.com/FOGProject/fogproject/issues/529