Skip to content

fix: Monitor for changes in supported letsencrypt locations#2279

Merged
polarathene merged 3 commits intodocker-mailserver:masterfrom
polarathene:fix/monitor-letsencrypt-locations
Nov 4, 2021
Merged

fix: Monitor for changes in supported letsencrypt locations#2279
polarathene merged 3 commits intodocker-mailserver:masterfrom
polarathene:fix/monitor-letsencrypt-locations

Conversation

@polarathene
Copy link
Copy Markdown
Member

@polarathene polarathene commented Nov 1, 2021

Description

This has been extracted from #2245 as it does not need to be part of that PR. It is related to addressing the problem, but I'm opting to piece-meal extract the bulk of the original PR out for easier review.

This fix is all @NorseGaud with minor fix by quote wrapping the vars and adding comments by me.


Previously we only monitored for $HOSTNAME in /etc/letsencrypt/live and only for hard-coded .pem filenames.

This ensures we check the locations of other locations that may not match $HOSTNAME, which we also support. Ideally in future at least the directory to look in would be better known in advance..


Current test failure is due to the lack of quote wrapping vars in check-for-changes.sh loop when SSL_DOMAIN contains a wildcard prefix (*.) for the FQDN. That was fixed in #2274 , once that's merged the test will pass.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes

Previously we only monitored for `$HOSTNAME` in `/etc/letsencrypt/live` and only for hard-coded `.pem` filenames.

This ensures we check the locations of other locations that may not match `$HOSTNAME`, which we also support. Ideally in future at least the directory to look in would be better known..
@polarathene

This comment has been minimized.

@casperklein
Copy link
Copy Markdown
Member

👍 for putting a large PR into smaller pieces.

casperklein
casperklein previously approved these changes Nov 1, 2021
Comment thread target/scripts/helper-functions.sh Outdated
@polarathene polarathene self-assigned this Nov 2, 2021
@polarathene polarathene added area/scripts kind/bug kind/improvement Improve an existing feature, configuration file or the documentation labels Nov 2, 2021
@polarathene polarathene added this to the v10.3.0 milestone Nov 2, 2021
Copy link
Copy Markdown
Member

@wernerfred wernerfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@polarathene polarathene merged commit 917f0f6 into docker-mailserver:master Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/scripts kind/improvement Improve an existing feature, configuration file or the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants