Skip to content

fix(controller): return Unimplemented from GetChangedTargetGraph stub (audit #5) - #253

Merged
sbalabanov merged 1 commit into
mainfrom
audit/fix-05-unimplemented-changedtargetgraph
Jul 30, 2026
Merged

fix(controller): return Unimplemented from GetChangedTargetGraph stub (audit #5)#253
sbalabanov merged 1 commit into
mainfrom
audit/fix-05-unimplemented-changedtargetgraph

Conversation

@sbalabanov

Copy link
Copy Markdown
Contributor

Summary

GetChangedTargetGraph was a silent stub that returned nil (empty success stream) while the proto contract promises a full streamed changed target graph. Metrics reported success, making it impossible to distinguish the stub from a real call.

  • Return a YARPC Unimplemented error classified as ErrorUser so clients get an explicit failure
  • Metrics lifecycle now reflects the failure: finish histogram tagged result=user, failure counter emitted with error_code=user
  • Proto RPC comment annotated with "NOT YET IMPLEMENTED"
  • Regenerated tangopbmock to include TangoServiceGetChangedTargetGraphYARPCServer
  • Added table-driven test asserting the YARPC error code and that no chunks are streamed
  • Updated existing metrics path-shape test to expect the new failure outcome

Test plan

  • go build ./... passes
  • go test ./controller/... passes (new + updated tests)
  • make gazelle BUILD files in sync

🤖 Generated with Claude Code

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Comment thread controller/getchangedtargetgraph.go Outdated
Comment thread controller/getchangedtargetgraph_test.go Outdated
The handler previously returned nil (success with an empty stream),
which silently lied to clients and reported success in metrics. Return
a YARPC Unimplemented error classified as a user error so clients get
an explicit failure, metrics reflect the real outcome (result=user),
and the failure counter is emitted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sbalabanov
sbalabanov force-pushed the audit/fix-05-unimplemented-changedtargetgraph branch from 4173271 to 5fc3a48 Compare July 30, 2026 03:33
@sbalabanov
sbalabanov marked this pull request as ready for review July 30, 2026 03:34
@sbalabanov
sbalabanov requested review from a team as code owners July 30, 2026 03:34
@sbalabanov
sbalabanov merged commit 5e02706 into main Jul 30, 2026
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants