-
Notifications
You must be signed in to change notification settings - Fork 576
Conversation
…tbot and supervisor
Thanks @akrmhrjn for the pull request! Per the CONTRIBUTING.md file displayed when you created this pull request, we need to add you to the list of approved contributors for the Mattermost project. Please help complete the Mattermost contribution license agreement? This is a standard procedure for many open source projects. Your form should be processed within 24 hours and reviewers for your pull request will be able to proceed. Please let us know if you have any questions. We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team. |
@@ -1,11 +1,23 @@ | |||
FROM nginx:mainline-alpine | |||
FROM nginx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any chance make it work with Alpine image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try to make work on alpine. Let me take a look on that.
# install cron, supervisor and certbot | ||
RUN echo "deb http://ftp.debian.org/debian stretch-backports main" | tee -a /etc/apt/sources.list | ||
RUN apt-get update | ||
RUN apt-get install -qy cron supervisor python-certbot-nginx -t stretch-backports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use a separate container instead of supervisor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need an extra container?
Please don't add unnecessary dependencies to the nginx container. There is already a Certbot container that can be used for it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think that we should not add dependencies to manage TLS certificates with Let's Encrypt inside the image, for 2 reasons:
- if you want to use LE, you can with the current image. Just use another container to manage with LE and put certificate and key on the correct folder
- the web image should be minimal to allow someone to quickly test to spawn a Mattermost server, but you should use another real reverse-proxy (like Traefik). Actually I'm thinking of dropping support for the web image.
@pichouk Another container seems to be the good option to include LE and manage certificate. And I agree with replacing web image with Traefik. It would be a good enhancement. It is minimal and has LE support as well. |
Hi,
I have added the code to automatically generate the SSL certificate from Let's Encrypt during docker-compose up. This might be useful as it is very easy to set up. I have also added what needs to be done to enable the automatic generation of SSL certificate. It also checks for SSL certificate expiration and automatically renews the SSL certificate.
Please review it and Let me know if I can be more useful.
Regards.