sensu-go icon indicating copy to clipboard operation
sensu-go copied to clipboard

Update resource PUT APIs to support 200 Created and 304 Not Modified

Open calebhailey opened this issue 3 years ago • 1 comments

Feature Suggestion

The POST APIs (e.g. CheckConfig POST) already return HTTP response code 201 Created and 409 Conflict for when the resource already exists. Extending the PUT APIs to indicate that a resource was either 201 Created, 200 OK (updated) or 304 Not Modified would enable better user experiences to be built when working with resources. Example: #3079

Context

Most core/v2 resource PUT APIs currently always return HTTP 201 when successful as the store interface does not facilitate communicating which action was taken.

Update these handlers to return HTTP 201 Created when the resource did not previously exist HTTP 200 OK when the resource was updated HTTP 304 Not Modified when the resource was unchanged

Tech Notes

Router:

The apid router will likely need rearranged to allow for multiple non-error status codes per route. The existing interface type actionHandlerFunc func(r *http.Request) (interface{}, error) only allows for variable error codes. Successful codes are hard coded in the router per-method.

Etcd Store:

The likely most performant solution in the etcd store to know whether or not a resource was created, updated or unchanged is to keep it in a single transaction and request prev_kv on the put operation. This could end up being flakey due to etcd shenanigans like compaction.

Store Interface Options:

The resources using the standard PUT handler all use the generic CreateOrUpdateResource store method today. The signature of this interface does not provide a way to communicate back whether or not a resource has been created, replaced or unchanged. Our options here are to update the store interface to return the previous value of the updated resource or to return a set of flags indicating what action was taken.

calebhailey avatar Jun 30 '21 16:06 calebhailey

@calebhailey after reviewing with eng, I think we're recommending that this feature be put on hold until 7.x.

Noted Concerns:

  • This change at the API level would necessitate a rather large refactor of our Store code, which could have large impacts. It may make sense to put a concerted effort into adopting and the store/v2 interface in 7.0 with some more thoughtful design put into supporting PUT and other tangentially related features (ETAG, patching, etc.)
  • The definition of "Not Modified" could use some defining for our purposes.

c-kruse avatar Feb 16 '22 21:02 c-kruse