Skip to content

Commit

Permalink
[RAM] Rule API Type Versioning POC - Create Rule (#158786)
Browse files Browse the repository at this point in the history
## Summary
Resolves: #157883

# Summary:
This PR acts as a POC for how we would want to version our rule API
types in preparation for the versioning of our HTTP endpoints.

There is now an ES persisted type (called `RuleAttributes`, akin to the
old `RawRule` type). This type should never be used at the business
logic level as we want the ability not to reference saved object
attributes directly in the application. Instead, we should transform
this saved-object to its corresponding domain object type.

HTTP types (`CreateBodyParams`, `RuleResponse`) are now located in
`x-pack/plugins/alerting/common/routes/rule` with a versioning structure
like:


![image](https://github.com/elastic/kibana/assets/74562234/1c8e886d-8983-40f2-9490-aa3864898535)

And follows the guideline here:
https://docs.elastic.dev/kibana-dev-docs/versioning-interfaces

Domain object types (rule for example) are derived from the
`config-schema` schemas using `TypeOf`, this was done to facilitate the
reuse of validation schemas that we might want to run for strict IO
validation, potentially at the `rulesClient` level.

## API:
Only the `createRule` endpoint has been converted for the sake of this
POC, other endpoints might have broken types that will be fixed once we
have a finalized pattern for dealing with versioned types.

At the API route level, I think it would be wise to import versioned
types in our APIs, this way, it forces the developer to deal with broken
types when we have a breaking change to our rule domain object.

The API route level is also responsible for transforming domain objects
to response types, usually, this just entails renaming property names
from camel case to snake case.

in `create_rule_route.ts`

```ts
import type { CreateRuleRequestBodyV1, CreateRuleRequestParamsV1 } from '../../../../common/routes/rule/create';
import type { RuleParamsV1 } from '../../../../common/routes/rule/rule_response';

...

// In the Handler:
const rulesClient = (await context.alerting).getRulesClient();

const createRuleData: CreateRuleRequestBodyV1<RuleParamsV1> = req.body;
const params: CreateRuleRequestParamsV1 = req.params;

const createdRule: Rule<RuleParamsV1> = (await rulesClient.create<RuleParamsV1>({
  data: transformCreateBodyV1<RuleParamsV1>(createRuleData),
  options: { id: params?.id },
})) as Rule<RuleParamsV1>;

const response: CreateRuleResponseV1<RuleParamsV1> = {
  body: transformRuleToRuleResponseV1<RuleParamsV1>(createdRule),
};

return res.ok(response);
```

### RulesClient -> Create
At the rules client level, we now need to transform saved-objects to the
domain objects after we perform CRUD operations on them. We can also
consider validating schemas here too since other solutions are using our
rules client directly instead of through the API.

I don't think we need to version rules client input and output types
since the route level types should deal with breaking changes already.
Therefore, we can freely import the latest of any domain object types
(Rule, for example)

```ts
import { Rule, RuleDomain, RuleParams } from '../types';
```

The flow is the following:
```
1. calling rulesClient.create() with API body and params.
2. perform schema validation on params
3. perform other validation not achievable using config-schema
4. perform rule create -> returns a `savedObject` type
5. convert `savedObject` ES type to the domain object type (`RuleDomain`)
6. convert the domain object type to a public domain object type (`Rule`)
7. We can also validate the created rule using config-schema
```

# Open questions:
- Should we validate input params at both the route and the rules client
level? I think we should since our rules client is shared.

- How do we want to version and validate rule params? Right now they're
typed as generics.

- Should we leave all date fields as `string`, even for domain objects?
Since config-schema is unable to define a `Date` type.

- Should we support partial rule domain object types at all? I can't
really think why we should even for `updates`, since we need to
reconstruct the overall rule to send back to the client.

- Should we have `undefined | null` types in the domain object? I know
we need `null` specifically for the ES types since there is a different
between updating a field as `undefined` or `null`, but this shouldn't
manifest itself in the business logic.

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
JiaweiWu and kibanamachine authored Jul 5, 2023
1 parent 9e52f70 commit 8de68af
Show file tree
Hide file tree
Showing 70 changed files with 2,951 additions and 460 deletions.
40 changes: 40 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/create/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export {
notifyWhenSchema,
actionFrequencySchema,
actionAlertsFilterSchema,
actionSchema,
createParamsSchema,
createBodySchema,
} from './schemas/latest';

export type {
CreateRuleAction,
CreateRuleActionFrequency,
CreateRuleRequestParams,
CreateRuleRequestBody,
CreateRuleResponse,
} from './types/latest';

export {
notifyWhenSchema as notifyWhenSchemaV1,
actionFrequencySchema as actionFrequencySchemaV1,
actionAlertsFilterSchema as actionAlertsFilterSchemaV1,
actionSchema as actionSchemaV1,
createParamsSchema as createParamsSchemaV1,
createBodySchema as createBodySchemaV1,
} from './schemas/v1';

export type {
CreateRuleAction as CreateRuleActionV1,
CreateRuleActionFrequency as CreateRuleActionFrequencyV1,
CreateRuleRequestParams as CreateRuleRequestParamsV1,
CreateRuleRequestBody as CreateRuleRequestBodyV1,
CreateRuleResponse as CreateRuleResponseV1,
} from './types/v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
99 changes: 99 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/create/schemas/v1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';
import { ruleNotifyWhenV1 } from '../../rule_response';
import {
validateNotifyWhenV1,
validateDurationV1,
validateHoursV1,
validateTimezoneV1,
} from '../../validation';

export const notifyWhenSchema = schema.oneOf(
[
schema.literal(ruleNotifyWhenV1.CHANGE),
schema.literal(ruleNotifyWhenV1.ACTIVE),
schema.literal(ruleNotifyWhenV1.THROTTLE),
],
{ validate: validateNotifyWhenV1 }
);

export const actionFrequencySchema = schema.object({
summary: schema.boolean(),
notify_when: notifyWhenSchema,
throttle: schema.nullable(schema.string({ validate: validateDurationV1 })),
});

export const actionAlertsFilterSchema = schema.object({
query: schema.maybe(
schema.object({
kql: schema.string(),
filters: schema.arrayOf(
schema.object({
query: schema.maybe(schema.recordOf(schema.string(), schema.any())),
meta: schema.recordOf(schema.string(), schema.any()),
state$: schema.maybe(schema.object({ store: schema.string() })),
})
),
dsl: schema.maybe(schema.string()),
})
),
timeframe: schema.maybe(
schema.object({
days: schema.arrayOf(
schema.oneOf([
schema.literal(1),
schema.literal(2),
schema.literal(3),
schema.literal(4),
schema.literal(5),
schema.literal(6),
schema.literal(7),
])
),
hours: schema.object({
start: schema.string({
validate: validateHoursV1,
}),
end: schema.string({
validate: validateHoursV1,
}),
}),
timezone: schema.string({ validate: validateTimezoneV1 }),
})
),
});

export const actionSchema = schema.object({
uuid: schema.maybe(schema.string()),
group: schema.string(),
id: schema.string(),
actionTypeId: schema.maybe(schema.string()),
params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }),
frequency: schema.maybe(actionFrequencySchema),
alerts_filter: schema.maybe(actionAlertsFilterSchema),
});

export const createBodySchema = schema.object({
name: schema.string(),
rule_type_id: schema.string(),
enabled: schema.boolean({ defaultValue: true }),
consumer: schema.string(),
tags: schema.arrayOf(schema.string(), { defaultValue: [] }),
throttle: schema.maybe(schema.nullable(schema.string({ validate: validateDurationV1 }))),
params: schema.recordOf(schema.string(), schema.maybe(schema.any()), { defaultValue: {} }),
schedule: schema.object({
interval: schema.string({ validate: validateDurationV1 }),
}),
actions: schema.arrayOf(actionSchema, { defaultValue: [] }),
notify_when: schema.maybe(schema.nullable(notifyWhenSchema)),
});

export const createParamsSchema = schema.object({
id: schema.maybe(schema.string()),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
37 changes: 37 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/create/types/v1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import type { TypeOf } from '@kbn/config-schema';
import { RuleParamsV1, RuleResponseV1 } from '../../rule_response';
import {
actionSchema as actionSchemaV1,
actionFrequencySchema as actionFrequencySchemaV1,
createParamsSchema as createParamsSchemaV1,
createBodySchema as createBodySchemaV1,
} from '..';

export type CreateRuleAction = TypeOf<typeof actionSchemaV1>;
export type CreateRuleActionFrequency = TypeOf<typeof actionFrequencySchemaV1>;

export type CreateRuleRequestParams = TypeOf<typeof createParamsSchemaV1>;
type CreateBodySchema = TypeOf<typeof createBodySchemaV1>;

export interface CreateRuleRequestBody<Params extends RuleParamsV1 = never> {
name: CreateBodySchema['name'];
rule_type_id: CreateBodySchema['rule_type_id'];
enabled: CreateBodySchema['enabled'];
consumer: CreateBodySchema['consumer'];
tags: CreateBodySchema['tags'];
throttle?: CreateBodySchema['throttle'];
params: Params;
schedule: CreateBodySchema['schedule'];
actions: CreateBodySchema['actions'];
notify_when?: CreateBodySchema['notify_when'];
}

export interface CreateRuleResponse<Params extends RuleParamsV1 = never> {
body: RuleResponseV1<Params>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export const ruleNotifyWhen = {
CHANGE: 'onActionGroupChange',
ACTIVE: 'onActiveAlert',
THROTTLE: 'onThrottleInterval',
} as const;

export const ruleLastRunOutcomeValues = {
SUCCEEDED: 'succeeded',
WARNING: 'warning',
FAILED: 'failed',
} as const;

export const ruleExecutionStatusValues = {
OK: 'ok',
ACTIVE: 'active',
ERROR: 'error',
WARNING: 'warning',
PENDING: 'pending',
UNKNOWN: 'unknown',
} as const;

export const ruleExecutionStatusErrorReason = {
READ: 'read',
DECRYPT: 'decrypt',
EXECUTE: 'execute',
UNKNOWN: 'unknown',
LICENSE: 'license',
TIMEOUT: 'timeout',
DISABLED: 'disabled',
VALIDATE: 'validate',
} as const;

export const ruleExecutionStatusWarningReason = {
MAX_EXECUTABLE_ACTIONS: 'maxExecutableActions',
MAX_ALERTS: 'maxAlerts',
} as const;

export type RuleNotifyWhen = typeof ruleNotifyWhen[keyof typeof ruleNotifyWhen];
export type RuleLastRunOutcomeValues =
typeof ruleLastRunOutcomeValues[keyof typeof ruleLastRunOutcomeValues];
export type RuleExecutionStatusValues =
typeof ruleExecutionStatusValues[keyof typeof ruleExecutionStatusValues];
export type RuleExecutionStatusErrorReason =
typeof ruleExecutionStatusErrorReason[keyof typeof ruleExecutionStatusErrorReason];
export type RuleExecutionStatusWarningReason =
typeof ruleExecutionStatusWarningReason[keyof typeof ruleExecutionStatusWarningReason];
64 changes: 64 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/rule_response/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export {
ruleParamsSchema,
actionParamsSchema,
mappedParamsSchema,
ruleExecutionStatusSchema,
ruleLastRunSchema,
monitoringSchema,
rRuleSchema,
ruleResponseSchema,
} from './schemas/latest';

export type { RuleParams, RuleResponse } from './types/latest';

export {
ruleNotifyWhen,
ruleLastRunOutcomeValues,
ruleExecutionStatusValues,
ruleExecutionStatusErrorReason,
ruleExecutionStatusWarningReason,
} from './constants/latest';

export type {
RuleNotifyWhen,
RuleLastRunOutcomeValues,
RuleExecutionStatusValues,
RuleExecutionStatusErrorReason,
RuleExecutionStatusWarningReason,
} from './constants/latest';

export {
ruleParamsSchema as ruleParamsSchemaV1,
actionParamsSchema as actionParamsSchemaV1,
mappedParamsSchema as mappedParamsSchemaV1,
ruleExecutionStatusSchema as ruleExecutionStatusSchemaV1,
ruleLastRunSchema as ruleLastRunSchemaV1,
monitoringSchema as monitoringSchemaV1,
rRuleSchema as rRuleSchemaV1,
ruleResponseSchema as ruleResponseSchemaV1,
} from './schemas/v1';

export {
ruleNotifyWhen as ruleNotifyWhenV1,
ruleLastRunOutcomeValues as ruleLastRunOutcomeValuesV1,
ruleExecutionStatusValues as ruleExecutionStatusValuesV1,
ruleExecutionStatusErrorReason as ruleExecutionStatusErrorReasonV1,
ruleExecutionStatusWarningReason as ruleExecutionStatusWarningReasonV1,
} from './constants/v1';

export type {
RuleNotifyWhen as RuleNotifyWhenV1,
RuleLastRunOutcomeValues as RuleLastRunOutcomeValuesV1,
RuleExecutionStatusValues as RuleExecutionStatusValuesV1,
RuleExecutionStatusErrorReason as RuleExecutionStatusErrorReasonV1,
RuleExecutionStatusWarningReason as RuleExecutionStatusWarningReasonV1,
} from './constants/v1';

export type { RuleParams as RuleParamsV1, RuleResponse as RuleResponseV1 } from './types/v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
Loading

0 comments on commit 8de68af

Please sign in to comment.