Skip to content

Commit

Permalink
[teraslice] Asset load dependency check (#3822)
Browse files Browse the repository at this point in the history
This PR makes the following changes:

- adds a check on asset upload to see if the asset is compatible with
the teraslice version
- creates a test asset with a high minimum teraslice version
- test the test asset to ensure an error is thrown
- fix a bug that leaves behind an s3 object in some situations if an
asset fails to upload

ref: #3685
  • Loading branch information
busma13 authored Nov 14, 2024
1 parent 91e0d62 commit c99f3e7
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 50 deletions.
Binary file added e2e/test/fixtures/assets/test_asset_json.zip
Binary file not shown.
53 changes: 30 additions & 23 deletions packages/teraslice/src/lib/storage/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ClientResponse, AssetRecord } from '@terascope/types';
import { TerasliceElasticsearchStorage, TerasliceESStorageConfig } from './backends/elasticsearch_store.js';
import { S3Store, TerasliceS3StorageConfig } from './backends/s3_store.js';
import { makeLogger } from '../workers/helpers/terafoundation.js';
import { saveAsset, AssetMetadata, isZipFile } from '../utils/file_utils.js';
import { saveAsset, AssetMetadata, isZipFile, deleteDir } from '../utils/file_utils.js';
import {
findMatchingAsset, findSimilarAssets, toVersionQuery,
getInCompatibilityReason
Expand Down Expand Up @@ -195,32 +195,39 @@ export class AssetsStorage {
const responseTimeout = this.context.sysconfig.teraslice.api_response_timeout as number;
const startTime = Date.now();

if (this.s3Backend) {
if (blocking) {
const elapsed = Date.now() - startTime;
const remaining = responseTimeout - elapsed;
await this.s3Backend.save(id, data, remaining);
} else {
await this.s3Backend.save(id, data, responseTimeout);
try {
if (this.s3Backend) {
if (blocking) {
const elapsed = Date.now() - startTime;
const remaining = responseTimeout - elapsed;
await this.s3Backend.save(id, data, remaining);
} else {
await this.s3Backend.save(id, data, responseTimeout);
}

this.logger.info(`asset id: ${id} has been saved to s3 store`);
}

this.logger.info(`asset id: ${id} has been saved to s3 store`);
}
const metaData = await saveAsset(
this.logger,
this.assetsPath,
id,
data,
_metaIsUnique(this.esBackend)
);

const metaData = await saveAsset(
this.logger,
this.assetsPath,
id,
data,
_metaIsUnique(this.esBackend)
);

const assetRecord = Object.assign({
_created: new Date().toISOString()
}, metaData);
const assetRecord = Object.assign({
_created: new Date().toISOString()
}, metaData);

await this._saveToEs(id, assetRecord, blocking, responseTimeout, startTime);
this.logger.info(`assets: ${metaData.name}, id: ${id} has been saved to assets_directory and elasticsearch`);
await this._saveToEs(id, assetRecord, blocking, responseTimeout, startTime);
this.logger.info(`assets: ${metaData.name}, id: ${id} has been saved to assets_directory and elasticsearch`);
} catch (err) {
// clean up s3 object or saved asset if a later step fails
await this.s3Backend?.remove(id);
await deleteDir(path.join(this.assetsPath, id));
throw err;
}
}

/**
Expand Down
63 changes: 36 additions & 27 deletions packages/teraslice/src/lib/utils/file_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ export interface AssetJSON {
arch?: string;
platform?: string;
node_version?: number;
minimum_teraslice_version?: string;
}

export type MetaCheckFN = (data: AssetMetadata) => Promise<AssetMetadata>;
Expand All @@ -80,46 +81,54 @@ export async function verifyAssetJSON(id: string, newPath: string): Promise<Asse
throw err;
}

let packageData;
try {
const packageData = await fse.readJson(path.join(newPath, 'asset.json'));
const metadata: AssetMetadata = Object.assign({ id }, packageData);
packageData = await fse.readJson(path.join(newPath, 'asset.json'));
} catch (_err) {
const err = new TSError(_err, {
message: 'Failure parsing asset.json, please ensure that\'s it formatted correctly',
statusCode: 422
});
throw err;
}

if (!metadata.name) {
throw new Error('Missing name');
}
const metadata: AssetMetadata = Object.assign({ id }, packageData);

metadata.version = semver.clean(metadata.version) as string;
if (!metadata.name) {
throw new Error('Missing name');
}

if (!semver.valid(metadata.version)) {
throw new Error(`Invalid version "${metadata.version}"`);
}
metadata.version = semver.clean(metadata.version) as string;

/**
if (!semver.valid(metadata.version)) {
throw new Error(`Invalid version "${metadata.version}"`);
}

/**
* If node_version, platform, or arch is set to a falsey
* value we should delete it so it is considered a wildcard.
*
* This is useful for making an asset bundle that isn't
* locked down.
*/
if (metadata.node_version) {
metadata.node_version = getMajorVersion(metadata.node_version);
} else {
delete metadata.node_version;
}
if (!metadata.platform) {
delete metadata.platform;
}
if (!metadata.arch) {
delete metadata.arch;
if (metadata.node_version) {
metadata.node_version = getMajorVersion(metadata.node_version);
} else {
delete metadata.node_version;
}
if (!metadata.platform) {
delete metadata.platform;
}
if (!metadata.arch) {
delete metadata.arch;
}
if (metadata.minimum_teraslice_version) {
const terasliceVersion = getPackageJSON().version;
if (semver.gt(metadata.minimum_teraslice_version, terasliceVersion)) {
throw new Error(`Asset requires teraslice version ${metadata.minimum_teraslice_version} or greater.`);
}
return metadata;
} catch (_err) {
const err = new TSError(_err, {
message: 'Failure parsing asset.json, please ensure that\'s it formatted correctly',
statusCode: 422
});
throw err;
}
return metadata;
}

async function _saveAsset(
Expand Down
7 changes: 7 additions & 0 deletions packages/teraslice/test/storage/assets_storage-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ describe('AssetsStorage using S3 backend', () => {
await expect(() => storage.save(buffer)).rejects.toThrow('Failed to save asset. File type not recognized as zip.');
});

it('will reject an asset if the minimum teraslice version is not met', async () => {
const filePath = 'e2e/test/fixtures/assets/test_asset_json.zip';
const buffer = fs.readFileSync(filePath);
await expect(() => storage.save(buffer)).rejects.toThrow('Asset requires teraslice version 999.9.9 or greater.');
expect(await storage.grabS3Info()).toEqual([]);
});

it('can save an asset to S3', async () => {
const filePath = 'e2e/test/fixtures/assets/example_asset_1.zip';
const buffer = fs.readFileSync(filePath);
Expand Down

0 comments on commit c99f3e7

Please sign in to comment.