atc: allow identifiers to start with numbers #9119
Conversation
There was a problem hiding this comment.
Why do we need to identify time.Duration-like strings? It feels like we're trying to resolve a very specific pain here instead of trying to determine if there's a larger ask/pain here.
Also, instead of rolling our own regex, why not use time.ParseDuration()?
After looking up where identifier gets used I'm guessing you have resources whose names are time.Duration-like strings?
Back to the larger ask, because I don't want this file to be committed full of exceptions for strings that look like X, then Y, then Z, etc.
Do we want to allow identifiers that start with numbers?
Having context for why we have our current limitation would be helpful to answer this question. Let's go explore the project history.
I looked through the commit history for this file, specifically looking at the lines where the regex are defined.
I looked at this commit e6c2ede.
Then the PR it came from: #5864
Ah, that PR references an RFC: concourse/rfcs#40
And the original motivation(s): #3985
So taking in all this context, it looks like identifier restrictions were introduced to ensure folks don't put weird characters in places that appear in URL's.
I'm not seeing any great reason to not allow identifiers to start with numbers, though it also seems like no one is really asking for that. That's always dangerous with yaml too because a bunch of numbers are going to be parsed as a number instead of a string.
a_number: 123
a_string: "123"Though I don't think anyone would name a pipeline/job/resource/task step JUST numbers.
I think it's reasonable to want to allow identifiers to start with numbers, given the one example here of having a time resource that you want to name 1h.
Would it break anything on the CLI level?
fly -t ci set-pipeline -p 1-pipeline-namePipeline names look weird, but I don't think this will break anything at the CLI level.
At the URL level this would be totally fine too. Having numbers at the start of any part of a URL path isn't breaking anything at the browser level or making anything harder for us to do within the web frontend, is it?
Do we want to allow an identifier to be ONLY numbers? A pipeline, job, or resource that's only numbers feels very strange and now I kinda worry it might break something...
I would be fine with allowing the identifier to start with numbers, but let's not allow the identifier to be only numbers. I think that would be a good middle ground here. What do you think?
Agreed. I'll change the PR. |
This change updates the identifier validation to permit identifiers that start with numbers (e.g., "1pipeline") while keeping other validation rules intact. All identifiers must now start with either a lowercase letter or a number, and the error messages have been updated accordingly. Signed-off-by: Mathias Bogaert <mathias.bogaert@gmail.com>
|
Docs here will need to be updated: https://github.com/concourse/docs/blob/70e8fd6ada55064074bc0d98666ca0219fa8d637/lit/docs/config/basic-schemas.lit#L79-L105 |
Signed-off-by: Taylor Silva <dev@taydev.net>
After concourse/concourse#9119 is released, identifiers can start with numbers, not just letters Signed-off-by: Taylor Silva <dev@taydev.net>
This change updates the identifier validation to permit identifiers that start with
numbers (e.g., "1pipeline") while keeping other validation rules intact. All identifiers
must now start with either a lowercase letter or a number, and the error messages have
been updated accordingly.
Release Note
a-z, otherwise Concourse would warn you that the identifier is invalid. An identifier cannot be only numbers though.543is invalid, but2-daysis valid.