fix(status): omit waypoint in service dump when none is configured by AswaniSahoo · Pull Request #1915 · kmesh-net/kmesh

AswaniSahoo · 2026-08-13T02:33:48Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

ConvertService always allocates a Waypoint, even when the service does not have one. The LoadBalancer branch three lines below already guards for the empty case, and ConvertWorkload uses omitempty for the same concept. So a service with neither currently dumps as:

"loadBalancer": null,
"waypoint": { "destination": "" }

Two optional fields, two different shapes. A client reading the dump cannot tell "no waypoint" from "a waypoint whose destination happens to be empty" without special-casing the empty string.

This guards the waypoint the same way s.LoadBalancing is guarded immediately below, so it serialises as null.

Also adds pkg/status/api_test.go, which the package did not have. It covers the three waypoint cases (absent, by address, by hostname), the resulting JSON shape, and the vips and loadBalancer conversion.

How I found it:

I was building an MCP client against /debug/config_dump/dual-engine and shaping test fixtures from the marshalled output. I wanted to list the services routed through a waypoint, checked for null, and got nothing back because every service had a waypoint object. Traced it to ConvertService.

Which issue(s) this PR fixes:

None, found while reading the code.

Special notes for your reviewer:

I could not run go test ./pkg/status/... locally. The package reaches the bpf2go packages that //go:embed the compiled .o artifacts, and those are not in the tree, so it does not build outside a full eBPF build. I checked the test's types, enum names and assertions against the real workloadapi types in a scratch harness and am relying on CI for the real run. Flagging it rather than implying I ran it.

If you would rather keep emitting the object for compatibility, I am happy to drop the api.go change and keep the tests documenting the current behaviour instead.

Does this PR introduce a user-facing change?:

The workload config dump now reports `"waypoint": null` for services that have no waypoint, instead of an object with an empty destination.

kmesh-bot · 2026-08-13T02:33:55Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

pkg/status/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kmesh-bot · 2026-08-13T02:34:00Z

Welcome @AswaniSahoo! It looks like this is your first PR to kmesh-net/kmesh 🎉

Copilot

Pull request overview

Fixes the status API service conversion so that services with no configured waypoint marshal "waypoint": null (instead of a waypoint object with an empty destination), aligning the JSON shape with how loadBalancer is represented and avoiding ambiguous client-side interpretation.

Changes:

Update ConvertService to only allocate/set Service.Waypoint when a non-empty waypoint destination is present.
Add pkg/status/api_test.go to cover waypoint conversion cases and verify the resulting marshaled JSON shape (plus basic vips/loadBalancer conversion).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/status/api.go	Conditionally sets `Waypoint` only when a real destination is present, so JSON shows `null` when absent.
pkg/status/api_test.go	Adds unit tests for waypoint conversion and JSON output shape for absent vs present waypoint.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

AswaniSahoo · 2026-08-15T11:15:53Z

+	assert.Contains(t, decoded, "waypoint")
+	assert.Nil(t, decoded["waypoint"])
+	assert.Nil(t, decoded["loadBalancer"])


Good catch. assert.Nil on a missing key passes for the wrong reason, so that test would have stayed green even if loadBalancer had been dropped entirely. Added the assert.Contains before it so presence is checked first, the same way the waypoint assertion above already does. Pushed in 6e45bea.

codecov · 2026-08-13T02:58:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.68%. Comparing base (c6357aa) to head (21f90c1).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Coverage Δ
pkg/status/api.go	`91.15% <100.00%> (+6.22%)`	⬆️

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec501a9...21f90c1. Read the comment docs.

🚀 New features to boost your workflow:

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

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

AswaniSahoo · 2026-08-24T16:43:58Z

Saw this on a live cluster today, not just from reading ConvertService.

kind with istio in ambient mode and kmesh in dual-engine, using the published image. Three services, none of them with a waypoint or a load balancer configured:

istio-system/istiod    "waypoint": {"destination": ""}   "loadBalancer": null
kube-system/kube-dns   "waypoint": {"destination": ""}   "loadBalancer": null
default/kubernetes     "waypoint": {"destination": ""}   "loadBalancer": null

So every service on a default install carries the empty waypoint object, and a client still has to check for the empty string to know there is no waypoint.

Happy to share the setup if that is useful.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := ConvertService(&workloadapi.Service{
+				Name:      "productpage",
+				Namespace: "default",
+				Hostname:  "productpage.default.svc.cluster.local",
+				Waypoint:  tc.waypoint,
+			})
+			assert.Equal(t, tc.expect, got.Waypoint)
+		})
+	}


+// service without load balancing already serialises loadBalancer as null.
+// Otherwise a consumer cannot tell "no waypoint" apart from "waypoint whose
+// destination happens to be empty".
+func TestConvertServiceOmitsEmptyWaypointInJSON(t *testing.T) {


+	data, err := json.Marshal(got)
+	assert.NoError(t, err)
+
+	var decoded map[string]interface{}
+	assert.NoError(t, json.Unmarshal(data, &decoded))


Signed-off-by: AswaniSahoo <aswanisahoo1012@gmail.com>

…it is null Signed-off-by: AswaniSahoo <aswanisahoo1012@gmail.com>

AswaniSahoo · 2026-09-06T02:53:55Z

@hzxuzhonghu @Okabe-Rintarou-0

Could you please take a look at this PR when you get a chance?

Regarding the single failing CI check (E2E Test(istio 1.28)): the failure was TestMixNsAndServiceWaypoint timing out on pod startup (container not found ("sleep")), while the same test passed cleanly on Istio 1.26, 1.27, and all IPv6 matrix runs. This PR only touches pkg/status/api.go and its unit tests to format absent waypoints as null, so the failure is unrelated.

Could you please trigger a /retest and review when convenient? Thanks!

AswaniSahoo · 2026-09-06T02:56:29Z

@hzxuzhonghu @Okabe-Rintarou-0

Could you please take a look at this PR when you get a chance?

Regarding the single failing CI check (E2E Test(istio 1.28)): the failure was TestMixNsAndServiceWaypoint timing out on pod startup (container not found ("sleep")), while the same test passed cleanly on Istio 1.26, 1.27, and all IPv6 matrix runs. This PR only touches pkg/status/api.go and its unit tests to format absent waypoints as null, so the failure is unrelated.

Could you please trigger a /retest and review when convenient? Thanks!

Copilot AI lite review requested due to automatic review settings August 13, 2026 02:33

kmesh-bot added the kind/bug Something isn't working label Aug 13, 2026

kmesh-bot requested review from Okabe-Rintarou-0 and hzxuzhonghu August 13, 2026 02:33

kmesh-bot added the size/L label Aug 13, 2026

Copilot started reviewing on behalf of AswaniSahoo August 13, 2026 02:34 View session

Copilot AI reviewed Aug 13, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 15, 2026 11:15

Copilot started reviewing on behalf of AswaniSahoo August 15, 2026 11:15 View session

Copilot AI reviewed Aug 15, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 24, 2026 16:44

kmesh-bot added the do-not-merge/contains-merge-commits label Aug 24, 2026

Copilot AI reviewed Aug 24, 2026

View reviewed changes

AswaniSahoo added 2 commits August 24, 2026 23:00

fix(status): omit waypoint in service dump when none is configured

c4bba5d

Signed-off-by: AswaniSahoo <aswanisahoo1012@gmail.com>

test(status): assert the loadBalancer key is present before checking …

21f90c1

…it is null Signed-off-by: AswaniSahoo <aswanisahoo1012@gmail.com>

AswaniSahoo force-pushed the fix/status-waypoint-nil branch from 5d3cd67 to 21f90c1 Compare August 24, 2026 17:34

kmesh-bot removed the do-not-merge/contains-merge-commits label Aug 24, 2026

Copilot started reviewing on behalf of AswaniSahoo August 24, 2026 17:47 View session

Navigation Menu

Uh oh!

fix(status): omit waypoint in service dump when none is configured - #1915

fix(status): omit waypoint in service dump when none is configured#1915
AswaniSahoo wants to merge 2 commits into
kmesh-net:mainfrom
AswaniSahoo:fix/status-waypoint-nil

AswaniSahoo commented Aug 13, 2026

Uh oh!

kmesh-bot commented Aug 13, 2026

Uh oh!

kmesh-bot commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

AswaniSahoo Aug 15, 2026

Uh oh!

codecov Bot commented Aug 13, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

AswaniSahoo commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

AswaniSahoo commented Sep 6, 2026

Uh oh!

AswaniSahoo commented Sep 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

AswaniSahoo commented Aug 13, 2026

Uh oh!

kmesh-bot commented Aug 13, 2026

Uh oh!

kmesh-bot commented Aug 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

AswaniSahoo Aug 15, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AswaniSahoo commented Aug 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AswaniSahoo commented Sep 6, 2026

Uh oh!

AswaniSahoo commented Sep 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Aug 13, 2026 •

edited

Loading