cockroachdb / cockroach

CockroachDB - the open source, cloud-native distributed SQL database.
https://www.cockroachlabs.com
Other
29.59k stars 3.71k forks source link

Cluster name cannot include dots, the error message says otherwise #126604

Open MagicalTux opened 3 weeks ago

MagicalTux commented 3 weeks ago

Describe the problem

Running cockroachdb with a cluster name containing dots produces this error message:

ERROR: invalid argument "xxx.xxx" for "--cluster-name" flag: cluster name must contain only letters, numbers or the "-" and "." characters

To Reproduce

  1. Pass --cluster-name a value that contains a dot.

Expected behavior

Either the dot should be accepted, or the error message changed to reflects dots aren't allowed.

Additional data / screenshots

Error message defined at: https://github.com/cockroachdb/cockroach/blob/master/pkg/cli/flags.go#L207

The regexp is a few lines below:

var errClusterNameInvalidFormat = errors.New(`cluster name must contain only letters, numbers or the "-" and "." characters`)

// clusterNameRe matches valid cluster names.
// For example, "a", "a123" and "a-b" are OK,
// but "0123", "a-" and "123a" are not OK.
var clusterNameRe = regexp.MustCompile(`^[a-zA-Z](?:[-a-zA-Z0-9]*[a-zA-Z0-9]|)$`)

The regexp includes hyphens, but no dots.

Jira issue: CRDB-39994

blathers-crl[bot] commented 3 weeks ago

Hi @MagicalTux, please add branch-* labels to identify which branch(es) this C-bug affects.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

blathers-crl[bot] commented 3 weeks ago

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I was unable to automatically find someone to ping.

If we have not gotten back to your issue within a few business days, you can try the following:

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

MagicalTux commented 3 weeks ago

Looking at the documentation it seems that allowing dots was supposed to be the right behavior, in which case the regexp can be updated as below (this would also filter a few things like forbidding hyphen groups).

var clusterNameRe = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9]*(?:[-.][a-zA-Z0-9]+)*$`)

The tests in pkg/cli/flags_test.go in func TestClusterNameFlag should also have (at least) one valid cluster name with a dot added to ensure this is handled properly and doesn't break in the future.

mohitsethia commented 1 week ago

Hi @rimadeodhar @MagicalTux , can I contribute to this?