Refactor how we set/restore ownership of drupal settings file to allow reuse... #179
Conversation
…w reuse in containers based on this one.
|
Here is the issue this refactoring helps to fix: Islandora-Devops/isle-dc#208 |
| if [ ! -z "${previous_owner_group}" ]; then | ||
| chown "${previous_owner_group}" "${site_directory}/settings.php" | ||
| fi | ||
| restore_settings_ownership ${site} ${previous_owner_group} | ||
|
|
||
| # Restrict access to settings.php | ||
| chmod 444 "${site_directory}/settings.php" |
There was a problem hiding this comment.
We should move the correction of permissions into restore_settings_ownership
# Restrict access to settings.php
chmod 444 "${site_directory}/settings.php"There was a problem hiding this comment.
Yes - good catch. I've made this change.
| # send debug output to stderr because the caller typically captures output from this function. | ||
| #>&2 echo "adjusting ownership of ${site_directory}/settings.php" | ||
| if [ -f "${site_directory}/settings.php" ]; then | ||
| previous_owner_group=$(stat -c "%u:%g" "${site_directory}/settings.php") |
There was a problem hiding this comment.
On review I don't know why we'd ever have an owner/group other than nginx:nginx (100:101)? With the possible exception if it's bind mounted. Possibly all files in /var/www/drupal have the hosts users group to allow for host edits or something... That being said it doesn't hurt anything. Just a thought as to why did we do this in the first place.
There was a problem hiding this comment.
Good catch re: permissions. I've moved this into the function.
Restoring ownership is only important when it's bind mounted. It ensures the file remains readable/writable by the host user. I've added a comment in the code to this effect.
| # Restore owner/group to previous value | ||
| if [ ! -z "${previous_owner_group}" ]; then | ||
| #echo "restoring ${site_directory}/settings.php" | ||
| #echo "previous_owner_group ${previous_owner_group}" |
There was a problem hiding this comment.
We can remove the commented out #echo statements.
There was a problem hiding this comment.
Agreed - I've taken them out.
...in containers based on this one.
The net effect of this PR on its own is none - everything works as before, I have simply factored out these two functions.
I'll be submitting a PR against isle-dc shortly where I make use of these functions to fix an issue occurring there.