Add changedetector functionality for ${SSL_TYPE} == manual
#2404
Conversation
The SSL configuration function is rather large. Outsourcing it provides the opportunity to run the function itself without sourcing the whole `setup-stack.sh`.
georglauterbach
added
kind/new feature
georglauterbach
requested review from
casperklein,
polarathene,
NorseGaud,
wernerfred and
a team
georglauterbach
changed the title
${SSL_TYPE}=manual${SSL_TYPE} == manual
Some difficulties on the way when it comes to changing the certiicate files led to another approach. The file is briefly changed on the host to see whether the changes are picked up.
Due to the changes in #2401 and the chages introduced here, `check-for-changes.sh` needs just a little bit longer.
The function now only computes what it ought to, and not more. This makes it a bit more efficient and less prone to problems.
|
@docker-mailserver/maintainers I know this looks like a big PR, but the main diffs come from the relocation of the |
|
Hey, sorry for the delay! I've been meaning to review it and will do so in about 10 hours time :) |
There was a problem hiding this comment.
LGTM. One thing to consider: Does this PR makes it harder to finish #2157?
|
Hey @casperklein , we can refactor as necessary |
As @NorseGaud mentioned, I think this PR only demands a (small) refactoring, nothing too dramatic. On a side-note: I'd say the |
There was a problem hiding this comment.
LGTM for the most part 😀
Sorry about the delay in reviewing! The only important review comments are the ones marked "change request" / "bug" and perhaps "feedback". If I did spot a bug, that's all that really needs to be address to merge.
There's some nitpicks that aren't important and can be ignored, and feedback comments are mostly preference or suggesting minor improvements or concerns.
The functions concerned with SSL/TLS setup were moved to `target/scripts/helpers/ssl.sh` because they actaully belong there. This is a non-issue as they are properly sourced by `index.sh` in `target/scripts/helpers/` anyway. This is a proper seperation of concerns.
This commit reverts a change previously introduced (that may got us some false positives). Furthermore, a link was added for a "TODO" that can be solved more easily in the future once these changes are resolved. And the line splitting is now less aggressive.
The function was revised again for better maintainability and readability. A second "staging" array was introduced which does a bit of the heavy lifting when it comes to adding the correct files for detecting changes. A missing `+` was added.
The `helper-functions.sh` script shall be splitted in the future. These changes take the first step in splitting the file for SSL related functions.
|
Documentation preview for this PR is ready! 🎉 Built with commit: bc7a9a7 |
georglauterbach
deleted the
feature/changedetector-for-ssl-type-manual
branch
Description
The additions of this PR enable users who use
${SSL_TYPE} == manual(like me) to use the changedetector as well. Since my change to K8s, I noticed that new certificates are not being picked up properly. This additions checks the${SSL_TYPE}and depending on the value does exactly what was done before for Let'sEncrypt, or when${SSL_TYPE} == manualchecks the corresponding locations certificate.Woke up today just to find out my certificates expired - an unpleasant experience...
Fixes #
Type of change
Checklist:
docs/)