Consider rejecting blind API writes

Luiz Oliveira (@laoj2)

Update calls for Substrate resources (Actor, ActorSnapshot, etc) expose an optimistic locking schema. Requests can specify metadata.version and/or metadata.uid, and the ate-apiserver will use those values to fence the update and make it only succeed if it matches the value in the DB. This check is implemented in a transactional read-modify-write.

Now, do we want to support this? Clients doing blind writes are dangerous as they can override each others requests (last request wins) when doing concurrent updates.

This task is to evaluate if we want to make metadata.version required for update methods. Also, should we require metadata.uid as well or just keep it optional?

Luiz Oliveira (@laoj2) Tim Hockin (@thockin) Joe Betz (@jpbetz)

ateapi - change preconditions to be an explicit field in Update... fu…

Julian Gutierrez Oschmann (@juli4n)

Julian Gutierrez Oschmann (@juli4n) just clarifying: the idea is to reject blind API writes only for Update methods, right? I'm not sure pinning a version makes sense for Delete methods

Ah, good clarification (I updated the title to reflect this better). Yes, this issue was mostly about Update calls.

I think deletes are different, and making them optional makes sense as requiring clients to always do a read just to delete seems excessive / unnecessary.

Ah, good clarification (I updated the title to reflect this better). Yes, this issue was mostly about Update calls.

I think deletes are different, and making them option makes sense as requiring clients to always do a read just to delete seems excessive / unnecessary.

This is very lifecycle dependent so you should make sure that callers have the option of only deleting if a precondition is met, but I would expect a lot of deletion use cases not to require the preconditions for safety.

ateapi - change preconditions to be an explicit field in Update... fu…

ateapi - reject blind updates

Haven Xia (@HavenXia)

Forcing RMW for updates amplifies the risk of data loss in the face of version skew. AFAICT there are 2 fixes - PATCH and preserving unknown fields in clients.

At this point in time I am OK to cut PATCH support (for scope - it will come back) and require preconditions, but we should decide very soon how we want clients to behave. We don't yet have an SDK beyond "here's a proto file", but I think we should make sure all our examples allow RMW to work correctly, without data loss.

Haven Xia (@HavenXia) since this intersects with upgrades.

ateapi - reject blind updates

At this point in time I am OK to cut PATCH support (for scope - it will come back) and require preconditions, but we should decide very soon how we want clients to behave. We don't yet have an SDK beyond "here's a proto file", but I think we should make sure all our examples allow RMW to work correctly, without data loss.

Sounds good. For now, I'll make sure the examples are correct and also add a note to the api-guide and api-style-guide docs to make this clearer

ateapi - change preconditions to be an explicit field in Update... fu…

ateapi - reject blind updates

ateapi - Require preconditions in Update methods (#1014)

AFAICT there are 2 fixes - PATCH and preserving unknown fields in clients.

Let me rationalize how I think about version skews and unknown fields, because I think there is a schema where we can guarantee that we never drop fields (by leveraging the fact that proto3 preserves unknown fields 1).

There are three possible scenarios we need to handle wrt version skews 1) the client is behind the server 2) the server is behind the client and 3) the server is behind other server replicas.

In this case, the client is doing a RMW, and the server sends fields that the client doesn't know. The client does a Get<Resource> RPC, gets the resource, mutates the proto (either in place or a proto.Clone of it) and calls Update<Resource>. This is safe, as long as the client doesn't do any transformation that drops unknown fields (e.g. protojson encode / decode), the fields will be round tripped back to the server.
In this case, the server is behind the client. The client calls Update<Resource> and provides a proto that the server doesn't know. This is rare, but I think it's nice if we can handle all the cases.
In this case, the server is behind other server replicas. The client calls Update<Resource> and provides a proto. This can happen during rolling updates of the ate-apiserver.

The solution to 2) and 3) are both in the way we "merge" the stored and the user provided resource proto. The server performs a RMW, and executes the mutate callback to "merge" both the server-owned fields and the client provided ones. It does by doing:

func (toUpdate *Actor) {
  before := proto.Clone(toUpdate)
  proto.Reset(toUpdate)
  proto.Merge(toUpdate, inActor) 
  toUpdate.Status = before.Status
  toUpdate.Metadata = newUpdateMetadata(before.Metadata)

The proto.Merge call makes sure we copy all user-owned fields, without dropping any. We only copy them because the Reset call before. This handles 2).

The last two statements that copy status and metadata fields make sure we don't drop unknown fields on server-owned fields. This handles 3).

One interesting (unexpected) thing about this: if we had a spec field to wrap all user-owned fields, this logic would be even simpler, no need for Reset/Merge, just a field assignment (I'm not proposing we do this though!).

In this case, the server is behind the client. The client calls Update and provides a proto that the server doesn't know. This is rare, but I think it's nice if we can handle all the cases.

For this to happen one of the following cases must be true:

a) The client is ahead of the server. It is asking for a field the server does not know about. Since this is known to not be a status field (we wipe those) it means the client is asking for something the server does not know. If we discard that we are lying. If we retain it but don't act on it, we are still lying.

b) The server is storing a field it doesn't understand (e.g. there was an update, someone used a new field, then there was a rollback). A forward-rev client might think the system is acting on that new field, but it's not. The server is lying.

c) There was a server rollback in the middle of the RMW cycle. The server gave you a field it knew about, but it doesn't know about it anymore.

In all 3 cases, I think we should reject unknown fields. Why? In case (a) accepting unknown fields implies that we are acting on those fields but we are not. We are lying. In case (b) we need to decide what is worse - losing information or carrying unknown data. Both are potentially lies. I think we should clear unknown fields on READ or at worst, we only accept unknown fields that have not been changed across an RMW. In case (c) I think it's totally legitimate to call it an update conflict and make the user try again, reducing it to (a) or (b).

Here's another problem with allowing unknown fields, or at least net-new unknowns -- does that mean a user can store anything they like in our API? Are we OK with that? It's an interesting extensibility model, but if we want to do this, we should do it intentionally (e.g. specific tag ranges, max size, etc).

In this case, the server is behind other server replicas. The client calls Update and provides a proto. This can happen during rolling updates of the ate-apiserver.

This is and will always be ugly. Retaining unknown data doesn't make it correct - a back-rev replica might act on a forward-rev request and again, we will be lying to users. The only solutions I know are two-phase (dark) rollouts or having olded replicas route to newer replicas. Even blue-green CP rollouts with atomic LB flips are not really atomic (unless we break long-lasting connections).

. In case (b) we need to decide what is worse - losing information or carrying unknown data. Both are potentially lies.

I like the losing information idea better than carrying unknown data, because at least the control plane is returning what it is currently enforcing and the data loss fails loudly.

So, if we can summarize it like this:

Reject on WRITE: reject the request to create/update an object with fields the server doesn't understand
Clear on READ: clear unknown fields before serving them to the clients

In case there's a:

Permanent rollback to V1:
* Client reads v2 object
* Client sees zero (unset?) values for V2 fields and knows the control plane is "not doing that anymore"
(should we mark fields as optional in the proto, so we can at least differentiate between set to zero vs unset?)
Temporary rollback to V1 / mid-RMW rollback:
* Client reads a V2 object,
* Server rolls back to V1
* Write of a V2 object fails
* Client retries RMW and now reads a pruned object
* Write succeeds, but V2 data is permanently lost (which is ok? when the server rolled back to v1 it lost the ability to handle that field).
Rolling upgrade (v1 -> v2): (yes, this is a complicated one, but I'm assuming not to be a problem for now)
* Object was stored at v2
* Client reads a pruned object from a v1 server
* Client later writes the object back to a v2 server
* v2 data is permanently lost

Late reply here, the upgrade proposal converged on the same rules this thread did, I planned to add an api-changes.md about API compatibility and provide clear guidance for contributor (filed #1185)

ateapi: reject unknown fields on Create requests

Navigation Menu

Repository navigation

Consider rejecting blind API writes #954

2 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Consider rejecting blind API writes #954

Description

Activity

laoj2 commented on Aug 18, 2026

juli4n commented on Aug 19, 2026

jpbetz commented on Aug 19, 2026

thockin commented on Aug 19, 2026

laoj2 commented on Aug 19, 2026

2 remaining items

juli4n commented on Aug 21, 2026

thockin commented on Aug 21, 2026

laoj2 commented on Aug 21, 2026

HavenXia commented on Aug 25, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions