[Security Solution] Initial migration of API endpoints to OpenAPI and code generation (#164482)

**Part of: https://github.com/elastic/security-team/issues/6726**

## Summary

Migrates the prebuilt rules and timelines status API route schema to
OpenAPI. This is exploratory work to assess the level of effort required
to migrate API route schemas from `io-ts` to `zod` generated by OpenAPI
codegen.

**Summary of the changes:**

- Added a CI job that runs code generation in Security Solution and
comments change if there are any.
- Migrated the `/api/detection_engine/rules/prepackaged/_status` route
to use generated `zod` schemas
- Updated schema tests
- Adjusted the code generator templates to handle `strict` schemas,
i.e., schemas that do not allow any extra params
- Updated the error transformation code to work with zod errors.
Validation errors are converted to string representations, like the
following:
<img width="627" alt="image"
src="93002573-972f-42e1-901d-01a19937f568">
This commit is contained in:
Dmitrii Shevchenko 2023-08-25 20:01:31 +02:00 committed by GitHub
parent 168e3dc5e9
commit 2171ecc719
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 159 additions and 95 deletions

View file

@ -11,7 +11,7 @@ steps:
- exit_status: '*'
limit: 1
artifact_paths:
- "target/kibana-security-solution/**/*"
- 'target/kibana-security-solution/**/*'
- command: .buildkite/scripts/steps/functional/security_solution_explore.sh
label: 'Explore - Security Solution Cypress Tests'
@ -25,7 +25,7 @@ steps:
- exit_status: '*'
limit: 1
artifact_paths:
- "target/kibana-security-solution/**/*"
- 'target/kibana-security-solution/**/*'
- command: .buildkite/scripts/steps/functional/security_solution_investigations.sh
label: 'Investigations - Security Solution Cypress Tests'
@ -39,7 +39,7 @@ steps:
- exit_status: '*'
limit: 1
artifact_paths:
- "target/kibana-security-solution/**/*"
- 'target/kibana-security-solution/**/*'
- command: .buildkite/scripts/steps/functional/security_solution_burn.sh
label: 'Security Solution Cypress tests, burning changed specs'
@ -52,4 +52,11 @@ steps:
automatic: false
soft_fail: true
artifact_paths:
- "target/kibana-security-solution/**/*"
- 'target/kibana-security-solution/**/*'
- command: .buildkite/scripts/steps/code_generation/security_solution_codegen.sh
label: 'Security Solution OpenAPI codegen'
agents:
queue: n2-2-spot
timeout_in_minutes: 60
parallelism: 1

View file

@ -31,7 +31,7 @@ check_for_changed_files() {
SHOULD_AUTO_COMMIT_CHANGES="${2:-}"
CUSTOM_FIX_MESSAGE="${3:-}"
GIT_CHANGES="$(git ls-files --modified -- . ':!:.bazelrc')"
GIT_CHANGES="$(git status --porcelain -- . ':!:.bazelrc')"
if [ "$GIT_CHANGES" ]; then
if ! is_auto_commit_disabled && [[ "$SHOULD_AUTO_COMMIT_CHANGES" == "true" && "${BUILDKITE_PULL_REQUEST:-}" ]]; then
@ -54,7 +54,7 @@ check_for_changed_files() {
git config --global user.name kibanamachine
git config --global user.email '42973632+kibanamachine@users.noreply.github.com'
gh pr checkout "${BUILDKITE_PULL_REQUEST}"
git add -u -- . ':!.bazelrc'
git add -A -- . ':!.bazelrc'
git commit -m "$NEW_COMMIT_MESSAGE"
git push

View file

@ -0,0 +1,12 @@
#!/usr/bin/env bash
set -euo pipefail
source .buildkite/scripts/common/util.sh
.buildkite/scripts/bootstrap.sh
echo --- Security Solution OpenAPI Code Generation
(cd x-pack/plugins/security_solution && yarn openapi:generate)
check_for_changed_files "yarn openapi:generate" true

View file

@ -8,6 +8,7 @@
import Boom from '@hapi/boom';
import { errors } from '@elastic/elasticsearch';
import { ZodError } from 'zod';
import { BadRequestError } from '../bad_request_error';
export interface OutputError {
@ -21,6 +22,15 @@ export const transformError = (err: Error & Partial<errors.ResponseError>): Outp
message: err.output.payload.message,
statusCode: err.output.statusCode,
};
} else if (err instanceof ZodError) {
const message = stringifyZodError(err);
return {
message,
// These errors can occur when handling requests after validation and can
// indicate of issues in business logic, so they are 500s instead of 400s
statusCode: 500,
};
} else {
if (err.statusCode != null) {
if (err.body != null && err.body.error != null) {
@ -50,3 +60,15 @@ export const transformError = (err: Error & Partial<errors.ResponseError>): Outp
}
}
};
export function stringifyZodError(err: ZodError<any>) {
return err.issues
.map((issue) => {
// If the path is empty, the error is for the root object
if (issue.path.length === 0) {
return issue.message;
}
return `${issue.path.join('.')}: ${issue.message}`;
})
.join(', ');
}

View file

@ -0,0 +1,49 @@
/*
* 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 { z } from 'zod';
/*
* NOTICE: Do not edit this file manually.
* This file is automatically generated by the OpenAPI Generator `yarn openapi:generate`.
*/
export type GetPrebuiltRulesAndTimelinesStatusResponse = z.infer<
typeof GetPrebuiltRulesAndTimelinesStatusResponse
>;
export const GetPrebuiltRulesAndTimelinesStatusResponse = z
.object({
/**
* The total number of custom rules
*/
rules_custom_installed: z.number().min(0),
/**
* The total number of installed prebuilt rules
*/
rules_installed: z.number().min(0),
/**
* The total number of available prebuilt rules that are not installed
*/
rules_not_installed: z.number().min(0),
/**
* The total number of outdated prebuilt rules
*/
rules_not_updated: z.number().min(0),
/**
* The total number of installed prebuilt timelines
*/
timelines_installed: z.number().min(0),
/**
* The total number of available prebuilt timelines that are not installed
*/
timelines_not_installed: z.number().min(0),
/**
* The total number of outdated prebuilt timelines
*/
timelines_not_updated: z.number().min(0),
})
.strict();

View file

@ -5,8 +5,8 @@ info:
paths:
/api/detection_engine/rules/prepackaged/_status:
get:
operationId: GetPrebuiltRulesStatus
x-codegen-enabled: false
operationId: GetPrebuiltRulesAndTimelinesStatus
x-codegen-enabled: true
summary: Get the status of Elastic prebuilt rules
tags:
- Prebuilt Rules API
@ -17,6 +17,7 @@ paths:
application/json:
schema:
type: object
additionalProperties: false
properties:
rules_custom_installed:
type: integer

View file

@ -5,11 +5,9 @@
* 2.0.
*/
import { left } from 'fp-ts/lib/Either';
import { pipe } from 'fp-ts/lib/pipeable';
import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts-utils';
import { GetPrebuiltRulesAndTimelinesStatusResponse } from './get_prebuilt_rules_and_timelines_status_route';
import { stringifyZodError } from '@kbn/securitysolution-es-utils';
import { expectParseError, expectParseSuccess } from '../../../../test/zod_helpers';
import { GetPrebuiltRulesAndTimelinesStatusResponse } from './get_prebuilt_rules_and_timelines_status_route.gen';
describe('Get prebuilt rules and timelines status response schema', () => {
test('it should validate an empty prepackaged response with defaults', () => {
@ -22,12 +20,10 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([]);
expect(message.schema).toEqual(payload);
expectParseSuccess(result);
expect(result.data).toEqual(payload);
});
test('it should not validate an extra invalid field added', () => {
@ -41,12 +37,12 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual(['invalid keys "invalid_field"']);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual(
"Unrecognized key(s) in object: 'invalid_field'"
);
});
test('it should NOT validate an empty prepackaged response with a negative "rules_installed" number', () => {
@ -59,14 +55,12 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "rules_installed"',
]);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual(
'rules_installed: Number must be greater than or equal to 0'
);
});
test('it should NOT validate an empty prepackaged response with a negative "rules_not_installed"', () => {
@ -79,14 +73,12 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "rules_not_installed"',
]);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual(
'rules_not_installed: Number must be greater than or equal to 0'
);
});
test('it should NOT validate an empty prepackaged response with a negative "rules_not_updated"', () => {
@ -99,14 +91,12 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "rules_not_updated"',
]);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual(
'rules_not_updated: Number must be greater than or equal to 0'
);
});
test('it should NOT validate an empty prepackaged response with a negative "rules_custom_installed"', () => {
@ -119,14 +109,12 @@ describe('Get prebuilt rules and timelines status response schema', () => {
timelines_not_installed: 0,
timelines_not_updated: 0,
};
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "-1" supplied to "rules_custom_installed"',
]);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual(
'rules_custom_installed: Number must be greater than or equal to 0'
);
});
test('it should NOT validate an empty prepackaged response if "rules_installed" is not there', () => {
@ -141,13 +129,9 @@ describe('Get prebuilt rules and timelines status response schema', () => {
};
// @ts-expect-error
delete payload.rules_installed;
const decoded = GetPrebuiltRulesAndTimelinesStatusResponse.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
const result = GetPrebuiltRulesAndTimelinesStatusResponse.safeParse(payload);
expect(getPaths(left(message.errors))).toEqual([
'Invalid value "undefined" supplied to "rules_installed"',
]);
expect(message.schema).toEqual({});
expectParseError(result);
expect(stringifyZodError(result.error)).toEqual('rules_installed: Required');
});
});

View file

@ -1,25 +0,0 @@
/*
* 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 * as t from 'io-ts';
import { PositiveInteger } from '@kbn/securitysolution-io-ts-types';
export type GetPrebuiltRulesAndTimelinesStatusResponse = t.TypeOf<
typeof GetPrebuiltRulesAndTimelinesStatusResponse
>;
export const GetPrebuiltRulesAndTimelinesStatusResponse = t.exact(
t.type({
rules_custom_installed: PositiveInteger,
rules_installed: PositiveInteger,
rules_not_installed: PositiveInteger,
rules_not_updated: PositiveInteger,
timelines_installed: PositiveInteger,
timelines_not_installed: PositiveInteger,
timelines_not_updated: PositiveInteger,
})
);

View file

@ -5,7 +5,7 @@
* 2.0.
*/
export * from './get_prebuilt_rules_and_timelines_status/get_prebuilt_rules_and_timelines_status_route';
export * from './get_prebuilt_rules_and_timelines_status/get_prebuilt_rules_and_timelines_status_route.gen';
export * from './get_prebuilt_rules_status/get_prebuilt_rules_status_route';
export * from './install_prebuilt_rules_and_timelines/install_prebuilt_rules_and_timelines_route';
export * from './perform_rule_installation/perform_rule_installation_route';

View file

@ -0,0 +1,20 @@
/*
* 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 { SafeParseError, SafeParseReturnType, SafeParseSuccess } from 'zod';
export function expectParseError<Input, Output>(
result: SafeParseReturnType<Input, Output>
): asserts result is SafeParseError<Input> {
expect(result.success).toEqual(false);
}
export function expectParseSuccess<Input, Output>(
result: SafeParseReturnType<Input, Output>
): asserts result is SafeParseSuccess<Output> {
expect(result.success).toEqual(true);
}

View file

@ -78,6 +78,7 @@ z.unknown()
{{@key}}: {{> schema_item requiredBool=(includes ../required @key)}},
{{/each}}
})
{{#if (eq additionalProperties false)}}.strict(){{/if}}
{{~/inline~}}
{{~#*inline "type_string"~}}

View file

@ -13,8 +13,8 @@ import type { SetupPlugins } from '../../../../../plugin';
import type { SecuritySolutionPluginRouter } from '../../../../../types';
import {
PREBUILT_RULES_STATUS_URL,
GetPrebuiltRulesAndTimelinesStatusResponse,
PREBUILT_RULES_STATUS_URL,
} from '../../../../../../common/api/detection_engine/prebuilt_rules';
import { getExistingPrepackagedRules } from '../../../rule_management/logic/search/get_existing_prepackaged_rules';
@ -89,16 +89,9 @@ export const getPrebuiltRulesAndTimelinesStatusRoute = (
timelines_not_updated: validatedPrebuiltTimelineStatus?.timelinesToUpdate.length ?? 0,
};
const [validatedBody, validationError] = validate(
responseBody,
GetPrebuiltRulesAndTimelinesStatusResponse
);
if (validationError != null) {
return siemResponse.error({ statusCode: 500, body: validationError });
} else {
return response.ok({ body: validatedBody ?? {} });
}
return response.ok({
body: GetPrebuiltRulesAndTimelinesStatusResponse.parse(responseBody),
});
} catch (err) {
const error = transformError(err);
return siemResponse.error({