Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Replace PrebuiltRuleAsset schema construction with Zod transform #188092

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented Jul 11, 2024

Summary

Pending work from: #186615

  • The previous implementation to create PrebuiltRuleAsset with some RuleResponse fields ommited from it had the disadvantage of being built with a discriminated union where all rule types had to be re-listed. If a new type was added, then it would have required manually adding the type to that union as well, which would have been surely forgotten.
  • This replaces that schema construction to use a Zod transform which simply eliminates the omitted fields using a Zod transform.

For maintainers

@jpdjere
Copy link
Contributor Author

jpdjere commented Jul 11, 2024

/ci

@jpdjere jpdjere self-assigned this Jul 11, 2024
@jpdjere jpdjere added release_note:skip Skip the PR/issue when compiling release notes refactoring backport:skip This commit does not require backporting Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team v8.16.0 labels Jul 11, 2024
@elasticmachine
Copy link
Contributor

elasticmachine commented Jul 11, 2024

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, output_index, threat] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, output_index, threat] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, version] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, version] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, note, version, and empty exceptions_list] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, note, version, and empty exceptions_list] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and exceptions_list] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and exceptions_list] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and non-existent exceptions_list] does validate with empty exceptions_list
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list [rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and non-existent exceptions_list] does validate with empty exceptions_list
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and invalid exceptions_list] does NOT validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema exception_list rule_id, description, from, to, index, name, severity, interval, type, filters, risk_score, note, version, and invalid exceptions_list] does NOT validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema note [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, note] does validate
  • [job] [logs] Jest Tests #15 / Prebuilt rule asset schema note [rule_id, description, from, to, index, name, severity, interval, type, filter, risk_score, note] does validate

History

cc @jpdjere

@jpdjere jpdjere marked this pull request as ready for review July 11, 2024 13:58
@jpdjere jpdjere requested a review from a team as a code owner July 11, 2024 13:58
@jpdjere jpdjere requested a review from dplumlee July 11, 2024 13:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@jpdjere
Copy link
Contributor Author

jpdjere commented Jul 11, 2024

/ci

@jpdjere jpdjere requested review from banderror and removed request for dplumlee July 11, 2024 13:59
@banderror banderror removed the backport:skip This commit does not require backporting label Jul 12, 2024
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Comment on lines +39 to +52
const TypeSpecificFields = TypeSpecificCreateProps.transform((val) => {
switch (val.type) {
case 'query': {
const { response_actions: _, ...rest } = val;
return rest;
}
case 'saved_query': {
const { response_actions: _, ...rest } = val;
return rest;
}
default:
return val;
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool, this is much simpler than reconstructing type specific props from scratch. And it even creates a correct type for TypeSpecificFields which I didn't expect! Also it's extensible: this way we could omit any other fields from other rule types. Very-very cool!

@jpdjere jpdjere added the backport:skip This commit does not require backporting label Jul 12, 2024
@jpdjere jpdjere merged commit ccfdd69 into elastic:main Jul 12, 2024
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting refactoring release_note:skip Skip the PR/issue when compiling release notes Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants