Send Classify claims via the claims chain

Vladimir Glafirovrequested to merge
vg/fix-topology-classify-claims-chain into main

What does this MR do and why?

Every Topology Service Classify call from GitLab Shell currently fails:

rpc error: code = InvalidArgument desc = invalid type: "UNSPECIFIED"

Found while enabling topology_service for GitLab Shell on GitLab.com staging (gstg-cny) — 6/6 classify attempts failed, zero successes. Full investigation in #864 (closed).

The failure is silent: the resolver catches the error and falls back to the default host, so Git-over-SSH keeps working, pods stay healthy (1/1, 0 restarts) and nothing surfaces to users. Only the logs and gitlab_shell_topology_requests_total{status="fail"} reveal it. In effect the Topology Service integration is a no-op wherever it is enabled today.

Root cause

The Topology Service introduced an ordered fallback chain in c4299cc3 and then removed the legacy single-claim path in aba4a3b4 (2026-07-07):

message ClassifyRequest {
  ClassifyType type = 2;
  string value = 3;

  reserved 4;
  reserved "claim";      // <-- what GitLab Shell still populated

  repeated types.v1.Claim claims = 5;   // <-- replacement
}

Client.Classify still set the now-reserved field 4, so the server decoded an effectively empty request: claims empty → classification falls through to the type/value switch → type defaults to UNSPECIFIED → rejected by the default branch in internal/services/classify/classify.go.

This went unnoticed because go.mod pinned topology-service at v0.0.0-20260522095121 (2026-05-22), roughly six weeks before the removal. The generated stubs at that revision still exposed ClassifyRequest_Claim, so the code compiled cleanly against a server that no longer accepts it.

How

  1. Bump topology-service v0.0.0-20260522095121 → v0.0.0-20260827174901.
  2. Send the claim as a single-element claims chain:
req := &pb.ClassifyRequest{
    Claims: []*types_proto.Claim{claim},
}

GitLab Shell resolves one claim per lookup, so a one-element chain preserves current semantics. Using the chain to collapse the route → SSH-fingerprint → username lookups into a single round trip is deliberately left as a follow-up to keep this fix reviewable.

  1. Add MockClassifyServer.LastClaim() so assertions read mock.LastClaim() rather than indexing GetClaims()[0] in nine places.
  2. Add a regression test.

Regression test

Table-driven across all five claim constructors (route, ssh_key, ssh_fingerprint, project_id, username), asserting the wire contract the server actually relies on:

  • claims contains exactly the claim, unmodified (proto.Equal)
  • the chain stays within the server's 5-claim limit
  • type/value are left unset so the claims path takes precedence

Verified the test fails without the client change — reverting Classify makes every subtest fail with "[]" should have 1 item(s), but has 0.

This is the check that was missing: the old assertions went through mock.LastRequest.GetClaim(), which mirrored whatever the client sent, so they passed regardless of whether the server would accept the request.

Test plan

Check Result
go build ./... clean
go vet ./... clean
gofmt -l . clean
go test ./... 49 packages, 0 failures
make lint (golangci-lint) clean
go mod tidy only the intended module changed (go.sum ±2 lines)

End-to-end validation against staging is tracked in #864 (closed). The gstg-cny canary is already configured and enabled, so the fix can be confirmed there as soon as a build lands — expect the classify failed after retries warnings to stop and status="ok" requests to appear.

Notes for reviewers

  • Also affects main. The bug is in v14.56.1 and current main (14.57.0), so a version bump alone does not fix it.
  • Suggested follow-ups (out of scope here): use the multi-claim chain to cut round trips, and add a CI guard so the pinned topology-service revision cannot silently drift behind the deployed service. This class of skew is invisible at compile time and degrades to a silent fallback at runtime.
  • The fallback path that masked this is the same one behind INC-11437, which is worth keeping in mind when relying on "SSH still works" as a health signal.

Related

/cc @vyaklushin @tkhandelwal3

Merge request reports