feat(httproute): add optional kind and group fields to parentRefs#349
feat(httproute): add optional kind and group fields to parentRefs#349pierre-jezegou wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for Gateway API v1.5 ListenerSet (and other non-default parent resources) by allowing kind and group to be specified on HTTPRoute.spec.parentRefs in the OpenCost Helm chart.
Changes:
- Extend
HTTPRoutetemplates to optionally renderparentRefs[].kindandparentRefs[].groupfor UI, API, and MCP HTTPRoutes. - Document the new optional
kind/groupfields invalues.yamlunder each HTTPRoute’sparentRefs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
charts/opencost/values.yaml |
Documents optional kind and group fields for httpRoute.parentRefs in UI/API/MCP values. |
charts/opencost/templates/httproute.yaml |
Renders optional kind/group fields into HTTPRoute.spec.parentRefs for UI/API/MCP routes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- if .kind }} | ||
| kind: {{ .kind }} | ||
| {{- end }} | ||
| {{- if .group }} | ||
| group: {{ .group }} |
|
@pierre-jezegou looks like some decent copilot findings, could you remediate? |
|
and please bump the patch version on the chart |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
charts/opencost/templates/httproute.yaml:73
- The
{{ if .kind }}/{{ end }}and{{ if .group }}/{{ end }}blocks don’t use whitespace-trimming ({{- ... }}), which will emit whitespace-only lines in the rendered manifest. With the repo’s yamllint config enablingtrailing-spaces, these space-only lines can causect lintto fail. Use trimmed control tags ({{- if ... }}/{{- end }}) consistent with the existingnamespace/sectionNameblocks so no trailing-space lines are rendered.
{{ if .kind }}
kind: {{ .kind | quote }}
{{ end }}
{{ if .group }}
group: {{ .group | quote }}
{{ end }}
charts/opencost/templates/httproute.yaml:123
- The
{{ if .kind }}/{{ end }}and{{ if .group }}/{{ end }}blocks don’t use whitespace-trimming ({{- ... }}), which will emit whitespace-only lines in the rendered manifest. With the repo’s yamllint config enablingtrailing-spaces, these space-only lines can causect lintto fail. Use trimmed control tags ({{- if ... }}/{{- end }}) consistent with the existingnamespace/sectionNameblocks so no trailing-space lines are rendered.
{{ if .kind }}
kind: {{ .kind | quote }}
{{ end }}
{{ if .group }}
group: {{ .group | quote }}
{{ end }}
charts/opencost/values.yaml:456
- These commented examples show
kind/groupas empty strings, but the PR description says the defaults arekind: Gateway(and default group implied by Gateway API). Using""here is misleading for users copying the example; consider updating the commented example to show the actual defaults (or explicitly document that omitting the fields uses the Gateway API defaults).
# kind: ""
# group: ""
charts/opencost/values.yaml:759
- These commented examples show
kind/groupas empty strings, but the PR description says the defaults arekind: Gateway(and default group implied by Gateway API). Using""here is misleading for users copying the example; consider updating the commented example to show the actual defaults (or explicitly document that omitting the fields uses the Gateway API defaults).
# kind: ""
# group: ""
| {{ if .kind }} | ||
| kind: {{ .kind | quote }} | ||
| {{ end }} | ||
| {{ if .group }} | ||
| group: {{ .group | quote }} | ||
| {{ end }} |
There was a problem hiding this comment.
@pierre-jezegou Agree with this, we should whitespace trim similar to how the rest of the template does.
| # kind: "" | ||
| # group: "" |
There was a problem hiding this comment.
That default is not determined by this Helm chart. Closing comment.
There was a problem hiding this comment.
Although we shouldn't set the default ourselves in the values.yaml. A comment here may help users? @pierre-jezegou?
parentRefs:
- name: ""
# kind: "Gateway" # Default when omitted
# group: "gateway.networking.k8s.io" # Default when omitted
Add
kindandgrouptoHTTPRouteparentRefsRequired to support
ListenerSetas a parent resource forHTTPRoute, promoted to Standard channel in Gateway API v1.5See https://gateway-api.sigs.k8s.io/guides/listener-set/
Changes
kindandgroupfields inhttproute.yaml(ui, api, mcp)values.yaml(defaults tokind: Gateway)