alerting(api): cross-field validation for PER_EXCHANGE + empty-targets guard
PER_EXCHANGE rules: 400 if reNotifyMinutes != 0 or forDurationSeconds != 0. Any rule: 400 if webhooks + targets are both empty (never notifies anyone). Turns green: AlertRuleControllerIT#createPerExchangeRule_with*NonZero_returns400, AlertRuleControllerIT#createAnyRule_withEmptyWebhooksAndTargets_returns400.
This commit is contained in:
@@ -22,6 +22,7 @@ import com.cameleer.server.core.alerting.AlertRuleRepository;
|
|||||||
import com.cameleer.server.core.alerting.AlertRuleTarget;
|
import com.cameleer.server.core.alerting.AlertRuleTarget;
|
||||||
import com.cameleer.server.core.alerting.ConditionKind;
|
import com.cameleer.server.core.alerting.ConditionKind;
|
||||||
import com.cameleer.server.core.alerting.ExchangeMatchCondition;
|
import com.cameleer.server.core.alerting.ExchangeMatchCondition;
|
||||||
|
import com.cameleer.server.core.alerting.FireMode;
|
||||||
import com.cameleer.server.core.alerting.WebhookBinding;
|
import com.cameleer.server.core.alerting.WebhookBinding;
|
||||||
import com.cameleer.server.core.outbound.OutboundConnection;
|
import com.cameleer.server.core.outbound.OutboundConnection;
|
||||||
import com.cameleer.server.core.outbound.OutboundConnectionService;
|
import com.cameleer.server.core.outbound.OutboundConnectionService;
|
||||||
@@ -126,6 +127,7 @@ public class AlertRuleController {
|
|||||||
HttpServletRequest httpRequest) {
|
HttpServletRequest httpRequest) {
|
||||||
|
|
||||||
validateAttributeKeys(req.condition());
|
validateAttributeKeys(req.condition());
|
||||||
|
validateBusinessRules(req);
|
||||||
validateWebhooks(req.webhooks(), env.id());
|
validateWebhooks(req.webhooks(), env.id());
|
||||||
|
|
||||||
AlertRule draft = buildRule(null, env.id(), req, currentUserId());
|
AlertRule draft = buildRule(null, env.id(), req, currentUserId());
|
||||||
@@ -147,6 +149,7 @@ public class AlertRuleController {
|
|||||||
|
|
||||||
AlertRule existing = requireRule(id, env.id());
|
AlertRule existing = requireRule(id, env.id());
|
||||||
validateAttributeKeys(req.condition());
|
validateAttributeKeys(req.condition());
|
||||||
|
validateBusinessRules(req);
|
||||||
validateWebhooks(req.webhooks(), env.id());
|
validateWebhooks(req.webhooks(), env.id());
|
||||||
|
|
||||||
AlertRule updated = buildRule(existing, env.id(), req, currentUserId());
|
AlertRule updated = buildRule(existing, env.id(), req, currentUserId());
|
||||||
@@ -258,6 +261,36 @@ public class AlertRuleController {
|
|||||||
// Helpers
|
// Helpers
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Cross-field business-rule validation for {@link AlertRuleRequest}.
|
||||||
|
*
|
||||||
|
* <p>PER_EXCHANGE rules: re-notify and for-duration are nonsensical (each fire is its own
|
||||||
|
* exchange — there's no "still firing" window and nothing to re-notify about). Reject 400
|
||||||
|
* if either is non-zero.
|
||||||
|
*
|
||||||
|
* <p>All rules: reject 400 if both webhooks and targets are empty — such a rule can never
|
||||||
|
* notify anyone and is a pure footgun.
|
||||||
|
*/
|
||||||
|
private void validateBusinessRules(AlertRuleRequest req) {
|
||||||
|
if (req.condition() instanceof ExchangeMatchCondition ex
|
||||||
|
&& ex.fireMode() == FireMode.PER_EXCHANGE) {
|
||||||
|
if (req.reNotifyMinutes() != null && req.reNotifyMinutes() != 0) {
|
||||||
|
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
|
||||||
|
"reNotifyMinutes must be 0 for PER_EXCHANGE rules (re-notify does not apply)");
|
||||||
|
}
|
||||||
|
if (req.forDurationSeconds() != null && req.forDurationSeconds() != 0) {
|
||||||
|
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
|
||||||
|
"forDurationSeconds must be 0 for PER_EXCHANGE rules");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
boolean noWebhooks = req.webhooks() == null || req.webhooks().isEmpty();
|
||||||
|
boolean noTargets = req.targets() == null || req.targets().isEmpty();
|
||||||
|
if (noWebhooks && noTargets) {
|
||||||
|
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
|
||||||
|
"rule must have at least one webhook or target — otherwise it never notifies anyone");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Validates that all attribute keys in an {@link ExchangeMatchCondition} match
|
* Validates that all attribute keys in an {@link ExchangeMatchCondition} match
|
||||||
* {@code ^[a-zA-Z0-9._-]+$}. Keys are inlined into ClickHouse SQL, making this
|
* {@code ^[a-zA-Z0-9._-]+$}. Keys are inlined into ClickHouse SQL, making this
|
||||||
|
|||||||
@@ -164,7 +164,8 @@ class AlertRuleControllerIT extends AbstractPostgresIT {
|
|||||||
{"name":"valid-attr","severity":"WARNING","conditionKind":"EXCHANGE_MATCH",
|
{"name":"valid-attr","severity":"WARNING","conditionKind":"EXCHANGE_MATCH",
|
||||||
"condition":{"kind":"EXCHANGE_MATCH","scope":{},
|
"condition":{"kind":"EXCHANGE_MATCH","scope":{},
|
||||||
"filter":{"status":"FAILED","attributes":{"order.type":"x"}},
|
"filter":{"status":"FAILED","attributes":{"order.type":"x"}},
|
||||||
"fireMode":"PER_EXCHANGE"}}
|
"fireMode":"PER_EXCHANGE"},
|
||||||
|
"targets":[{"kind":"USER","targetId":"test-operator"}]}
|
||||||
""";
|
""";
|
||||||
|
|
||||||
ResponseEntity<String> resp = restTemplate.exchange(
|
ResponseEntity<String> resp = restTemplate.exchange(
|
||||||
@@ -324,10 +325,12 @@ class AlertRuleControllerIT extends AbstractPostgresIT {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private static String routeMetricRuleBody(String name) {
|
private static String routeMetricRuleBody(String name) {
|
||||||
|
// Includes a USER target so the rule passes the "at least one webhook or target" guard.
|
||||||
return """
|
return """
|
||||||
{"name":"%s","severity":"WARNING","conditionKind":"ROUTE_METRIC",
|
{"name":"%s","severity":"WARNING","conditionKind":"ROUTE_METRIC",
|
||||||
"condition":{"kind":"ROUTE_METRIC","scope":{},
|
"condition":{"kind":"ROUTE_METRIC","scope":{},
|
||||||
"metric":"ERROR_RATE","comparator":"GT","threshold":0.05,"windowSeconds":60}}
|
"metric":"ERROR_RATE","comparator":"GT","threshold":0.05,"windowSeconds":60},
|
||||||
|
"targets":[{"kind":"USER","targetId":"test-operator"}]}
|
||||||
""".formatted(name);
|
""".formatted(name);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user