Hi all

This is a proposal for discussion, not a patch. The developer documentation asks that plans be discussed before starting work, so I'd like to check whether this would be acceptable before writing it.

GET /api2/json/admin/datastore/{store}/snapshots returns only the snapshots in the requested namespace, so a client wanting a datastore-wide view has to call /admin/datastore/{store}/namespace first and then issue one snapshots request per namespace. I ran into this writing a third-party monitoring client.

Tested against PBS 4.2:

/admin/datastore/pbs/snapshots -> 485 snapshots (root only) 

/admin/datastore/pbs/snapshots?ns=test -> 1 snapshot 

/admin/datastore/pbs/snapshots?max-depth=7 -> 400 parameter verification failed - 'max-depth': schema does not allow additional properties

Proposal: accept an optional max-depth using the existing NS_MAX_DEPTH_SCHEMA, defaulting to 0 so behaviour is unchanged for current callers.

This looks anticipated in the code. As of 5d95fc20e, src/api2/admin/datastore.rs lines 571, 575 and 583 each carry a "// FIXME: Recursion" on the group construction in list_snapshots_blocking, with a further one at 565 about filtering by owner before collecting. ListAccessibleBackupGroups already takes max_depth and applies the per-namespace privilege and owner checks during iteration, and get_snapshots_count uses it in roughly the shape list_snapshots would need.

I'd send it as two patches: the refactor onto ListAccessibleBackupGroups with no behavioural change, then max-depth on top. I can test on a 4.2 instance including with a token holding DatastoreAudit on the root namespace only, to confirm namespaces the token cannot see stay absent from a recursive listing.

Questions for the list

1. Is adding max-depth to the snapshots endpoint acceptable in principle?

2. Is ListAccessibleBackupGroups the right vehicle here, or is there a reason list_snapshots does not already use it?

3. Snapshot entries carry no namespace field, so a recursive listing is ambiguous unless the client tracks it per request. Adding one would mean a new field on SnapshotListItem, which is also decoded from remote instances in server/sync.rs and server/push.rs

If you'd rather this were solved another way or not at all I'm happy to hear it before writing anything.

Regards Thomas Moore