fix(gatewayapi): validate listener fields in IsHTTPRouteReady#1950
fix(gatewayapi): validate listener fields in IsHTTPRouteReady#1950vibhor-5 wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR updates ChangesHTTPRoute Readiness Consistency with Listener-Specific ParentRef Validation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
internal/gatewayapi/utils_test.go (1)
824-976: ⚡ Quick winAdd explicit negative cases for controller and gateway identity mismatches.
The new suite covers listener
SectionName/Portwell, but it doesn’t directly assert the new controller-name and gateway identity filters. Adding those two cases would harden regression protection for the core change.Suggested additional test cases
{ + name: "Not Ready - controller name mismatch", + httpRoute: &gatewayapiv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Status: gatewayapiv1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1.RouteStatus{ + Parents: []gatewayapiv1.RouteParentStatus{ + { + ControllerName: gatewayapiv1.GatewayController("other/controller"), + ParentRef: gatewayapiv1.ParentReference{ + Name: "my-gateway", + }, + Conditions: []metav1.Condition{{ + Type: string(gatewayapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }, + }, + }, + }, + }, + gateway: gateway, + listener: listenerHttp, + controllerName: controllerName, + expected: false, + }, + { + name: "Not Ready - gateway namespace mismatch", + httpRoute: &gatewayapiv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Status: gatewayapiv1.HTTPRouteStatus{ + RouteStatus: gatewayapiv1.RouteStatus{ + Parents: []gatewayapiv1.RouteParentStatus{ + { + ControllerName: controllerName, + ParentRef: gatewayapiv1.ParentReference{ + Name: "my-gateway", + Namespace: ptr.To(gatewayapiv1.Namespace("other-ns")), + }, + Conditions: []metav1.Condition{{ + Type: string(gatewayapiv1.RouteConditionAccepted), + Status: metav1.ConditionTrue, + }}, + }, + }, + }, + }, + }, + gateway: gateway, + listener: listenerHttp, + controllerName: controllerName, + expected: false, + },🤖 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 `@internal/gatewayapi/utils_test.go` around lines 824 - 976, Add two negative test cases to the existing testCases slice in utils_test.go to assert controller-name and gateway identity filters: (1) a case where the RouteParentStatus.ControllerName is set to a different controller (not equal to the local controllerName variable) and the ParentRef.Name matches "my-gateway" but expected is false; (2) a case where ControllerName is correct but ParentRef.Name is changed to a different gateway name (e.g., "other-gateway") with the same SectionName/Port as listenerHttp and expected is false; place these alongside the existing HTTPRoute entries so they exercise the same matching logic used by the functions that read HTTPRoute.Status.Parents (refer to the testCases slice, controllerName, gateway, listenerHttp, and ParentReference.ControllerName/Name fields).
🤖 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.
Nitpick comments:
In `@internal/gatewayapi/utils_test.go`:
- Around line 824-976: Add two negative test cases to the existing testCases
slice in utils_test.go to assert controller-name and gateway identity filters:
(1) a case where the RouteParentStatus.ControllerName is set to a different
controller (not equal to the local controllerName variable) and the
ParentRef.Name matches "my-gateway" but expected is false; (2) a case where
ControllerName is correct but ParentRef.Name is changed to a different gateway
name (e.g., "other-gateway") with the same SectionName/Port as listenerHttp and
expected is false; place these alongside the existing HTTPRoute entries so they
exercise the same matching logic used by the functions that read
HTTPRoute.Status.Parents (refer to the testCases slice, controllerName, gateway,
listenerHttp, and ParentReference.ControllerName/Name fields).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1b7584dc-f976-4bc4-ad28-9931bb69298b
📒 Files selected for processing (5)
internal/controller/auth_policy_status_updater.gointernal/controller/ratelimit_policy_status_updater.gointernal/controller/tokenratelimitpolicy_status_updater.gointernal/gatewayapi/utils.gointernal/gatewayapi/utils_test.go
Align IsHTTPRouteReady with IsGRPCRouteReady by checking SectionName and Port in ParentReferences. Update status updaters to pass listener. Closes Kuadrant#1868 Signed-off-by: vibhor kumar <vibhork1105@gmail.com>
vibhor-5
force-pushed
the
fix-1868-httproute-listener-validation
branch
from
a21b470 to
97d772f
Compare
1 participant
Align IsHTTPRouteReady with IsGRPCRouteReady by checking SectionName and Port in ParentReferences. Update status updaters to pass listener.
Closes #1868
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests