fix(aci milestone 3): check for resolution condition in metric detector validator #102922
Conversation
github-actions
bot
added
the
Scope: Backend
| ( | ||
| Condition.GREATER, | ||
| Condition.LESS, | ||
| Condition.GREATER_OR_EQUAL, |
There was a problem hiding this comment.
this and the condition below it are used for resolution
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #102922 +/- ##
===========================================
+ Coverage 79.61% 80.65% +1.03%
===========================================
Files 9015 9015
Lines 392406 392455 +49
Branches 24968 24968
===========================================
+ Hits 312425 316516 +4091
+ Misses 79584 75542 -4042
Partials 397 397 |
| if "condition_group" in attrs: | ||
| conditions = attrs.get("condition_group", {}).get("conditions") | ||
| if len(conditions) > 2: | ||
| if len(conditions) > 3: |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| ) and not any(condition["type"] == Condition.ANOMALY_DETECTION for condition in value): | ||
| raise serializers.ValidationError( | ||
| "Resolution condition required for metric issue detector." | ||
| ) |
There was a problem hiding this comment.
Bug: Validation bypass: raw input used after validation
The validate_conditions method validates the condition data with MetricIssueComparisonConditionValidator but discards the validated data. The resolution check then uses the raw input value instead of the validated data where condition_result has been properly converted to DetectorPriorityLevel enum. This causes the validation to incorrectly fail when condition_result is passed as a string (e.g., "0") rather than an integer, since string-to-enum comparison returns false.
| class MetricIssueConditionGroupValidator(BaseDataConditionGroupValidator): | ||
| conditions = serializers.ListField(required=True) | ||
|
|
||
| def validate_conditions(self, value): |
There was a problem hiding this comment.
just wondering out loud (not feedback on this specific PR): do we have any validation on the backend that the condition values are valid against each other? There's a lot of logic on the frontend to prevent it, but it could be an issue for API users or if we have a bug on the frontend
There was a problem hiding this comment.
Good point, I'm not sure we do 🤔 I'll add some more validation in a follow up PR.
…or validator (#102922) Verify on the backend that metric detectors have a resolution condition.
…or validator (#102922) Verify on the backend that metric detectors have a resolution condition.
Verify on the backend that metric detectors have a resolution condition.