LFXV2-211 - query-service - migrate IngressRoute to the Gateway API HTTPRoute#6
Conversation
…ic port values - add HTTPRoute resource Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe changes transition the lfx-v2-query-service Helm chart from using a Traefik IngressRoute to a Gateway API HTTPRoute for ingress. Service and deployment port configurations are made dynamic via Helm values. The values.yaml is restructured to support the new Gateway API configuration, including domain, namespace, and service port settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Gateway
participant HTTPRoute
participant Service
participant Pod
Client->>Gateway: HTTP request to /query or /query/...
Gateway->>HTTPRoute: Forwards request based on host/path
HTTPRoute->>Service: Routes to lfx-v2-query-service on configured port
Service->>Pod: Proxies request to application container
Pod-->>Service: Responds
Service-->>Gateway: Forwards response
Gateway-->>Client: Returns response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the query-service from Traefik IngressRoute to the newer Gateway API HTTPRoute, following the same pattern established in project-service. The migration involves replacing legacy ingress configuration with modern Gateway API standards while maintaining the same routing functionality.
- Replaces legacy IngressRoute with Gateway API HTTPRoute resource
- Updates service configuration to use configurable ports instead of hardcoded values
- Adds new configuration sections for traefik, lfx, and service settings in values.yaml
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| values.yaml | Replaces ingress configuration with traefik, lfx, and service configuration sections |
| service.yaml | Updates service port to use configurable value from values.yaml |
| ingressroute.yaml | Removes legacy Traefik IngressRoute configuration |
| httproute.yaml | Adds new Gateway API HTTPRoute configuration with equivalent routing rules |
| deployment.yaml | Updates container port to use configurable value from values.yaml |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
charts/lfx-v2-query-service/values.yaml (1)
25-28: Document the integer expectation forservice.portAdd a short comment that the value must be numeric (no quotes) since code casts it to
int.
Helps chart consumers avoid subtle template failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/lfx-v2-query-service/templates/deployment.yaml(1 hunks)charts/lfx-v2-query-service/templates/httproute.yaml(1 hunks)charts/lfx-v2-query-service/templates/ingressroute.yaml(0 hunks)charts/lfx-v2-query-service/templates/service.yaml(1 hunks)charts/lfx-v2-query-service/values.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- charts/lfx-v2-query-service/templates/ingressroute.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-v2-query-service/templates/httproute.yaml
[error] 25-25: syntax error: could not find expected ':'
(syntax)
🔇 Additional comments (1)
charts/lfx-v2-query-service/templates/httproute.yaml (1)
24-31: YAML-lint failure hints at indentation risk around the conditionalfiltersblockWhen
.Values.heimdall.enabledisfalse, thefilters:key disappears but the surrounding indentation stays, potentially leaving extra blank lines that YAML-lint flags (could not find expected ':').
Wrap the whole block withnindentto guarantee correct indent, e.g.:{{- if .Values.heimdall.enabled }} {{- nindent 4 "filters:\n- type: ExtensionRef\n extensionRef:\n group: traefik.io\n kind: Middleware\n name: heimdall" }} {{- end }}This yields valid YAML in both code paths.
| ports: | ||
| - containerPort: 8080 | ||
| - containerPort: {{ .Values.service.port }} | ||
| name: web |
There was a problem hiding this comment.
Cast the configurable port to an int to avoid type-mismatch errors
If a user overrides .Values.service.port with a quoted value ("8080"), Helm will render a string.
containerPort must be an integer; sending a string will fail validation against the K8s API.
- - containerPort: {{ .Values.service.port }}
+ - containerPort: {{ .Values.service.port | int }}(Apply the same filter anywhere the value is used as a numeric port.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ports: | |
| - containerPort: 8080 | |
| - containerPort: {{ .Values.service.port }} | |
| name: web | |
| ports: | |
| - containerPort: {{ .Values.service.port | int }} | |
| name: web |
🤖 Prompt for AI Agents
In charts/lfx-v2-query-service/templates/deployment.yaml around lines 36 to 38,
the containerPort value is rendered directly from .Values.service.port, which
can be a string if overridden with quotes, causing Kubernetes validation errors.
Fix this by casting .Values.service.port to an integer using the Helm int
function wherever it is used as a numeric port to ensure the value is always an
integer.
| ports: | ||
| - name: web | ||
| port: 80 | ||
| port: {{ .Values.service.port }} | ||
| targetPort: web |
There was a problem hiding this comment.
Same int-casting needed for the Service port
spec.ports[].port is also an integer field. Cast it the same way to keep template behaviour consistent.
- port: {{ .Values.service.port }}
+ port: {{ .Values.service.port | int }}🤖 Prompt for AI Agents
In charts/lfx-v2-query-service/templates/service.yaml around lines 11 to 14, the
service port value is not explicitly cast to an integer, which can cause
inconsistencies since spec.ports[].port expects an integer. Update the template
to cast .Values.service.port to an integer using the int function, ensuring
consistent and correct type handling in the YAML output.
Generated with [GitHub Copilot](https://github.com/features/copilot) Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
Overview
Similar to the migration done in project-service (see PR #7), we need to migrate the query-service from using Traefik IngressRoute to the newer Gateway API HTTPRoute.
This follows the same pattern as LFXV2-49 and involves:
Reference Implementation: The project-service PR provides a good example of the changes needed: https://github.com/linuxfoundation/lfx-v2-project-service/pull/7/files