Onpremise: Let port binding management to docker-compose.override.yml instead of variable usage

[soullivaneuh]

Summary

Port binding of the nginx server is currently defined with a env variable: https://github.com/getsentry/onpremise/blob/ae9378862caead70b620bb347bf5510833889478/docker-compose.yml#L199-L200

Instead, we should remove the port binding completely and let the user set it up on docker-compose.override.yml like this:

version: '3.4'

services:
  nginx:
    ports:
      - 80 # Do whatever you want here.

Motivation

Because of a lack of functionnality of docker-compose we are currently not able to remove the port binding.

As you suggested on your documentation I would like to setup a reverse proxy by adding a Traefik router service on the docker stack.

In that case, I do not want to expose the nginx port directly for security reason and because it will become totally useless.

I technically can not do that expect if I modify your docker-compose.yml directly, but this is not the proper way and I will always have a git diff to stash at each update.

Additional Context

Any other context or screenshots or API request payload/responses that
pertain to the feature.

[soullivaneuh]

Here is my working docker-compose.override.yml with Traefik setup:

version: '3.4'

services:
  router:
    # Private image based on Traefik
    image: registry.gitlab.com/company/traffic:1.1.5
    restart: always
    volumes:
      - router:/data
      - /var/run/docker.sock:/var/run/docker.sock:ro
    labels:
      - traefik.http.routers.api.rule=Host(`router.sentry.company.net`)
    ports:
      - "80:80"
      - "443:443"

  nginx:
    labels:
      - traefik.enable=true
      - traefik.docker.network=company
      - traefik.http.routers.sentry.rule=Host(`sentry.company.net`)
      - traefik.http.routers.sentry.tls.certresolver=letsencrypt

networks:
  default:
    name: company

volumes:
  router: ~ 

With this config, I can access Sentry on https://sentry.company.net but also on http://sentry.company.net:9000 that I don't want to.

[BYK]

Hmm, this is actually a good solution. I think the best non-breaking way forward would be to handle this in the install script: detect if there is an already existing docker-compose.override.yml file and if not, create one with the current port binding.

If there _is_ an already existing override, try to see if it has anything related to nginx and patch it if it doesn't. Otherwise issue a big warning saying you need a port binding.

What do you think? Also, would you be willing to submit PR for this?

[BYK]

Ping @soullivaneuh

[soullivaneuh]

detect if there is an already existing docker-compose.override.yml file and if not, create one with the current port binding.

I am not sure about this solution because it will be a problem for possible future BC breaking change.

Plus, some users may already have a docker-compose.override.yml but are using the current port binding.

However, as you strongly recommend to put sentry behind a reverse proxy, why let the default possibility to bind a port?

Also, would you be willing to submit PR for this?

I unfortunately can't guarantee to have the time to do it for now.

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Accepted, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― _Ella Wheeler Wilcox_ 🥀

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Accepted, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― _Ella Wheeler Wilcox_ 🥀