Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sps-Idempotency-Key Header Proposal #98

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Sps-Idempotency-Key Header Proposal #98

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02bd31e
Added Idempotency-Key suggestion
nicholas-s-perkins Dec 11, 2024
561b2be
Improved Idempotency-Key description.
nicholas-s-perkins Dec 12, 2024
8d721e1
Update standards/request-response.md
nicholas-s-perkins Dec 12, 2024
0b7a62b
Updated some notes
nicholas-s-perkins Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions standards/request-response.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,32 @@ Access-Control-Allow-Methods: *

<hr />

```note
**KEEP AN EYE ON IT**: The [Idempotency-Key Request header](https://tools.ietf.org/id/draft-idempotency-header-01.html) is in an experimental state but getting lots of attention as a pattern of making fault-tolerant resilient requests for traditionally non-idempotent methods like `POST`.
#### [Idempotency-Key Request header](https://datatracker.ietf.org/doc/html/draft-idempotency-header-01)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### [Idempotency-Key Request header](https://datatracker.ietf.org/doc/html/draft-idempotency-header-01)
#### Idempotency-Key

Give this RFC is expired and archived, we have some questions to answer:

  • Should this be SPS-Idempotency-Key instead? (move this to the custom header section)?
  • Do we link and reference the RFC given its state (useful as a side reference no doubt, but not to be confused with).

**Type**: Request

**Support**: OPTIONAL

**Description**:
The HTTP Idempotency-Key request header field can be used to carry an "idempotency key" to make non-idempotent HTTP methods such as POST or PATCH fault-tolerant.
nicholas-s-perkins marked this conversation as resolved.
Show resolved Hide resolved
This is an experimental header, as there is common usage of it, but no accepted spec for it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify this experimental header comment a bit, by that you mean that there is an existing unaccepted RFC? (is it worth linking that at all)? Lets move the link above down to this part instead).

`Idempotency-Key` should typically be a V4 UUID as a string, or another random string with enough entropy to avoid collisions, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be stated somewhere that the idempotency key is always generated by the client?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

should be no longer than 255 characters long.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave this out as its a detail for the list below.

An API must in some way persist the Idempotency-Key in a stateful way to manage the idempotency.
It is common to include the key as part of the primary key or as a secondary index for the resource.

- The header value **MUST** follow this regex format: `[a-z0-9-]{20,255}`
- An API **MUST** return a `400` when the header value is an invalid format.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Is there a restriction in the types of Verbs you should use this header with, such as POST, PATCH (PUT?DELETE?)
  • Is this considered sensitive data that should not be logged?
  • Any consideration around the expiration of idempotency keys at all? Should I expect that if it works today it can dedupe that same key a week from now on the request (crazy delay in between)?
  • Any scope considerations... is an idempotency key scoped to a resource? to an endpoint? other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a restriction in the types of Verbs you should use this header with, such as POST, PATCH (PUT?DELETE?)

Not sure about restrictions. The draft spec mentioned PATCH as another possibility, but I honestly don't know of a use case to use the header outside of a POST. You can use an idempotency-key for any request that could possibly not be idempotent, and it might be hard to imagine all of the use cases or want to lock it down if there is some weird reason you might want to use it for a DELETE or something. I don't think there's any harm to being vague about it's usage outside of it being a request header.

Is this considered sensitive data that should not be logged?
It should, in general, not be sensitive data. And it would be OK to log.

Any consideration around the expiration of idempotency keys at all

I think it would be reasonable for some APIs to have short lived ones, and reasonable for some APIs to have indefinate ones. It seems reasonable that an API might want to just have a short lived one for a particular request, and once the request is fully processed, not really need to save the key anymore, and you can save some space and get index speed efficiency that way.
payload-storage-service, on the other hand, will bind a key to a payload for the entire retention policy of that payload, since it gets baked into the ID of the payload.

Any scope considerations... is an idempotency key scoped to a resource? to an endpoint? other?

That might be a bit hard to say. In a pure definitional sense it's an ID that identifies a particular unique "request", including retries, almost like a request session of a sort. Perhaps a better way to think about it is that it works a bit more like a way to identify a unique "action", which you could imagine an action having multiple requests (if anything, a single endpoint with multiple retries). I think almost always it will just be a way to have a unique POST behavior.

- On conflict, an API **MUST** return a `2xx` if the endpoint is idempotent, or a `409` if the endpoint is not idempotent.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought all usages with it should be idempotent... is it worth breaking this into two statements to clarify an idempotent usage/ non idempotent usage?

Copy link
Contributor Author

@nicholas-s-perkins nicholas-s-perkins Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's valid reasons that some cases are not idempotent.
The key indicates that you are trying to do some unique "action" with an ID. There's a lot of cases where an API could assess if the action a client is trying to do is invalid.
In payload-storage-service's case, when a client POSTs a new payload with an Idempotency-Key, this is idempotent, initially. The client can retry the POST with the same request multiple times, and will always get a 201 back without ever creating duplicate payloads.
But if client uses the same key, but then tries to change the payload content, this is an illegal action. It's only idempotent as long as the client is uploading the same payload content. A payload content is immutable, so if a naughty API accidentally changed out the payload content from the request every time it retried in a retry loop, the API needs to indicate that it is doing something that won't work, so it will reject it with a 409.

- An API **MAY** treat this as the final primary ID of the resource, or as some part of the ID.
Copy link
Contributor Author

@nicholas-s-perkins nicholas-s-perkins Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The [a-z0-9-]{20,255} regex is included more as a proposal for debate.

I was on the fence about whether this should be strictly enforced as a UUID, but after some thought, it seemed like a better idea to make it UUID-compatible rather than UUID-mandated.

There's advantages to it being a strict UUID. An API could parse it as a UUID and store it in UUID formats and use possibly more optimal storage for indexes.

There's disadvantage to forcing UUID in that there could be other styles of keys you might want to support, or use keys from various kinds of frameworks.

I think regardless of the format choice, I think it's useful to treat it like it could be a reference ID, and not allow weird non-url safe characters. That does limit you to to essentially a hex format of some sort.

Like a v4 UUID hex format:

e9ac6f49-4963-4f20-a038-539c66b6f5db

or a sha-256 hex format

83d0437862c69e1b7d280b25ab68d8003f7b8af12a7ac3a47dd8c5a090c8c65c

or a long decimal format

157627141979612915626333577105111854232

What this particular regex would exclude would be things like base64 formats or any of the other interesting bases.

For my use case, I want to encode this into the S3 key of an object and be reference-able in a way that's unique. If I make it more flexible, I could be sneaking in a problem where people don't really understand how unique an object is and when it might expire, but that could also be powerful to have the option and be compatible with a few different kinds of systems. It could be useful to call out that there are reasons to narrow the validation, but also it's useful to have a shared pattern across apps for this.

Copy link
Member

@travisgosselin travisgosselin Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya that is interesting.

There's disadvantage to forcing UUID in that there could be other styles of keys you might want to support, or use keys from various kinds of frameworks.

My thoughts immediately went to this, combined with your base64 example of how some usages of it might be represented.

If we don't have a strong opinion or clear direction on this, I think I'm more inclined to relax the format a bit.


```
// CORRECT
Idempotency-Key: d4f885c5-2196-49c0-ba69-bc70008585ad
Idempotency-Key: d4f885c5-2196-49c0-ba69-bc70008585ad-custom

// INCORRECT
Idempotency-Key: a // not enough entropy
Idempotency-Key: KG5Lxw!@#$&*()FBepaKHyUD // non-url-safe special characters can be limiting for usage or later reference
```

### Custom Headers
Expand Down
Loading