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

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

Merged
merged 6 commits into from
Feb 27, 2023

Conversation

georglauterbach
Copy link
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
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
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