fix: harden cell fan-out placement routing (trail #794 findings)

eaa6b32→main·

Soph·1w ago·3 files·+58 added/-13 removed

Two low-severity review findings on the mirror fan-out:

• groupReposByCell identified the home placement by string-matching p.Cell == r.Cell to decide which placement inherits the top-level cluster slug. Switch to the contract-guaranteed RepoPlacement.Mirror flag (home = false), so the precise slug->catalog join still works if the index ever omits the top-level Cell alongside the placement array. (finding 019f40a4-e98)

• resolveCellBaseURLs' last-resort jurisdiction-default fallback can bind a group to a non-default cell within its jurisdiction, a silent mirror miss. Add a debug log when that fallback fires so the miss is diagnosable. (finding 019f40a4-fce)

Adds TestGroupReposByCell_PlacementSlugFromMirrorFlag covering an index entry with an empty top-level Cell whose home is identified by Mirror.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

01KX0FZ3BTJJ2M65QE1PF93H0DView transcript

Changes

3

• cmd/entire/cli

• Mcell_fanout.go+18/-8

• Mcell_fanout_test.go+36/-2

• Msearch_cmd_test.go+4/-3

91 unmodified lines

for _, r := range repos { if len(r.Placements) > 0 { homeCell := strings.ToLower(strings.TrimSpace(r.Cell)) for _, p := range r.Placements { // Placements don't carry a cluster slug; the top-level // slug applies only to the home placement. Pass it when // the placement's cell matches the top-level cell (home), // empty otherwise — resolveCellBaseURLs falls back to // jurisdiction matching for groups without a slug. // Placements don't carry a cluster slug; the top-level slug // applies only to the home placement. RepoPlacement.Mirror is // the contract-guaranteed home(false)/mirror(true) marker, so // assign the slug to the home placement and leave mirrors // slugless — resolveCellBaseURLs falls back to cell/jurisdiction // matching for groups without a slug. (Keying off Mirror rather // than p.Cell == r.Cell means the join still works if the index // omits the top-level Cell alongside the placement array.) slug := "" if strings.ToLower(strings.TrimSpace(p.Cell)) == homeCell { if !p.Mirror { slug = r.ClusterSlug } addToGroup(p.ID, p.Cell, p.Jurisdiction, slug) } } } }

// Last resort: jurisdiction-level fallback using the default // cluster. Less precise, but still routes to the right // jurisdiction when the cell name doesn't appear in any URL. cl, ok = byJurisdiction[cells[i].jurisdiction] if cl, ok = byJurisdiction[cells[i].jurisdiction]; ok { // This binds the group to the jurisdiction's DEFAULT cluster, // which may not be the cell hosting this placement's repo. If // the placement lives in a non-default cell of the jurisdiction // the query can hit a cell that returns nothing — a silent // mirror miss. Log it so such a miss is diagnosable. logging.Debug(ctx, "cell fan-out: jurisdiction-default fallback used (cell name not in any catalog URL); may mis-route within jurisdiction", "cell", cells[i].cell, "jurisdiction", cells[i].jurisdiction, "resolved_cluster", cl.Slug) }

if !ok { logging.Debug(ctx, "cell fan-out: cluster not in catalog, using jurisdiction routing",

Mcmd/entire/cli/cell_fanout.go+18/-8

46 unmodified lines

if got := strings.Join(us.repoIDs, ","); got != "01B,01C" {
    t.Fatalf("us repoIDs = %q, want 01B,01C", got)
}
if us.clusterSlug != "us-prod" || us.jurisdiction != "us" {
    if us.clusterSlug != testClusterSlugUS || us.jurisdiction != "us" {
        t.Fatalf("us group coordinates = %+v, want us-prod/us", us)
    }
}

// TestGroupReposByCell_PlacementSlugFromMirrorFlag verifies the home
// placement's cluster slug is assigned via RepoPlacement.Mirror rather than by
// string-matching the top-level Cell. When the index omits the top-level Cell
// alongside the placement array, the string-match would find no home and drop
// every group to the fuzzier fallback; keying off Mirror keeps the precise
// slug->catalog join.
func TestGroupReposByCell_PlacementSlugFromMirrorFlag(t *testing.T) {
    t.Parallel()
    repos := []coreapi.RepoIndexEntry{
        {
            // Top-level Cell intentionally empty; the home placement is
            // identified by Mirror=false, not by matching the top-level Cell.
            ID: "01US", ClusterSlug: "us-prod", Jurisdiction: "us",
            Placements: []coreapi.RepoPlacement{
                {ID: "01US", Cell: "aws-us-east-2", Jurisdiction: "us", Mirror: false},
                {ID: "01EU", Cell: "aws-eu-central-1", Jurisdiction: "eu", Mirror: true},
            },
        },
    }
    cells := groupReposByCell(repos)
    if len(cells) != 2 {
        t.Fatalf("groups = %d, want 2: %+v", len(cells), cells)
    }
    // Sorted: aws-eu-central-1 < aws-us-east-2.
    eu := cells[0]
    us := cells[1]
    if us.cell != "aws-us-east-2" || us.clusterSlug != testClusterSlugUS {
        t.Fatalf("home group = %+v, want cell aws-us-east-2 with slug us-prod", us)
    }
    if eu.cell != "aws-eu-central-1" || eu.clusterSlug != "" {
        t.Fatalf("mirror group = %+v, want cell aws-eu-central-1 with empty slug", eu)
    }
}

// TestResolveCellBaseURLs_RefusesBaseURLWithoutJurisdiction pins the guard: a
// concrete baseURL is only usable together with the jurisdiction its token
// must be minted for; a catalog row with no jurisdiction leaves the group on