Skip to content

Env-driven Elasticsearch wiring + dev override (alternative to #52) - #54

Open
rtb-12 wants to merge 1 commit into
masterfrom
elasticsearch-env-wiring
Open

Env-driven Elasticsearch wiring + dev override (alternative to #52)#54
rtb-12 wants to merge 1 commit into
masterfrom
elasticsearch-env-wiring

Conversation

@rtb-12

@rtb-12 rtb-12 commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Re-opened from the org-repo branch (was previously #53 from a fork). Same commits, same diff.

Summary

  • Wires the noticeboard Elasticsearch connection settings through an `env_file` on both Django services so the same image works in dev (local sidecar) and prod (separate ES VM with TLS + auth) without code changes.
  • Adds `noticeboard/elasticsearch_stencil.env` + `.gitignore` + `README.md` following the per-service convention already used by `postgres/` and `rabbitmq/`.
  • Adds `docker-compose.override.yml.example` as an opt-in dev sidecar that addresses the security and operational issues raised in Elasticsearch integration to the Docker Compose setup #52 (localhost-only port binding, JVM heap cap, healthcheck `start_period`, no fixed `container_name`).
  • Adds `docker-compose.override.yml` to root `.gitignore` so each developer keeps their own overrides locally without polluting the canonical compose file.

Why this shape

Posted as a follow-up to #52 (and an alternative shape for it) based on review feedback. Three things drove the design:

  1. ES belongs on a separate host in production. Putting the service inline in the canonical `docker-compose.yml` (as Elasticsearch integration to the Docker Compose setup #52 does) makes that hard — every deploy of the stack also deploys ES on the same box, defeating the goal of not loading the noticeboard host with index workload. The override pattern lets dev opt in to a local sidecar while prod points at a remote VM via the same env vars.
  2. The hardcoded `Elasticsearch(['http://elastic:9200'])` in This PR enhances the Elasticsearch integration. omniport-app-noticeboard#22 needs to come from settings/env. This PR provides the channel; the corresponding app-side change reads `ELASTICSEARCH_HOST` etc. from `os.environ`.
  3. Security defaults matter. The example override binds 9200 to `127.0.0.1` only and caps the JVM heap, so a developer copy-pasting it doesn't accidentally expose an unauthenticated cluster to the internet.

What this PR does NOT do

  • Does not add Python deps to `pyproject.toml` — that lives in the app PR (omniport-app-noticeboard#22) so the dep change ships with the code that uses it.
  • Does not add Django settings — also lives in the app PR.
  • Does not modify or replace Elasticsearch integration to the Docker Compose setup #52's inline service — leaves that decision to maintainers; the two approaches can be compared side by side.

Test plan

  • `cp noticeboard/elasticsearch_stencil.env noticeboard/elasticsearch.env`, fill in dev values
  • `cp docker-compose.override.yml.example docker-compose.override.yml`
  • `docker-compose up -d elasticsearch` — confirm container becomes healthy
  • `curl http://127.0.0.1:9200/_cluster/health\` — confirm reachable from host
  • `docker-compose exec intranet-server env | grep ELASTICSEARCH_` — confirm env vars populate inside the container
  • `nmap -p 9200 ` — confirm 9200 is not exposed publicly

Wires the noticeboard ES connection through an env_file on both Django
services so the same image runs in dev (local sidecar) and prod (separate
ES VM with TLS + auth) without code changes.

- noticeboard/elasticsearch_stencil.env: connection-settings template
- noticeboard/.gitignore + README: matches the existing per-service
  pattern used by postgres/ and rabbitmq/
- docker-compose.yml: env_file passthrough on intranet-server and
  internet-server
- docker-compose.override.yml.example: opt-in dev sidecar that binds
  9200 to 127.0.0.1 only and caps the JVM heap, with .gitignore for
  per-developer overrides

Coordinates with omniport-app-noticeboard#22 (which needs to read these
env vars from settings) and is an alternative shape for the inline
service in omniport-docker#52.
@pelagiann

Copy link
Copy Markdown

Agreed on this shape — I have closed #52 in favour of it. The reasoning about keeping Elasticsearch out of the canonical compose file, and about the connection coming from the environment, both hold up.

The application side already matches this PR. settings/third_party/elastic.py in IMGIITRoorkee/omniport-backend#238 reads ELASTICSEARCH_HOST, ELASTICSEARCH_USER, ELASTICSEARCH_PASSWORD, ELASTICSEARCH_VERIFY_CERTS, ELASTICSEARCH_TIMEOUT and ELASTICSEARCH_CA_CERT from os.environ, with http://elastic:9200 only as the fallback default. Its docstring already points at noticeboard/elasticsearch_stencil.env, so once this merges the reference resolves; until then it names a file that does not exist yet.

The security point is not hypothetical. On the deployment running #52's inline service today:

docker port elastic          9200/tcp -> 0.0.0.0:9200
                             9300/tcp -> 0.0.0.0:9300
curl http://<host>:9200/_cat/indices   ->  200, no credentials

xpack.security.enabled is false, so anything that can reach the host can read the index and, more importantly, delete or poison it. The index rebuilds from PostgreSQL with the search_index management command, so it is a denial-of-service rather than data loss — but the 127.0.0.1 binding in your example override is the right default and would have prevented it.

Two things this PR needs:

  1. A rebase onto current master — it is currently marked as conflicting and has not moved since April.
  2. A home for deployment-specific Django variables. Beyond the Elasticsearch ones, OMNIPORT_CSRF_COOKIE_NAME needs to be set per deployment: the frontend reads its CSRF cookie under a deployment-chosen name, so Django has to write the same one or every POST is rejected. I am currently setting it through a local docker-compose.override.yml, which works with the pattern you introduce here, but it may be worth mentioning in the README alongside the Elasticsearch variables.

Happy to help with the rebase if useful.

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.

2 participants