feat: add Gateway API support in hydra - #889
Conversation
|
|
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds Gateway API HTTPRoute support to the Hydra Helm chart, introducing conditional templates for public and admin routes, default disabled values with hostname placeholders, and an additional issuer URL derivation branch that uses the public HTTPRoute hostname when enabled. ChangesHTTPRoute feature
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)flowchart TD
ValuesYaml --> HelmTemplate
HelmTemplate --> HttprouteAdminYaml
HelmTemplate --> HttproutePublicYaml
HttproutePublicYaml --> IssuerHelper
IssuerHelper --> ConfigUrlsIssuer
Related Issues: None referenced. Related PRs: None referenced. Suggested labels: helm, enhancement Suggested reviewers: None determinable from the provided information. 🐰 A hop, a route, a gateway new, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
helm/charts/hydra/templates/httproute-admin.yaml (2)
17-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDefault empty
parentRefsproduces an invalid HTTPRoute.Same concern as flagged for
httproute-public.yamlandvalues.yaml.Also applies to: 27-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/charts/hydra/templates/httproute-admin.yaml` around lines 17 - 20, The httproute-admin template currently renders an empty parentRefs block when .Values.httproute.admin.parentRefs is unset, which makes the HTTPRoute invalid. Update the template around parentRefs in the httproute-admin YAML to only emit that section when there is at least one entry, using the same conditional pattern used for httproute-public.yaml, and ensure the related defaults in values.yaml do not leave parentRefs empty by default.
5-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSame
labels/annotationsnesting bug ashttproute-public.yaml.Identical control-flow issue: the
if .Release.Namespace(line 7) is only closed at line 15, solabels:(9-10) andannotations:(11-14) are nested inside it, unlikeservice-admin.yaml's pattern where the namespace conditional closes immediately after thenamespace:line.🐛 Proposed fix
metadata: name: {{ $fullName }}-admin {{- if .Release.Namespace }} namespace: {{ .Release.Namespace }} + {{- end }} labels: {{- include "hydra.labels" . | nindent 4 }} {{- with .Values.httproute.admin.annotations }} annotations: {{- tpl (toYaml .) $ | nindent 4 }} {{- end }} - {{- end }} spec:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/charts/hydra/templates/httproute-admin.yaml` around lines 5 - 15, The metadata block in httproute-admin has the same nesting mistake as httproute-public: the .Release.Namespace conditional in the template keeps labels and annotations inside it instead of matching the service-admin pattern. Update the httproute-admin template so the namespace guard only wraps the namespace field, then render labels and annotations unconditionally afterward, using the existing metadata structure and symbols like .Release.Namespace, .Values.httproute.admin.annotations, and include "hydra.labels".helm/charts/hydra/templates/httproute-public.yaml (1)
17-20: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDefault empty
parentRefsproduces an invalid HTTPRoute.See corresponding comment on
values.yaml— with the defaultparentRefs: [], thiswithblock renders no content, leavingparentRefs:with a null value, which is invalid since Gateway API HTTPRoutes require at least oneparentRefto attach to a Gateway.Also applies to: 27-32
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/charts/hydra/templates/httproute-public.yaml` around lines 17 - 20, The HTTPRoute templates are rendering an empty parentRefs field when .Values.httproute.public.parentRefs (and the same pattern in the other affected template block) is left at its default empty list, which produces an invalid route. Update the templating around the parentRefs section in the HTTPRoute manifest so it either always renders at least one valid parentRef or conditionally omits the entire HTTPRoute when no parentRefs are configured, using the existing .Values.httproute.public.parentRefs lookup to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@helm/charts/hydra/templates/httproute-public.yaml`:
- Around line 5-15: The `httproute-public.yaml` template has `labels` and
`annotations` incorrectly nested under the `.Release.Namespace` conditional.
Update the `metadata` block so the `if .Release.Namespace` in the
`httproute-public` template only wraps the `namespace:` line, then close it
before `labels:` and the `with .Values.httproute.public.annotations` block,
matching the pattern used in `service-public.yaml`.
---
Duplicate comments:
In `@helm/charts/hydra/templates/httproute-admin.yaml`:
- Around line 17-20: The httproute-admin template currently renders an empty
parentRefs block when .Values.httproute.admin.parentRefs is unset, which makes
the HTTPRoute invalid. Update the template around parentRefs in the
httproute-admin YAML to only emit that section when there is at least one entry,
using the same conditional pattern used for httproute-public.yaml, and ensure
the related defaults in values.yaml do not leave parentRefs empty by default.
- Around line 5-15: The metadata block in httproute-admin has the same nesting
mistake as httproute-public: the .Release.Namespace conditional in the template
keeps labels and annotations inside it instead of matching the service-admin
pattern. Update the httproute-admin template so the namespace guard only wraps
the namespace field, then render labels and annotations unconditionally
afterward, using the existing metadata structure and symbols like
.Release.Namespace, .Values.httproute.admin.annotations, and include
"hydra.labels".
In `@helm/charts/hydra/templates/httproute-public.yaml`:
- Around line 17-20: The HTTPRoute templates are rendering an empty parentRefs
field when .Values.httproute.public.parentRefs (and the same pattern in the
other affected template block) is left at its default empty list, which produces
an invalid route. Update the templating around the parentRefs section in the
HTTPRoute manifest so it either always renders at least one valid parentRef or
conditionally omits the entire HTTPRoute when no parentRefs are configured,
using the existing .Values.httproute.public.parentRefs lookup to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a0eed4a-142e-40ee-9910-937260d7825b
📒 Files selected for processing (4)
helm/charts/hydra/templates/_helpers.tplhelm/charts/hydra/templates/httproute-admin.yamlhelm/charts/hydra/templates/httproute-public.yamlhelm/charts/hydra/values.yaml
|
@Demonsthere @piotrmsc any chance I could get your attention? |
2 participants
Related Issue or Design Document
partially implements #845
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.com) from the maintainers to push the changes.
Further comments
This PR adds Gateway API HTTPRoutes in
hydrafor the API and proxy.The objective was to provide a drop-in replacement for Ingress resources.
One discepancy is in
_helpers.tpland thehydra.config.urls.issuerhelper. HTTPRoutes don't contain information about TLS in their spec so it's impossible to determine the protocol just from it. Since you can work around this by using.Values.hydra.config.urls.self.issuerI did not consider it a problem but I'm happy to get feedback.This PR is a bit selfish. I just need to get this done one way or another and thought that contributing back was the best way to get it done.
Summary by CodeRabbit
New Features
Documentation