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

Fix permission problem at runtime level once and for all (allow switching between webserver without any action) #98

Merged
merged 1 commit into from
Sep 26, 2022

Conversation

krysstof
Copy link
Contributor

Hello
I was stuck with permission problem, no pre-creating the volumes or folders was working, no chmod/chown was working and I was stuck.
I compared with a dockerfile for roudcube that uses apache and sqlite and they did the image a bit differently, ie : they got the source package uncompressed in another folder (/usr/src/...) and they were using the entrypoint to copy the files in the /var/www folder then applying the permissions.

this take care of a lot of issue as the presented volume contain no data overwritten by the package
the folder are set to the correct permission before the webserver starts and after the docker started running
this apply the permission on the fly in the presented volumes allowing switching from nginx to apache without any action

changes made :

  • Docker Files now do unzip in /usr/src/baikal
  • Docker Files now run "ENTRYPOINT" instead of CMD to allow passing parameters, nginx now uses start.sh like apache
  • start.sh is now taking a parameter that set which webserver is used
  • it starts with the copy from src to www then depending on the parameter passed to the entrypoint it change permission and start the webserver

I tested the 4 dockerfile, I could swtich from any version without losing configuration or data

@ckulka ckulka self-assigned this Sep 25, 2022
@ckulka ckulka changed the base branch from master to features/fix-permissions September 25, 2022 23:45
@ckulka
Copy link
Owner

ckulka commented Sep 26, 2022

Hi @krysstof, the idea is really good, thanks! I'll merge it (also because I want to loose your commit in the overall git history), but there are a few changes I'll look into:

  • There are additional scripts that the default nginx-image runs (/docker-entrypoint.d) that wouldn't be run anymore. I'll see that fixing ownership is done as another startup script there
  • Since Apach doesn't have /docker-entrypoint.sh, I'll mimick the same functionality there
  • Changing the entrypoint like this breaks running other commands during container start, e.g. docker run -it ckulka/baikal bash
  • I've seen issues here where users change/override .htaccess files, which would be overridden if the original source files are copied over

@ckulka ckulka changed the base branch from features/fix-permissions to master September 26, 2022 02:57
@ckulka ckulka merged commit 6a10cd0 into ckulka:master Sep 26, 2022
@krysstof
Copy link
Contributor Author

for the .htaccess :
the script in the start.sh could simply verify the presence of the files in the folder instead of copying blindly every time (like the roundcube dockerfile does) I didn't thought of the .htaccess as it's not in a persistent storage, sorry about that.
the copy only needs to happen once, it's the chown that counts

and for the Entrypoint, i'm really not an expert on docker, i know you can specify your own, like I did here, but if it break the nginx image yeah the solution is not great. I didn't see any issue though but I mainly tested the apache image

@ckulka
Copy link
Owner

ckulka commented Sep 26, 2022

Nginx's entrypoint scripts aren't super-important and two of them aren't relevant for Baikal, but one of them (as far as I can tell) does some performance tuning based on the resources available to the container.

Not a big deal though, I've already taken care of it in #99.

@ckulka
Copy link
Owner

ckulka commented Oct 1, 2022

Hi @krysstof, with the scripts merged into master, could you please help me test it? I've already done it on my machine, but would like to have a second set of eyes on it.

The images are

  • ckulka/baikal:experimental-nginx
  • ckulka/baikal:experimental-apache
  • ckulka/baikalexperimental-nginx-php8.0
  • ckulka/baikal:experimental-apache-php8.0

I'm also looking into writing an automated test for it, but I'm not sure yet when I'm done with that.

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.

2 participants