add heimdall middleware #12
Conversation
This is needed if this service is installed in a namespace that doesn't otherwise have the middleware, since there is no current way to reference a middleware in the gateway api spec outside of the current namespace. Signed-off-by: Jordan Evans <jevans@linuxfoundation.org>
WalkthroughAdds an optional Helm template and values to create two Traefik forwardAuth middlewares for Heimdall, and bumps chart version from 0.2.3 to 0.2.4. Also includes minor YAML linting comment edits in CI workflow files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant T as Traefik
participant M as Middleware (Heimdall forwardAuth)
participant H as Heimdall
participant S as lfx-v2-query-service
rect rgba(224,240,255,0.5)
note over T,M: Conditional middleware path (created when heimdall.add_middleware = true)
C->>T: HTTP request
T->>M: forwardAuth (forwards Authorization header)
M->>H: Auth request (optionally includes body for heimdall-forward-body)
H-->>M: Auth allow/deny
alt Allowed
T->>S: Forward request
S-->>T: Response
T-->>C: Response
else Denied
T-->>C: 401/403
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)
✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds optional Heimdall middleware configuration to the LFX V2 Query Service Helm chart, addressing namespace isolation issues where middleware resources cannot be referenced across namespaces in Gateway API specifications.
- Added
add_middlewareboolean flag andurlconfiguration option to values.yaml - Created new middleware template with two variants (with/without body forwarding)
- Bumped chart version from 0.2.3 to 0.2.4
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| charts/lfx-v2-query-service/values.yaml | Added configuration options for middleware creation and Heimdall URL |
| charts/lfx-v2-query-service/templates/heimdall-middleware.yaml | New template for creating Heimdall middleware resources |
| charts/lfx-v2-query-service/Chart.yaml | Version bump to reflect new functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
andrest50
left a comment
andrest50
left a comment
There was a problem hiding this comment.
I'll approve after the megalinter job passes. There's just some lint errors.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
charts/lfx-v2-query-service/values.yaml (1)
56-58: Make header behavior configurable and validate URL when enabling middleware.
- Consider adding heimdall.authRequestHeaders and heimdall.authResponseHeaders so header pass-through isn’t hardcoded in templates. Add a guard to fail if add_middleware=true and url is empty.
Apply this diff:
heimdall: enabled: true add_middleware: false - url: http://lfx-platform-heimdall.lfx.svc.cluster.local:4456 + url: http://lfx-platform-heimdall.lfx.svc.cluster.local:4456 + # Headers from the original request to forward to Heimdall (e.g., credentials) + authRequestHeaders: + - Authorization + - Cookie + # Headers from Heimdall’s response to propagate to the upstream request (identity/claims) + authResponseHeaders: []charts/lfx-v2-query-service/templates/heimdall-middleware.yaml (4)
3-3: Gate creation on both add_middleware and enabled.Prevents accidental creation when heimdall.enabled=false.
Apply this diff:
-{{ if .Values.heimdall.add_middleware }} +{{- if and .Values.heimdall.enabled .Values.heimdall.add_middleware }}
16-21: Add optional TLS config and make forwardBody a value.Provide tls.insecureSkipVerify and toggle forwardBody via values for large payload routes.
Apply this diff and wire values:
forwardAuth: address: "{{ .Values.heimdall.url }}" + tls: + insecureSkipVerify: {{ default false .Values.heimdall.tlsInsecureSkipVerify }} authRequestHeaders: - Authorization - Cookie - forwardBody: true + forwardBody: {{ default true .Values.heimdall.forwardBody }}
22-35: DRY: reduce duplication with a named template.Both resources differ only by name and forwardBody. Consider a define/template to render both variants.
5-9: Comment phrasing.“parentRef requiring authentication is in the request body” is confusing. Suggest clarifying that some auth decisions may depend on request body content.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
charts/lfx-v2-query-service/Chart.yaml(1 hunks)charts/lfx-v2-query-service/templates/heimdall-middleware.yaml(1 hunks)charts/lfx-v2-query-service/values.yaml(2 hunks)
🔇 Additional comments (3)
charts/lfx-v2-query-service/Chart.yaml (1)
8-9: Version bump looks good.Patch bump to 0.2.4 matches a backwards-compatible Helm addition.
charts/lfx-v2-query-service/values.yaml (1)
24-31: Check both HTTPRoute and Middleware templates setmetadata.namespaceto.Values.lfx.namespace.Without explicit matching namespaces, the HTTPRoute’s
extensionRefwon’t resolve the Middleware under Gateway API whenadd_middleware=true. Please confirm each template has:metadata: namespace: {{ .Values.lfx.namespace }}charts/lfx-v2-query-service/templates/heimdall-middleware.yaml (1)
16-21: Ensure Traefik version supports forwardAuth.forwardBody
forwardAuth.forwardBody is available in Traefik v2.x and in the v3 series from v3.3 onward. Confirm your cluster is running one of these versions; otherwise upgrade Traefik or remove the forwardBody option.
Signed-off-by: Jordan Evans <jevans@linuxfoundation.org>
This is needed if this service is installed in a namespace that doesn't otherwise have the middleware, since there is no current way to reference a middleware in the gateway api spec outside of the current namespace.