Skip to content

scripts: housekeeping & cleanup setup (1/2)#3121

Merged
georglauterbach merged 6 commits intomasterfrom
scripts/cleanup-setup
Feb 27, 2023
Merged

scripts: housekeeping & cleanup setup (1/2)#3121
georglauterbach merged 6 commits intomasterfrom
scripts/cleanup-setup

Conversation

@georglauterbach
Copy link
Copy Markdown
Member

@georglauterbach georglauterbach commented Feb 26, 2023

Description

(First) Follow up of #3115. This is mostly a cleanup & housekeeping PR; changes are partially split from #3112 to make reviewing easier.

Commits are cleanly separated; reviewing commit-by-commit is advised.

In f30f5c2, I recommend looking at setup.d/security/misc.sh not only via the diff view, but also directly at the file on the branch, because the diff is a bit fuzzy.

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
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Not all functionality was migrated with this commit; the next commit
will enhance the security setup, and in the next commit, the fixes are
included.
This greatly improves readability and consistency in our security setup
procedure. I am trying to start a new "calling convention" here, whereby
the callee (the function called) will perform checks on what it actually
needs to do. This way, we relief the caller from this burder, which in
turn simplifies `_register_functions`. Moreover, the functions become
uniform, because we can log appropriate messages for all cases!

Follow-up PR will adopt this new convention step-by-step.
Remove functionality from `start-mailserver.sh` to put it into the
appropriate stacks; this way, `start-mailserver.sh` does not need to
know about any arrays at all and the arrays specific for each stack are
in each stack. The functions for registering were moved as well, since
this nicely encapsulates all state for a stack now in the stack file.
@georglauterbach georglauterbach added area/scripts kind/improvement Improve an existing feature, configuration file or the documentation labels Feb 26, 2023
@georglauterbach georglauterbach added this to the v12.0.0 milestone Feb 26, 2023
@georglauterbach georglauterbach self-assigned this Feb 26, 2023
@georglauterbach georglauterbach changed the title Scripts/cleanup setup scripts: housekeeping & cleanup setup (1/2) Feb 26, 2023
1. Split OpenDKIM & OpenDMARC functions and added proper comments
2. Made log messages uniform and added comment on why the log messages
   are as they are
3. Moved registering of functions (this is done in an effort to simplify
   the many small functions for Postfix into bigger ones in the future;
   moreover, the Postfix setup is now closer together)
@georglauterbach
Copy link
Copy Markdown
Member Author

@casperklein do you want to review this PR too? Currently, auto-merge is enabled, which would cause this PR to be merged with one approval. If you want to review this PR, just disable auto-merge :)

Copy link
Copy Markdown
Member

@polarathene polarathene left a comment

Choose a reason for hiding this comment

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

The refactor commit had a few things going on that could have been better diffed as multiple commits, but other than that was easy enough to review through commits 👍

Good work 😀

Appease the lint gods

@georglauterbach georglauterbach merged commit 4b04c3e into master Feb 27, 2023
@georglauterbach georglauterbach deleted the scripts/cleanup-setup branch February 27, 2023 19:21
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