Skip to content
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

Add optional alerts validations #4

Merged

Conversation

assafad
Copy link
Collaborator

@assafad assafad commented Feb 28, 2024

Add the following custom alert validations:

  • Validation for limiting alerts names length to 50 characters. We want to prevent alerts names from being too long, so they will fit on the TOC in the docs.
  • Validation for the existence of runbook_url annotation.
  • Validation for existence of operator_health_impact, kubernetes_operator_part_of and kubernetes_operator_component labels.

Fixes https://issues.redhat.com/browse/CNV-31132.

@sradco
Copy link

sradco commented Feb 28, 2024

@assafad please update the PR description and add the link to Jira in a separate line.

@assafad
Copy link
Collaborator Author

assafad commented Feb 28, 2024

@assafad please update the PR description and add the link to Jira in a separate line.

Done.

Copy link
Owner

@machadovilaca machadovilaca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a unit test for this verification

also, do you think this should be a mandatory rule for every operator that uses this library, or it should be optional?

@sradco
Copy link

sradco commented Feb 28, 2024

please add a unit test for this verification

also, do you think this should be a mandatory rule for every operator that uses this library, or it should be optional?

I think we should make it optional.

@assafad assafad force-pushed the alert-name-length-validation branch 5 times, most recently from d8f32a3 to b8f254c Compare February 28, 2024 13:24
@assafad assafad changed the title Add validation for alert name length Add optional alerts validations Feb 28, 2024
pkg/testutil/alert_custom_validations.go Outdated Show resolved Hide resolved
pkg/testutil/alert_custom_validations.go Show resolved Hide resolved
pkg/testutil/alert_custom_validations.go Show resolved Hide resolved
pkg/testutil/alert_validation_test.go Outdated Show resolved Hide resolved
@assafad assafad force-pushed the alert-name-length-validation branch 3 times, most recently from 30c3831 to 100a2c7 Compare February 28, 2024 14:17
pkg/testutil/alert_custom_validation_test.go Outdated Show resolved Hide resolved
pkg/testutil/alert_custom_validations.go Outdated Show resolved Hide resolved
pkg/testutil/alert_custom_validations.go Outdated Show resolved Hide resolved
Signed-off-by: assafad <aadmi@redhat.com>
Copy link
Owner

@machadovilaca machadovilaca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the PR and for the fixes in the previous code
good work

@machadovilaca machadovilaca merged commit 6720f1d into machadovilaca:main Feb 28, 2024
1 check passed
machadovilaca added a commit that referenced this pull request Feb 28, 2024
After #4, this commit mentions in  AlertsAndRecordingRulesValidation.md the availability of the new custom alert validation rules created
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants