table, server: report the origin-eq condition from OriginEq - #3616
Conversation
toStatementApi in internal/pkg/table checks that the statement has an origin-eq condition, but then switches on Actions.BgpActions.SetRouteOrigin to fill Conditions.Origin. ListPolicy therefore returns the origin of the set-route-origin action as the condition, and leaves the condition unset when the statement has no such action. The copy of toStatementApi in pkg/server, used by ListStatement, does not convert the origin-eq condition at all, so ListStatement never returns it. Read the condition from Conditions.BgpConditions.OriginEq in the first and add the same conversion to the second. A policy read back through either call and added again now keeps its origin condition.
|
The zapi-v3 failure on this pull request does not come from the change. Rather than ask you to look into it, I ran the job here: That matches what the diff does. It only changes how an existing policy condition is reported over the API: What I cannot reproduce here is CI timing, and a netns scenario on a loaded runner is exactly where test isolation tends to bite. So if a re-run of that job comes back green, this comment is nothing you need to act on — I just thought you should have the result rather than the question. Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5. |
|
Thanks! |
I was comparing the two copies of
toStatementApiand noticed that the origin-eq condition does not survive being listed back through the API.In
internal/pkg/table/policy.go, the function checksConditions.BgpConditions.OriginEq, but the switch that fillsConditions.OriginreadsActions.BgpActions.SetRouteOrigin. This has been the case since the origin predicates were added in #2816.ListPolicygoes through this copy, so it reports the set-route-origin action's origin as the condition. If the statement has no such action, the condition comes back unset. The copy inpkg/server/grpc_server.go, whichListStatementuses, has no conversion for the origin condition at all.I added
TestListPolicyOriginCondition, which adds a policy with two statements: one matches EGP and sets IGP, the other only matches INCOMPLETE. The test then reads both statements back throughListPolicyandListStatement. Before the change, it fails on all four checks:With the change, it passes, and
go test ./pkg/server/ ./internal/pkg/table/ ./pkg/config/...andgo veton the two touched packages pass too.The fix doesn't change any function signature or API message. Only the value that
ListPolicyandListStatementput inConditions.Originchanges, and it now matches whatAddPolicy/AddStatementwere given. That matters for anything that reads a policy back and adds it again: today, that round trip silently drops the origin condition or replaces it with the action's origin.Found by a defect-hunting pipeline I build and run (Dev-next-gen), using Claude Code with Anthropic's Claude Opus 5.