feat: migrate ingress from NGINX to Envoy Gateway#80
feat: migrate ingress from NGINX to Envoy Gateway#80laurigates wants to merge 2 commits intomasterfrom
Conversation
Replace ingress-nginx with Gateway API (Envoy Gateway) in both backend and frontend Helm values files by enabling the gateway flag supported by the helm-webapp chart. - Add gateway.enabled: true to backend and frontend values - Disable explicit ingress (chart suppresses it when gateway is enabled) - Remove NGINX-specific annotation from frontend; SPA routing (try_files) must be handled at the nginx container level, not at the ingress Closes #62 Co-authored-by: Lauri Gates <laurigates@users.noreply.github.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request transitions the OLMap backend and frontend services from using Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request migrates the application's ingress from NGINX Ingress to Envoy Gateway by updating the Helm values for the backend and frontend deployments. The changes correctly enable the Gateway API configuration and disable the legacy Ingress resources. However, I've identified a critical issue where related application configuration, such as DJANGO_ALLOWED_HOSTS and CORS settings, has not been updated to reflect the new hostname-based routing scheme introduced by this migration. This will likely break the application. A specific comment has been left on the relevant file with details on the required fixes.
| ingress: | ||
| # Gateway API configuration (Envoy Gateway) | ||
| # Replaces ingress-nginx; creates HTTPRoute pointing to shared dataportal-gateway | ||
| # Hostname auto-generated as olmap-backend.dataportal.fi from release name |
There was a problem hiding this comment.
This comment indicates the backend will be served from olmap-backend.dataportal.fi. This is a change from the previous single-host setup (olmap.dataportal.fi).
This change requires corresponding updates to other configuration values in this file, which appear to have been missed:
DJANGO_ALLOWED_HOSTS(line 80) should includeolmap-backend.dataportal.fi.DJANGO_CORS_ALLOWED_ORIGINS(line 82) should be updated to the new frontend URL, which seems to behttps://olmap-frontend.dataportal.fibased on the changes indeploy/frontend-values.yaml.FRONTEND_ROOT(line 98) should also be updated tohttps://olmap-frontend.dataportal.fi.
Without these changes, the application will likely fail with CORS errors, incorrect redirects, and other issues.
|
Let's address the recommendations in the comments. Evaluate which pieces of criticism and recommendations are substantive and valid. Act accordingly and refute invalid points. Create follow-up issue for the currently failing tests |
|
Claude finished @laurigates's task in 2m 43s —— View job Addressing Review Comments
Gemini feedback evaluationValid — addressed: Gemini's concern about hostname mismatches is correct. The migration changes routing from a single shared domain
The following env vars in
Without these changes, Django would reject requests arriving at the new backend hostname and CORS would block the frontend. CI failure analysisThe
I attempted to create a follow-up issue but the
| Branch |
Update DJANGO_ALLOWED_HOSTS, DJANGO_CORS_ALLOWED_ORIGINS, and FRONTEND_ROOT to reflect the new Envoy Gateway hostname-based routing scheme: - Backend: olmap-backend.dataportal.fi (was olmap.dataportal.fi with path routing) - Frontend: olmap-frontend.dataportal.fi (was olmap.dataportal.fi) Without these changes, Django would reject requests from the new backend hostname and CORS would block frontend requests from the new frontend origin. Co-authored-by: Lauri Gates <laurigates@users.noreply.github.com>
Migrate OLMap backend and frontend from ingress-nginx to Envoy Gateway (Gateway API).
gateway.enabled: trueto bothdeploy/backend-values.yamlanddeploy/frontend-values.yamlconfiguration-snippetannotation from frontendImportant
Before merging, verify the frontend nginx container image handles SPA routing (
try_files $uri $uri/ /index.html) internally in its nginx.conf.Closes #62
Generated with Claude Code