cli/auth: address review nits on entire-api cell routing · Entire
cli/auth: address review nits on entire-api cell routing
8a978d3·
Soph·2w ago·2 files·+30 added/-6 removed
Follow-ups from the 715 review:
- Case-fold the home_jurisdiction claim from the login JWT before the strict [a-z0-9-] label check. The repo-target path already lowercases; the JWT path did not, so an uppercase claim would hard-fail instead of routing.
- List the cluster catalog in the home-jurisdiction fallback against the discovered login core (selected.CoreURL) rather than the templated jurisdiction core — the login JWT is signed by the former, so in a multi-core setup the templated core could reject it. The exchange core is unchanged.
- Add the ErrNoCellForJurisdiction sentinel and wrap the "no cell / no apiUrl" errors with it, so callers with a data-API fallback (activity, recap next) can degrade instead of failing when a region has no cell.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Sessions
e94d8b46f42cView transcript
Changes
2
cmd/entire/cli/auth
Mcell_data_api.go+24/-6
Mcell_data_api_test.go+6
156 unmodified lines
157
158
159
160
160
161
162
163
164
165
166
167
25 unmodified lines
193
194
195
196
197
198
199
200
201
202
203
4 unmodified lines
208
209
210
202
203
211
212
213
214
215
216
217
1 unmodified line
219
220
221
211
222
223
224
225
148 unmodified lines
374
375
376
377
378
379
380
381
382
383
384
385
386
40 unmodified lines
427
428
429
412
430
431
414
432
433
434
435
156 unmodified lines
return nil, err
}
cellBaseURL, err := resolveTargetCellBaseURL(ctx, target, dataOrigin, jurisdiction, coreURL, loginJWT, httpClient)
// The home-jurisdiction fallback lists the cluster catalog with loginJWT,
// which is signed by selected.CoreURL — so list there, not at the templated
// jurisdiction core (coreURL), which in a multi-core setup could differ and
// reject the token. coreURL still governs the token exchange below.
cellBaseURL, err := resolveTargetCellBaseURL(ctx, target, dataOrigin, jurisdiction, selected.CoreURL, loginJWT, httpClient)
if err != nil {
return nil, err
}
25 unmodified lines
return "", err
}
}
// Case-fold before the label check and any URL templating: the target path
// already lowercases (resolveExpertsCellTarget), but the JWT claim arrives
// verbatim, so an uppercase home_jurisdiction would otherwise hard-fail the
// strict \[a-z0-9-\] label check instead of routing.
jurisdiction = strings.ToLower(strings.TrimSpace(jurisdiction))
if jurisdiction == "" {
return "", errors.New("login token has no home_jurisdiction claim; cannot route to entire-api cell")
}
4 unmodified lines
}
// resolveTargetCellBaseURL decides which cell origin to dial. See
// NewEntireAPICellClient's precedence doc.
func resolveTargetCellBaseURL(ctx context.Context, target *CellTarget, dataOrigin, jurisdiction, coreURL, loginJWT string, httpClient *http.Client) (string, error) {
// NewEntireAPICellClient's precedence doc. listCoreURL is the core the
// home-jurisdiction fallback lists the cluster catalog against; it must be a
// core that accepts loginJWT (i.e. the discovered login core).
func resolveTargetCellBaseURL(ctx context.Context, target *CellTarget, dataOrigin, jurisdiction, listCoreURL, loginJWT string, httpClient *http.Client) (string, error) {
if target != nil && strings.TrimSpace(target.BaseURL) != "" {
return strings.TrimRight(target.BaseURL, "/"), nil
}
1 unmodified line
// Already a cell URL, or a loopback local-dev host: keep it verbatim.
return strings.TrimRight(dataOrigin, "/"), nil
}
return resolveCellAPIBaseURL(ctx, coreURL, loginJWT, jurisdiction, httpClient)
return resolveCellAPIBaseURL(ctx, listCoreURL, loginJWT, jurisdiction, httpClient)
}
// isBFFOrigin reports whether origin is a BFF / apex host that fronts multiple
148 unmodified lines
APIURL string `json:"apiUrl"`
}
// ErrNoCellForJurisdiction signals that the caller's home jurisdiction has no
// entire-api cell in the cluster catalog (or its row carries no apiUrl). It is
// not fatal: callers that also have a data-API path (e.g. activity/recap) treat
// it as "entire-api isn't serving this region yet" and fall back rather than
// failing the command. errors.Is unwraps it from the contextual message.
var ErrNoCellForJurisdiction = errors.New("no entire-api cell configured for jurisdiction")
// resolveCellAPIBaseURL is the home-jurisdiction fallback cell resolver: it
// lists the caller's clusters and picks the apiUrl for `jurisdiction` (default
// cluster first). It hand-parses GET /api/v1/clusters rather than reusing the
40 unmodified lines
if sawJurisdiction {
// A cluster row exists for the jurisdiction but carries no apiUrl —
// a schema/deploy problem, distinct from "no cell for jurisdiction".
return "", fmt.Errorf("cluster for jurisdiction %q advertises no apiUrl (entire-api cell not configured?)", jurisdiction)
return "", fmt.Errorf("%w %q: cluster advertises no apiUrl (entire-api cell not configured?)", ErrNoCellForJurisdiction, jurisdiction)
}
return "", fmt.Errorf("no entire-api cell configured for jurisdiction %q", jurisdiction)
return "", fmt.Errorf("%w %q", ErrNoCellForJurisdiction, jurisdiction)
}
chosen := matches[0]
for _, row := range matches {