Conversation
Add a default HTTPListenerPolicy targeting the default-gateway in the data-plane Helm chart. Enable it by default, gated on gatewayClassName being "kgateway". This allows workloads with Websocket endpoints to function correctly out of the box without requiring manual cluster-wide operations. Also, remove the manual HTTPListenerPolicy creation steps from the doclet and echo-websocket-service sample READMEs. Refs openchoreo#4068, openchoreo#3915 Signed-off-by: kavix <[email protected]>
📝 WalkthroughWalkthroughThis PR adds a Helm template that conditionally creates an HTTPListenerPolicy for WebSocket upgrade support on the default gateway, along with corresponding values.yaml and values.schema.json entries. Two sample READMEs are updated to remove now-unnecessary manual WebSocket-enabling steps and renumber subsequent steps. ChangesHTTPListenerPolicy chart support and sample doc updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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
🤖 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
`@install/helm/openchoreo-data-plane/templates/gateway/httplistenerpolicy.yaml`:
- Around line 1-19: The chart is still rendering the deprecated
HTTPListenerPolicy resource; update the gateway template to emit ListenerPolicy
instead. Keep the existing gating around .Values.gateway.enabled and
.Values.gateway.httpListenerPolicy.enabled, but change the manifest from
HTTPListenerPolicy to ListenerPolicy and move the websocket upgrade
configuration under ListenerPolicy.spec.default.httpSettings. Ensure the
targetRefs still point at the default Gateway and preserve the existing
labels/metadata structure in the gateway template.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 03edfca7-ad2a-433e-918e-16eee77d8a01
📒 Files selected for processing (5)
install/helm/openchoreo-data-plane/templates/gateway/httplistenerpolicy.yamlinstall/helm/openchoreo-data-plane/values.schema.jsoninstall/helm/openchoreo-data-plane/values.yamlsamples/from-image/doclet/README.mdsamples/from-image/echo-websocket-service/README.md
| {{- if and .Values.gateway.enabled (eq (.Values.gateway.gatewayClassName | default "kgateway") "kgateway") .Values.gateway.httpListenerPolicy.enabled }} | ||
| apiVersion: gateway.kgateway.dev/v1alpha1 | ||
| kind: HTTPListenerPolicy | ||
| metadata: | ||
| name: default-httplistenerpolicy | ||
| namespace: {{ .Release.Namespace }} | ||
| labels: | ||
| {{- include "openchoreo-data-plane.componentLabels" (dict "context" . "component" "gateway") | nindent 4 }} | ||
| spec: | ||
| # Target the default gateway created by this chart | ||
| targetRefs: | ||
| - group: gateway.networking.k8s.io | ||
| kind: Gateway | ||
| name: gateway-default | ||
| # Enable WebSocket upgrades by default on all listeners of the target Gateway | ||
| upgradeConfig: | ||
| enabledUpgrades: | ||
| - websocket | ||
| {{- end }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the kgateway version pinned by this chart to check HTTPListenerPolicy deprecation status
rg -n 'tag:|appVersion|kgateway' install/helm/openchoreo-data-plane/Chart.yaml install/helm/openchoreo-data-plane/values.yamlRepository: openchoreo/openchoreo
Length of output: 670
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the gateway/controller values block and any kgateway API references.
sed -n '1,120p' install/helm/openchoreo-data-plane/values.yaml
printf '\n---\n'
sed -n '200,260p' install/helm/openchoreo-data-plane/values.yaml
printf '\n---\n'
rg -n 'HTTPListenerPolicy|ListenerPolicy|kgateway|gatewayClassName|controller.image.tag' install/helm/openchoreo-data-planeRepository: openchoreo/openchoreo
Length of output: 7439
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Query upstream docs/changelog for HTTPListenerPolicy deprecation and ListenerPolicy replacement.
python3 - <<'PY'
import urllib.request, urllib.parse, ssl
queries = [
"kgateway HTTPListenerPolicy deprecated v2.2.0 ListenerPolicy httpSettings",
"site:kgateway.dev HTTPListenerPolicy deprecated ListenerPolicy",
]
for q in queries:
print("QUERY:", q)
PYRepository: openchoreo/openchoreo
Length of output: 310
🌐 Web query:
kgateway HTTPListenerPolicy deprecated ListenerPolicy v2.2.0 site:kgateway.dev OR site:docs.kgateway.dev OR site:github.com/kgateway-dev
💡 Result:
In Kgateway, the HTTPListenerPolicy resource is officially deprecated [1][2][3]. Users are instructed to migrate their configurations to the ListenerPolicy resource instead [1][2]. Specifically, configuration previously handled by HTTPListenerPolicy should now be managed using the httpSettings field within a ListenerPolicy [2][3]. This change was noted in the release notes for version 2.2.0, which formalised the deprecation of HTTPListenerPolicy [2].
Citations:
- 1: https://kgateway.dev/docs/envoy/2.2.x/about/policies/httplistenerpolicy/
- 2: https://kgateway.dev/docs/envoy/2.2.x/reference/release-notes/
- 3: https://kgateway.dev/docs/envoy/2.2.x/reference/api/
Migrate HTTPListenerPolicy to ListenerPolicy kgateway deprecates HTTPListenerPolicy in 2.2.0+, so this chart keeps emitting a deprecated API. Move the websocket upgrade config under ListenerPolicy.spec.default.httpSettings.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 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 `@install/helm/openchoreo-data-plane/templates/gateway/httplistenerpolicy.yaml`
around lines 1 - 19, The chart is still rendering the deprecated
HTTPListenerPolicy resource; update the gateway template to emit ListenerPolicy
instead. Keep the existing gating around .Values.gateway.enabled and
.Values.gateway.httpListenerPolicy.enabled, but change the manifest from
HTTPListenerPolicy to ListenerPolicy and move the websocket upgrade
configuration under ListenerPolicy.spec.default.httpSettings. Ensure the
targetRefs still point at the default Gateway and preserve the existing
labels/metadata structure in the gateway template.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Purpose
Websocketis a first-class workload endpoint type in OpenChoreo, but connections failed out of the box because KGateway (based on Envoy) disables WebSocket upgrades by default. Enabling upgrades requires anHTTPListenerPolicy(gateway.kgateway.dev/v1alpha1) on the Gateway listener.This PR deploys a default
HTTPListenerPolicyin the data-plane Helm chart so that WebSocket upgrades work out-of-the-box for all users without manual cluster-wide operations.Approach
install/helm/openchoreo-data-plane/templates/gateway/httplistenerpolicy.yamlwhich creates anHTTPListenerPolicytargeting the default gateway (gateway-default) with WebSocket upgrades enabled.gateway.enabledandgateway.gatewayClassName == "kgateway"to align with the KGateway-specific CRD availability and prevent installation failures on non-KGateway configurations.gateway.httpListenerPolicyconfiguration undergatewayinvalues.yamlandvalues.schema.jsonto allow users to toggle the policy.HTTPListenerPolicyfromsamples/from-image/doclet/README.mdandsamples/from-image/echo-websocket-service/README.md.Related Issues
Closes #4068
Refs #3915
Checklist
backport/<release-branch>label if this should be backported (e.g.,backport/release-v1.0)