From 488dfc25724a769e1bd91348e1c6f4829e79c6f4 Mon Sep 17 00:00:00 2001 From: kaiyou Date: Sat, 31 Aug 2019 19:15:06 +0200 Subject: [PATCH 1/2] Start writing the contribution guidelines --- docs/contributors/guidelines.rst | 205 ++++++++++++++++++ docs/contributors/{guide.rst => workflow.rst} | 31 +-- docs/index.rst | 3 +- 3 files changed, 214 insertions(+), 25 deletions(-) create mode 100644 docs/contributors/guidelines.rst rename docs/contributors/{guide.rst => workflow.rst} (84%) diff --git a/docs/contributors/guidelines.rst b/docs/contributors/guidelines.rst new file mode 100644 index 00000000..093d23ad --- /dev/null +++ b/docs/contributors/guidelines.rst @@ -0,0 +1,205 @@ +Project guidelines +================== + +Why these guidelines +-------------------- + +This page is written as an evolving memo of the main development guidelines +that we follow or should follow as contributors. Some are basic development +constraints, some are architectural opinions. + +If you contribute to this project, please read through them all and read the +architectural documentation. Reviewers will check that your contributions +match these guidelines amont other things. + +If you feel one of the guidelines is wrong or incomplete, please come and +discuss on our messaging channel, then open a pull request including the +change you would like to make and the reasons why in the description, we can +then formally discuss it and review it following our usual review process. + +General +------- + +What features should be included +```````````````````````````````` + +Mailu is a mail server. That said, mail works in an ecosystem and we should set +clear boundaries on what Mailu is and what it is not. Beyond mail, we include +features that mail is dependent on in some way, not the other way aroud. + +For instance, we include contact management and a dav service for synchronizing +contacts, because sending mail is made easier with manageable contacts. We do +not include a calendar management app or file sharing app, because mail is not +dependent on them. + +Examples of features we will not include in Mailu are: calendars, documents and +file sharing, full-sized groupware, instant messaging, password management. If +you want and success in connecting those to Mailu and want to share, please +write some useful documentation for others to do the same. + +What behavior is tolerated +`````````````````````````` + +Be respectful of others and others contributions. If you find a whole part of +Maiu is terribly written or badly designed, please open an issue to discuss it +calmly or come and have a chat on our channel. + +Any PR that just rewrites a huge part of the code by making mostly cosmetic or +opinionated changes without prior discussion will be received with a polite +"no". + +Architecture +------------ + +What setups should be supported +``````````````````````````````` + +Mailu supports out-of-the-box Docker Compose, Docker Swarm and Kubernetes +environments. In those environments, it consists of many containers and +supports hosting some of those containers in a separate environment. + +It supports hosting the data outside of the environment, in as many types of +database servers as can be documented. + +It supports spreading and scaling the containers across hosts, except for +those that share volumes. The intention is to stop sharing volumes between +containers. + +What is a Mailu container +````````````````````````` + +A Mailu container should provide one service and run one type of process only. +A new Webmail should be in a separate container, a new antivirus or a new +antispam should be in a separate container. + +A container is developped as a singled directory under the proper category in +the main repository, the only exception being service containers that should +only use official Docker images. + +A container image name must explicitely state the technology being used. +Container versions are synchronized and all containers are always built at +once. The service name associated in the Compose file or Kubernetes configuration +should match the container image name. + +How configuration should be managed +`````````````````````````````````` + +Anything that is static should be managed using environment variables. +Configuration files should be compiled at runtime by the container `start.py` +script and all conditional syntax should be handled using Jinja logic. + +The `socrate` Python package should include relevant functions for container +lifecycle management. + +Anything that is not static, i.e. able to change at runtime, either due to +configuration in the admin ui or user behavior, should take advantage of the +admin API. The `podop` package binds mail specific sofware (Postfix and Dovecot +at the moment) to the admin API, other containers should use specific API calls. + +What traffic should go through the nginx container +````````````````````````````````````````````````to + +All of it. All traffic, including HTTP(S), IMAP, SMTP, POP3, should go through +the front container. + +More generally, the front container is responsible for routing that traffic based +on the incoming port or the HTTP browsing logic. It handles protocol rewriting +for security, authentication, rejects based on identity or IP address. + +How browsing should be managed +`````````````````````````````` + +The nginx container is responsible for routing all Web traffic. Web traffic should +go directly from the nginx container to the target container. + +All traffic to a container should be accessed at ``/`` for a given +container. If multiple types of traffic are routed for a specific container, they +should be accessed at ``//`` and +``//``, for instance ``/admin/ui`` or +``/admin/api``. + +Traffic directed to ``/`` should be routed to an endpoint on the admin container +responsible for dynamically redirecting it. + + +How authentication should work +`````````````````````````````` + +Authentication should be managed by the nginx container, based on the admin +container API. The only exception is authentication to the admin container +itself, that handled directly. + +Authentication API in the admin container should contain the logic for +account management (allowed features, etc.) and rate limiting. + +Configuration +------------ + +Where should configuration files be stored +```````````````````````````````````````` + +Configuration files should ideally be stored in the most standard place for the +tool being configured. For instance, if the tool usually accepts configuration +files in ``/etc``, then configuration should be written there. + +Should we use default configuration +`````````````````````````````````` + +Some tools ship with default configuration, that handles the standard behavior. +Using this configuration is prone to later changes and unexpected side effects. +We should always provide all required configuration, including the base files, +and not rely on default configuration files form the distribution. + +For that reason, in case the tool looks for specific files and include them +automatically, we should overwrite them or delete them. + +How should configuration be overridden +````````````````````````````````````` + +Some containers support configuration override. For this feature, we should +ideally look for conditional configuration inclusion in the configuration syntax +and use it. If the tool supports multiple methods of overrides, we should use +the one that supports overriding the more configuration. + +In case the tool does not support conditional inclusion, we can add the +override logic in the `start.py` script. + +How much should configuration be documented +`````````````````````````````````````````` + +We should not keep default documentation included by the distribution when +providing configuration files. +We should organize configuration files in section relevant to the type of things +we configure. +We should add comments, and point to Github issues or public documentation when +required, in order to make our choices explicit. + +Coding +------ + +Coding standards +```````````````` + +All Python code should comply with PEP-8. We should review our code using +pylint. + +We should comply with architectural recommendation from the Flask +documentation. + +Models and database +``````````````````` + +All model classes should only use generic types that are compatible with most +supported database backends. + +No database specific configuration should be included in the models, no table +name should be forced and no schema specifics should be configured. These +should be handled by the migration scripts and only used when absolutely +necessary. + +Updating the dependencies +````````````````````````` + +Every major change to the admin Python code should be preceded by an upgrade +of the dependencies. The dependency upgrade should be tested then provided +as a separate PR before the actual changes. diff --git a/docs/contributors/guide.rst b/docs/contributors/workflow.rst similarity index 84% rename from docs/contributors/guide.rst rename to docs/contributors/workflow.rst index 0ae15c03..ba1b3d00 100644 --- a/docs/contributors/guide.rst +++ b/docs/contributors/workflow.rst @@ -1,25 +1,8 @@ -Development guidelines -====================== - -Philosophy ----------- - -The mailserver is designed as a whole, some images are therefore not best -suited for reuse outside this project. All images should however follow -Docker best practices and be as generic as possible : - - - even if not suited for reuse, they should be simple enough to - fit as base images for other projects, - - interesting settings should be available as environment variables - - base images should be well-trusted (officiel Alpine or Debian for instance). - -.. _git_workflow: - -Git workflow ------------- +Contribution workflow +==================== Forking vs. committing -`````````````````````` +---------------------- Linus' way of things sounds fine for now: if you find yourself implementing a new feature, either send me an email with a bunch of commits or use Github @@ -28,7 +11,7 @@ for trust in a specific branch of the project, we can switch to a shared repository and add a couple of trusted committers. Commits -``````` +------- This is a community project, thus commits should be readable enough for any of the contributors to guess the content by simply reading the comment or find a @@ -39,7 +22,7 @@ additional multiline if required (keep in mind that the most important piece of information should fit in the first line). Branches -```````` +-------- You are of course free of naming you branches to your taste or even using master directly if you find this appropriate. Still, keep in mind that: @@ -53,7 +36,7 @@ master directly if you find this appropriate. Still, keep in mind that: either by the name of the Github issue or a short and meaningful name. PR Workflow -```````````` +---------- All pull requests have to be against the main ``master`` branch. The PR gets build by Travis and some primitive auto-testing is done. @@ -70,7 +53,7 @@ At the end of every milestone, a new stable branch will be created from ``master or any previous commit that matches the completion of the milestone. CHANGELOG -````````` +--------- Adding entries in the CHANGELOG is an automated process which requires creation of a file under ``towncrier/newsfragments`` directory. diff --git a/docs/index.rst b/docs/index.rst index 300bacbf..476cf7fa 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -72,7 +72,8 @@ the version of Mailu that you are running. :maxdepth: 2 :caption: Contributors - contributors/guide + contributors/workflow + contributors/guidelines contributors/environment contributors/database contributors/memo From d5ad1cb4499f9103dd94ee77b10ec24e477d6f78 Mon Sep 17 00:00:00 2001 From: kaiyou Date: Tue, 17 Sep 2019 21:11:30 +0200 Subject: [PATCH 2/2] Fix many typos and take the review into account --- docs/contributors/guidelines.rst | 33 +++++++++++++++++++------------- docs/contributors/workflow.rst | 6 ++++-- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/docs/contributors/guidelines.rst b/docs/contributors/guidelines.rst index 093d23ad..8f302fb8 100644 --- a/docs/contributors/guidelines.rst +++ b/docs/contributors/guidelines.rst @@ -10,7 +10,7 @@ constraints, some are architectural opinions. If you contribute to this project, please read through them all and read the architectural documentation. Reviewers will check that your contributions -match these guidelines amont other things. +match these guidelines among other things. If you feel one of the guidelines is wrong or incomplete, please come and discuss on our messaging channel, then open a pull request including the @@ -74,7 +74,11 @@ antispam should be in a separate container. A container is developped as a singled directory under the proper category in the main repository, the only exception being service containers that should -only use official Docker images. +only use official Docker images. Categories are: + +- core, for mandatory components +- optional, for optional components +- webmail, for webmails A container image name must explicitely state the technology being used. Container versions are synchronized and all containers are always built at @@ -82,7 +86,7 @@ once. The service name associated in the Compose file or Kubernetes configuratio should match the container image name. How configuration should be managed -`````````````````````````````````` +``````````````````````````````````` Anything that is static should be managed using environment variables. Configuration files should be compiled at runtime by the container `start.py` @@ -97,7 +101,7 @@ admin API. The `podop` package binds mail specific sofware (Postfix and Dovecot at the moment) to the admin API, other containers should use specific API calls. What traffic should go through the nginx container -````````````````````````````````````````````````to +`````````````````````````````````````````````````` All of it. All traffic, including HTTP(S), IMAP, SMTP, POP3, should go through the front container. @@ -119,7 +123,10 @@ should be accessed at ``//`` and ``/admin/api``. Traffic directed to ``/`` should be routed to an endpoint on the admin container -responsible for dynamically redirecting it. +responsible for dynamically redirecting it. That last redirect must use a +302. + +Any traffic to a non existing endpoint must return a 404. How authentication should work @@ -127,34 +134,34 @@ How authentication should work Authentication should be managed by the nginx container, based on the admin container API. The only exception is authentication to the admin container -itself, that handled directly. +itself, that is handled directly. Authentication API in the admin container should contain the logic for account management (allowed features, etc.) and rate limiting. Configuration ------------- +------------- Where should configuration files be stored -```````````````````````````````````````` +`````````````````````````````````````````` Configuration files should ideally be stored in the most standard place for the tool being configured. For instance, if the tool usually accepts configuration files in ``/etc``, then configuration should be written there. Should we use default configuration -`````````````````````````````````` +``````````````````````````````````` Some tools ship with default configuration, that handles the standard behavior. Using this configuration is prone to later changes and unexpected side effects. We should always provide all required configuration, including the base files, -and not rely on default configuration files form the distribution. +and not rely on default configuration files froms the distribution. For that reason, in case the tool looks for specific files and include them automatically, we should overwrite them or delete them. How should configuration be overridden -````````````````````````````````````` +`````````````````````````````````````` Some containers support configuration override. For this feature, we should ideally look for conditional configuration inclusion in the configuration syntax @@ -165,7 +172,7 @@ In case the tool does not support conditional inclusion, we can add the override logic in the `start.py` script. How much should configuration be documented -`````````````````````````````````````````` +``````````````````````````````````````````` We should not keep default documentation included by the distribution when providing configuration files. @@ -183,7 +190,7 @@ Coding standards All Python code should comply with PEP-8. We should review our code using pylint. -We should comply with architectural recommendation from the Flask +We should comply with architectural recommendations from the Flask documentation. Models and database diff --git a/docs/contributors/workflow.rst b/docs/contributors/workflow.rst index ba1b3d00..16dcef52 100644 --- a/docs/contributors/workflow.rst +++ b/docs/contributors/workflow.rst @@ -1,5 +1,7 @@ Contribution workflow -==================== +===================== + +.. _git_workflow: Forking vs. committing ---------------------- @@ -36,7 +38,7 @@ master directly if you find this appropriate. Still, keep in mind that: either by the name of the Github issue or a short and meaningful name. PR Workflow ----------- +----------- All pull requests have to be against the main ``master`` branch. The PR gets build by Travis and some primitive auto-testing is done.