-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Clean up docker compose file for selfhosting #1022
Conversation
Removed nginx container (might want to put it back?) Changed spacing to tabs for better readability Changed the order, first the important stuff (plane) and later the database/redis All containers should be in the same format (first container_name, then image, then restart, etc.). Removed links because deprecated since compose version 2, all containers are in one docker network Removed build from plane-api Removed ports from redis and postgresql Removed PGDATA directory because that's the default one Renamed redis and db to plane-redis and plane-db
@Robin-Sch is attempting to deploy a commit to the Plane Team on Vercel. A member of the Team first needs to authorize it. |
Use tabs instead of spaces is a bad idea for YAML files
Totally agree with that. Currently, compose file looks too excess because of environment variables (most of them are duplicated from .env file by directives like |
Thank you, folks, for your contributions. We are still testing different options, and this temporary unstable fix is just one of them. Our team is actively working on finding a permanent solution. We appreciate your contribution and will review it and merge it soon. |
I'd like to second this change - because the nginx instance and all of Plane's services are on the same Docker network, binding host ports is not required for communication between services. Exposing these services via bound host ports represents a security issue in single-server production environments; only the nginx proxy port needs to be accessible outside of the docker network for upstream traffic. |
My bad, I changed it to 4 spaces after I noticed tabs not working (that's why the multiple fix spacing commits) |
I'm not sure which direction you want to go, include the nginx container or not. Afaik, many, if not all selfhosted services don't include a nginx container and just expect people to be already running their own reverse proxy. Personally, I think it's the best to leave the container out, but yeah, let me know what you think of it. |
Hi @Robin-Sch, thanks for your contribution. We will keep the nginx container and update the documentation for the people already running their own reverse proxy. |
Merging this off, thank you for you contribution @Robin-Sch. Will update the documentation for users with and without nginx. |
See @require-dev's suggestion too |
Removed nginx container (might want to put it back?)
I think the majority of people who will selfhost plane are selfhosting more than just plane, and this way the nginx container doesn't conflict with their current setup (port 80 is most likely already in use).
Maybe show/guide on how to setup a simple nginx server (in case they haven't done so)?
Changed spacing to 4 spaces for better readability
Changed the order, first the important stuff (plane) and later the database/redis
For example, to upgrade to a different version or change port(s) you don't have to scroll all the way down anymore
Also I don't think anyone will ever change something with the redis/postgres instance?
All containers are in the same format
First container_name, then image, then restart etc..
Just standardization, and looks better
Removed links
Links are deprecated since compose version 2, all containers are in one docker network
Removed build from plane-api
Now you don't have to download the whole repo anymore, just for the api image
Removed ports from redis and postgresql
They can communicate freely with each other without exposing the port, increases the security since the postgres and redis instances aren't accessible from the outside
Removed PGDATA environment variable
It's the default one and by removing it's less clutter in the file
Renamed redis and db to plane-redis and plane-db
This way you know the redis and postgres containers belong to plane and aren't just named "redis" and "db"
Could still be improved
The environment variables could be included in the
.env
file to remove them from the compose file and making the compose file more readable and smaller see thisThis way you can change all environment variables from the
.env
file and they aren't spread across two files.#1019