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.

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

Bug Report: CreateShard allows shard names with "/", which breaks other commands/components #12842

Closed
ajm188 opened this issue Apr 5, 2023 · 0 comments · Fixed by #12843
Closed

Comments

@ajm188
Copy link
Contributor

ajm188 commented Apr 5, 2023

Overview of the Issue

TopoServer's CreateShard does only minimal validation on the shard name; if you create a shard with a "/" character (e.g. commerce/a/b), but without a "-" character, it is assumed to be non-range-based sharding and is therefore valid. other methods and commands will parse this as a keyspace with name commerce and a shard named a/b, which breaks those commands.

Reproduction Steps

  1. use vtadmin to create a shard named commerce/a/b
  2. Load vtadmin, keyspace tab shows nothing (even if you had other keyspaces previously)

Binary Version

`main`, `v16`, probably every version prior

Operating System and Environment details

n/a

Log Fragments

No response

@ajm188 ajm188 self-assigned this Apr 5, 2023
ajm188 pushed a commit to planetscale/vitess that referenced this issue Apr 5, 2023
Fixes vitessio#12842.

Signed-off-by: Andrew Mason <andrew@planetscale.com>
deepthi added a commit that referenced this issue Apr 6, 2023
* Disallow the slash character in shard names

Fixes #12842.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* add release note

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Update go/vt/topo/shard.go

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

* Update changelog/17.0/17.0.0/summary.md

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Signed-off-by: Andrew Mason <amason@hey.com>
Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
ajm188 pushed a commit to planetscale/vitess that referenced this issue Apr 6, 2023
* Disallow the slash character in shard names

Fixes vitessio#12842.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* add release note

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Update go/vt/topo/shard.go

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

* Update changelog/17.0/17.0.0/summary.md

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Signed-off-by: Andrew Mason <amason@hey.com>
Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
deepthi added a commit that referenced this issue Apr 12, 2023
…12858)

* [topo] Disallow the slash character in shard names (#12843)

* Disallow the slash character in shard names

Fixes #12842.

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* add release note

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* Update go/vt/topo/shard.go

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

* Update changelog/17.0/17.0.0/summary.md

Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Signed-off-by: Andrew Mason <amason@hey.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Signed-off-by: Andrew Mason <amason@hey.com>
Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>

* add changelog stub

Signed-off-by: Andrew Mason <andrew@planetscale.com>

* fix version name

Signed-off-by: Andrew Mason <andrew@planetscale.com>

---------

Signed-off-by: Andrew Mason <andrew@planetscale.com>
Signed-off-by: Andrew Mason <amason@hey.com>
Co-authored-by: Deepthi Sigireddi <deepthi.sigireddi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant