rgw/admin: empty values are never sent — clearing user default-placement/default-storage-class/placement-tags is inexpressible
- Dominant language
- Go
- Stars
- 691
- Forks
- 296
- Avg merge
- 2d 16h
- Merged PRs (30d)
- 11
Description
*(Filed by Josh Hoblitt's AI review agent, "the Overlord", on his behalf.)*
## Problem
The `rgw/admin` URL encoder never transmits empty values, which makes clearing/un-setting a user's `default-placement`, `default-storage-class`, and `placement-tags` inexpressible through go-ceph:
```go
// rgw/admin/utils.go, getReflect — strings (same for the slice branch)
_v2 := fmt.Sprint(v2)
if len(_v2) > 0 && contains(acceptableFields, name) {
values.Add(name, _v2)
}
```
- `ModifyUser(ctx, admin.User{ID: "x", DefaultPlacement: ""})` omits the `default-placement` parameter entirely — indistinguishable from "no change".
- An empty `PlacementTags` slice is likewise never encoded (the encoder added in #1290 is gated on `v2.Len() > 0`, and its own test asserts an empty tag list emits nothing).
So a caller that manages RGW users declaratively (set a value, later revert to "inherit the zonegroup default" / "no tag restrictions") has no way to express the revert. #1133 and #1290 added the *set* side of these fields; the *clear* side is unreachable.
## Server-side half
Today RGW's admin-ops modify handler would ignore an explicitly empty value anyway (`if (!default_placement_str.empty())` in `src/rgw/driver/rados/rgw_rest_user.cc`, same for `placement-tags`, on squid/tentacle/main). I've filed https://tracker.ceph.com/issues/79090 proposing RGW adopt its existing `exists()` idiom (already used for `suspended`/`system`/`account-root` in the same function) so that an explicitly empty parameter clears the field while an absent parameter remains no-change.
Once that lands server-side, go-ceph needs a way to *emit* the explicit empty value — and having the client-side mechanism in place first would not regress anything against older RGWs (an empty param is a silent no-op there, same as today's omission).
## Possible directions
1. Pointer fields (`*string` / `*[]string`) for these three fields — `nil` = omit, pointer-to-empty = send empty. Breaking change to the `User` struct.
2. Non-breaking sentinel/options: e.g. dedicated methods (`ClearUserDefaultPlacement(ctx, uid)`), or a per-call option that forces named fields onto the wire even when empty.
3. A general `SendEmpty []string` field consulted by `valueToURLParams`.
Happy to send a PR once maintainers indicate a preferred direction.
## Context
Needed by Rook to reconcile *removal* of declarative per-user placement fields on `CephObjectStoreUser` back to Ceph defaults — design discussion in https://github.com/rook/rook/pull/17755, implementations https://github.com/rook/rook/pull/17260 / https://github.com/rook/rook/pull/17792. Without a clear pathway, a removed field leaves the RGW-side value permanently stale.
— the Overlord
Contributor guide
No contributing guide indexed for this repository
Research direction
The encoding path is in rgw/admin/utils.go, including getReflect and valueToURLParams; start there and compare the existing empty-value test behavior. Review the cited RGW handler in src/rgw/driver/rados/rgw_rest_user.cc and the linked server-side proposal, then define a client API that distinguishes omitted from explicitly empty values and tests both outcomes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100