Skip to content

Conversation

@TrafeX
Copy link
Owner

@TrafeX TrafeX commented Dec 31, 2023

I moved the wp-secrets.php file to the volume mount next to wp-content to make this file persistent between container recreations. WIth a symlink I point WordPress to the right location.

@Morpheus636
Copy link
Contributor

Morpheus636 commented Dec 31, 2023

I like this solution, though with a couple things to note:

  • The nginx config on this project shouldn't expose so-secrets.php in wp-content (it should 404, I believe) but I'm in favor of assuming that the wp-content directory is public just in case something got missed.
  • Would it be possible to retain the check for environment variables (the inner if statement) from Generate salts every time the container starts, unless they're set via environment variables #49? Or something similar guarding the require_once in wp-config.php. That will prevent warnings for deployments that used environment variables as a workaround for this bug.

@Morpheus636
Copy link
Contributor

Morpheus636 commented Dec 31, 2023

Update: I thought you were storing it in wp-content. Per the README, the volume is directly mounted to /var/www/wp-content, so files in /var/www/ shouldn't be persisted.

This also doesn't fix the issue in cases where the volume already exists, such as migrating from a different WP installation or upgrading from a previous version of this container.

@TrafeX TrafeX force-pushed the persistent-wp-secrets branch from b55fa6b to f7d6f0a Compare January 1, 2024 08:15
@TrafeX
Copy link
Owner Author

TrafeX commented Jan 1, 2024

@Morpheus636 Thank you for the feedback!
You're right, it wasn't on the right location. I've moved it to wp-content. It's still safe because the Nginx config doesn't allow for files other than index.php to be executed.

I also added the way you wrote the wp-secrets.php file so that the file is always created when it doesn't exist and not when the environment variables are set.

Copy link
Contributor

@Morpheus636 Morpheus636 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the changes, LGTM!

@TrafeX TrafeX merged commit 804d2d4 into master Jan 1, 2024
@TrafeX
Copy link
Owner Author

TrafeX commented Jan 1, 2024

@Morpheus636 Cool, thanks!

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.

3 participants