Skip to content

fix: lower nginx catch-all ingressroute priority to restore /api routing#3202

Open
Viczei wants to merge 1 commit intomasterfrom
fix/api-search-ingress-priority
Open

fix: lower nginx catch-all ingressroute priority to restore /api routing#3202
Viczei wants to merge 1 commit intomasterfrom
fix/api-search-ingress-priority

Conversation

@Viczei
Copy link
Copy Markdown
Contributor

@Viczei Viczei commented Apr 13, 2026

Contexte

Signalé par un utilisateur de Annuaire Entreprises : l'endpoint https://egapro.travail.gouv.fr/api/search?q=... renvoie un 404 HTML (page Next.js) au lieu du JSON de l'API Python.

Diagnostic

Dans .kontinuous/env/prod/archive/http.yaml, deux IngressRoutes Traefik matchent sur l'host egapro.travail.gouv.fr :

  • api (PathPrefix(/api) → service Python) : priority 100
  • nginx (catch-all sur l'host → Next.js) : priority 10000

Chez Traefik la priority la plus haute gagne, donc le catch-all nginx mange tout /api/*, sauf les règles explicitement re-routées à 10000 vers nginx (/api/auth, /api/monitoring, /index-egapro/recherche).

Correction

On baisse la priority du catch-all nginx à 50 pour que l'ordre de résolution devienne :

Path Règle gagnante Service
/api/auth/* nginx /api/auth (10000) nginx
/api/monitoring/* nginx /api/monitoring (10000) nginx
/index-egapro/recherche/* nginx (10000) nginx
/api/search, /api/stats, autres /api/* api (100) api Python
reste nginx catch-all (50) nginx (Next.js)

⚠️ Note : le fichier est actuellement sous archive/ (archivé dans #3097), donc Helm ne le rend plus. Les IngressRoutes en prod sont des orphelines du cluster. Un kubectl apply -f manuel est prévu pour appliquer le correctif — à discuter si on désarchive le fichier dans la foulée pour que Helm reprenne la main.

Test plan

  • kubectl apply -f .kontinuous/env/prod/archive/http.yaml en prod
  • curl https://egapro.travail.gouv.fr/api/search?q=356000000 renvoie du JSON
  • curl https://egapro.travail.gouv.fr/api/stats renvoie du JSON
  • Vérifier que /api/auth/* (ProConnect) fonctionne toujours
  • Vérifier que / charge toujours le Next.js

The nginx catch-all rule (priority 10000) was winning over the api
IngressRoute (priority 100), so /api/* requests (except the explicit
/api/auth and /api/monitoring overrides) were being routed to the
Next.js app and returning 404. Drop the catch-all priority to 50 so
the api route wins while keeping the nginx-specific overrides.
@Viczei Viczei requested review from a team as code owners April 13, 2026 11:40
@revu-bot revu-bot Bot requested a review from revu-bot April 13, 2026 11:40
Copy link
Copy Markdown
Collaborator

@revu-bot revu-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ PR Review Skipped

3 validation issues found. Review thresholds can be adjusted in .revu.yml.

See why it was skipped and detailed metrics

Issues Found

1. This PR changes 189 files, which exceeds the limit of 25 files.

Suggestion: Consider breaking this PR into smaller, more focused changes. Large PRs are harder to review effectively and may contain unrelated changes.

2. This PR has 52877 lines of diff, which exceeds the limit of 15000 lines.

Suggestion: Consider splitting this PR into smaller chunks. Large diffs are difficult to review thoroughly and may hide important issues.

3. This PR contains files that exceed the size limit: '.kontinuous/charts/redis/values.schema.json' (3299 lines of changes), 'pnpm-lock.yaml' (12609 lines of changes), 'yarn.lock' (15749 lines of changes), which exceeds the limit of 3000. The limit is 3000 lines per file.

Suggestion: Consider refactoring large changes into smaller, more focused modifications. Large file changes are harder to review and understand.

PR Metrics

  • Total files changed: 189
  • Reviewable files: 189
  • Diff size: 52877 lines
  • Documentation files: 5
  • Largest file change: 15749 lines
  • Addition/Deletion ratio: 1.59

This validation helps ensure the bot focuses on PRs where automated review provides the most value.

@Viczei Viczei changed the base branch from alpha to master April 13, 2026 11:44
@Viczei Viczei requested a review from revu-bot April 13, 2026 11:45
Copy link
Copy Markdown
Collaborator

@revu-bot revu-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Lower nginx catch-all IngressRoute priority to 50

This is a minimal, targeted fix for a production routing bug where the nginx catch-all IngressRoute (priority 10000) was intercepting /api/* requests before the dedicated api IngressRoute (priority 100) could handle them.

The one-line change is correct and the routing table in the PR description accurately reflects the intended behavior after the fix. However, there are a few important concerns worth raising.

File Lines Severity Issue
.kontinuous/env/prod/archive/http.yaml 111 CRITICAL File is archived — Helm won't apply this; manual kubectl apply creates drift
.kontinuous/env/prod/archive/http.yaml 111 IMPORTANT Priority 50 is dangerously close to Traefik's auto-computed default; consider a safer value
.kontinuous/env/prod/archive/http.yaml 108–111 IMPORTANT files-public and files-restricted routes also use priority 100 — verify they aren't shadowed by the new catch-all at 50

@tokenbureau
Copy link
Copy Markdown

tokenbureau Bot commented Apr 13, 2026

🎉 Deployment for commit 8758298 :

Ingresses
Docker images
  • 📦 docker pull busybox:1.36
  • 📦 docker pull harbor.fabrique.social.gouv.fr/egapro/egapro/api:sha-8758298e921d2b75244384d4dade1e82e9ff3ef7
  • 📦 docker pull harbor.fabrique.social.gouv.fr/egapro/egapro/app:sha-8758298e921d2b75244384d4dade1e82e9ff3ef7
  • 📦 docker pull harbor.fabrique.social.gouv.fr/egapro/egapro/files:sha-8758298e921d2b75244384d4dade1e82e9ff3ef7
  • 📦 docker pull harbor.fabrique.social.gouv.fr/egapro/egapro/nginx:sha-8758298e921d2b75244384d4dade1e82e9ff3ef7
  • 📦 docker pull maildev/maildev:2.1.0
  • 📦 docker pull quay.io/keycloak/keycloak:26.0
  • 📦 docker pull sosedoff/pgweb:0.14.1
Debug

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.

4 participants