Support wider range of application names #31752
Conversation
This extends the validation for application names to allow an optional
suffix of "-" (or "_") followed by any number of printable, visible
ascii characters.
The purpose of this is to support multiple kibana instances against a
single ES cluster where the name of each kibana application is
"kibana-${kibana-index}", assuming some reasonable limits on the
Kibana index name.
tvernum
added
>enhancement
review
:Security/Authorization
|
Pinging @elastic/es-security |
|
CC: @kobelb |
| * - Then any number of printable, visible ASCII characters (letter, numbers, symbols) other than '*' | ||
| */ | ||
| private static final Pattern VALID_APPLICATION = Pattern.compile("^[a-z][A-Za-z0-9]{2,}([_-][\\p{Graph}&&[^*]]*)?$"); | ||
| private static final Pattern VALID_APPLICATION_OR_WILDCARD = Pattern.compile("^[a-z*][A-Za-z0-9*]*([_-]\\p{Graph}*)?$"); |
There was a problem hiding this comment.
The \p{Graph} thing is probably too broad for indices names
. I would leave it as it is though, treading on regex is long haul.VALID_APPLICATION_OR_WILDCARD is again too broad, it accepts multiple * intertwined with text, so more like a regex than an wildcard.
This one I would change. I would use the same regex as VALID_APPLICATION but ending with [*]?$ instead of $. Also, a test for this would be nice.
There was a problem hiding this comment.
Are you suggesting that we only support wildcards that end in *?
That wasn't my intent, but it solves all the use cases for which wildcards are required, so it sounds like a reasonable approach, although I do need to work out an appropriate regex for it.
I do wonder whether trying to do all this with a regex is becoming a little futile...
There was a problem hiding this comment.
Also, a test for this would be nice.
What specifically did you want a test for? There are tests for wildcards
- https://github.com/elastic/elasticsearch/pull/31752/files#diff-965f80992d9820eff5becc043e4cb96aR42
- https://github.com/elastic/elasticsearch/pull/31752/files#diff-965f80992d9820eff5becc043e4cb96aR50
- https://github.com/elastic/elasticsearch/pull/31752/files#diff-965f80992d9820eff5becc043e4cb96aR59
There was a problem hiding this comment.
Yes, I meant the * to only be allowed at the end, and the test will check that * is not allowed anywhere else. It was a reflex, I haven't considered the implication of it, though...
The Pattern based validation was overly permissive and gave bad error messages. It has been rewritten to be a combination of Patterns and explicit logic
|
Thanks @albertzaharovits |
I had used "astrix" to match Strings.validFileNameExcludingAstrix but since this method doesn't call that utility method, it seemed silly to keep the misspelling.
This extends the validation for application names to allow an optional
suffix of "-" (or "_") followed by any number of "filename safe characters"
(other than '*').
The purpose of this is to support multiple kibana instances against a
single ES cluster where the name of each kibana application is
"kibana-${kibana-index}", assuming some reasonable limits on the
Kibana index name.