Skip to content

Commit

Permalink
[Fleet] Split unpacking an archive and caching its files into separat…
Browse files Browse the repository at this point in the history
…e functions (#83085)

## 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
  • Loading branch information
John Schulz authored Nov 10, 2020
1 parent 44d45ff commit d66ca49
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 27 deletions.
52 changes: 34 additions & 18 deletions x-pack/plugins/fleet/server/services/epm/archive/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@ import {
deleteArchiveFilelist,
} from './cache';
import { ArchiveEntry, getBufferExtractor } from '../registry/extract';
import { parseAndVerifyArchive } from './validation';
import { parseAndVerifyArchiveEntries } from './validation';

export * from './cache';

export async function loadArchivePackage({
export async function getArchivePackage({
archiveBuffer,
contentType,
}: {
archiveBuffer: Buffer;
contentType: string;
}): Promise<{ paths: string[]; archivePackageInfo: ArchivePackage }> {
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);

setArchiveFilelist(archivePackageInfo.name, archivePackageInfo.version, paths);

return {
Expand All @@ -37,26 +39,40 @@ export async function loadArchivePackage({

export async function unpackArchiveToCache(
archiveBuffer: Buffer,
contentType: string,
filter = (entry: ArchiveEntry): boolean => true
contentType: string
): Promise<string[]> {
const entries = await unpackArchiveEntries(archiveBuffer, contentType);
return addEntriesToMemoryStore(entries);
}

function addEntriesToMemoryStore(entries: ArchiveEntry[]) {
const paths: string[] = [];
entries.forEach((entry) => {
const { path, buffer } = entry;
if (buffer) {
cacheSet(path, buffer);
paths.push(path);
}
});

return paths;
}

export async function unpackArchiveEntries(
archiveBuffer: Buffer,
contentType: string
): Promise<ArchiveEntry[]> {
const bufferExtractor = getBufferExtractor({ contentType });
if (!bufferExtractor) {
throw new PackageUnsupportedMediaTypeError(
`Unsupported media type ${contentType}. Please use 'application/gzip' or 'application/zip'`
);
}
const paths: string[] = [];
const entries: ArchiveEntry[] = [];
try {
await bufferExtractor(archiveBuffer, filter, (entry: ArchiveEntry) => {
const { path, buffer } = entry;
// skip directories
if (path.endsWith('/')) return;
if (buffer) {
cacheSet(path, buffer);
paths.push(path);
}
});
const onlyFiles = ({ path }: ArchiveEntry): boolean => !path.endsWith('/');
const addToEntries = (entry: ArchiveEntry) => entries.push(entry);
await bufferExtractor(archiveBuffer, onlyFiles, addToEntries);
} catch (error) {
throw new PackageInvalidArchiveError(
`Error during extraction of package: ${error}. Assumed content type was ${contentType}, check if this matches the archive type.`
Expand All @@ -65,12 +81,12 @@ export async function unpackArchiveToCache(

// While unpacking a tar.gz file with unzipBuffer() will result in a thrown error in the try-catch above,
// unpacking a zip file with untarBuffer() just results in nothing.
if (paths.length === 0) {
if (entries.length === 0) {
throw new PackageInvalidArchiveError(
`Archive seems empty. Assumed content type was ${contentType}, check if this matches the archive type.`
);
}
return paths;
return entries;
}

export const deletePackageCache = (name: string, version: string) => {
Expand Down
31 changes: 24 additions & 7 deletions x-pack/plugins/fleet/server/services/epm/archive/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,29 @@ import {
RegistryVarsEntry,
} from '../../../../common/types';
import { PackageInvalidArchiveError } from '../../../errors';
import { pkgToPkgKey } from '../registry';
import { cacheGet } from './cache';
import { ArchiveEntry, pkgToPkgKey } from '../registry';

const MANIFESTS: Record<string, Buffer> = {};
const MANIFEST_NAME = 'manifest.yml';

// TODO: everything below performs verification of manifest.yml files, and hence duplicates functionality already implemented in the
// package registry. At some point this should probably be replaced (or enhanced) with verification based on
// https://github.com/elastic/package-spec/
export async function parseAndVerifyArchiveEntries(
entries: ArchiveEntry[]
): Promise<{ paths: string[]; archivePackageInfo: ArchivePackage }> {
const paths: string[] = [];
entries.forEach(({ path, buffer }) => {
paths.push(path);
if (path.endsWith(MANIFEST_NAME) && buffer) MANIFESTS[path] = buffer;
});

return {
archivePackageInfo: parseAndVerifyArchive(paths),
paths,
};
}

export function parseAndVerifyArchive(paths: string[]): ArchivePackage {
// The top-level directory must match pkgName-pkgVersion, and no other top-level files or directories may be present
const toplevelDir = paths[0].split('/')[0];
Expand All @@ -31,10 +48,10 @@ export function parseAndVerifyArchive(paths: string[]): ArchivePackage {
});

// The package must contain a manifest file ...
const manifestFile = `${toplevelDir}/manifest.yml`;
const manifestBuffer = cacheGet(manifestFile);
const manifestFile = `${toplevelDir}/${MANIFEST_NAME}`;
const manifestBuffer = MANIFESTS[manifestFile];
if (!paths.includes(manifestFile) || !manifestBuffer) {
throw new PackageInvalidArchiveError('Package must contain a top-level manifest.yml file.');
throw new PackageInvalidArchiveError(`Package must contain a top-level ${MANIFEST_NAME} file.`);
}

// ... which must be valid YAML
Expand Down Expand Up @@ -97,8 +114,8 @@ function parseAndVerifyDataStreams(
dataStreamPaths = uniq(dataStreamPaths);

dataStreamPaths.forEach((dataStreamPath) => {
const manifestFile = `${pkgKey}/data_stream/${dataStreamPath}/manifest.yml`;
const manifestBuffer = cacheGet(manifestFile);
const manifestFile = `${pkgKey}/data_stream/${dataStreamPath}/${MANIFEST_NAME}`;
const manifestBuffer = MANIFESTS[manifestFile];
if (!paths.includes(manifestFile) || !manifestBuffer) {
throw new PackageInvalidArchiveError(
`No manifest.yml file found for data stream '${dataStreamPath}'`
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
} from '../../../errors';
import { getPackageSavedObjects } from './get';
import { appContextService } from '../../app_context';
import { loadArchivePackage } from '../archive';
import { getArchivePackage } from '../archive';
import { _installPackage } from './_install_package';

export async function installLatestPackage(options: {
Expand Down Expand Up @@ -283,7 +283,7 @@ async function installPackageByUpload({
archiveBuffer,
contentType,
}: InstallUploadedArchiveParams): Promise<AssetReference[]> {
const { paths, archivePackageInfo } = await loadArchivePackage({ archiveBuffer, contentType });
const { paths, archivePackageInfo } = await getArchivePackage({ archiveBuffer, contentType });

const installedPkg = await getInstallationObject({
savedObjectsClient,
Expand Down

0 comments on commit d66ca49

Please sign in to comment.