-
Notifications
You must be signed in to change notification settings - Fork 96
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
Input Validation for Monitor Fields #771
base: main
Are you sure you want to change the base?
Input Validation for Monitor Fields #771
Conversation
Signed-off-by: vikhy-aws <[email protected]>
…elds Signed-off-by: vikhy-aws <[email protected]>
Signed-off-by: vikhy-aws <[email protected]>
@@ -75,14 +75,22 @@ fun validateIamRoleArn(roleArn: String) { | |||
require(roleArnRegex.matcher(roleArn).find()) { "Invalid AWS role ARN: $roleArn " } | |||
} | |||
|
|||
fun isValidName(name: String): Boolean { | |||
fun isValidQueryName(name: String): Boolean { |
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.
Could you add a short doc comment that explains this function validates the doc level monitor DocLevelQuery name specifically? We use the term "query" in many places, so a quick comment can help prevent confusion.
https://github.com/opensearch-project/common-utils/blob/main/src/main/kotlin/org/opensearch/commons/alerting/model/DocLevelQuery.kt#L16
import org.junit.jupiter.api.Assertions.assertTrue | ||
import org.junit.jupiter.api.Test | ||
|
||
class ValidationHelpers { |
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.
Nitpick: Could you rename the test suite and it file to something like ValiationHelpersTests
? That will follow the naming pattern we've used for other unit test suites.
https://github.com/opensearch-project/common-utils/blob/main/src/test/kotlin/org/opensearch/commons/alerting/MonitorTests.kt
Description
This PR adds a validation helper function to validate the name fields in a monitor. This helper function is used in Alerting to validate the monitor name and trigger name fields.
Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
--signoff
.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.