add additionalMounts and additionalVolumeMounts to values and deployment

[rspier]

This is to simplify injecting semi-dynamic configuration files (for example
postfix maps modified by an external program) into the container.

example usage:

additionalVolumes:
  - name: "custommaps-postfix"
    persistentVolumeClaim:
      claimName: custommaps-postfix
              
additionalVolumeMounts:
  - name: custommaps-postfix
    mountPath: /etc/postfix/custommaps

And then custommaps-postfix can be a shared volume that a CronJob updates periodically with new maps.

[funkypenguin]

Sorry about the delay here, I received the notification for the review request among a flurry of other GitHub notifications on my phone, and then completely missed it thereafter. (I find GitHub notifications tricky to manage!). This looks sensible to me, and I don't see any harm - I've enabled the running of the lint-test, which I see is currently failing. This is because any change to the chart requires a version bump in Chart.yaml. You might also want to consider adding a section in the README detailing how the additional volumes are intended to be used ;)

[rspier]

I think GitHub alerts on my phone would drive me crazy. :) But I'm one of those people who likes email, despite running mailservers.

Good idea to update the README.md, but which one? Looks like there are two, and they've diverged slightly. I'm happy to send a fix to sync them and eliminate one, but want to know which one you'd prefer to preserve. The top-level one or the nested one? (See diff below.)

I sent another PR to to fix the local lint instructions. Then I discovered the local lint isn't configured to do the version increment.

 diff README.md charts/docker-mailserver/README.md 
43a44
> * Supports integration with external HAProxy, HAProxy Ingress Controller, or [poor-mans-k8s-lb](https://www.funkypenguin.co.nz/project/a-simple-free-load-balancer-for-your-kubernetes-cluster/)
45c46
< * Bundles in [RainLoop](https://www.rainloop.net) for webmail access (disabled by default)
---
> * Bundles in [RainLoop](https://www.rainloop.net) for webmail access (enabled by default)
47,48c48
< * CI/CD tested against Kubernetes 1.18,1.19, and 1.20 : ![Lint and Test Charts](https://github.com/funkypenguin/helm-docker-mailserver/workflows/Lint%20and%20Test%20Charts/badge.svg)
< * 
---
> 
51c51
< - Kubernetes 1.16+ (*CI validates against > 1.18.0*)
---
> - Kubernetes 1.5+ (*CI validates against 1.12.0*)
213c213,214
< | `pod.dockermailserver.hostPID`                    | Not really sure. TBD.                                                                                                                                                                | `None`                                               |                                           |
---
> | `pod.dockermailserver.hostPID`                    | Not really sure. TBD.                                                                                                                                                                | `None`                                               |
> | `pod.dockermailserver.securityContext.privileged` | Whether to run this pod in "privileged" mode.                                                                                                                                        | `false`                                              |

[funkypenguin]

Thanks @rspier!

Thinking it through, it probably makes the most sense to put the README at the root of the repository, and just copy the README to the charts/docker-server directory. That way, users who land on the repository will see up-to-date instructions re how to use the helm chart.

As for which README is the most up-to-date, I'd go with the one in the charts/docker-mailserver directory - it seems to have been kept more up-to-date than the README in the root of the repo :)

Thanks!
D

[funkypenguin]

LGTM!

[funkypenguin]

(Oh, but you'll want to bump the chart version to satisfy the lint test, which will probably re-request an approval!)

[rspier]

Thanks @rspier!

Thinking it through, it probably makes the most sense to put the README at the root of the repository, and just copy the README to the charts/docker-server directory. That way, users who land on the repository will see up-to-date instructions re how to use the helm chart.

What do you mean by "copy" here? I think the duplication may have been the cause of this skew situation in the first place. It looks like ArtifactHub wants it in the charts/docker-mailserver directory (see https://artifacthub.io/packages/helm/geek-cookbook/docker-mailserver). Is there a "release" process before uploading it where there's a chance to do the copy?

Looking at the commit history, it looks like a symlink was tried, but didn't work out.

It might be simpler (or at least less error prone) to make the top-level README.md have brief summary and a links to the charts/docker-mailserver directory file?

As for which README is the most up-to-date, I'd go with the one in the charts/docker-mailserver directory - it seems to have been kept more up-to-date than the README in the root of the repo :)

I was going to suggest going the other direction. It looks like the top-level README.md is more up to date, with the exception of line line about HAProxy support.

$ diff -U0 README.md charts/docker-mailserver/README.md 
--- README.md	2021-10-23 09:13:08.000000000 -0700
+++ charts/docker-mailserver/README.md	2021-10-23 09:13:08.000000000 -0700
@@ -43,0 +44 @@
+* Supports integration with external HAProxy, HAProxy Ingress Controller, or [poor-mans-k8s-lb](https://www.funkypenguin.co.nz/project/a-simple-free-load-balancer-for-your-kubernetes-cluster/)
@@ -45 +46 @@
-* Bundles in [RainLoop](https://www.rainloop.net) for webmail access (disabled by default)
+* Bundles in [RainLoop](https://www.rainloop.net) for webmail access (enabled by default)
@@ -47,2 +48 @@
-* CI/CD tested against Kubernetes 1.18,1.19, and 1.20 : ![Lint and Test Charts](https://github.com/funkypenguin/helm-docker-mailserver/workflows/Lint%20and%20Test%20Charts/badge.svg)
-* 
+
@@ -51 +51 @@
-- Kubernetes 1.16+ (*CI validates against > 1.18.0*)
+- Kubernetes 1.5+ (*CI validates against 1.12.0*)
@@ -213 +213,2 @@
-| `pod.dockermailserver.hostPID`                    | Not really sure. TBD.                                                                                                                                                                | `None`                                               |                                           |
+| `pod.dockermailserver.hostPID`                    | Not really sure. TBD.                                                                                                                                                                | `None`                                               |
+| `pod.dockermailserver.securityContext.privileged` | Whether to run this pod in "privileged" mode.                                                                                                                                        | `false`                                              |

Thanks-

[rspier]

Rebased against master.

How would you feel about merging this even without adding the docs and solving the duplicated README issue? It at least gets the functionality out there.

[funkypenguin]

Sorry, this stalled again due to my lack of attention :( I see what you mean about artifacthub's expectation - and yes, I'm inclined to agree that we should avoid duplicating a README, and just let the repo-level README point to the chart README (which also seems to be the format I've been increasingly noticing in other charts).

I'll merge these functional changes now, and then we can do a separate PR for the README update :)

D