clear stale policy status after the policy is orphaned #12883

alexliu541 · 2025-11-13T19:44:25Z

Description

When TrafficPolicy or HTTPListenerPolicy become orphan, ie all its TargetRefs are all invalid, it will not be picked up by translation phase due to its reverse lookup mechanism and thus will not receive any status update. This means any previous status message becomes stale and still remains on the policy's status. This PR resolves this problem by implementing the status collection approach discussed in the design doc linked below. Now the policy's status will be correctly cleared if the policy ever become orphaned.

Design doc: EP-11747

Fixes #11402

Changes

Add status collection for TrafficPolicy and HTTPListenerPolicy plugin, which will generate a marker if the CR has parent referring to kGateway resource in existing status
Add callback function in plugin fields
Fixes a problem where ancestor ref in status cannot be nil
Proxy syncer will process the markers and merge with translation report map
Add E2E test for orphaned policies status clear

Change Type

/kind fix

Changelog

Clear stale TrafficPolicy and HTTPListenerPolicy status after the policy has all invalid TargetRefs

shashankram · 2025-11-17T18:45:21Z

internal/kgateway/extensions2/plugins/httplistenerpolicy/httplistener_plugin.go

 		}

+		// Create status marker if existing status has kgateway controller
+		var statusMarker *struct{}


let's define this as a concrete type StatusMarker that can be reused across different files

Defined in #12852, will update this once the other merged

shashankram · 2025-11-17T18:47:43Z

pkg/pluginsdk/types.go

-	GlobalPolicies func(krt.HandlerContext) ir.PolicyIR
+	Policies krt.Collection[ir.PolicyWrapper]
+	// ProcessPolicyStatusMarkers add empty reports for policies to clear stale status
+	ProcessPolicyStatusMarkers func(krt.HandlerContext, *reports.ReportMap)


The name could be changed to something more expressive, such as AddStaleStatusMarker

This function is not to add the marker but the call back to process the markers. I think its important to specify its for policy in the name, so do you think changing this to ProcessPolicyStaleStatusMarkers is a bit too long?

On reading this further, it appears that the function simply creates an empty report for policies with status referencing our controller and relies on BuildPolicyStatus to remove the stale status. Is that correct? If not, where does the clearing of stale status happen?

ProcessPolicyStaleStatusMarker seems reasonable

Yes that is correct, empty policy report entry in reportmap means empty status, so status syncer essentially clears the statuses.
Will change to ProcessPolicyStaleStatusMarker

It would make sense to add a unit test for BuildPolicyStatus to ensure that an empty report does not add the status from currentStatus passed to it

Done and done

shashankram · 2025-11-17T18:49:15Z

test/e2e/features/cors/suite.go

 }
+
+// TestTrafficPolicyClearStaleStatus verifies that stale status is cleared when targetRef becomes invalid
+func (s *testingSuite) TestTrafficPolicyClearStaleStatus() {


Can we test these using translator tests or a new suite for statuses? This test is not specific to the Cors feature.

Agree, will move to a new suite

I actually think we should consider moving the e2e tests for stale policy and route status to a standalone status suite or to a "scenario" style e2e test.

Never mind, I see the updated structure. I think this makes sense as defined in this PR for the policy tests.

I still think we should reevaluate where the HTTPRoute stale status test lives, but unrelated to this PR.

Signed-off-by: Alexander Liu <[email protected]>

internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go

internal/kgateway/extensions2/plugins/backendtlspolicy/plugin.go

lgadban · 2025-11-23T17:25:12Z

test/e2e/features/cors/suite.go

 }
+
+// TestTrafficPolicyClearStaleStatus verifies that stale status is cleared when targetRef becomes invalid
+func (s *testingSuite) TestTrafficPolicyClearStaleStatus() {


Never mind, I see the updated structure. I think this makes sense as defined in this PR for the policy tests.

I still think we should reevaluate where the HTTPRoute stale status test lives, but unrelated to this PR.

internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go

pkg/reports/policy.go

internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go

lgadban · 2025-11-23T17:47:02Z

internal/kgateway/proxy_syncer/proxy_syncer.go

 		s.commonCols.Routes.ProcessHTTPRouteStatusMarkers(objStatus, merged)

+		for _, plugin := range s.plugins.ContributesPolicies {
+			if plugin.ProcessPolicyStaleStatusMarkers != nil && plugin.ProcessBackend == nil {


is this done to exclude backend policy plugins?

Yes, backend policy and traffic policies uses different status reporting queue

I have a small concern with this approach, if we have plugins that contribute both backends and policy will we potentially lose this functionality?

Wondering how we can make this more explicit.

One option would be to have an explicit processBackendPolicyMarkers vs. processPolicyMarkers.

What do you think? Is that overkill?

I think the functionality will be fine as long as we make sure to report / generate its status in either backend or general status report, and given its gonna contribute to backend most likely it will be reported as a part of backend status reporting, which is what I have here.

We might need to have explicit processBackendPolicyMarkers vs processPolicyMarkers and detailed if check in that case, but right now I didn't want to add more fields into the plugin unless unnecessary, so I went with a check here. Not entirely a fan of checking the ProcessBackend either so if you think have separate is better we can do that

I agree with your assessment.

Given it's not likely we'll hit this specific scenario anytime soon, I think we can move forward with this as-is.

Signed-off-by: Alexander Liu <[email protected]>

lgadban

Great work, thanks!

I'll approve with one minor question.
Let's chat tomorrow and either merge or follow up.

internal/kgateway/extensions2/plugins/backendconfigpolicy/plugin.go

lgadban · 2025-11-25T03:28:29Z

internal/kgateway/proxy_syncer/proxy_syncer.go

 		s.commonCols.Routes.ProcessHTTPRouteStatusMarkers(objStatus, merged)

+		for _, plugin := range s.plugins.ContributesPolicies {
+			if plugin.ProcessPolicyStaleStatusMarkers != nil && plugin.ProcessBackend == nil {


I have a small concern with this approach, if we have plugins that contribute both backends and policy will we potentially lose this functionality?

Wondering how we can make this more explicit.

One option would be to have an explicit processBackendPolicyMarkers vs. processPolicyMarkers.

What do you think? Is that overkill?

gateway-bot added kind/fix Categorizes issue or PR as related to a bug. release-note labels Nov 13, 2025

shashankram reviewed Nov 17, 2025

View reviewed changes

alexliu541 mentioned this pull request Nov 18, 2025

report policy status when targetrefs are invalid #12708

Closed

alexliu541 self-assigned this Nov 20, 2025

alexliu541 requested a review from lgadban November 20, 2025 16:35

alexliu541 added 5 commits November 21, 2025 09:51

clear stale policy status after the policy is orphaned

29f4143

Signed-off-by: Alexander Liu <[email protected]>

use setup option controller name instead of default

bdcefcf

Signed-off-by: Alexander Liu <[email protected]>

resolve comments

5e1cdb6

Signed-off-by: Alexander Liu <[email protected]>

backend policy stale status

b6b27ea

Signed-off-by: Alexander Liu <[email protected]>

use StatusMarker type and update tests

a0d74fe

Signed-off-by: Alexander Liu <[email protected]>

alexliu541 force-pushed the alexliu541/policystatus branch from 2b45988 to a0d74fe Compare November 21, 2025 18:11

lgadban reviewed Nov 23, 2025

View reviewed changes

alexliu541 added 2 commits November 24, 2025 16:50

resolve comments

58bf6cd

Signed-off-by: Alexander Liu <[email protected]>

fix verify

5b8199d

Signed-off-by: Alexander Liu <[email protected]>

lgadban self-requested a review November 25, 2025 02:57

lgadban approved these changes Nov 25, 2025

View reviewed changes

lgadban added this pull request to the merge queue Nov 26, 2025

Merged via the queue into kgateway-dev:main with commit 33d2bad Nov 26, 2025
30 checks passed

alexliu541 deleted the alexliu541/policystatus branch November 26, 2025 18:51

clear stale policy status after the policy is orphaned #12883

clear stale policy status after the policy is orphaned #12883

Uh oh!

Conversation

alexliu541 commented Nov 13, 2025

Description

Changes

Change Type

Changelog

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexliu541 Nov 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexliu541 Nov 17, 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!

Choose a reason for hiding this comment

Uh oh!

alexliu541 Nov 17, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

lgadban left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexliu541 Nov 18, 2025 •

edited

Loading

alexliu541 Nov 17, 2025 •

edited

Loading

alexliu541 Nov 17, 2025 •

edited

Loading