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

[jhoblitt]

(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:

// 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 rgw/admin: support placement-tags for user creation/modification #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 rook/rook#17755, implementations rook/rook#17260 / rook/rook#17792. Without a clear pathway, a removed field leaves the RGW-side value permanently stale.

— the Overlord

[anoopcs9]

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).

I would rather wait for the server-side implementation to land in at least the main branch.

[phlogistonjohn]

Without thinking too hard, my gut preferences would be: 2 - most preferable, 3 - middle, 1 - least preferable.

The mention of a sentinel value also made me think that if there was a special string you never wanted to sent the server we could use that as a way to indicate we want to retain the key plus an empty string value. Here's my dumb little experiment:

package main

import "fmt"

var EMPTY = string([]byte{0})

type Data struct {
	Foo string
	Bar string
	Baz string
}

func p(s string) {
	fmt.Printf("[%s]", s)
	if s == "" {
		fmt.Printf(" empty")
	} else if s == EMPTY {
		fmt.Printf(" EMPTY!")
	}
	fmt.Printf("\n")
}

func x(d Data) {
	p(d.Foo)
	p(d.Bar)
	p(d.Baz)
}

func main() {
	x(Data{"wow", EMPTY, ""})
}

Maybe that's too hacky... I dunno just a brainwave I had while replying :-)

[jhoblitt]

I think it would be convenient to have accessors/mutators for each independent value regardless. I am not completely opposed to option 1 as something like a User2 that exists in parallel with the current type.