Skip to content

Kuery api strict validation#161064

Merged
criamico merged 51 commits intoelastic:mainfrom
criamico:kuery_api_strict_validation
Jul 24, 2023
Merged

Kuery api strict validation#161064
criamico merged 51 commits intoelastic:mainfrom
criamico:kuery_api_strict_validation

Conversation

@criamico
Copy link
Copy Markdown
Member

@criamico criamico commented Jul 3, 2023

Summary

Require validation for endpoints accepting kuery as a parameter in requests.

IMPORTANT: This PR is part of the work needed to prepare the APIs for Serverless.

Some context

The initial idea was to completely remove any KQL queries from being exposed in the endpoints, but after some discussion we came to the agreement that they can stay but need to be validated, so only allowed parameters can be sent. A similar approach is being followed by other teams as well.

Impacted endpoints:

  • GET api/fleet/agents
  • GET api/fleet/agent_status
  • GET api/fleet/agent_policies
  • GET api/fleet/package_policies
  • GET api/fleet/enrollment_api_keys
  • GET api/fleet/agent_status

All these endpoints accept as a parameter ListWithKuery. It was originally being deprecated but it was then decided to keep it and add validation to the endpoints instead.

The endpoint api/fleet/agents/action_status doesn't accept kuery anymore, since it was not being passed internally.

What's changing

The KQL passed to these endpoints will be accepted in two possible formats:

GET kbn:api/fleet/agents?kuery=local_metadata.agent.version="8.8.0"

GET kbn:api/fleet/agents?kuery=fleet-agents.local_metadata.agent.version="8.8.0"

Note that originally only the second format was going to accepted, but we decided to avoid enforcing it as it would introduce a breaking change, possibly breaking many customers automations.

How it works

The code for ValidateFilterKueryNode has been adapted from a similar function already used in Kibana core. I added several tests where with some common queries that are performed in the UI just to be sure that they would pass validation. Additional queries can be validated by these tests in the future.

ValidateFilterKueryNode needs to have the SO or index and a mapping with the parameters to validate against. I copied over the mappings for the necessary entities; if in the future we intend to expose a new mapping parameter in the endpoints, it will be necessary to add it there as well, or the validation will fail.

UI

I also checked that the UI doesn't fail when using the KQL search boxes for Agents, Agent policies and Enrollment keys and made sure that they expose the same values present in the mappings.

Testing

From dev tools, you can test the affected endpoints by entering the following queries:

# agents
GET kbn:api/fleet/agents?kuery=fleet-agents.active:true
GET kbn:api/fleet/agents?kuery=active:true

# tags
GET kbn:api/fleet/agents/tags?kuery=fleet-agents.tags:tag1
GET kbn:api/fleet/agents/tags?kuery=tags:tag1

# agent status
GET kbn:/api/fleet/agent_status?kuery=fleet-agents.policy_id:fleet-server-policy
GET kbn:/api/fleet/agent_status?kuery=policy_id:fleet-server-policy

# package policies
GET kbn:/api/fleet/package_policies?kuery=ingest-package-policies.package.name:fleet_server

# agent policies
GET kbn:/api/fleet/agent_policies?kuery=ingest-agent-policies.name:"Fleet Server Policy"
GET kbn:/api/fleet/agent_policies?kuery=name:"Fleet Server Policy"

# enrollment keys
GET kbn:/api/fleet/enrollment_api_keys?kuery=fleet-enrollment-api-keys.policy_id:policy1
GET kbn:/api/fleet/enrollment_api_keys?kuery=policy1

These should all pass validation; modifying the parameters (for instance with non existing ones) should fail validation

Checklist

@criamico criamico self-assigned this Jul 3, 2023
@ghost
Copy link
Copy Markdown

ghost commented Jul 3, 2023

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@criamico
Copy link
Copy Markdown
Member Author

criamico commented Jul 3, 2023

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

criamico commented Jul 5, 2023

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

merge conflict between base and head

@criamico criamico added the Team:Fleet Team label for Observability Data Collection Fleet team label Jul 13, 2023
@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@criamico
Copy link
Copy Markdown
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
fleet 1066 1067 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
fleet 1012.2KB 1012.1KB -90.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fleet 136.2KB 136.2KB +62.0B
Unknown metric groups

API count

id before after diff
fleet 1182 1183 +1

Unreferenced deprecated APIs

id before after diff
fleet 1 0 -1

History

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

cc @criamico

@criamico criamico merged commit 82a1776 into elastic:main Jul 24, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Jul 24, 2023
@criamico criamico deleted the kuery_api_strict_validation branch July 24, 2023 14:50
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
## Summary

Require validation for endpoints accepting `kuery` as a parameter in
POST or PUT requests.

**IMPORTANT**: This PR is part of the work needed to prepare the APIs
for Serverless.

### Some context
The initial idea was to completely remove any KQL queries from being
exposed in the endpoints, but after some discussion we came to the
agreement that they can stay but need to be validated, so only allowed
parameters can be sent. A similar approach is being followed by other
teams as well.

Impacted endpoints:
- `GET api/fleet/agents`
- `GET api/fleet/agent_status`
- `GET api/fleet/agent_policies`
- `GET api/fleet/package_policies`
- `GET api/fleet/enrollment_api_keys`
- `GET api/fleet/agent_status`

All these endpoints accept as a parameter `ListWithKuery`. It was
originally being deprecated but it was then decided to keep it and add
validation to the endpoints instead.

The endpoint `api/fleet/agents/action_status` doesn't accept `kuery`
anymore, since it was not being passed internally.


### What's changing

The KQL passed to these endpoints will be accepted in two possible
formats:
```
GET kbn:api/fleet/agents?kuery=local_metadata.agent.version="8.8.0"

GET kbn:api/fleet/agents?kuery=fleet-agents.local_metadata.agent.version="8.8.0"
```
Note that originally only the second format was going to accepted, but
we decided to avoid enforcing it as it would introduce a breaking
change, possibly breaking many customers automations.

### How it works
The code for `ValidateFilterKueryNode` has been adapted from a [similar
function](https://github.com/elastic/kibana/blob/45a483f49643bcca4ff130d9f100c38a1a2181e7/packages/core/saved-objects/core-saved-objects-api-server-internal/src/lib/search/utils/filter_utils.ts#L102)
already used in Kibana core. I added several tests where with some
common queries that are performed in the UI just to be sure that they
would pass validation. Additional queries can be validated by these
tests in the future.

`ValidateFilterKueryNode` needs to have the SO or index and a mapping
with the parameters to validate against. I copied over the mappings for
the necessary entities; if in the future we intend to expose a new
mapping parameter in the endpoints, it will be necessary to add it there
as well, or the validation will fail.

### UI
I also checked that the UI doesn't fail when using the KQL search boxes
for Agents, Agent policies and Enrollment keys and made sure that they
expose the same values present in the mappings.


### Testing

From dev tools, you can test the affected endpoints by entering the
following queries:
```
# agents
GET kbn:api/fleet/agents?kuery=fleet-agents.active:true
GET kbn:api/fleet/agents?kuery=active:true

# tags
GET kbn:api/fleet/agents/tags?kuery=fleet-agents.tags:tag1
GET kbn:api/fleet/agents/tags?kuery=tags:tag1

# agent status
GET kbn:/api/fleet/agent_status?kuery=fleet-agents.policy_id:fleet-server-policy
GET kbn:/api/fleet/agent_status?kuery=policy_id:fleet-server-policy

# package policies
GET kbn:/api/fleet/package_policies?kuery=ingest-package-policies.package.name:fleet_server

# agent policies
GET kbn:/api/fleet/agent_policies?kuery=ingest-agent-policies.name:"Fleet Server Policy"
GET kbn:/api/fleet/agent_policies?kuery=name:"Fleet Server Policy"

# enrollment keys
GET kbn:/api/fleet/enrollment_api_keys?kuery=fleet-enrollment-api-keys.policy_id:policy1
GET kbn:/api/fleet/enrollment_api_keys?kuery=policy1
```

These should all pass validation; modifying the parameters (for instance
with non existing ones) should fail validation

### Checklist
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com>
criamico added a commit that referenced this pull request Aug 11, 2023
## Summary
In #161064 the deprecation of the
parameter `kuery` was removed but I forgot to update the openapi specs.
Here I'm fixing it.


### Checklist
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
@jillguyonnet jillguyonnet mentioned this pull request Jun 11, 2024
1 task
jillguyonnet added a commit that referenced this pull request Jun 18, 2024
## Summary

Closes #178069

This PR fixes agent policies KQL filtering in Fleet UI. Because agent
policy data is retrieved from Saved Objects, the policy fields require
the SO prefix (`ingest-agent-policies`), which was removed in
#161064, to be present (see
#178069 (comment)
for details). [A further
ER](#185921) captures the
requirements for displaying custom labels without the prefix.

In Fleet UI, the `SearchBar` component that uses KQL filtering is used
in three tabs:
- Agents
- Agent policies
- Enrollment tokens

Note that the search inputs in the Uninstall tokens and Data streams
tabs are simple text filtering, not KQL.

The filtering behaviour with this fix matches the one in 8.11.0 and is
captured in the screen recording below:
- Agents tab: agent fields (e.g. `policy_id` or `agent.version`)
- Agent policies tab: agent policy fields prefixed with
`ingest-agent-policies` (e.g. `ingest-agent-policies.name`)
- Enrollment tokens tab: token fields (e.g. `name` or `policy_id`)

### Screen recording

This screen recording shows working KQL filtering and suggestions for
the three tabs (fixed for Agent policies):


https://github.com/elastic/kibana/assets/23701614/db4a7de7-a098-497a-a3c8-075ed5d0425e

### Testing

1. Create a few agent policies and enroll a couple of agents.
2. Test that the expected fields are shown in the KQL search bars for
agents, agent policies and enrollment tokens. For each, check that
suggestions are shown when you select a particular field with existing
values.
3. For agent policies in particular, also check that KQL syntax works as
expect. For instance, if you have an agent policy named "Test agent
policy", the query `ingest-agent-policies.name : *agent*` should
correctly filter for it.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants