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

Feat: Optional change installation directory #2710

Open
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

LEstradioto
Copy link
Contributor

@LEstradioto LEstradioto commented Jun 28, 2024

/claim #2708

@Silvea12 raised a concern about installing Coolify on an immutable filesystem, such as openSUSE MicroOS

Changes made

  • Updated all references from /data/coolify to retrieve the value from .env, allowing users to specify a custom path.

  • Example installation command:
    curl -fsSL https://cdn.coollabs.io/coolify/install.sh | env COOLIFY_ROOT_PATH=/custom/path/to/coolify bash

  • Users can now change the base directory if needed.


Observations for openSUSE MicroOS (self-installing host)

edit: READ HERE TOO

Even with this patch, setting up on this OS can be challenging. The following steps worked for me:

  1. Change the ID attribute from opensuse-microos to opensuse-tumbleweed:
transactional-update shell
# Edit /usr/lib/os-release
  1. Install Docker before running install.sh:
transactional-update pkg in docker
sudo usermod -aG docker $(whoami)
sudo service docker start
  1. Modify SSH configuration:
echo "PermitRootLogin yes" | sudo tee /etc/ssh/sshd_config.d/coolify.conf
  1. Install necessary packages:
transactional-update pkg in curl wget git jq
  1. Comment or delete these lines from install.sh:
# zypper refresh >/dev/null
# zypper install -y curl wget git jq >/dev/null
  1. Run install.sh

Further improve

If approved, I can further improve install.sh, and reduce this steps, and to support more OSes by using ID_LIKE for derivatives, as I saw in this issue

Breaking Changes

  • BASE_CONFIG_PATH in .env must be renamed to COOLIFY_ROOT_PATH.

@ayntk-ai
Copy link

Does this also work on NixOS: https://nixos.org/ ?

@Silvea12
Copy link

Silvea12 commented Jun 28, 2024

The install steps above actually can break MicroOS - you don't want to be changing the release as it can cause parts of the system that differentiate between Tumbleweed and MicroOS to misbehave - so we definitely want to specify more aliases above. You also don't need to set up the docker group, as the intent is generally to just have a root user if you're running it on a server, so there's not a lot of point. Just needs to be tweaked a little.

That being said, at least with a quick glance over it, this seems to cover most of what is needed minus documentation and some additional changes I'd suggest adding to install.sh (which I have listed at the end)

My own testing steps - these are not ideal either, but they work for now:

# Set up system - install docker and nano (or your editor of choice) + coolify dependencies, remove podman if installed (not required but having 2 runtimes makes things fight sometimes - ideally we get native podman support at some point) as well as other dependencies
transactional-update run sh -c 'zypper ref && zypper dup -y && zypper in -y docker nano curl wget git jq && zypper rm -y podman'
# Reboot into new OS snapshot
reboot

# Enable docker
systemctl enable --now docker
# Set up SSH stuff
echo -e '# Coolify requires root login over SSH to manage this server\nPermitRootLogin yes' > /etc/ssh/sshd_config.d/70-coolify.conf

# Clone the repo
git clone https://github.com/LEstradioto/coolify.git -b Feat--Customize-Install-Directory
cd coolify
nano scripts/install.sh
# Comment out the zypper-related lines as mentioned
# Add opensuse-microos as a supported OS in the $OS_TYPE check for the support list - this should be fixed in this PR, see below for suggestions

# Start it up!
mkdir /var/coolify
COOLIFY_ROOT_PATH=/var/coolify ./scripts/install.sh

It still references /data for some stuff here, but this is an issue of it still pulling from the CDN - as a result, I could not test end to end, as it'd require modifying the install script substantially, and I don't have time to spend on testing to that degree right now.

A few suggested improvements for @LEstradioto:

  • Add a flag or variable that allows skipping of things like dependency installation and platform checks, maybe ALLOW_UNSUPPORTED_PLATFORMS=1 or something, which instead prints a warning and says "Hey if these are missing this may fail, be sure these are available" - a trapdoor for those who are doing funky stuff is always appreciated ❤️
  • Optionally: add explicit support for opensuse-microos in all areas - if it's microos, add these to the script:

    Note, this may be a bad idea unless MicroOS is a platform that support should be dedicated to going forward - it's fairly niche, so I'd just add the trapdoor and call it a day, rather than adding explicit support.

    transactional-update pkg in -y curl wget git jq
    transactional-update apply
  • Add the SSH lines to /etc/ssh/sshd_config.d/70-coolify.conf on all platforms - all major distributions support reading from this path in their default installs, so there's no reason to modify the primary sshd_config file when we can have overrides written here.
  • Move or change the writable path check - this will always fail in its current state, as a directory not existing (which it won't at first in most cases) will fail this check. Could also just check the exit code of mkdir -p "$COOLIFY_ROOT_PATH" to the if check, as it'll return OK even if it exists already - it'll only fail if it's not writable.

If the user doesn't have docker installed, this should probably be managed by the user, not by the install script.
Additionally, the install of curl + wget + git + jq should probably be in a toolbox container or something (MicroOS's fully mutable temporary environment for doing things like this) but it seems to only support docker as a runtime when run with -S (sandboxed), which prevents it being used for this purpose.

With the above changes, it should make the installation a little smoother for people on MicroOS (and potentially other systems too re: the config file change for sshd)

@Silvea12
Copy link

@ayntk-ai While the patchset could help with NixOS packaging, due to the complexity of the change supporting NixOS may be out of scope of the bounty I have put on this issue. The only real key change to this is finishing the prior work on allowing a custom host installation directory. This is by no means a "Support all immutable OSes" bounty, just a key part in making support possible at all. The average user doing curl file.sh | bash is not the average user using things like MicroOS or NixOS, so I wouldn't expect native support.

@LEstradioto
Copy link
Contributor Author

Thanks for the informations.

The error you got after successful install.sh is because code changes must reflect on Prod. That should succeed with no worries if merged.
For now, I think you could clone my repo and run like this:

cp scripts/install.sh install.sh
cp scripts/upgrade.sh upgrade.sh
sed -i 's/^CDN=.*/CDN="."/g' install.sh
sed -i 's/^CDN=.*/CDN="."/g' upgrade.sh
COOLIFY_ROOT_PATH=/var/coolify ALLOW_UNSUPPORTED_PLATFORMS=1 SKIP_DEPENDENCY_INSTALLATION=1 ./install.sh 4.0.0-beta.307

Ive already did a check on the writable of root path, so it fails if no write permission

We could add opensuse-microos. @Silvea12 could help me adding aliases (perhaps check ID_LIKE too)
But, I agree that for any explicit support we should wait @andrasbacsai position

Changes made:

  • Refactor encapsulation to install.sh
  • Added new flags, ALLOW_UNSUPPORTED_PLATFORMS, SKIP_DEPENDENCY_INSTALLATION
  • About ssh lines, actually install.sh do not change the PermitRootLogin, so I just updated the warning message suggesting the override on /etc/ssh/sshd_config.d/70-coolify.conf

@LEstradioto
Copy link
Contributor Author

LEstradioto commented Jun 28, 2024

I did a refactor on install.sh too and added Logo and Colors 😄 @andrasbacsai

@Silvea12
Copy link

Silvea12 commented Jun 29, 2024

Due to the sheer number of changes in the second commit, I'm hesitant to even test this properly to see if it passes the needs of the original issue, as I may be testing code paths that may be canned. @andrasbacsai can you comment on this change? Not really sure what to do here, the second commit to this PR makes a lot of behavioral changes. I'm going to put this on ice on my end and wait for a comment back from you.

Worth a note too, your mentioned instructions don't work, because changing the CDN URL to pull from the local dir doesn't work when fed into curl.

@LEstradioto
Copy link
Contributor Author

Sorry guys.

I will turn this into Draft. I did more stuff then the requested, my bad. The correct is to separate these in another PRs. So I will do this first

BREAKING CHANGES: env var BASE_CONFIG_PATH must be updated to COOLIFY_ROOT_PATH

- eye-checked and changed all hard-coded /data/coolify to the right config
- updated install.sh and upgrade.sh scripts and ensured .env is updated accordingly
@LEstradioto LEstradioto force-pushed the Feat--Customize-Install-Directory branch from 5fcfab8 to 5e1d603 Compare July 1, 2024 20:39
@LEstradioto
Copy link
Contributor Author

Okay, that's it. I undo the last commit.

The command now is like this:
COOLIFY_ROOT_PATH=/another/path SKIP_OS=1 sudo -E scripts/install.sh

Only one flag now, just SKIP_OS (this skips os check and dependencies installs). As there are very few cases where using them separately or individually is necessary.

Copy link

gitguardian bot commented Jul 1, 2024

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
12428579 Triggered Generic Password ea2b6fb app/Http/Controllers/Api/DatabasesController.php View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@Silvea12
Copy link

Silvea12 commented Jul 4, 2024

Aside from $SKIP_OS being an unintuitive flag name to me (but that's personal preference), LGTM? I'm happy to pay out for this on merge by maintainers (once they run their own review)

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

3 participants