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.

Sign up for GitHub

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

Jump to bottom

[Fleet] Add tests for parseAndVerifyArchive method #147050

Closed
kpollich opened this issue Dec 5, 2022 · 4 comments
Closed

[Fleet] Add tests for parseAndVerifyArchive method #147050

kpollich opened this issue Dec 5, 2022 · 4 comments
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture

Comments

@kpollich
Copy link
Member

kpollich commented Dec 5, 2022

Add tests to the parseAndVerify archive method that reads package manifest.yml files and generates packageInfo objects from them. We want to ensure that we're parsing all expected content as expected from package manifests.

kibana/x-pack/plugins/fleet/server/services/epm/archive/parse.ts

Line 175 in a433b64

function parseAndVerifyArchive(

This is a somewhat challenging method to test, as we rely on reading YML buffers from the archive cache, so a substantial amount of mocking is required. Consider refactoring to make this process more testable as it's a crucial step in Fleet's business logic.
@kpollich kpollich added the Team:Fleet Team label for Observability Data Collection Fleet team label Dec 5, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@kpollich kpollich mentioned this issue Dec 5, 2022
Merged
@jen-huang jen-huang added the technical debt Improvement of the software architecture and operational architecture label Dec 6, 2022
@jsoriano
Copy link
Member

jsoriano commented Dec 6, 2022

May be related: elastic/package-spec#322

@kpollich kpollich mentioned this issue Jan 23, 2023
Merged
kpollich added a commit that referenced this issue Jan 24, 2023
@kpollich
[Fleet] Fix parsing logic for data_stream.hidden manifest field (#149357
61ce24d 
)

## Summary

Fixes package archive parsing logic for the
`index_template_data_stream.hidden` property, implemented in line with
other `index_template.*` "dotted property name" values from package
manifests.

## To test

Create a test package that includes
`elasticsearch.index_template.data_stream.hidden: true` in a
`data_stream/**/manifest.yml` file, install the package, then verify
(via dev tools - NOT stack management!!!) that the generated index
template contains `data_stream.hidden: true`


![image](https://user-images.githubusercontent.com/6766512/214155715-1a78a7f6-cde0-4cb1-a6e3-fd1ac3749f1e.png)

It'd be great to have automated tests around this, but testing our
archive parsing logic is very challenging due to the nature of reading
directories/files and how much mocking is required. See
#147050. I'll see about adding
an integration test for this.
@jillguyonnet
Copy link
Contributor

Hey @kpollich I see unit tests were added recently for this method here: #150888. Could this issue be closed?

@kpollich
Copy link
Member Author

kpollich commented May 3, 2023

Seems like it. We can reopen if more tests are needed.

@kpollich kpollich closed this as completed May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Team:Fleet Team label for Observability Data Collection Fleet team technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jsoriano @jen-huang @kpollich @elasticmachine @jillguyonnet