[Security Solution] Replaced the incorrect runtime type used for ruleSource#184004
Merged
xcrzx merged 1 commit intoelastic:mainfrom May 23, 2024
xcrzx:fix-rule-schema
Merged
[Security Solution] Replaced the incorrect runtime type used for ruleSource#184004xcrzx merged 1 commit intoelastic:mainfrom xcrzx:fix-rule-schema
xcrzx merged 1 commit intoelastic:mainfrom
xcrzx:fix-rule-schema
Conversation
Contributor
|
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Contributor
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
Contributor
|
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
nikitaindik
approved these changes
May 22, 2024
Contributor
nikitaindik
left a comment
There was a problem hiding this comment.
Thanks @xcrzx! Changes LGTM.
banderror
reviewed
May 22, 2024
| TimestampOverrideFallbackDisabled, | ||
| } from '../../../../../common/api/detection_engine/model/rule_schema'; | ||
| import type { SERVER_APP_ID } from '../../../../../common/constants'; | ||
| import { convertObjectKeysToCamelCase } from '../../../../utils/object_case_converters'; |
Contributor
There was a problem hiding this comment.
Should we also do something with this utility? For example:
- Add a comment to
convertObjectKeysToCamelCasethat it doesn't work as it was originally intended. Explain how it works. - Create a ticket for reworking it or reopen [Security Solution] Implement a Zod transformation from snake_case to camelCase and vice versa #180121. Add a comment to
convertObjectKeysToCamelCasewith a link to the ticket. - Delete the utility.
Contributor
Author
There was a problem hiding this comment.
This function works as expected. It accepts any object and converts its keys to camel case. It has nothing to do with Zod and can be used (and is used) in other places, so I believe we can leave it as is.
mikecote
approved these changes
May 22, 2024
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @xcrzx |
yctercero
approved these changes
May 22, 2024
89 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Needed for: #180141 and #180128
Summary
This PR replaces the incorrect Zod schema for the
ruleSourcerule param.Previously, the rule source field schema was implemented using a Zod transformation that automatically converted the snake-cased
is_customizedattribute to its camel-cased versionisCustomized.However, this meant that the expected input type for the schema was snake-cased, as the transformation happened only after validation.
Valid payload before:
To overcome this issue, the rule source schema was implemented without using the transformation (revert #180121).
Valid payload after:
Important Note
This rule param schema change is considered safe because we do not currently use this field in the code. All values of this field are currently
undefined. However, to ensure a Serverless release rollout without breaking changes, we need to release this schema change before we start writing any actual data.