search

serversideup / docker-php

🐳 Production-ready Docker images for PHP. Optimized for Laravel, WordPress, and more!
https://serversideup.net/open-source/docker-php/
GNU General Public License v3.0
1.67k stars 109 forks source link

Add check for Unit custom certs #321

Closed jbxonline closed 4 months ago

jbxonline commented 4 months ago

I'm not convinced we need the first check on line 157 at all as I don't know how a cert would ever get into the config directory at this stage. However, maybe it is useful if someone is building a custom image and copying their SSL directly into the config directory. So I've left it in place.

However, we previously then went straight to creating the self signed cert, without ever actually taking a look inside /etc/ssl/private to see if there was a custom cert in there. So this PR basically adds that check to use a custom cert if provided before moving on to generating a self signed.

jaydrogers commented 4 months ago

This looks great!!

I'm not convinced we need the first check on line 157 at all as I don't know how a cert would ever get into the config directory at this stage. However, maybe it is useful if someone is building a custom image and copying their SSL directly into the config directory. So I've left it in place.

I intentionally put that in there because I was running into weird things with container caching.

However, we previously then went straight to creating the self signed cert, without ever actually taking a look inside /etc/ssl/private to see if there was a custom cert in there. So this PR basically adds that check to use a custom cert if provided before moving on to generating a self signed.

I love how you did this and I apologize for missing that earlier. I was writing a lot of code really fast and learning something at the time of creating that script. It's a dangerous combo 🤣

Merged into our "dev" images

This has been merged and is building here: https://github.com/serversideup/docker-php/actions/runs/8800388318

They will be made available here: https://hub.docker.com/r/serversideup/php-dev

Once that job is done, let me know that everything works for you with the dev image 👍

I greatly appreciate your contribution and will gladly accept more PRs in the future!

jbxonline commented 4 months ago

Thanks for the quick merge! Really helpful, hopefully I can ship a beta today pulling from your dev repo rather than my personal repos :)

I intentionally put that in there

Perfect - if it's needed - it's needed 👍🏻

Once that job is done, let me know that everything works for you with the dev image

Yeah, don't you just hate how slow docker builds are in ci environments!
Anyways, once it is built, I'll test it. Hopefully later this week I will get some time to test some more features of this variation and I'll let you know if I spot any issues. Also, if anyone raises an issue with the unit variation and you're short on time, feel free to tag me and I'll take a look for you.

jaydrogers commented 4 months ago

Thanks a ton!

Yeah, don't you just hate how slow docker builds are in ci environments!

Yes, I tried to solve this yesterday and failed 😆 https://github.com/serversideup/docker-php/pull/316

Images should be built, so keep me posted if everything works well 👍