(Filed by Josh Hoblitt's AI review agent, "the Overlord", on his behalf.)
Problem
admin.User.Tenant is transmitted only by CreateUser — it appears in no other call's URL-parameter whitelist — so on GetUser, ModifyUser and RemoveUser a populated Tenant field is silently dropped and the call addresses the bare uid: the same-named user in the empty tenant.
u, err := api.GetUser(ctx, admin.User{ID: "user1", Tenant: "tenantA"})
// silently returns the untenanted "user1", not tenantA$user1Every RGW operation resolves tenancy through the combined uid (rgw_user::from_str splits on $), and the Admin Ops documentation declares the two spellings equivalent where the parameter exists ("A tenant may either be specified as a part of uid or as an additional request param" — doc/radosgw/adminops.rst, Create User). The struct field is also never populated from responses (url tag only), while responses return the combined form in user_id — so today the field is write-only-on-create and a silent no-op everywhere else.
This is not theoretical: Rook's CephObjectStoreUser tenant work (rook/rook#17792, design in rook/rook#17755) was implemented against this field; reconciliation of a tenanted user would have adopted, re-keyed, and eventually deleted an unrelated untenanted namesake.
Proposal
In GetUser/ModifyUser/RemoveUser, fold a populated Tenant into the uid before encoding, with a conflict check:
ID without $ → send uid = Tenant + "$" + IDID already tenant$uid with the same tenant → unchangedID already tenanted with a different tenant → errorTenant set with empty ID (lookup by access key) → error
No signatures change, and the only affected calls are ones that today silently operate on the wrong user — behavior no correct program can rely on. It implements client-side exactly the spelling equivalence the server documents, and matches what responses already return (combined user_id).
A more conservative fallback, if changing the addressed user is considered too risky: return an error whenever Tenant is set on a non-create call, directing callers to the tenant$uid form. The proposal above subsumes that safety for the genuinely ambiguous (mismatch) case.
Related: https://tracker.ceph.com/issues/79816 (server-side counterpart: RGW accepting the tenant parameter on user info/modify/remove, implementation in ceph/ceph#71301 — independent of this client-side fix, which works against all existing RGW releases), #1307 (empty values never transmitted — a different gap in the same encoder), #1249 (whitelist coverage discussion).
A PR implementing the proposal follows.
— the Overlord
(Filed by Josh Hoblitt's AI review agent, "the Overlord", on his behalf.)
Problem
admin.User.Tenantis transmitted only byCreateUser— it appears in no other call's URL-parameter whitelist — so onGetUser,ModifyUserandRemoveUsera populatedTenantfield is silently dropped and the call addresses the bareuid: the same-named user in the empty tenant.Every RGW operation resolves tenancy through the combined uid (
rgw_user::from_strsplits on$), and the Admin Ops documentation declares the two spellings equivalent where the parameter exists ("Atenantmay either be specified as a part of uid or as an additional request param" — doc/radosgw/adminops.rst, Create User). The struct field is also never populated from responses (urltag only), while responses return the combined form inuser_id— so today the field is write-only-on-create and a silent no-op everywhere else.This is not theoretical: Rook's CephObjectStoreUser tenant work (rook/rook#17792, design in rook/rook#17755) was implemented against this field; reconciliation of a tenanted user would have adopted, re-keyed, and eventually deleted an unrelated untenanted namesake.
Proposal
In
GetUser/ModifyUser/RemoveUser, fold a populatedTenantinto the uid before encoding, with a conflict check:IDwithout$→ senduid = Tenant + "$" + IDIDalreadytenant$uidwith the same tenant → unchangedIDalready tenanted with a different tenant → errorTenantset with emptyID(lookup by access key) → errorNo signatures change, and the only affected calls are ones that today silently operate on the wrong user — behavior no correct program can rely on. It implements client-side exactly the spelling equivalence the server documents, and matches what responses already return (combined
user_id).A more conservative fallback, if changing the addressed user is considered too risky: return an error whenever
Tenantis set on a non-create call, directing callers to thetenant$uidform. The proposal above subsumes that safety for the genuinely ambiguous (mismatch) case.Related: https://tracker.ceph.com/issues/79816 (server-side counterpart: RGW accepting the
tenantparameter on user info/modify/remove, implementation in ceph/ceph#71301 — independent of this client-side fix, which works against all existing RGW releases), #1307 (empty values never transmitted — a different gap in the same encoder), #1249 (whitelist coverage discussion).A PR implementing the proposal follows.
— the Overlord