mirror of https://github.com/apache/superset.git
fix: add alert report timeout limits (#12926)
* prevent working timeout and grace period from being set to negative numbers * add extra validation * lint * fix black * fix isort * add js tests * fix lint + more python schema validation * add report schema test for timeout limits * add extra test for null grace period
This commit is contained in:
parent
741219e84d
commit
fc180ab2a6
|
@ -260,4 +260,39 @@ describe('AlertReportModal', () => {
|
||||||
expect(addWrapper.find('input[name="grace_period"]')).toExist();
|
expect(addWrapper.find('input[name="grace_period"]')).toExist();
|
||||||
expect(wrapper.find('input[name="grace_period"]')).toHaveLength(0);
|
expect(wrapper.find('input[name="grace_period"]')).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('only allows grace period values > 1', async () => {
|
||||||
|
const props = {
|
||||||
|
...mockedProps,
|
||||||
|
isReport: false,
|
||||||
|
};
|
||||||
|
|
||||||
|
const addWrapper = await mountAndWait(props);
|
||||||
|
|
||||||
|
const input = addWrapper.find('input[name="grace_period"]');
|
||||||
|
|
||||||
|
input.simulate('change', { target: { name: 'grace_period', value: 7 } });
|
||||||
|
expect(input.instance().value).toEqual('7');
|
||||||
|
|
||||||
|
input.simulate('change', { target: { name: 'grace_period', value: 0 } });
|
||||||
|
expect(input.instance().value).toEqual('');
|
||||||
|
|
||||||
|
input.simulate('change', { target: { name: 'grace_period', value: -1 } });
|
||||||
|
expect(input.instance().value).toEqual('1');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('only allows working timeout values > 1', () => {
|
||||||
|
const input = wrapper.find('input[name="working_timeout"]');
|
||||||
|
|
||||||
|
input.simulate('change', { target: { name: 'working_timeout', value: 7 } });
|
||||||
|
expect(input.instance().value).toEqual('7');
|
||||||
|
|
||||||
|
input.simulate('change', { target: { name: 'working_timeout', value: 0 } });
|
||||||
|
expect(input.instance().value).toEqual('');
|
||||||
|
|
||||||
|
input.simulate('change', {
|
||||||
|
target: { name: 'working_timeout', value: -1 },
|
||||||
|
});
|
||||||
|
expect(input.instance().value).toEqual('1');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -42,6 +42,7 @@ import {
|
||||||
} from './types';
|
} from './types';
|
||||||
|
|
||||||
const SELECT_PAGE_SIZE = 2000; // temporary fix for paginated query
|
const SELECT_PAGE_SIZE = 2000; // temporary fix for paginated query
|
||||||
|
const TIMEOUT_MIN = 1;
|
||||||
|
|
||||||
type SelectValue = {
|
type SelectValue = {
|
||||||
value: string;
|
value: string;
|
||||||
|
@ -837,6 +838,23 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
|
||||||
updateAlertState(target.name, target.value);
|
updateAlertState(target.name, target.value);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const onTimeoutVerifyChange = (
|
||||||
|
event: React.ChangeEvent<HTMLTextAreaElement | HTMLInputElement>,
|
||||||
|
) => {
|
||||||
|
const { target } = event;
|
||||||
|
const value = +target.value;
|
||||||
|
|
||||||
|
// Need to make sure grace period is not lower than TIMEOUT_MIN
|
||||||
|
if (value === 0) {
|
||||||
|
updateAlertState(target.name, null);
|
||||||
|
} else {
|
||||||
|
updateAlertState(
|
||||||
|
target.name,
|
||||||
|
value ? Math.max(value, TIMEOUT_MIN) : value,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
const onSQLChange = (value: string) => {
|
const onSQLChange = (value: string) => {
|
||||||
updateAlertState('sql', value || '');
|
updateAlertState('sql', value || '');
|
||||||
};
|
};
|
||||||
|
@ -1283,10 +1301,11 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
|
||||||
<div className="input-container">
|
<div className="input-container">
|
||||||
<input
|
<input
|
||||||
type="number"
|
type="number"
|
||||||
|
min="1"
|
||||||
name="working_timeout"
|
name="working_timeout"
|
||||||
value={currentAlert ? currentAlert.working_timeout : ''}
|
value={currentAlert?.working_timeout || ''}
|
||||||
placeholder={t('Time in seconds')}
|
placeholder={t('Time in seconds')}
|
||||||
onChange={onTextChange}
|
onChange={onTimeoutVerifyChange}
|
||||||
/>
|
/>
|
||||||
<span className="input-label">seconds</span>
|
<span className="input-label">seconds</span>
|
||||||
</div>
|
</div>
|
||||||
|
@ -1297,10 +1316,11 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
|
||||||
<div className="input-container">
|
<div className="input-container">
|
||||||
<input
|
<input
|
||||||
type="number"
|
type="number"
|
||||||
|
min="1"
|
||||||
name="grace_period"
|
name="grace_period"
|
||||||
value={currentAlert?.grace_period || ''}
|
value={currentAlert?.grace_period || ''}
|
||||||
placeholder={t('Time in seconds')}
|
placeholder={t('Time in seconds')}
|
||||||
onChange={onTextChange}
|
onChange={onTimeoutVerifyChange}
|
||||||
/>
|
/>
|
||||||
<span className="input-label">seconds</span>
|
<span className="input-label">seconds</span>
|
||||||
</div>
|
</div>
|
||||||
|
|
|
@ -17,8 +17,9 @@
|
||||||
from typing import Any, Dict, Union
|
from typing import Any, Dict, Union
|
||||||
|
|
||||||
from croniter import croniter
|
from croniter import croniter
|
||||||
|
from flask_babel import gettext as _
|
||||||
from marshmallow import fields, Schema, validate, validates_schema
|
from marshmallow import fields, Schema, validate, validates_schema
|
||||||
from marshmallow.validate import Length, ValidationError
|
from marshmallow.validate import Length, Range, ValidationError
|
||||||
|
|
||||||
from superset.models.reports import (
|
from superset.models.reports import (
|
||||||
ReportRecipientType,
|
ReportRecipientType,
|
||||||
|
@ -158,14 +159,22 @@ class ReportSchedulePostSchema(Schema):
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
validator_config_json = fields.Nested(ValidatorConfigJSONSchema)
|
validator_config_json = fields.Nested(ValidatorConfigJSONSchema)
|
||||||
log_retention = fields.Integer(description=log_retention_description, example=90)
|
log_retention = fields.Integer(
|
||||||
|
description=log_retention_description,
|
||||||
|
example=90,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
|
)
|
||||||
grace_period = fields.Integer(
|
grace_period = fields.Integer(
|
||||||
description=grace_period_description, example=60 * 60 * 4, default=60 * 60 * 4
|
description=grace_period_description,
|
||||||
|
example=60 * 60 * 4,
|
||||||
|
default=60 * 60 * 4,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
)
|
)
|
||||||
working_timeout = fields.Integer(
|
working_timeout = fields.Integer(
|
||||||
description=working_timeout_description,
|
description=working_timeout_description,
|
||||||
example=60 * 60 * 1,
|
example=60 * 60 * 1,
|
||||||
default=60 * 60 * 1,
|
default=60 * 60 * 1,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
)
|
)
|
||||||
|
|
||||||
recipients = fields.List(fields.Nested(ReportRecipientSchema))
|
recipients = fields.List(fields.Nested(ReportRecipientSchema))
|
||||||
|
@ -225,15 +234,22 @@ class ReportSchedulePutSchema(Schema):
|
||||||
)
|
)
|
||||||
validator_config_json = fields.Nested(ValidatorConfigJSONSchema, required=False)
|
validator_config_json = fields.Nested(ValidatorConfigJSONSchema, required=False)
|
||||||
log_retention = fields.Integer(
|
log_retention = fields.Integer(
|
||||||
description=log_retention_description, example=90, required=False
|
description=log_retention_description,
|
||||||
|
example=90,
|
||||||
|
required=False,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
)
|
)
|
||||||
grace_period = fields.Integer(
|
grace_period = fields.Integer(
|
||||||
description=grace_period_description, example=60 * 60 * 4, required=False
|
description=grace_period_description,
|
||||||
|
example=60 * 60 * 4,
|
||||||
|
required=False,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
)
|
)
|
||||||
working_timeout = fields.Integer(
|
working_timeout = fields.Integer(
|
||||||
description=working_timeout_description,
|
description=working_timeout_description,
|
||||||
example=60 * 60 * 1,
|
example=60 * 60 * 1,
|
||||||
allow_none=True,
|
allow_none=True,
|
||||||
required=False,
|
required=False,
|
||||||
|
validate=[Range(min=1, error=_("Value must be greater than 0"))],
|
||||||
)
|
)
|
||||||
recipients = fields.List(fields.Nested(ReportRecipientSchema), required=False)
|
recipients = fields.List(fields.Nested(ReportRecipientSchema), required=False)
|
||||||
|
|
|
@ -433,6 +433,8 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||||
"recipient_config_json": {"target": "channel"},
|
"recipient_config_json": {"target": "channel"},
|
||||||
},
|
},
|
||||||
],
|
],
|
||||||
|
"grace_period": 14400,
|
||||||
|
"working_timeout": 3600,
|
||||||
"chart": chart.id,
|
"chart": chart.id,
|
||||||
"database": example_db.id,
|
"database": example_db.id,
|
||||||
}
|
}
|
||||||
|
@ -443,6 +445,8 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||||
created_model = db.session.query(ReportSchedule).get(data.get("id"))
|
created_model = db.session.query(ReportSchedule).get(data.get("id"))
|
||||||
assert created_model is not None
|
assert created_model is not None
|
||||||
assert created_model.name == report_schedule_data["name"]
|
assert created_model.name == report_schedule_data["name"]
|
||||||
|
assert created_model.grace_period == report_schedule_data["grace_period"]
|
||||||
|
assert created_model.working_timeout == report_schedule_data["working_timeout"]
|
||||||
assert created_model.description == report_schedule_data["description"]
|
assert created_model.description == report_schedule_data["description"]
|
||||||
assert created_model.crontab == report_schedule_data["crontab"]
|
assert created_model.crontab == report_schedule_data["crontab"]
|
||||||
assert created_model.chart.id == report_schedule_data["chart"]
|
assert created_model.chart.id == report_schedule_data["chart"]
|
||||||
|
@ -514,6 +518,78 @@ class TestReportSchedulesApi(SupersetTestCase):
|
||||||
rv = self.client.post(uri, json=report_schedule_data)
|
rv = self.client.post(uri, json=report_schedule_data)
|
||||||
assert rv.status_code == 400
|
assert rv.status_code == 400
|
||||||
|
|
||||||
|
# Test that report can be created with null grace period
|
||||||
|
report_schedule_data = {
|
||||||
|
"type": ReportScheduleType.ALERT,
|
||||||
|
"name": "new3",
|
||||||
|
"description": "description",
|
||||||
|
"crontab": "0 9 * * *",
|
||||||
|
"recipients": [
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.EMAIL,
|
||||||
|
"recipient_config_json": {"target": "target@superset.org"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.SLACK,
|
||||||
|
"recipient_config_json": {"target": "channel"},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
"working_timeout": 3600,
|
||||||
|
"chart": chart.id,
|
||||||
|
"database": example_db.id,
|
||||||
|
}
|
||||||
|
uri = "api/v1/report/"
|
||||||
|
rv = self.client.post(uri, json=report_schedule_data)
|
||||||
|
assert rv.status_code == 201
|
||||||
|
|
||||||
|
# Test that grace period and working timeout cannot be < 1
|
||||||
|
report_schedule_data = {
|
||||||
|
"type": ReportScheduleType.ALERT,
|
||||||
|
"name": "new3",
|
||||||
|
"description": "description",
|
||||||
|
"crontab": "0 9 * * *",
|
||||||
|
"recipients": [
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.EMAIL,
|
||||||
|
"recipient_config_json": {"target": "target@superset.org"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.SLACK,
|
||||||
|
"recipient_config_json": {"target": "channel"},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
"working_timeout": -10,
|
||||||
|
"chart": chart.id,
|
||||||
|
"database": example_db.id,
|
||||||
|
}
|
||||||
|
uri = "api/v1/report/"
|
||||||
|
rv = self.client.post(uri, json=report_schedule_data)
|
||||||
|
assert rv.status_code == 400
|
||||||
|
|
||||||
|
report_schedule_data = {
|
||||||
|
"type": ReportScheduleType.ALERT,
|
||||||
|
"name": "new3",
|
||||||
|
"description": "description",
|
||||||
|
"crontab": "0 9 * * *",
|
||||||
|
"recipients": [
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.EMAIL,
|
||||||
|
"recipient_config_json": {"target": "target@superset.org"},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": ReportRecipientType.SLACK,
|
||||||
|
"recipient_config_json": {"target": "channel"},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
"grace_period": -10,
|
||||||
|
"working_timeout": 3600,
|
||||||
|
"chart": chart.id,
|
||||||
|
"database": example_db.id,
|
||||||
|
}
|
||||||
|
uri = "api/v1/report/"
|
||||||
|
rv = self.client.post(uri, json=report_schedule_data)
|
||||||
|
assert rv.status_code == 400
|
||||||
|
|
||||||
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
|
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
|
||||||
def test_create_report_schedule_chart_dash_validation(self):
|
def test_create_report_schedule_chart_dash_validation(self):
|
||||||
"""
|
"""
|
||||||
|
|
Loading…
Reference in New Issue