mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 09:48:58 -04:00
[8.x] [ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params (#217440) (#217526)
# Backport This will backport the following commits from `main` to `8.x`: - [[ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params (#217440)](https://github.com/elastic/kibana/pull/217440) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Christos Nasikas","email":"christos.nasikas@elastic.co"},"sourceCommit":{"committedDate":"2025-04-08T13:58:26Z","message":"[ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params (#217440)\n\n## Summary\n\nA PR introduced into 8.18/9.0\n(https://github.com/elastic/kibana/pull/205507) changed the way we\nvalidate the log threshold rule type parameters. The validation happens\non rule params and changes a loose validation to a strict validation, so\nthose users who’ve inserted excess fields before 8.18/9.0 will see rules\nstarting to fail to run, their rule page failing to load and the API\nstarting to reject calls with excess fields.\n\nFixes: https://github.com/elastic/kibana/issues/217384\n\n## Testing instructions\n\n1. Start Kibana on 8.17 and create the following rule using the API. Let\nthe rule run.\n\n<details><summary>Rule</summary>\n\n```\n{\n \"name\": \"[QAF] Observability rule 3\",\n \"tags\": [\n \"metrics\",\n \"threshold\",\n \"qaf\"\n ],\n \"rule_type_id\": \"logs.alert.document.count\",\n \"consumer\": \"alerts\",\n \"schedule\": {\n \"interval\": \"1m\"\n },\n \"actions\": [],\n \"params\": {\n \"timeSize\": 8,\n \"timeUnit\": \"h\",\n \"count\": {\n \"value\": 1,\n \"comparator\": \"more than\"\n },\n \"criteria\": [\n {\n \"field\": \"bytes\",\n \"comparator\": \"more than\",\n \"value\": 1\n }\n ],\n \"logView\": {\n \"logViewId\": \"log-view-reference-0\",\n \"type\": \"log-view-reference\"\n },\n \"groupBy\": [\n \"geo.dest\"\n ],\n \"outputIndex\": \".alerts-observability.logs.alerts-default\"\n }\n}\n```\n\n</details> \n\n2. Start Kibana on 8.18. Verify that you cannot create the same rule and\nthe rule created in step 1 starts failing.\n3. Start Kibana on this PR and that you can create the same rule and the\nrule created in step 1 is working as expected.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"2a01722cfa42772b584e195eaebf4078e0cc5be2","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","Team:ResponseOps","v9.0.0","backport:version","v8.18.0","v9.1.0","v8.19.0"],"title":"[ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params","number":217440,"url":"https://github.com/elastic/kibana/pull/217440","mergeCommit":{"message":"[ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params (#217440)\n\n## Summary\n\nA PR introduced into 8.18/9.0\n(https://github.com/elastic/kibana/pull/205507) changed the way we\nvalidate the log threshold rule type parameters. The validation happens\non rule params and changes a loose validation to a strict validation, so\nthose users who’ve inserted excess fields before 8.18/9.0 will see rules\nstarting to fail to run, their rule page failing to load and the API\nstarting to reject calls with excess fields.\n\nFixes: https://github.com/elastic/kibana/issues/217384\n\n## Testing instructions\n\n1. Start Kibana on 8.17 and create the following rule using the API. Let\nthe rule run.\n\n<details><summary>Rule</summary>\n\n```\n{\n \"name\": \"[QAF] Observability rule 3\",\n \"tags\": [\n \"metrics\",\n \"threshold\",\n \"qaf\"\n ],\n \"rule_type_id\": \"logs.alert.document.count\",\n \"consumer\": \"alerts\",\n \"schedule\": {\n \"interval\": \"1m\"\n },\n \"actions\": [],\n \"params\": {\n \"timeSize\": 8,\n \"timeUnit\": \"h\",\n \"count\": {\n \"value\": 1,\n \"comparator\": \"more than\"\n },\n \"criteria\": [\n {\n \"field\": \"bytes\",\n \"comparator\": \"more than\",\n \"value\": 1\n }\n ],\n \"logView\": {\n \"logViewId\": \"log-view-reference-0\",\n \"type\": \"log-view-reference\"\n },\n \"groupBy\": [\n \"geo.dest\"\n ],\n \"outputIndex\": \".alerts-observability.logs.alerts-default\"\n }\n}\n```\n\n</details> \n\n2. Start Kibana on 8.18. Verify that you cannot create the same rule and\nthe rule created in step 1 starts failing.\n3. Start Kibana on this PR and that you can create the same rule and the\nrule created in step 1 is working as expected.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"2a01722cfa42772b584e195eaebf4078e0cc5be2"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.x"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/217440","number":217440,"mergeCommit":{"message":"[ResponseOps][Rules] Ignore unknowns in the schema of the log threshold params (#217440)\n\n## Summary\n\nA PR introduced into 8.18/9.0\n(https://github.com/elastic/kibana/pull/205507) changed the way we\nvalidate the log threshold rule type parameters. The validation happens\non rule params and changes a loose validation to a strict validation, so\nthose users who’ve inserted excess fields before 8.18/9.0 will see rules\nstarting to fail to run, their rule page failing to load and the API\nstarting to reject calls with excess fields.\n\nFixes: https://github.com/elastic/kibana/issues/217384\n\n## Testing instructions\n\n1. Start Kibana on 8.17 and create the following rule using the API. Let\nthe rule run.\n\n<details><summary>Rule</summary>\n\n```\n{\n \"name\": \"[QAF] Observability rule 3\",\n \"tags\": [\n \"metrics\",\n \"threshold\",\n \"qaf\"\n ],\n \"rule_type_id\": \"logs.alert.document.count\",\n \"consumer\": \"alerts\",\n \"schedule\": {\n \"interval\": \"1m\"\n },\n \"actions\": [],\n \"params\": {\n \"timeSize\": 8,\n \"timeUnit\": \"h\",\n \"count\": {\n \"value\": 1,\n \"comparator\": \"more than\"\n },\n \"criteria\": [\n {\n \"field\": \"bytes\",\n \"comparator\": \"more than\",\n \"value\": 1\n }\n ],\n \"logView\": {\n \"logViewId\": \"log-view-reference-0\",\n \"type\": \"log-view-reference\"\n },\n \"groupBy\": [\n \"geo.dest\"\n ],\n \"outputIndex\": \".alerts-observability.logs.alerts-default\"\n }\n}\n```\n\n</details> \n\n2. Start Kibana on 8.18. Verify that you cannot create the same rule and\nthe rule created in step 1 starts failing.\n3. Start Kibana on this PR and that you can create the same rule and the\nrule created in step 1 is working as expected.\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"2a01722cfa42772b584e195eaebf4078e0cc5be2"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Christos Nasikas <christos.nasikas@elastic.co>
This commit is contained in:
parent
ccaf01ed71
commit
8bdecd6641
2 changed files with 191 additions and 38 deletions
|
@ -0,0 +1,145 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the "Elastic License
|
||||
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
|
||||
* Public License v 1"; you may not use this file except in compliance with, at
|
||||
* your election, the "Elastic License 2.0", the "GNU Affero General Public
|
||||
* License v3.0 only", or the "Server Side Public License, v 1".
|
||||
*/
|
||||
|
||||
import { omit } from 'lodash';
|
||||
import { logThresholdParamsSchema } from './v1';
|
||||
|
||||
describe('logThresholdParamsSchema', () => {
|
||||
const base = {
|
||||
timeSize: 8,
|
||||
timeUnit: 'h',
|
||||
count: {
|
||||
value: 1,
|
||||
comparator: 'more than',
|
||||
},
|
||||
logView: {
|
||||
logViewId: 'log-view-reference-0',
|
||||
type: 'log-view-reference',
|
||||
},
|
||||
groupBy: ['geo.dest'],
|
||||
};
|
||||
|
||||
const countParams = {
|
||||
...base,
|
||||
criteria: [
|
||||
{
|
||||
field: 'bytes',
|
||||
comparator: 'more than',
|
||||
value: 1,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const ratioParams = {
|
||||
...base,
|
||||
criteria: [
|
||||
[
|
||||
{
|
||||
field: 'bytes',
|
||||
comparator: 'more than',
|
||||
value: 1,
|
||||
},
|
||||
],
|
||||
],
|
||||
};
|
||||
|
||||
const countParamsWithExcess = {
|
||||
...countParams,
|
||||
outputIndex: '.alerts-observability.logs.alerts-default',
|
||||
};
|
||||
|
||||
const ratioParamsWithExcess = {
|
||||
...ratioParams,
|
||||
outputIndex: '.alerts-observability.logs.alerts-default',
|
||||
};
|
||||
|
||||
it('validates count params correctly', () => {
|
||||
expect(() => logThresholdParamsSchema.validate(countParams)).not.toThrow();
|
||||
});
|
||||
|
||||
it('validates ratio params correctly', () => {
|
||||
expect(() => logThresholdParamsSchema.validate(ratioParams)).not.toThrow();
|
||||
});
|
||||
|
||||
it('does not throw with excess fields in count params', () => {
|
||||
const result = logThresholdParamsSchema.validate(countParamsWithExcess);
|
||||
expect(result).toEqual(omit(countParamsWithExcess, 'outputIndex'));
|
||||
});
|
||||
|
||||
it('does not throw with excess fields in ration params', () => {
|
||||
const result = logThresholdParamsSchema.validate(ratioParamsWithExcess);
|
||||
expect(result).toEqual(omit(ratioParamsWithExcess, 'outputIndex'));
|
||||
});
|
||||
|
||||
it('strips out excess fields in count params', () => {
|
||||
const result = logThresholdParamsSchema.validate(countParamsWithExcess);
|
||||
expect(result).toEqual(omit(countParamsWithExcess, 'outputIndex'));
|
||||
});
|
||||
|
||||
it('strips out excess fields in ratio params', () => {
|
||||
const result = logThresholdParamsSchema.validate(ratioParamsWithExcess);
|
||||
expect(result).toEqual(omit(ratioParamsWithExcess, 'outputIndex'));
|
||||
});
|
||||
|
||||
it.each(['criteria', 'count', 'timeUnit', 'timeSize', 'logView'])(
|
||||
'fails without %s required field in count params',
|
||||
(field) => {
|
||||
expect(() => logThresholdParamsSchema.validate(omit(countParams, field))).toThrow();
|
||||
}
|
||||
);
|
||||
|
||||
it.each(['groupBy'])('does not fail without %s optional field in count params', (field) => {
|
||||
expect(() => logThresholdParamsSchema.validate(omit(countParams, field))).not.toThrow();
|
||||
});
|
||||
|
||||
it.each(['criteria', 'count', 'timeUnit', 'timeSize', 'logView'])(
|
||||
'fails without %s required field in ratio params',
|
||||
(field) => {
|
||||
expect(() => logThresholdParamsSchema.validate(omit(ratioParams, field))).toThrow();
|
||||
}
|
||||
);
|
||||
|
||||
it.each(['groupBy'])('does not fail without %s optional field in ratio params', (field) => {
|
||||
expect(() => logThresholdParamsSchema.validate(omit(ratioParams, field))).not.toThrow();
|
||||
});
|
||||
|
||||
it('trips out excess fields in logView', () => {
|
||||
expect(() =>
|
||||
logThresholdParamsSchema.validate({
|
||||
...countParams,
|
||||
logView: { ...countParams.logView, excessField: 'foo' },
|
||||
})
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('trips out excess fields in threshold', () => {
|
||||
expect(() =>
|
||||
logThresholdParamsSchema.validate({
|
||||
...countParams,
|
||||
count: { ...countParams.count, excessField: 'foo' },
|
||||
})
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it('trips out excess fields in criteria', () => {
|
||||
expect(() =>
|
||||
logThresholdParamsSchema.validate({
|
||||
...countParams,
|
||||
criteria: [
|
||||
{
|
||||
field: 'bytes',
|
||||
comparator: 'more than',
|
||||
value: 1,
|
||||
excessField: 'foo',
|
||||
},
|
||||
],
|
||||
})
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
|
@ -9,10 +9,13 @@
|
|||
|
||||
import { schema } from '@kbn/config-schema';
|
||||
|
||||
const persistedLogViewReferenceSchema = schema.object({
|
||||
logViewId: schema.string(),
|
||||
type: schema.literal('log-view-reference'),
|
||||
});
|
||||
const persistedLogViewReferenceSchema = schema.object(
|
||||
{
|
||||
logViewId: schema.string(),
|
||||
type: schema.literal('log-view-reference'),
|
||||
},
|
||||
{ unknowns: 'ignore' }
|
||||
);
|
||||
|
||||
// Comparators //
|
||||
enum Comparator {
|
||||
|
@ -41,16 +44,22 @@ const ComparatorSchema = schema.oneOf([
|
|||
schema.literal(Comparator.NOT_MATCH_PHRASE),
|
||||
]);
|
||||
|
||||
const ThresholdSchema = schema.object({
|
||||
comparator: ComparatorSchema,
|
||||
value: schema.number(),
|
||||
});
|
||||
const ThresholdSchema = schema.object(
|
||||
{
|
||||
comparator: ComparatorSchema,
|
||||
value: schema.number(),
|
||||
},
|
||||
{ unknowns: 'ignore' }
|
||||
);
|
||||
|
||||
const criterionSchema = schema.object({
|
||||
field: schema.string(),
|
||||
comparator: ComparatorSchema,
|
||||
value: schema.oneOf([schema.string(), schema.number()]),
|
||||
});
|
||||
const criterionSchema = schema.object(
|
||||
{
|
||||
field: schema.string(),
|
||||
comparator: ComparatorSchema,
|
||||
value: schema.oneOf([schema.string(), schema.number()]),
|
||||
},
|
||||
{ unknowns: 'ignore' }
|
||||
);
|
||||
|
||||
const countCriteriaSchema = schema.arrayOf(criterionSchema);
|
||||
const ratioCriteriaSchema = schema.arrayOf(countCriteriaSchema);
|
||||
|
@ -65,34 +74,33 @@ const timeUnitSchema = schema.oneOf([
|
|||
const timeSizeSchema = schema.number();
|
||||
const groupBySchema = schema.arrayOf(schema.string());
|
||||
|
||||
const RequiredRuleParamsSchema = schema.object({
|
||||
// NOTE: "count" would be better named as "threshold", but this would require a
|
||||
// migration of encrypted saved objects, so we'll keep "count" until it's problematic.
|
||||
count: ThresholdSchema,
|
||||
timeUnit: timeUnitSchema,
|
||||
timeSize: timeSizeSchema,
|
||||
logView: persistedLogViewReferenceSchema, // Alerts are only compatible with persisted Log Views
|
||||
});
|
||||
|
||||
const OptionalRuleParamsSchema = schema.object({
|
||||
groupBy: schema.maybe(groupBySchema),
|
||||
});
|
||||
|
||||
const countRuleParamsSchema = schema.intersection([
|
||||
schema.object({
|
||||
const countRuleParamsSchema = schema.object(
|
||||
{
|
||||
criteria: countCriteriaSchema,
|
||||
}),
|
||||
RequiredRuleParamsSchema,
|
||||
OptionalRuleParamsSchema,
|
||||
]);
|
||||
// NOTE: "count" would be better named as "threshold", but this would require a
|
||||
// migration of encrypted saved objects, so we'll keep "count" until it's problematic.
|
||||
count: ThresholdSchema,
|
||||
timeUnit: timeUnitSchema,
|
||||
timeSize: timeSizeSchema,
|
||||
logView: persistedLogViewReferenceSchema, // Alerts are only compatible with persisted Log Views
|
||||
groupBy: schema.maybe(groupBySchema),
|
||||
},
|
||||
{ unknowns: 'ignore' }
|
||||
);
|
||||
|
||||
const ratioRuleParamsSchema = schema.intersection([
|
||||
schema.object({
|
||||
const ratioRuleParamsSchema = schema.object(
|
||||
{
|
||||
criteria: ratioCriteriaSchema,
|
||||
}),
|
||||
RequiredRuleParamsSchema,
|
||||
OptionalRuleParamsSchema,
|
||||
]);
|
||||
// NOTE: "count" would be better named as "threshold", but this would require a
|
||||
// migration of encrypted saved objects, so we'll keep "count" until it's problematic.
|
||||
count: ThresholdSchema,
|
||||
timeUnit: timeUnitSchema,
|
||||
timeSize: timeSizeSchema,
|
||||
logView: persistedLogViewReferenceSchema, // Alerts are only compatible with persisted Log Views
|
||||
groupBy: schema.maybe(groupBySchema),
|
||||
},
|
||||
{ unknowns: 'ignore' }
|
||||
);
|
||||
|
||||
export const logThresholdParamsSchema = schema.oneOf([
|
||||
countRuleParamsSchema,
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue