Expand jetstream KV to allow : in key

nats-io / nats-architecture-and-design

Architecture and Design Docs

Apache License 2.0

177 stars 20 forks source link

Expand jetstream KV to allow : in key #77

Closed matthewdevenny closed 2 years ago

matthewdevenny commented 2 years ago

Overview

jetstream KV keys should be allowed to contain a : this would provide feature parity with etcd.

The behavior is documented in ADR-8.

Clients and Tools

[ ] Schemas @ripienaar
[ ] CLI @ripienaar
[ ] Terraform @ripienaar
[ ] Github Actions @ripienaar
[ ] Go @wallyqs
[ ] Java @scottf
[ ] JavaScript @aricart
[ ] .Net @ColinSullivan1 @scottf
[ ] C @kozlovic
[ ] Python @wallyqs
[ ] Ruby @wallyqs
[ ] Rust @matthiashanel @derekcollison

Other Tasks

[ ] docs.nats.io updated
[ ] Update ADR to Implemented
[ ] Update client features spreadsheet

Client authors please update with your progress. If you open issues in your own repositories as a result of this request, please link them to this one by pasting the issue URL in a comment or main issue description.

ripienaar commented 2 years ago

Basically this is just to support what etcd supports for the Kine compatibility, i dont see a reason not to support : so seems fine to me.

scottf commented 2 years ago

@ripienaar Do we want to expand to include printable like a regular subject?

printable        = all printable ascii (33 to 126 inclusive)
term             = (printable except dot, asterisk or gt)+
limited-term     = (A-Z, a-z, 0-9, dash, underscore, fwd-slash, equals)+
restricted-term  = (A-Z, a-z, 0-9, dash, underscore)+

...

kv-key-name = term (dot term)*

ripienaar commented 2 years ago

Not sure, reason we limit it is because other backends also limit it, if we later want to support future backends and we expand this too far we can be in trouble.

matthewdevenny commented 2 years ago

It also appears that etcd allows a trailing . in a key

ripienaar commented 2 years ago

We cannot support that. There's some thinking around encoded/encrypted keys which would allow you to put whatever you like into keys, I can show you examples of what I had for that

scottf commented 2 years ago

Please see PR to Add colon to limited term to expand what is allowed for kv key name into ADR-6 naming conventions https://github.com/nats-io/nats-architecture-and-design/pull/78

derekcollison commented 2 years ago

I am ok with supporting ':'

ripienaar commented 2 years ago

Appears there's nothing to do here, we cannot meet all the requirements @boxboatmatt has without encoding the keys into something like Base64, other issues for example they need to support a watch on /x/y/z which to us would mean x.y.z.> and worse they need . in keys and . in end of keys.

So us supporting : wins nothing, all has to be encoded anyway.

So I think we can close this, agree @boxboatmatt ?

matthewdevenny commented 2 years ago

Agree