[Osquery] 7.14 bug squash #105387
[Osquery] 7.14 bug squash #105387
Conversation
lykkin
added
bug
|
Pinging @elastic/security-asset-management (Team:Asset Management) |
x-pack/plugins/osquery/public/action_results/action_results_summary.tsx
Outdated
Show resolved
Hide resolved
| export const useSavedQueryForm = ({ defaultValue, handleSubmit }: UseSavedQueryFormProps) => | ||
| useForm({ | ||
| export const useSavedQueryForm = ({ defaultValue, handleSubmit }: UseSavedQueryFormProps) => { | ||
| const { data } = useSavedQueries({}); |
There was a problem hiding this comment.
can we move it to the backend?
There was a problem hiding this comment.
i like the idea of having it fail validation eagerly, but i also like the idea of having redundant protections. i'll add a check in the backend for both this and the scheduled query endpoint
There was a problem hiding this comment.
i added additional logic to the saved query endpoints, but skipped on the scheduled query endpoints since it seems like they are commented out currently. it seems like all the saved object write logic is done on the frontend anyway, from what i can tell; the saved query ui elements don't actually call into the backend, unless i'm missing something.
There was a problem hiding this comment.
you're right 🤦
maybe let's move the validation fix to 7.15 and in this PR only merge telemetry, expired message, and agents count?
as we're going to introduce new logic for packs in 7.15 maybe we will be able to solve the validation differently
There was a problem hiding this comment.
i'll rip out the backend logic, but i think it's worthwhile to keep the frontend validation in place to avoid the duplicate id problem. we can always rip it out when we implement something more general purpose.
There was a problem hiding this comment.
maybe we could move it to
https://github.com/patrykkopycinski/kibana/blob/feat/cases-lens-markdown-plugin/x-pack/plugins/osquery/public/saved_queries/use_create_saved_query.ts#L40
and
https://github.com/patrykkopycinski/kibana/blob/feat/cases-lens-markdown-plugin/x-pack/plugins/osquery/public/saved_queries/use_update_saved_query.ts#L39
and then we just try to savedObjectsClient.find({'id': yyyy}) and if results are empty then it means that the id is unique, what do you think?
There was a problem hiding this comment.
it makes a lot of sense to me to put defensive logic around the points where the data is created/modified, so that sounds great. i'll add that.
...ugins/osquery/public/scheduled_query_groups/queries/use_scheduled_query_group_query_form.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/osquery/server/routes/saved_query/create_saved_query_route.ts
Outdated
Show resolved
Hide resolved
| id: SAVED_QUERY_FORM_ID + uuid.v4(), | ||
| schema: formSchema, | ||
| onSubmit: handleSubmit, | ||
| onSubmit: async (formData, isValid) => { |
There was a problem hiding this comment.
the forms were submitting even when they were invalid before, so i gated them on an isValid check. let me know if there's a better place to put this logic.
| const ids = useMemo<string[]>( | ||
| () => | ||
| data?.items | ||
| .find((value) => value.id === scheduledQueryGroupId) |
There was a problem hiding this comment.
as we are validating it now against the same scheduledQueryGroupId maybe we could just pass idSet directly to useScheduledQueryGroupQueryForm ?
There was a problem hiding this comment.
currently idSet is only used by the form to validate, the hook retrieving the saved queries doesn't do any of that validation. were you thinking that the hook could be responsible for populating the set and pass it back?
i'm going to remove the find call and pass that into the hook at the least, as that will be less wasteful.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Summary
Fixes:
Telemetry stats now accurately reflect query cardinality in groups + will no longer error when the saved objects go missing.
Saved query and scheduled query IDs must now be unique.
Unhealthy agents will now appear in the live query agent selector, making the list more consistent with the aggregate group numbers.
Added an error message to expired live queries.
