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

[Fleet] Return empty agents list when submitting a kuery with no keys #93844

Merged
merged 4 commits into from
Mar 9, 2021

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Mar 5, 2021

Summary

Fixes #92853

When submitting a query string that would result in a The key is empty error, the SavedObjects API instead swallows the error and returns no results.

The syntax to actually filter agents by name in the SavedObject system is very complicated and not suggested by autocomplete (fleet-agents.local_metadata.host.hostname), but given that this syntax is going to change in Fleet Server, I'm not sure if it's worth the engineering complexity to convert raw string input to a field lookup.

@Zacqary Zacqary added release_note:fix v8.0.0 Feature:Fleet Fleet team's agent central management project Team:Fleet Team label for Observability Data Collection Fleet team v7.12.0 v7.13.0 labels Mar 5, 2021
@Zacqary Zacqary requested a review from a team as a code owner March 5, 2021 21:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Feature:Fleet)

@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@jen-huang
Copy link
Contributor

jen-huang commented Mar 8, 2021

I think this is a regression coming from where we used to wrap the kuery keys with the SO name. We did this for all our endpoints which accepted kuery:

filters.push(normalizeKuery(AGENT_SAVED_OBJECT_TYPE, kuery));

@nchaulet This looks like it was removed as part of #86179, was this intentional? Is this fix still needed with the incoming changes to Fleet server (since we move to ES indices instead of SOs, but do we need to keep SO kuerying for migration)?

Edit: I think we should put this wrapping back at least for 7.12, regardless of what happens with Fleet server in 7.13.

@nchaulet
Copy link
Member

nchaulet commented Mar 9, 2021

We are still be doing the normalizeKuery part in 7.12, not in 7.13 as there is no saved object anymore.

I guess the error was always present but before 7.11 we were swallowing errors happening when we fetch agents, if you search for ?kuery=win for example it failed before too

@nchaulet
Copy link
Member

nchaulet commented Mar 9, 2021

@Zacqary I am wondering if it's better to handle 400 errors client side here and swallow the error, or mark the search bar as invalid as there is a user error and the user can potentially fix that kuery

@Zacqary
Copy link
Contributor Author

Zacqary commented Mar 9, 2021

@nchaulet In 7.13 a query without keys doesn't throw a 400 error at all, now that it's not querying SavedObjects. This PR, as-is, would replicate that same behavior in 7.12.

Do we want to change 7.13 so that it throws a 400 error for a keyless query, and handle that on the client side by turning the search bar red?

@nchaulet
Copy link
Member

nchaulet commented Mar 9, 2021

@nchaulet In 7.13 a query without keys doesn't throw a 400 error at all, now that it's not querying SavedObjects. This PR, as-is, would replicate that same behavior in 7.12.

I think the fix is correct for 7.12 👍

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

🚀

Zacqary added 3 commits March 9, 2021 13:12
…-emptykey

# Conflicts:
#	x-pack/test/fleet_api_integration/apis/agents/list.ts
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Zacqary

@Zacqary Zacqary merged commit 5d119cf into elastic:master Mar 9, 2021
@Zacqary Zacqary deleted the 92853-so-suppress-emptykey branch March 9, 2021 22:10
Zacqary added a commit that referenced this pull request Mar 11, 2021
…no keys (#93844) (#94218)

* [Fleet] Return empty agents list when submitting a kuery with no keys (#93844)

* Fix test merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Fleet Fleet team's agent central management project release_note:fix Team:Fleet Team label for Observability Data Collection Fleet team v7.12.0 v7.13.0 v8.0.0
Projects
None yet
5 participants