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] Split unpacking an archive and caching its files into separate functions #83085

Merged
merged 3 commits into from
Nov 10, 2020

Conversation

jfsiii
Copy link
Contributor

@jfsiii jfsiii commented Nov 10, 2020

Summary

  • Separate unpacking an archive from caching it or other side-effects
  • Parse and validate an archive before caching it
  • Validation has no coupling with caching side-effects or any other code outside the validation.ts file
-  const paths = await unpackArchiveToCache(archiveBuffer, contentType);
-  const archivePackageInfo = parseAndVerifyArchive(paths);
+  const entries = await unpackArchiveEntries(archiveBuffer, contentType);
+  const { archivePackageInfo } = await parseAndVerifyArchiveEntries(entries);
+  const paths = addEntriesToMemoryStore(entries);

Checklist

@jfsiii jfsiii changed the title [Fleet] Separate unpack buffers from cache side-effects [Fleet] Split unpacking an archive and caching its files into separate functions Nov 10, 2020
paths,
};
}

export function parseAndVerifyArchive(paths: string[]): ArchivePackage {
Copy link
Contributor Author

@jfsiii jfsiii Nov 10, 2020

Choose a reason for hiding this comment

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

parseAndVerifyArchive needs to read any manifest.yml files in the archive to do its validation. Instead of sharing the existing memory cache, use a local map. It only holds a few entries only needs them while parsing. Now it's not coupled to any other implementation or order.

@jfsiii jfsiii marked this pull request as ready for review November 10, 2020 20:20
@jfsiii jfsiii requested a review from a team November 10, 2020 20:20
@jfsiii jfsiii self-assigned this Nov 10, 2020
@jfsiii jfsiii requested review from skh and neptunian November 10, 2020 20:21
@jfsiii jfsiii added 7.11.0 release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.11.0 and removed 7.11.0 labels Nov 10, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@botelastic botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@jfsiii jfsiii merged commit d66ca49 into elastic:master Nov 10, 2020
jfsiii pushed a commit that referenced this pull request Nov 11, 2020
…e functions (#83085) (#83153)

## Summary

 * Separate unpacking an archive from caching it or other side-effects
 * Parse and validate an archive before caching it 
 * Validation has no coupling with caching side-effects or any other code outside the `validation.ts` file

```diff
-  const paths = await unpackArchiveToCache(archiveBuffer, contentType);
-  const archivePackageInfo = parseAndVerifyArchive(paths);
+  const entries = await unpackArchiveEntries(archiveBuffer, contentType);
+  const { archivePackageInfo } = await parseAndVerifyArchiveEntries(entries);
+  const paths = addEntriesToMemoryStore(entries);
```
### Checklist
- [ ] [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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants