Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added creation of domains.conf in Dockerfile #60

Merged
merged 1 commit into from
Aug 24, 2019
Merged

Added creation of domains.conf in Dockerfile #60

merged 1 commit into from
Aug 24, 2019

Conversation

Akkarine
Copy link
Contributor

@Akkarine Akkarine commented Aug 17, 2019

This PR contains:

  • Those, who ran docker image before creation of mounting folders (as me), firstly will run into error:
    image
    And then, when they copy their file and delete directory, will see: /etc/letsencrypt/domains.conf\\" caused \\"not a directory\\"\""
    This problem described here: https://stackoverflow.com/a/47099098/8537739
    To prevent this behavior we can add creation of domains.conf into Dockerfile.
  • For docker-compose template I found confusing to create certs directory in $ROOT, because there is duplication of domains.conf.
  • Added LEXICON_SLEEP_TIME to docker-compose template because it is important setting.
  • Made small improvements to README.md.

- Removed certs directory from docker-compose template
- Added LEXICON_SLEEP_TIME to docker-compose template
- Updated README.md
@Akkarine Akkarine changed the title - Added creation of domains.conf in Dockerfile Added creation of domains.conf in Dockerfile Aug 17, 2019
@teohhanhui
Copy link

It's better if there's a way to avoid using domains.conf altogether, in keeping with this Twelve-Factor App principle: https://12factor.net/config

Practically speaking, I want to be able to include this container in our Docker Compose setup, even for local development where using Let's Encrypt / HTTPS should be entirely optional. Currently, that means the user has to do touch path/to/domains.conf, otherwise Docker Compose creates a directory named domains.conf since we mount from host (same problem as you've described above @Akkarine).

@Akkarine
Copy link
Contributor Author

Akkarine commented Aug 23, 2019

@teohhanhui hello. Thank you for review. I got the point about dynamic configs. But for this particulary application existing of domains.conf is a mandatory. All what it does is a creation of certificates, based on information in this file. So I don't understand, what supposed to mean, that you may not want to use HTTPS. So in that case I think we can help a little to user.

@teohhanhui
Copy link

I mean, if the entries in domains.conf could be specified through environment variables instead.

@Akkarine
Copy link
Contributor Author

But now it is impossible. I leave this to @adferrand.

Copy link
Owner

@adferrand adferrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beside a section in README.md that disappeared, it's LGTM.

Indeed I was saving some lines in the Dockerfile by not creating domains.conf, and it was working for me, because usually I use my Docker by making a host mount only on etc/letsencrypt, and create the domains.conf inside it directly. However explicit mount of /etc/letsencrypt/domains.conf can lead to a directory be created if the path on the host does not exist.

Thanks for this correction, and for various errors in README.md.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
@adferrand
Copy link
Owner

To respond more generally on why domains.conf is necessary or not.

When I started to create Dockers, I was quite in favor of pure environment variable driven configuration.

However as the time goes, I start thinking that is not that a great idea, as a given Docker add more and more functionalities. I see mainly to drawbacks:

First, you can end having a lot of environment variables. Really, a lot. See for instance the list of supported variables for a Docker I appreciate a lot: https://github.com/sameersbn/docker-gitlab#available-configuration-parameters. Despite my good feeling about the strong construction of this Docker, the number of variables is just insane. This is problematic, in particular because there is no real way to make a validation of them: as the maintainer, it is quite hard to ensure that you did not miss any variable that can be consumed in you code, as a user, you may miss some of the functionalities, or misconfigure your instance because of an error in a variable name, without warning.

Second, what you can store in the variables is just a string. So write structured data is really hard. An structure data is something really useful. In one of the projects I maintain for instance, https://github.com/AnalogJ/lexicon, you can have several layers of structured value as a configuration object. Describe them as environment variables is not easy. That's why we introduced a YAML configuration file that can be consumed by the program.

Also note that nowadays, attaching a configuration file to a Docker becomes easier, in particular in the Kubernetes environment.

So, I am more in favor in fact to move all the configuration of my Docker in a centralized configuration file, and drop any use of an environment variable.

This is a big part of the major rewriting that I still plan to do of docker-letsencrypt-dns: https://github.com/adferrand/docker-letsencrypt-dns/wiki/Project-V3-specifications,-aka-Namedcrypt

@teohhanhui
Copy link

@adferrand Thanks for the explanation.

I think it'd be great if we could have:

  • /etc/namedcrypt/profiles.d
  • /etc/namedcrypt/certificates.d

in addition to the monolithic config file.

Also, I think the proposed name is really confusing. It invokes an image of file encryption, like TrueCrypt / VeraCrypt.

@adferrand
Copy link
Owner

Yes forget about the name, it was totally temporary ^^

Thanks for the idea of an exploded configuration.

@adferrand
Copy link
Owner

@teohhanhui I thought a little about your proposition. I think I will add a providers_directory and certificates_directory in the main configuration file. The system will look for any *.yml file in them to enrich the relevant sections with the content of the files found.

This is an approach borrowed from supervisor and circus, two process managers that I use a lot.

@Akkarine
Copy link
Contributor Author

So, @adferrand, do you want me to make any changes?

@adferrand
Copy link
Owner

No, that's great! Thank you.

@adferrand
Copy link
Owner

I prepare a release with your changes for tomorrow.

@adferrand adferrand merged commit 268bc0e into adferrand:master Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants