chore: improve HTTPRoute status errors by zhaohuabing · Pull Request #5747 · envoyproxy/gateway

zhaohuabing · 2025-04-17T00:54:23Z

This PR includes the following improvements:

Fixes tests inputs in the Gateway API translator tests. Some tests were missing services and endpointSlices resources, which led to 500 errors in the output files. This PR adds the missing services and endpointSlices to ensure the expected behavior.
Improve the HTTPRoute status: Translation errors of all the HTTPRouteRules are now surfaced to the HTTPRoute status. This makes it easier for users to find out which parts of their config are invalid.
Consolidate all the HTTPRoute translation error handling in a single location (processHTTPRouteParentRefs) rather than updating status throughout the code. This simplifies the HTTPRoute translation logic and allows us to aggregate all errors into a single status message for better clarity. This is not totally done yet, will clean them up in a follow-up PR after handling TCP/TLS/UDP route statuses.

To keep the scope managed and make this easier to review, this PR only modifies the HTTPRoute. If this makes sense, I'll raise a follow-up PR to also clean up the status for other routes.

codecov · 2025-04-17T01:16:38Z

Codecov Report

Attention: Patch coverage is 62.03390% with 112 lines in your changes missing coverage. Please review.

Project coverage is 65.15%. Comparing base (096cb8d) to head (19fdade).
Report is 33 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/gatewayapi/status/error.go	0.00%	88 Missing ⚠️
internal/gatewayapi/route.go	86.73%	12 Missing and 1 partial ⚠️
internal/gatewayapi/validate.go	92.50%	5 Missing and 1 partial ⚠️
internal/gatewayapi/backend.go	80.95%	4 Missing ⚠️
internal/gatewayapi/listener.go	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5747      +/-   ##
==========================================
- Coverage   65.19%   65.15%   -0.05%     
==========================================
  Files         214      217       +3     
  Lines       34321    34865     +544     
==========================================
+ Hits        22377    22717     +340     
- Misses      10591    10753     +162     
- Partials     1353     1395      +42

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

...gatewayapi/testdata/gateway-with-listener-with-udproute-with-mismatch-port-protocol.out.yaml

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

internal/gatewayapi/status/error.go

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml

arkodg · 2025-04-17T21:13:19Z

internal/gatewayapi/testdata/httproute-with-multiple-gateways-from-different-ns.out.yaml

        mergeSlashes: true
      port: 10080
      routes:
-      - directResponse:


doesnt look right

This change is actually irrelevant to the status: I found that httproute-with-multiple-gateways-from-different-ns.in.yaml is invalid, missing the backend, so I added it.

The purpose of this test is to verify we have the right namespace for the parent Gateway in the route status, not 500 direct response, so it should have valid backends in it.

see: #4592

arkodg · 2025-04-17T21:33:01Z

@zhaohuabing can we try to make sure only the status messages are updated in the test file and not the reasons or conditions, we'll need to revisit the entire status spec again if changes are made In that area

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing · 2025-04-18T01:37:42Z

internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.out.yaml

+        message: 'Failed to process route rule 0 backendRef 0: no ca found in configmap
+          no-ca-cmap.'
+        reason: InvalidBackendTLS
+        status: "False"


We should set ResolvedResf to false here, since according to the Gateway API, "ResolvedRefs which is set to true when all references to other resources have been successfully resolved."

Surfacing the TLS-related failures to Route status will help users - it gives them a single place to understand why a Route isn't working, rather than having to dig through all the related resources.

Reference: https://gateway-api.sigs.k8s.io/geps/gep-1364/#all-conditions-positive

zhaohuabing · 2025-04-18T01:47:32Z

internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid.out.yaml

+          secret for client auth: envoy-gateway-system/client-auth-not-found specified
+          in EnvoyProxy envoy-gateway-system/test.'
+        reason: InvalidBackendTLS
+        status: "False"


zhaohuabing · 2025-04-18T01:48:41Z

...gatewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml

      - lastTransitionTime: null
-        message: 'backend Service validation failed: TCP Port 8081 not found on Service
-          default/service-1'
+        message: TCP Port 8081 not found on Service default/service-1


I plan to handle this in a follow-up PR, and use the same message format as HTTPRoute.

...testdata/httproute-attaching-to-listener-with-backend-backendref-mixed-address-type.out.yaml

zhaohuabing · 2025-04-18T02:52:29Z

internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml

+        message: Resolved all the Object references for the Route
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs


The behavior follows the existing logic: Even though the Accepted condition is false, the ResolveRefs can be true if all the Backends are successfully resolved.

Existing tests:

gateway/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-secret-in-other-namespace.out.yaml

Lines 65 to 76 in 652b6b4

- conditions:

- lastTransitionTime: null

message: There are no ready listeners for this parent ref

reason: NoReadyListeners

status: "False"

type: Accepted

- lastTransitionTime: null

message: Resolved all the Object references for the Route

reason: ResolvedRefs

status: "True"

type: ResolvedRefs

controllerName: gateway.envoyproxy.io/gatewayclass-controller

zhaohuabing · 2025-04-18T03:28:35Z

internal/gatewayapi/testdata/envoyproxy-tls-settings-invalid-ns.out.yaml

+          Secret is not located in the same namespace as Envoyproxy. Secret namespace:
+          envoy-gateway-user-ns does not match Envoyproxy namespace: envoy-gateway-system.'
+        reason: InvalidBackendTLS
+        status: "False"


zhaohuabing · 2025-04-18T03:31:23Z

...ute-attaching-to-listener-with-multiple-serviceimport-backendrefs-diff-address-type.out.yaml

        type: Accepted
      - lastTransitionTime: null
-        message: Mixed endpointslice address type between backendRefs is not supported
-        reason: ResolvedRefs


zhaohuabing · 2025-04-18T03:31:38Z

...ta/httproute-attaching-to-listener-with-serviceimport-backendref-mixed-address-type.out.yaml

        type: Accepted
      - lastTransitionTime: null
-        message: mixed endpointslice address type for the same backendRef is not supported
-        reason: ResolvedRefs


Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing · 2025-04-18T03:42:26Z

internal/gatewayapi/testdata/httproute-with-invalid-backend-ref-invalid-mixed-kind.out.yaml

        type: Accepted
      - lastTransitionTime: null
-        message: Mixed endpointslice address type between backendRefs is not supported
-        reason: ResolvedRefs


zhaohuabing · 2025-04-18T03:43:07Z

internal/gatewayapi/testdata/httproute-with-invalid-regex.out.yaml

        reason: UnsupportedValue
        status: "False"
        type: Accepted
+      - lastTransitionTime: null


Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing · 2025-04-18T04:05:51Z

@zhaohuabing can we try to make sure only the status messages are updated in the test file and not the reasons or conditions, we'll need to revisit the entire status spec again if changes are made In that area

@arkodg I’ve reverted the standard status reasons back to their original values. A few non-standard ones have been updated to better reflect the specific errors — but if you prefer keeping the older ones for consistency, I’m happy to revert those as well.

arkodg · 2025-04-23T19:18:08Z

internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml

                name: policy-btls/envoy-gateway-ca
              sni: example.com
            weight: 1
-        directResponse:


why did this change ?

Fixes tests inputs in the Gateway API translator tests. Some tests were missing services and endpointSlices resources, which led to 500 errors in the output files. This PR adds the missing services and endpointSlices to ensure the expected behavior.

arkodg · 2025-04-23T19:19:31Z

internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions.out.yaml

        type: Accepted
      - lastTransitionTime: null
-        message: Service envoy-gateway/service-1 not found
-        reason: BackendNotFound


should be BackendNotFound https://github.com/kubernetes-sigs/gateway-api/blob/efec20dbaa16f49f7227c576eb332c539715c6d3/apis/v1beta1/shared_types.go#L143

looks like you are intentionally cleaning up / fixing these tests ? are these tests for the happy path or error path ?

is this same as #5747 (comment) ?

Yes, I fixed these tests, which are missing services and endpoints in their inputs.

Fixes tests inputs in the Gateway API translator tests. Some tests were missing services and endpointSlices resources, which led to 500 errors in the output files. This PR adds the missing services and endpointSlices to ensure the expected behavior.

arkodg · 2025-04-23T19:19:47Z

internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions.out.yaml

        mergeSlashes: true
      port: 10080
      routes:
-      - directResponse:


why did these get removed ?

Fixes tests inputs in the Gateway API translator tests. Some tests were missing services and endpointSlices resources, which led to 500 errors in the output files. This PR adds the missing services and endpointSlices to ensure the expected behavior.

arkodg

LGTM thanks

guydc

LGTM

zhaohuabing requested a review from a team as a code owner April 17, 2025 00:54

zhaohuabing marked this pull request as draft April 17, 2025 00:54

zhaohuabing force-pushed the status-errors branch from 28a1b84 to 1dd25b5 Compare April 17, 2025 01:01

zhaohuabing changed the title ~~Status errors~~ chore: improve HTTPRoute status errors Apr 17, 2025

zhaohuabing force-pushed the status-errors branch 3 times, most recently from 18a6b48 to 4267457 Compare April 17, 2025 01:07

zhaohuabing marked this pull request as ready for review April 17, 2025 01:08

zhaohuabing mentioned this pull request Apr 17, 2025

feat: impl dynamic resolver backend type #5725

Merged

zhaohuabing requested a review from arkodg April 17, 2025 01:17

zhaohuabing force-pushed the status-errors branch 3 times, most recently from 07edb2a to 536d226 Compare April 17, 2025 01:28

zhaohuabing commented Apr 17, 2025

View reviewed changes

...gatewayapi/testdata/gateway-with-listener-with-udproute-with-mismatch-port-protocol.out.yaml Outdated Show resolved Hide resolved

zhaohuabing force-pushed the status-errors branch from 536d226 to 32c8846 Compare April 17, 2025 02:17

zhaohuabing added 2 commits April 17, 2025 03:13

Improve the HTTPRoute status errors.

6587f05

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

move status error to the status package

f37c41c

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing force-pushed the status-errors branch from cd532e8 to f37c41c Compare April 17, 2025 03:28

shawnh2 reviewed Apr 17, 2025

View reviewed changes

internal/gatewayapi/status/error.go Show resolved Hide resolved

fix lint

fdf6420

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing requested a review from shawnh2 April 17, 2025 04:36

arkodg reviewed Apr 17, 2025

View reviewed changes

internal/gatewayapi/testdata/httproute-with-single-rule-with-multiple-rules.out.yaml Show resolved Hide resolved

arkodg reviewed Apr 17, 2025

View reviewed changes

fix accepted status

82b14ab

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing commented Apr 18, 2025

View reviewed changes

...testdata/httproute-attaching-to-listener-with-backend-backendref-mixed-address-type.out.yaml Outdated Show resolved Hide resolved

zhaohuabing commented Apr 18, 2025

View reviewed changes

zhaohuabing requested a review from arkodg April 18, 2025 03:13

zhaohuabing commented Apr 18, 2025

View reviewed changes

fix accepted reason

00e254c

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing commented Apr 18, 2025

View reviewed changes

zhaohuabing added 2 commits April 18, 2025 03:53

fix accepted reason

08f5a0a

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

fix lint

39a29a6

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>

zhaohuabing requested review from a team April 18, 2025 07:29

zhaohuabing added 3 commits April 21, 2025 01:22

Merge remote-tracking branch 'origin/main' into status-errors

4c211f9

Merge remote-tracking branch 'origin/main' into status-errors

ecc9204

Merge remote-tracking branch 'origin/main' into status-errors

19fdade

arkodg reviewed Apr 23, 2025

View reviewed changes

arkodg approved these changes Apr 23, 2025

View reviewed changes

arkodg requested review from a team April 23, 2025 19:27

guydc approved these changes Apr 23, 2025

View reviewed changes

zhaohuabing merged commit a80f5f8 into envoyproxy:main Apr 24, 2025
25 checks passed

zhaohuabing deleted the status-errors branch April 24, 2025 00:26

zhaohuabing mentioned this pull request Apr 24, 2025

chore: improve xRoute status errors #5803

Merged

zhaohuabing mentioned this pull request May 8, 2025

fix: httproute status.parents is not cleaned up #5679

Closed

arkodg mentioned this pull request Nov 10, 2025

1 Invalid BackendRef causes 500 for all requests #7483

Closed

zirain mentioned this pull request Feb 12, 2026

Mixed IP and UDS endpoints rejected #8229

Open

	- conditions:
	- lastTransitionTime: null
	message: There are no ready listeners for this parent ref
	reason: NoReadyListeners
	status: "False"
	type: Accepted
	- lastTransitionTime: null
	message: Resolved all the Object references for the Route
	reason: ResolvedRefs
	status: "True"
	type: ResolvedRefs
	controllerName: gateway.envoyproxy.io/gatewayclass-controller

Conversation

zhaohuabing commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg commented Apr 17, 2025

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing commented Apr 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

guydc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

zhaohuabing commented Apr 17, 2025 •

edited

Loading

codecov bot commented Apr 17, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing Apr 18, 2025 •

edited

Loading

zhaohuabing commented Apr 18, 2025 •

edited

Loading