Release: fix 500s on /api/auth/user/info/ and /api/search/aimodel/ - #191
Merged
Merged
Conversation
Renamed the workflow's display name from "Deploy Backend to Dev EC2" to "Deploy Backend to EC2". One workflow now handles both: push to dev deploys to development (unchanged behavior), push to main deploys to production -- environment and image tag derive from github.ref_name. Same shape DataSpaceFrontend/ParakhAPI/ParakhAI-frontend's pipelines already use. No script changes needed: ci-deploy.sh/ci-rollback.sh/ci-finalize.sh are already fully environment-agnostic (single docker-compose.yml, no dev/prod split the way ParakhAPI needed ENV_SUFFIX for). prod-cds also already uses the same ~/DataExchange/DataExBackend submodule layout as dev-cds (confirmed via a recent read-only survey), so no host migration script is needed either -- this is the simplest of the four sibling pipelines. New repo var PROD_API_BASE_URL (https://api.datakeep.civicdays.in -- confirmed via a prior survey, NOT api.civicdataspace.in as the naming pattern would suggest) for the smoke-tests job, which is a `uses:` job and can't see environment-scoped vars. Needs a `production` GH environment with EC2_HOST/EC2_USERNAME/ EC2_PRIVATE_KEY before the first push to main can deploy -- tracked separately, not part of this change (touches prod-cds, not mine to set up). Also flagged inline: prod-cds currently has 12 uncommitted files in its checkout that an image-based deploy will silently discard -- diff those before the first real prod run. actionlint: zero findings.
ci: generalize deploy-backend.yml to cover dev and prod (dev companion to #155)
prod-cds's checkout had 12 uncommitted changes (found while auditing it before trusting an image-based deploy there, which would have silently discarded all of them). Sorted into what to keep vs. discard; these 3 are real, confirmed-needed fixes: - ALLOWED_HOSTS: add prod-parakh's IP (13.235.189.143). ParakhAPI/ ParakhAI-frontend (both hosted there) make server-side calls to this backend; without this Django 400s them as a disallowed host. - rate_limit middleware: exempt health checks and keycloak login/refresh from rate limiting. Also fixed the exempt path list itself while restoring it: the original prod hotfix exempted "/api/auth/keycloak/refresh/", but the real route (DataSpace/urls.py -> api/urls.py) is "/api/auth/token/refresh/" -- that exemption never actually matched anything. Added the /auth/-mounted equivalents too (authorization.urls exposes its own keycloak/login + token/refresh under /auth/, separate from api.urls's copy under /api/) since it wasn't obvious which one is actually in current use -- exempting both is safe, missing the real one risks locking users out under load. - keycloak_utils: log Elasticsearch bulk errors on a user-sync failure, not just the top-level exception message. Deliberately NOT carried forward: a Dockerfile change reordering `chmod +x docker-entrypoint.sh` earlier in the build. Both the original and moved position chmod the file before ENTRYPOINT ever uses it, so there's no behavioral difference either way, and nobody could recall why it was made -- dropped rather than guess at an undocumented rationale. Also on the host, confirmed safe to leave un-committed (backups, stray build/debug artifacts, not source): rate_limit.py.bak.* files, a stray `db.sqlite3;`, logs/, static/, staticfiles/, render.html, snapshot.html.
opub-kc.civicdatalab.in doesn't match the confirmed current production Keycloak domain (auth.civicdatalab.in) -- stale across 6 files (SDK docstrings, README, QUICKSTART, AUTHENTICATION.md, one example script). Swept all 16 occurrences. Also fixed AUTHENTICATION.md's troubleshooting section, which hedged between with/without a /auth path prefix: current Keycloak versions drop that context path by default (confirmed: auth.civicdatalab.in's real realm URLs have no /auth segment), so state that plainly instead of "try both", with a one-line note for anyone still on an older standalone Keycloak that kept the legacy path.
…-dev fix: recover 3 real hotfixes made directly on prod-cds (dev companion to #157)
…ocs-dev docs: fix stale Keycloak domain and /auth prefix (dev companion to #159)
MIDDLEWARE conditionally added debug_toolbar's middleware when DEBUG is True, but INSTALLED_APPS never registered the app itself -- git history shows this pairing existed before and the INSTALLED_APPS half got dropped somewhere along this repo's messy merge/revert history. Broke every build since (confirmed via 3 failed CI runs today, all at the same step): the sanity-check step's `manage.py check` runs with DEBUG defaulting to True (no DEBUG env var set), and any request/check path that touches debug_toolbar's models crashes with "Model class debug_toolbar.models.HistoryEntry doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS" -- the middleware imports the app's models without the app being registered.
…-apps-dev fix: register debug_toolbar in INSTALLED_APPS (dev companion to #161)
…ners Blocked the first real prod deploy attempt today: `$COMPOSE up -d backend_db elasticsearch redis` failed with "Conflict. The container name '/DataExBackendElastic' is already in use" -- confirmed live via `docker ps` compose-project labels that on prod-cds these three are long-lived containers (DataExBackendDb up 2 months, DataExBackendElastic up 6 months, DataExBackendRedis up 14 months) owned by the `dataexchange` project (the top-level superproject), not this pipeline's own `dataexbackend` project. Docker container names are host-global, not project-scoped, so compose doesn't recognize an externally-owned same-named container as already satisfying the service and tries (and fails) to create a new one. The script aborted immediately on this (set -euo pipefail) before ever touching the running `backend` app container -- confirmed live, prod stayed healthy and untouched throughout. Fix: check each dependency container by name first, only run `up -d` for whichever is genuinely absent (a fresh host with nothing running yet). Verified live against the real prod-cds container state (with sudo, matching how the script actually runs there): correctly detects all three as already running and would skip the `up -d` call entirely.
…ntainer-ownership-dev fix: ci-deploy.sh dependency container ownership (dev companion to #163)
Blocked the DataSpaceBackend production deploy: the `release` step's migrate command failed with `could not translate host name "backend_db" to address: Temporary failure in name resolution`. Root cause: this compose file's `backend_db`/`elasticsearch`/`redis` services are, on both dev-cds and prod-cds, deliberately never started by this pipeline (ci-deploy.sh's earlier fix, PR #163, correctly skips them since same-named containers already exist under the separate `dataexchange` superproject). Those real containers live on `dataexchange_default`, a network this project never declared -- `backend`/`release` only ever joined the implicit default network for this project (`dataexbackend_default` on the real hosts), which has no route to them. The currently-running backend container only works today because of an undocumented `docker network connect dataexchange_default DataSpace` done by hand on the host at some point -- confirmed live via `docker inspect`. That connection is not captured anywhere in code, doesn't apply to the `release` container (a fresh container per run), and would NOT survive a real `--force-recreate` of `backend` either. Fix: declare `dataexchange_default` as an external network and attach both `backend` and `release` to it alongside the project's own default network -- covers both a genuinely fresh host (where this file's own backend_db/elasticsearch/redis come up on `default`) and dev-cds/prod-cds (where the real ones live on dataexchange_default). Verified both networks exist on both real hosts via `docker network ls`, and that the rendered config (`docker compose config` / `--profile release config`) resolves both services onto both networks correctly. Caveat, not addressed here: `external: true` means `docker compose up` would fail outright on a machine that has neither network (e.g. a fresh local dev setup never connected to the DataExchange superproject) -- `docker compose config` itself still validates fine without it existing, only `up` needs it. Not fixing speculatively since both confirmed-real deploy targets (dev-cds, prod-cds) already have it.
…ent-dev fix: backend network attachment (dev companion to #165)
Blocked the production deploy's release step: `search_index --populate` crashed with `relation "publication" does not exist`. Root cause traced much further than expected. .gitignore had `api/migrations/*` and `authorization/migrations/*` -- every migration generated for either app since whenever that was added has been silently invisible to git, no matter how many times someone ran `git add`. This repo's `api/migrations/` only ever had `0001_initial.py` committed (likely force-added once, or predates the ignore rule); `authorization/migrations/` had nothing at all, not even `__init__.py`. Confirmed via git log this wasn't always broken: `30480c6 "reset migrations"` deliberately squashed history down to one clean `0001_initial.py` at some point (a legitimate, intentional cleanup, probably around a Postgres migration). But nothing was ever committed again afterward -- every migration generated since (via normal `makemigrations` + `git add`, silently swallowed by the ignore rule) only ever existed on whichever host it was generated on. dev-cds's live checkout had the full, real, working set: 46 files for `api` (0002-0047, ending in the Publication/ResourceType migration this deploy needed) and all 5 for `authorization` (0001-0004 + __init__.py, defining `authorization.User` itself -- AUTH_USER_MODEL). Pulled all 51 files directly from dev-cds (read-only), verified: - Every file compiles clean (`py_compile`). - The full api dependency chain is linear and unbroken, 0001 -> 0047, no gaps, no branches. - authorization's 4 files chain cleanly from 0001, which itself correctly depends on api/0023 (a real cross-app FK relationship) -- confirms the two apps' histories are genuinely interleaved and consistent, not independently drifted. - No overlap with the deliberately-excluded fetch_resource_data feature from the earlier dev/main reconciliation (PR #153) -- the two incidental substring matches found were unrelated migration filenames. Removed both ignore rules. Django migrations must always be committed; there is no legitimate reason to exclude them. This also means any genuinely fresh database (a new contributor's local setup, a CI job with --create-db, a disaster-recovery rebuild) has been silently unable to reach current schema this whole time -- would have stopped dead at whatever 0001_initial.py defines. That risk is now closed.
…ration-dev fix: commit missing migrations (dev companion to #167)
The Smoke Tests job passed the repo-level HOME_URL_DEV (the dev site) on main too, so every prod deploy's consumer and provider suites tested dev: prod's nginx logged 0 test-browser requests during a prod run's smoke window, dev's logged 8840. Prod rollbacks were being triggered by dev timeouts, and prod's UI was never actually tested. On main, pass the prod URL and set skip_provider, since provider-smoke creates content with the test account.
…-dev fix: point prod smoke tests at prod and skip provider-smoke there
dev: smoke first; functional runs only if smoke passes; either failing rolls back, both passing finalizes. main: only the readonly allowlist runs against prod (provider never), replacing skip_provider. Finalize uses !failure() because functional-tests is skipped on main by design.
Runs the full suite (smoke, functional, regression) against dev on every PR into main. deployed_sha is the PR head, so the gate fails unless dev is running exactly the code being merged.
Provider functional tests fail at shifting points against dev (autosave races, slow create flows), so gating on them would roll back dev deploys spuriously. The gating functional stage and the PR gate now skip provider; separate provider jobs run with only_provider and report, without being a dependency of rollback/promote or a required check.
feat: deploy gating model — two-stage dev, readonly prod, PR gate
Finalize and rollback did not list the report-only provider job in needs, so a deploy asked for finalize approval while provider tests were still running. Both now wait for it, and condition on the gating jobs' results explicitly instead of failure()/!failure(), which would count the report-only job's result.
…tests-dev fix: wait for every test before finalizing or rolling back
org_add_permission needs a Keycloak token to check canAdd. A called workflow only sees secrets its caller passes, and the report-only job passed none, so every org-create test skipped on both workers -- the provider report looked green while never exercising org flows.
…provider-report Mirrors main: contents/pull-requests read (the CodeQL finding), and KEYCLOAK_CLIENT_SECRET for provider-report's org-create tests.
…cret-dev fix: give report-only provider jobs the Keycloak secret
UserInfoView declared no permission_classes, so the project-wide AllowAny default let anonymous requests reach the handler, where reading .email off AnonymousUser raised AttributeError and returned 500 instead of 401.
cache.set pickles the value. Serialized hits keep AttrList/AttrDict/InnerDoc values for nested fields, and those classes are rebuilt per document type, so pickling raised "it's not the same object as elasticsearch_dsl.document .InnerDoc" and the except clause turned it into a 500 on /api/search/aimodel/.
…ch-cache fix: 500s on /api/auth/user/info/ and /api/search/aimodel/
options was passed twice and JS true was used instead of True, in five places, so this file never ran - and it took the whole suite down with it.
The SDK workflows run a few tests with -p no:django, so views, schema and permissions had no CI coverage at all. Known-stale chart and metadata tests are excluded, tracked in #189.
…ch-cache fix: 500s on /api/auth/user/info/ and /api/search/aimodel/, plus Django tests in CI
Ships the /api/auth/user/info/ and /api/search/aimodel/ 500 fixes, their tests, and the Django Tests CI job. Conflicts: pr-gate.yml takes dev (adds the Django Tests job); deploy-backend.yml keeps main (retains the CodeQL permissions block). dataset_data stays reverted on main (01bf459); this merge does not undo that.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ntain permissions' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes #185 to prod. Both endpoints return 500 on prod today.
What ships (6 files vs main):
/api/auth/user/info/→ 401 for anonymous callers instead of 500 (permission_classes = [IsAuthenticated])./api/search/aimodel/→ search results cached as plain data, socache.setno longer fails to pickle nestedInnerDocvalues.tests/test_search_cache_and_user_info.py— each test fails without its fix.django-tests.yml+ a Django Tests job inpr-gate.yml; syntax repair intest_grouped_bar_chart.py.Verified on dev:
user/info401,search/aimodel200; dev deploy 35066030763 fully green; Django Tests 216 passed.Merge notes:
devintomainrather than a direct dev→main PR, because two workflow files conflicted.pr-gate.ymltakes dev (adds Django Tests).deploy-backend.ymlkeeps main (retains the CodeQLpermissionsblock prod already deploys with).dataset_datastays reverted on main (01bf459 Revert "Dev"). Git treats that commit as already in main's history, so this merge does not reintroduce it.eaf5a1c, and this PR's head is the merge commit.