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

Only Generate/Regenerate config.php at the End of Installation #31362

Open
GuyPaddock opened this issue Feb 25, 2022 · 1 comment
Open

Only Generate/Regenerate config.php at the End of Installation #31362

GuyPaddock opened this issue Feb 25, 2022 · 1 comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: install and update

Comments

@GuyPaddock
Copy link

GuyPaddock commented Feb 25, 2022

How to use GitHub

  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.

I'm always frustrated when Nextcloud overwrites its configuration file with an incomplete one, leading to users receiving the following message until an admin comes along and resurrects the configuration file from a backup (if they have one!):
2022-02-25

How and Why Does this Happen?

When Nextcloud is running in a container, the /var/www/html/config folder necessarily has to be mounted from a volume so that it persists across container restarts and is available to all replicas. That means that on a provider like Azure or GCP, this is coming from an Azure Files or NFS volume.

Unfortunately, the volume can become inaccessible during normal use. For example, a connection might become idle, get throttled due to IOPS, or become disconnected due to a transient network fault. Unfortunately, when this happens, Nextcloud will think that it is not installed because it can't read the config.php file. Instead of failing with a 500 Internal Server Error, it stupidly writes out a new config.php in place of the old one, which usually is enough to get the host OS to re-establish connectivity to the SMB or NFS share and overwrite the actual copy. From then on, the Nextcloud instance is down no matter how many times the containers are restarted.

Before #14965 was committed, this represented a tremendous security risk because literally anyone happening to drop by after this happened could install Nextcloud and seize control of the existing files hosted within.

Describe the solution you'd like

Nextcloud should NOT write out a new config.php file whenever it realizes one is missing. Instead, this file should ONLY be generated during the installation process, or at the end of the installation. This way, if the configuration volume is inaccessible due to a connection drop, a container orchestrator like Kubernetes can catch it during health checks and automatically cycle the pod to restore the connection, rather than the application being down indefinitely until an admin comes along to restore the overwritten config.

I believe this would also resolve #24301, #25175, and provide an alternate solution to #27492.

Describe alternatives you've considered

Since 2019, to mitigate this issue, we have had to mount the configuration folder read-only (in the volume mount of the container) to prevent it from overwriting its config under load. We have to mount it read-write only during Nextcloud upgrades, and then have to quickly return it to read-only when the upgrade is done. This makes upgrades take a bit longer than they should and makes them a bit more complicated than just rolling out a new image.

Without this in place, if our Azure Files share gets under any load (which can happen several times during peak hours), Nextcloud can wind up overwriting its configuration file 3-5 times PER DAY.

@GuyPaddock GuyPaddock added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Feb 25, 2022
@joshtrichards
Copy link
Member

Hi @GuyPaddock - This was partially addressed in #33921.

Some further robustness attempts were initiated in #34009, but reverted temporarily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: install and update
Projects
None yet
Development

No branches or pull requests

2 participants