Outsourcing Environment Variables to the Documentation by georglauterbach · Pull Request #1948 · docker-mailserver/docker-mailserver

georglauterbach · 2021-05-07T09:06:34Z

Description

Environment variables are now found in the documentation and README links to this documentation. As a consequence, ENVIRONMENT.md has been deleted.

Furthermore, I reworked the whole system for variables. In the past, users we're confused by two different .env files which serve two different purposes. Therefore, the README was adjusted and compose.env becomes history. This should make it more straight-forward and the setup easier.

Type of change

Improvement (non-breaking change that does improve existing functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

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 ENVIRONMENT.md or the documentation)
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

georglauterbach · 2021-05-07T09:06:59Z

@docker-mailserver/maintainers Contributions and discussions are welcome :D

wernerfred · 2021-05-07T09:21:07Z

Like stated multiple times from my side (e.g. here) I really appreciate this change. I'm super busy right now but I am looking forward to contribute/review this weekend on this topic 👍🏻
Thanks for starting this PR.

casperklein · 2021-05-07T11:39:02Z

Now that compose.env --> .env is going to be removed, I noticed, that at some point env_file: .env was removed from docker-compose.yml.

That leads to, that $DOMAINNAME is now no longer available in the container.

I did a quick grep. Some scripts do something like DOMAINNAME=$(hostname -d), but others seem to depend on $DOMAINNAME already set?

IMO this can never be true:

docker-mailserver/target/scripts/postsrsd-wrapper.sh

Line 11 in 74068fe

elif [[ -n ${DOMAINNAME} ]]

Or do I overlook something?

https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label

georglauterbach · 2021-05-07T11:56:46Z

Now that compose.env --> .env is going to be removed, I noticed, that at some point env_file: .env was removed from docker-compose.yml.

That leads to, that $DOMAINNAME is now no longer available in the container.

I did a quick grep. Some scripts do something like DOMAINNAME=$(hostname -d), but others seem to depend on $DOMAINNAME already set?

IMO this can never be true:

docker-mailserver/target/scripts/postsrsd-wrapper.sh

Line 11 in 74068fe

elif [[ -n ${DOMAINNAME} ]]

Or do I overlook something?

No, you're quite correct. But according to blame, I think I removed this a rather long while back, but no complaints were raised. But we should definitely do something about this behavior.

I propose to just parse it from the hostname command or from the OVERRIDE_HOSTNAME variable or something like this.

polarathene

Thanks for kickstarting this! 😃

Here's my feedback so far. Some of it's outdated now, was half way through it last night but got interrupted 😬

georglauterbach · 2021-05-08T11:48:34Z

OPEN TASKS

Check that all links are correct
Refactor $DOMAINNAME

Co-authored-by: Brennan Kinney <[email protected]>

georglauterbach · 2021-05-11T07:17:19Z

This should now be ready for merging.

wernerfred

LGTM 👍🏻
Thanks

polarathene

Absolute URLs should not be used to reference internal docs, those should be relative file paths for error detection.
1 typo to correct.
Optional: Punctuation consistency. The . at end of lines for environment.md (I didn't review the whole document, just noticed inconsistent usage through highlighted diff).

georglauterbach · 2021-05-11T09:02:33Z

Absolute URLs should not be used to reference internal docs, those should be relative file paths for error detection.

1 typo to correct.

Optional: Punctuation consistency. The . at end of lines for environment.md (I didn't review the whole document, just noticed inconsistent usage through highlighted diff).

You're right, but I'm low on time. Can you commit the Last changes you proposed on the environment branch?:)

wernerfred · 2021-05-11T09:05:31Z

There is a new feature now to commit suggestions directly:

(or am i seeing a preview feature that is not public right now)

georglauterbach · 2021-05-11T09:09:21Z

There is a new feature now to commit suggestions directly:

(or am i seeing a preview feature that is not public right now)

No, you're right. I already used this for the last suggestions. But this would mean 11 new commits. Okay, they are squashed, but I think it's cleaner with one or two commits. But go ahead @wernerfred and merge them :D I have no objections.

Co-authored-by: Brennan Kinney <[email protected]>

wernerfred · 2021-05-11T09:28:31Z

@polarathene I added your changes and enabled auto-merge. If the changes are fine with you hit the approve button ✅

started outsourcing env variables to the documentation

7bc2067

georglauterbach requested review from casperklein, polarathene and wernerfred May 7, 2021 09:06

georglauterbach self-assigned this May 7, 2021

georglauterbach marked this pull request as draft May 7, 2021 09:07

georglauterbach changed the title ~~started outsourcing env variables to the documentation~~ Outsourcing Environment Variables to the Documentation May 7, 2021

wernerfred and others added 2 commits May 7, 2021 11:43

fix: update link in issue templates to new location

356e0a6

Merge branch 'master' into environment

e8b4fb5

fix selinux syntax

3df32fc

https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label

casperklein reviewed May 7, 2021

View reviewed changes

Comment thread README.md Outdated

casperklein reviewed May 7, 2021

View reviewed changes

Comment thread README.md Outdated

casperklein reviewed May 7, 2021

View reviewed changes

Comment thread README.md Outdated

casperklein added 2 commits May 7, 2021 14:08

Update procedure updated

1744850

sample domain changed

8439404

casperklein mentioned this pull request May 7, 2021

Update check #1951

Merged

11 tasks

polarathene requested changes May 8, 2021

View reviewed changes

adjusted files to @polararthene's suggestions

83c6561

adjusted links

639eb34

georglauterbach and others added 3 commits May 10, 2021 19:31

Update README.md

860b3d4

Co-authored-by: Brennan Kinney <[email protected]>

Update README.md

5b0200e

Co-authored-by: Brennan Kinney <[email protected]>

Update docs/content/config/advanced/auth-ldap.md

6f4ddf5

Co-authored-by: Brennan Kinney <[email protected]>

georglauterbach requested a review from polarathene May 10, 2021 17:36

georglauterbach enabled auto-merge (squash) May 10, 2021 18:01

georglauterbach disabled auto-merge May 10, 2021 18:02

wernerfred previously approved these changes May 11, 2021

View reviewed changes

polarathene requested changes May 11, 2021

View reviewed changes

chore(docs): cleanup + streamline

8a157c2

Co-authored-by: Brennan Kinney <[email protected]>

wernerfred dismissed their stale review via 8a157c2 May 11, 2021 09:18

wernerfred and others added 10 commits May 11, 2021 11:18

chore(docs): cleanup + streamline

9902451

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

45a9524

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

7761e6f

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

1b4b19c

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

6244f56

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

6265424

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

0fd82c8

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

e905e47

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

3319113

Co-authored-by: Brennan Kinney <[email protected]>

chore(docs): cleanup + streamline

2e8aa26

Co-authored-by: Brennan Kinney <[email protected]>

wernerfred enabled auto-merge (squash) May 11, 2021 09:27

wernerfred merged commit 5449efd into master May 11, 2021

polarathene approved these changes May 11, 2021

View reviewed changes

wernerfred deleted the environment branch May 11, 2021 10:15

wernerfred added this to the v10.0.0 milestone May 18, 2021

Uh oh!

Conversation

georglauterbach commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Uh oh!

georglauterbach commented May 7, 2021

Uh oh!

wernerfred commented May 7, 2021

Uh oh!

casperklein commented May 7, 2021

Uh oh!

Uh oh!

georglauterbach commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

polarathene left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

georglauterbach commented May 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

georglauterbach commented May 11, 2021

Uh oh!

wernerfred left a comment

Choose a reason for hiding this comment

Uh oh!

polarathene left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

georglauterbach commented May 11, 2021

Uh oh!

wernerfred commented May 11, 2021

Uh oh!

georglauterbach commented May 11, 2021

Uh oh!

wernerfred commented May 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

georglauterbach commented May 7, 2021 •

edited

Loading

georglauterbach commented May 7, 2021 •

edited

Loading

polarathene left a comment •

edited

Loading

georglauterbach commented May 8, 2021 •

edited

Loading

polarathene left a comment •

edited

Loading