Discussion: Consumer and Stream Name Validation

aricart commented 1 year ago

Overview

Initial validation for streams and consumer names included ., *, or > in the names. The ADR for the KV specified bucket names to be ^[a-zA-Z0-9_-]+$. This regex is not a bad choice because it pairs well with https://www.ibm.com/docs/en/zos/2.1.0?topic=locales-posix-portable-file-name-character-set.

The main concern is how to clamp such validations because it may mean that assets that the server accepted and a client used now becomes bricked by a new version of the client.

Clients and Tools

[ ] Schemas @ripienaar
[ ] CLI @ripienaar
[ ] Terraform @ripienaar
[ ] Github Actions @ripienaar
[ ] Go @piotrpio
[ ] Java @scottf
[ ] JavaScript @aricart
[ ] .Net @scottf
[ ] C @levb
[ ] Python @wallyqs
[ ] Ruby @wallyqs
[ ] Rust @Jarema @caspervonb

Other Tasks

[ ] docs.nats.io updated @jnmoyne
[ ] 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 1 year ago

KV has never accepted anything other than that from day one. It's not a later addition.

Jarema commented 1 year ago

Apart from the discussion around special characters that are now allowed in stream and consumer name (diacritics, Chinese characters, etc) - which restricting makes sense, but might break clients for some users, I think we need to add space too.

aricart commented 1 year ago

With tooling that lets you backup and move streams to different OS etc, this is going to be a source of pain. The POC for KV from Roland did the right thing from the get-go. JavaScript clients for sure have the bug and need to clamp. The server added validation for /. But likely any such validation will need a major version update.

scottf commented 1 year ago

It was also captured here: https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-6.md which has also been recently updated to reflect the state of the server regarding stream, durable and consumer names.

aricart commented 1 year ago

It was also captured here: https://github.com/nats-io/nats-architecture-and-design/blob/main/adr/ADR-6.md which has also been recently updated to reflect the state of the server regarding stream, durable and consumer names.

The bucket name/etc listed there must also be file restricted names, as they are files.

scottf commented 1 year ago

Bucket names are restricted-term so are very limited. Here is an excerpt:

restricted-term    = (A-Z, a-z, 0-9, dash, underscore)+
filename-safe      = (printable except dot, asterisk, gt, fwd-slash, backslash)+ maximum 255 characters

...

stream-name        = filename-safe
durable-name       = filename-safe
consumer-name      = filename-safe
account-name       = filename-safe 

...

kv-bucket-name     = restricted-term
os-bucket-name     = restricted-term

Jarema commented 1 year ago

@aricart @scottf let's move forward with this. Helped users this week whose consumer creation stopped working after client update to new consume API with really cryptic error because of space in consumer name. No way they could find it.

I'm fine with mentioned regex / restricted-term.

nats-io / nats-architecture-and-design