Move basic enviroment variables into a configuration file

[jakicoll]

This PR moves the essential environment variables, everyone will need to configure, into a dedicated mailman.cfg file, that is read by docker-compose via the added .env symlink.

The current approach of defining the environment variables directly in the docker-compose.yml file complects the composition of the mailman application with it's configuration. Problems are:

• Repetitions of the of the same values within the docker-compose.yml (not DRY)
• Manual edits of the docker-compose.yml file are likely to cause merge conflicts on project updates.
• Secrets in a (at least by default) word-readable file.

Would you give me some feedback on this idea? If you'd consider merging it, I would also adapt the other two compose files (-mysql and -postorious) to the new format and write some lines on these changes for the NEWS.md and how existing installations can migrate their configuration (they will get merge conflicts, since they changed the compose files.)

[almereyda]

This is a good refactoring, and conforms to good practices for providing a level of indirection for configuration parameters.

Symlinking .env to mailman.cfg is especially neat for human understanding.

[almereyda]

Would it be possible to add the missing line break here?

Also Docker Compose interprets values behind the = equal sign literally, that means the double quotation marks " must be avoided.

[jakicoll]

Ups, that's right. Done.

[almereyda]

Do you think it would make (aesthetic) sense to switch from array to object notation for the environment key?

For example

    environment:
    - POSTGRES_DB=${POSTGRES_DB}
    - POSTGRES_USER=${POSTGRES_USER}
    - POSTGRES_PASSWORD=${POSTGRES_PASSWORD}

becomes

    environment:
      POSTGRES_DB:       ${POSTGRES_DB}
      POSTGRES_USER:     ${POSTGRES_USER}
      POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}

which I find a bit more readable sometimes.

[maxking]

Thanks for taking time to work on this :-)

I am fine with the idea to move the env vars into a separate file, although, let's not call it 'mailman.cfg`, which is the name of the config file for Mailman 3 core and is bound to cause confusion with the users. I'd let it be '.env' so folks familiar with the docker/compose ecosystem guess the purpose fast. For folks who aren't, they'll need to learn about it anyway.

[almereyda]

Indeed. The symlink from the conventional .env to the unconventional mailman.cfg, which incidentally is also the name of that file in the data volume, only invites for confusion, which can be avoided.

Anyone who needs this gimmick is always free to establish it for themselves.

Following Occam's razor, when the same result can be achieved with less boilerplate, it's better to choose the simpler option.

[jakicoll]

I've got a question regarding the two other compose files (-mysql.yaml, and -postorius.yaml):

From reading the README, I could not really understand the pupose of the -postorius.yaml. What's it for?

Also, i've noticed some inconsitencies:

• -postorius.yaml misses the "HYPERKITTY_API_KEY", which is present in the others
• -postorius.yaml has the DATABASE_URL_SCHEMA set to postgres, while it's postgresql in the default file ([BUG] docker-compose-postorius.yaml is unmaintained #605)

I'd propose to remove the -postorius.yaml and apply the changes to the default docker-compose.yaml to the -mysql.yaml. What do you think?

[jakicoll]

Another option might be to remove the MySQL Support entirely. Since the database is setup automatically in a container and exclusively for this application, I don't really see the point in letting the user choose here.

[maxking]

I've got a question regarding the two other compose files (-mysql.yaml, and -postorius.yaml):

-mysql is pretty evident i guess, which implies using mysql. It is not a lot of maintenance to keep it and some people can use MySQL if they are more comfortable with it.

From reading the README, I could not really understand the pupose of the -postorius.yaml. What's it for?

It does not have Hyperkitty in it, it only includes the Web UI for managing Mailman Core.

Also, i've noticed some inconsitencies:

• -postorius.yaml misses the "HYPERKITTY_API_KEY", which is present in the others

Yes, because the postorius container does not include HK like mailman-web container does.

• -postorius.yaml has the DATABASE_URL_SCHEMA set to postgres, while it's postgresql in the default file ([BUG] docker-compose-postorius.yaml is unmaintained #605)

Definitely a bug.

I'd propose to remove the -postorius.yaml and apply the changes to the default docker-compose.yaml to the -mysql.yaml. What do you think?

[jakicoll]

I'd consider this ready to merge. Please review again. :)

What still needs to be done prior to releasing: Write some paragraphs on how existing installations can migrate their configuration from within the docker-compose.yaml into the new .env file. On upgrading, they will get merge conflicts, since they changed the compose files. So that should be addressed.

[almereyda]

Thank you for the changes. One last detail about the environment configuration:

Over the years it has become useful to us that (1) .env is not commited to the git tree, and .gitignore takes care of keeping our secrets out of there and (2) that we could always diff and patch our existing .env against an upstream version of .env.example that changed after a pull.

Does it seem useful to rename the file to .env.example/.env.sample and add an instruction to the readme to copy one's own .env file from it?

About the other subjects:

• It should surface to the user at least in the README that they have a choice of databases, and that the presented configuration is merely an example. Maybe it can be good to have the default setup bare minimum, without an additional database container and less environmental variables, and to only use SQLite there? Which would open the option for a more production-like -postgresql environment.
• The recent change in the name of the Postgres scheme in SQAlchemy database URLs is a ripple effect from a previous version upgrade upstream, and a likely reason for an instance to break, if not well communicated in the release notes, which happened before.
• Would it seem imaginable to run the Postorius management interface and the Hyperkitty archiver + interface in separate processes, and as such separate containers? Backing down from "fat" containers and destructuring them into more composable parts is often useful to us.

[jakicoll]

Does it seem useful to rename the file to .env.example/.env.sample and add an instruction to the readme to copy one's own .env file from it?

Sure. Done that.

Regarding your other points, I'd suggest to handle these as seperate issues. IMHO they are not linked to the objective of this pull request, which is to remove environment values from the docker-compose.yml

[maxking]

Can you rebase with the main branch? I think there is an issue related to django-allauth that makes the existing configs fail.

[maxking]

nvm, I realized GH allows me to do that, so I have just updated the PR

[maxking]

Actually, I now realize that the tests are failing because we might need to update the tests as well.

[nextgen-networks]

Found that discussion by furtune ... this was the onyl way to achive a running docker config with the -postorius.yaml
@maxking Any chance to update the already mentioned tests to be able to merge this PR?