-
Notifications
You must be signed in to change notification settings - Fork 62
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
Added validations for indexPattern in autofollow API #1358
Conversation
Hi All, Please let me know if the above made changes are good to go. Feedback is much appreciated :) |
Hi @monusingh-1 , Any update ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking out the time to contribute this fix @skumarp7
Have few minor comments on the PR. Rest LGTM.
* Validate the pattern against the rules that we have for indexPattern name. | ||
*/ | ||
|
||
fun validatePattern(pattern: String?, validationException: ValidationException) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's name this as validateAutofollowPattern
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -294,6 +294,32 @@ class UpdateAutoFollowPatternIT: MultiClusterRestTestCase() { | |||
.hasMessageContaining(errorMsg) | |||
} | |||
|
|||
fun `test auto follow should fail on indexPattern validation failure`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure we're not regressing, Can you please add the tests for various valid patterns as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
fun validatePattern(pattern: String?, validationException: ValidationException) { | ||
|
||
if (!Strings.validFileNameExcludingAstrix(pattern)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The characters in the pattern doesn't have to a valid filename right? Wanted to understand the how you arrived at this criteria.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tried to adapt similar validations done in opensearch on indexPatterns at index template level where the index Pattern must not contain invalid filename characters.
Enabled approval workflow |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for
Security tests have started to fail, investigating |
Description
This PR adds validations for the indexPattern configured in autofollow API
Existing behavior of the autofollow API with invalid indexPattern:
Behaviour after adding validations on indexPattern:
Issues Resolved
#1034
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.