ceph / ceph/go-ceph

rgw/admin: empty values are never sent — clearing user default-placement/default-storage-class/placement-tags is inexpressible

Open
#1,307 3 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.