LemmyNet / lemmy-ansible

A docker deploy for ansible
GNU Affero General Public License v3.0
248 stars 92 forks source link

Clean up networks in docker-compose.yml #102

Closed christopher-conley closed 1 year ago

christopher-conley commented 1 year ago

This commit removes the "internal" boolean from the "lemmyinternal" network, explicitly defines hostnames for each container (matching existing hostnames if found elsewhere like in env variables), ensures the nginx container connects to the "lemmyexternalproxy" network first, and places the containers in the appropriate network(s).

I'm really not sure what the "internal" boolean on the "lemmyinternal" network brings, as the containers that need WAN access wouldn't be able to get it unless they're also connected to the "lemmyexternalproxy" network and could egress through that route, which defeats the purpose of having them connected to a restricted network. If the idea is to restrict access/traffic to only the containers within that network, but still allow WAN egress from them, it might make more sense to have the internal nginx container also act as a forward proxy for those containers in addition to the reverse proxy role it's already serving.

Not every container had an explicit hostname, so sometimes traffic meant for one container would actually go to a different one. The postfix container also didn't have an explicit network assignment, so it would fall off onto its own subnet in the default network.

christopher-conley commented 1 year ago

I've made a new commit to the PR addressing the comments. I also tested it to a new VM at a VPC, and it deployed and federated with other instances successfully.

Nutomic commented 1 year ago

Honestly I would just get rid of all the network sections. They dont seem to add any value, as every service needs to connect to the external network in one case or another. Better to keep it simple and use the default docker network.

christopher-conley commented 1 year ago

Honestly I would just get rid of all the network sections. They dont seem to add any value, as every service needs to connect to the external network in one case or another. Better to keep it simple and use the default docker network.

I think we can close this PR, it looks like that was already done with this commit: 6ebe1a1. One downside to this approach is if someone has multiple docker compose files in the same directory, the containers launched from those different compose files will all be connected to the same network. If networks aren't defined and services assigned to them explicitly in the compose files, docker will create a default network for it based on the parent directory name. Has the potential to cause problems in the future.

Also, if the idea was to keep the lemmy/lemmy-ui/postgres containers segregated on their own network and have the nginx container be the only point of network ingress/egress for them, I'm not sure if the internal nginx container serves a purpose after removal of the docker networks. With the networks gone, it looks like that container could be removed entirely and just move the "nginx_internal.conf" config into one of the host's nginx include directories with minor alterations (replace references to lemmy/lemmy-ui with localhost and port, essentially).

Nutomic commented 1 year ago

I would say t he internal nginx is mainly to make configuration easier.