SSL documentation contains a small bug #2381

[Twist235]

Update of example to be more consistent with the next one

Description

Update of examples

Fixes #2381

Type of change

• Improvement (non-breaking change that does improve existing functionality)

Checklist:

• [x ] My code follows the style guidelines of this project
• [x ] I have performed a self-review of my own code
• [x ] I have commented my code, particularly in hard-to-understand areas
• [x ] I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• [x ] If necessary I have added tests that prove my fix is effective or that my feature works
• [N/A no functional change ] New and existing unit tests pass locally with my changes

[polarathene]

Hey, thanks for taking the time to raise an issue and even contribute a PR to resolve it! Very much appreciated 😀

However in this case there appears to be a misunderstanding, and perhaps we could better adjust the docs to clarify that instead?

---

The path to a local /etc/letsencrypt system path is correct. On a Linux system this is where certbot run locally without docker would install certs to, as per the link to certbot docs in that section.

The example was showing how you could mount your local certs to your docker container this way. Whereas the examples that follow after it are provisioned via separate docker containers (one being certbot). I can understand why that might seem confusingly similar and trip a reader up, especially if they're less familiar with either Linux or running certbot outside of a container.

---

That said, since we're a docker focused project, perhaps we don't need to cover this difference and could just focus on the dockerized certbot example that follows?

I did not write the original docs, only revised them. Did part of the confusion also come from the switching between docker-compose and the docker CLI examples?

There is a feature that our docs generator provides for tabbed content (additional related docs), it could probably be updated at a later point to include both, while defaulting to docker-compose config? No need to worry about that for this PR, but your input would be appreciated :)

---

TL;DR: I am reluctant to merge this given the above context, the change is not correct. I am open towards a revision that better clears up any confusion in our docs however.

[Twist235]

I followed the example and used the cerbot docker image, as you said it does not store the data at /etc/letsencrypt like normal certbot so I was confused why mailserver crashed.....

Maybe it could make sense to change the docs to add a placeholder in the local path and explain how this might be changed?
e.g.
volumes:

• :/etc/letsencrypt

where is either
/etc/letsencrypt if cerbot is used directly
./docker-data/certbot/certs/ if cerbot docker image is used

Or we could use the following parameters for the local certbot call:
--config-dir ./docker-data/certbot/certs
--work-dir ./docker-data/certbot/work
--logs-dir ./docker-data/certbot/log

And on a different topic:
My setup uses automated letsencrypt DNS challenges because http ports were already in use and forwarding the letsencrypt port via nginx did not work (for sure some config mistake I made).
I could add a short description with links to docs used for my setup, maybe this could also help someone? (of course the DNS provider need to have an API for it)

[polarathene]

Maybe it could make sense to change the docs to add a placeholder in the local path and explain how this might be changed?

I think it'd make more sense just to drop the non-docker certbot example. It's not going to really be lost due to version control in case we ever need to restore it.

Or we could use the following parameters for the local certbot call

I suppose? I imagine the example was intended for users already using certbot elsewhere and familiar with it, or as a quick option if not using the other provisioners.

I haven't looked through the history for when each guide was introduced to the prior Github Wiki before migration and revising to our current docs generator. Most of the content was community contributed and had fallen out of date, I've trimmed some of it away since.

Should be safe to trim away the non-docker certbot portion, or make it a collapsed by default section.

---

I could add a short description with links to docs used for my setup, maybe this could also help someone? (of course the DNS provider need to have an API for it)

Contributions are welcome. If you can fit it in nicely for the SSL docs that's great, otherwise we seem to allow general tips/guides in the examples section.

[Twist235]

I made an update to my commit based on comment from polarathene: For me, it could also make sense to create a second example for the letsencrypt with docker and keep normal letsencrypt as it was before? Or we could also hide the other example or re-order it?
Edit: Regarding the certbot changes I made for letsencrypt with DNS: I think I could add those to the mentioned examples section later. Adding this to the ssl.md might overload it too much.

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: e40d732

[polarathene]

I'm not too fond of the mostly duplicate/repeated example here where the only change is the local folder being mounted into the container.

The initial LE section I think is meant to be less about Certbot and a general overview of what's required to use LE certs (set the hostname and optional domainname, set the SSL_TYPE env and mount your certs to the expected location).

After that were examples of popular LE provisioners (not too sure about the NAS one).

---

If you feel this reduces confusion, I'm happy to approve it. I've been short of time lately, but I do recall my backlog tasks cover refactoring our SSL/TLS support to simplify configuration, at which point I'll revise the docs again too.

[polarathene]

For me, it could also make sense to create a second example for the letsencrypt with docker and keep normal letsencrypt as it was before?

That is what the PR currently does right? I've approved it.

Or we could also hide the other example or re-order it?

The current order of non-docker, then docker is preferred. The non-docker description looks more of a terse overview with more details for dockerized approaches, it just didn't communicate it that well, causing some confusion to the reader unfortunately.

I'm happy to merge this and make changes if we get any more feedback about it.

---

Regarding the certbot changes I made for letsencrypt with DNS: I think I could add those to the mentioned examples section later. Adding this to the ssl.md might overload it too much.

It should be ok as long as it has a heading to add it to the sidebar for a quick navigation overview. Most readers are likely to ignore anything not relevant to them. If the content would be larger than the nginx sections or you feel is too niche, you might want to write a separate guide in the examples page and add a mention of it on the SSL docs page instead.

---

I just noticed that the section being added to here has a bug (before your contribution).

It's rendering everything as bullet points, even though only the first two items should be, then an ordered list. Probably needs something to interrupt the two lists, perhaps part of 1. line can be split before introducing 1. to start the ordered list.

Also with your addition, the last ordered list item is now rendering as 1. which is not good.. That can be resolved by indenting your example block of content (all lines) by 4 spaces I think. It will indent the example admonition into that list item it was intended for.

[georglauterbach]

Oops, figured this is fine to merge because of the approval... I will take care of the rendering if necessary to clean up my premature merge.