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

chore: Refactor setup-stack.sh case SSL_TYPE=letsencrypt #2278

Conversation

polarathene
Copy link
Member

Description

This has been extracted from #2245 as it does not need to be part of that PR.

To function correctly, it depends on a helper-function.sh method added in #2274 , tests will pass once that is merged.


Mostly cleans up the code and documents it better, although there are some minor fixes for handling SSL_DOMAIN ENV and additional logging added for spotting issues related to it in future when troubleshooting.

Commits are scoped with context messages for easing review if necessary. Overview of changes:

Traefik specific:

  • Logic extracted out into it's own function.
  • Conditional reworked to assist with debugging.
  • SSL_DOMAIN must not be empty when attempting to extract.
  • Added additional notes.

SSL_TYPE=letsencrypt case:

  • Revised top note block.
  • Correct handling for SSL_DOMAIN.
  • Removed some unnecessary nesting.
  • Less repetitive error message for LETSENCRYPT_DOMAIN.
  • Added use of panics where appropriate (kept return 1 so failures still exit functionality early).
  • Improved inline docs.

Type of change

  • 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

polarathene and others added 7 commits November 1, 2021 13:23
- Don't attempt to extract for `SSL_DOMAIN` if the value is empty.
- Added `EXTRACTED_DOMAIN` and error + debug log output for better troubleshooting.
- Removed some unnecessary nesting.
- Correct handling for `SSL_DOMAIN`.
- Less repetitive error message for `LETSENCRYPT_DOMAIN`.
- Added use of panics where appropriate (kept `return 1` so failures still exit functionality early).
Correct the indentation mistake spotted by CI lint job.
georglauterbach
georglauterbach previously approved these changes Nov 1, 2021
Copy link
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@georglauterbach georglauterbach added area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Nov 1, 2021
@georglauterbach georglauterbach added this to the v10.3.0 milestone Nov 1, 2021
@georglauterbach
Copy link
Member

Please label your PRs to make it more obvious what they are about even when not having them opened :)

target/scripts/startup/setup-stack.sh Outdated Show resolved Hide resolved
Co-authored-by: Casper <casperklein@users.noreply.github.com>
@polarathene
Copy link
Member Author

Please label your PRs to make it more obvious what they are about even when not having them opened :)

Oh sure! Sorry I never thought about that 😅

@polarathene polarathene merged commit bdb35dd into docker-mailserver:master Nov 2, 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.

None yet

3 participants