AndrewPaglusch / FlashPaper

One-time encrypted password/secret sharing
MIT License
374 stars 60 forks source link

Added Docker healthcheck + cleaned up Dockerfile #81

Closed modem7 closed 1 year ago

modem7 commented 2 years ago

Added Docker healthcheck Reduced the amount of repetition required when updating Dockerfile + entrypoint.sh

modem7 commented 2 years ago

@mattburchett - would it be possible for you to have a look at this as well as it builds upon your work in https://github.com/AndrewPaglusch/FlashPaper/pull/67?

modem7 commented 2 years ago

We could probably condense it even further by having a single ENV/ARG variable.

Instead of:

ENV PHP_VER=php8
ENV PHPFPM_VER=php-fpm8

Just have:

ENV PHP_VER=8 or ARG PHP_VER=8.

I don't know enough about PHP naming conventions to know if this is safe further down the line however.

I think two variables looks neater, but it's up to you on what you'd prefer.

modem7 commented 2 years ago

Also note that the healthcheck is only checking the NGINX portion of things.

If the PHP side of things breaks, it won't catch it.

Maybe something more complex more is suitable, or maybe it's overcomplicating matters for small returns vs effort spent implementing, especially given the additional bloat of that particular solution.

AndrewPaglusch commented 2 years ago

This is a fantastic improvement to the project. I wonder if we could check the actual FlashPaper application that would be served through nginx and php-fpm to see if some well-known text is displayed on the page. This would test all components of the container. Perhaps a portion of text in the footer towards the bottom of the page could be looked for?

Something like this (I haven't tested this yet):

HEALTHCHECK --interval=30s --timeout=10s --retries=3 --start-period=10s \
    CMD curl -fSs http://127.0.0.1:80 | grep -q 'freely available' || exit 1
modem7 commented 2 years ago

This is a fantastic improvement to the project. I wonder if we could check the actual FlashPaper application that would be served through nginx and php-fpm to see if some well-known text is displayed on the page. This would test all components of the container. Perhaps a portion of text in the footer towards the bottom of the page could be looked for?

Something like this (I haven't tested this yet):

HEALTHCHECK --interval=30s --timeout=10s --retries=3 --start-period=10s \
    CMD curl -fSs http://127.0.0.1:80 | grep -q 'freely available' || exit 1

Realistically, we'd want to test the actual services themselves rather than specific files (especially if something changes further down the line).

Imo https://github.com/renatomefi/php-fpm-healthcheck is still the best bet.

But getting it working on the other hand........Is not proving to be so easy.

At some point this week, I'll see if I can investigate https://github.com/erseco/alpine-php-webserver to see how they're doing it, and see if I can implement the above.

AndrewPaglusch commented 2 years ago

By checking for text on the site, we are checking Nginx and PHP-FPM, since both of those are required for that text to be returned.

AndrewPaglusch commented 1 year ago

@modem7 It looks like there hasn't been any movement on this PR for a while. Do you plan on completing it, or would you like me to take over from here? Thanks!

modem7 commented 1 year ago

Heya,

Just started a new job on this end, so time (and leftover brainpower!) is unfortunately quite low!

If you could take over with whatever you require, and if there's any inputs you need, I'm happy to chip in if needs be!

Thanks!