From c5cea8da3049b2195996cc3695f6c09bb9bdf678 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 17 Aug 2023 15:41:20 -0700 Subject: [PATCH 01/24] Stub catalog reader interface Signed-off-by: Simeon Widdis --- .../integrations/repository/catalog_reader.ts | 10 +++++++ .../repository/local_catalog_reader.ts | 28 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 server/adaptors/integrations/repository/catalog_reader.ts create mode 100644 server/adaptors/integrations/repository/local_catalog_reader.ts diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts new file mode 100644 index 000000000..c8cdbb39a --- /dev/null +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -0,0 +1,10 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +interface CatalogReader { + readFile: (filename: string) => Promise; + readDir: (filename: string) => Promise; + isDir: (filename: string) => Promise; +} diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts new file mode 100644 index 000000000..b3d9d796d --- /dev/null +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * A CatalogReader that reads from the local filesystem. + * Used to read Integration information when the user uploads their own catalog. + */ +class LocalCatalogReader implements CatalogReader { + directory: string; + + constructor(directory: string) { + this.directory = directory; + } + + async readFile(_dirname: string): Promise { + return ''; + } + + async readDir(_dirname: string): Promise { + return []; + } + + async isDir(_dirname: string): Promise { + return false; + } +} From 77ad89b68e22295095f9382e300321738267f783 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 17 Aug 2023 16:00:13 -0700 Subject: [PATCH 02/24] Add basic catalog functionality to new catalog reader Signed-off-by: Simeon Widdis --- .../integrations/repository/catalog_reader.ts | 3 +- .../repository/local_catalog_reader.ts | 41 ++++++++++++++++--- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts index c8cdbb39a..9710ce7fd 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -6,5 +6,6 @@ interface CatalogReader { readFile: (filename: string) => Promise; readDir: (filename: string) => Promise; - isDir: (filename: string) => Promise; + isIntegration: (filename: string) => Promise; + isRepository: (filename: string) => Promise; } diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts index b3d9d796d..2634e7e62 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -3,6 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as fs from 'fs/promises'; +import path from 'path'; +import sanitize from 'sanitize-filename'; +import { Integration } from './integration'; + /** * A CatalogReader that reads from the local filesystem. * Used to read Integration information when the user uploads their own catalog. @@ -14,15 +19,41 @@ class LocalCatalogReader implements CatalogReader { this.directory = directory; } - async readFile(_dirname: string): Promise { - return ''; + // Use before any call to `fs` + _prepare(filename: string): string { + return path.join(this.directory, sanitize(filename)); + } + + async readFile(filename: string): Promise { + return await fs.readFile(this._prepare(filename), { encoding: 'utf-8' }); } - async readDir(_dirname: string): Promise { - return []; + async readDir(dirname: string): Promise { + // TODO return empty list if not a directory + return await fs.readdir(this._prepare(dirname)); } - async isDir(_dirname: string): Promise { + async isDirectory(dirname: string): Promise { + return (await fs.lstat(this._prepare(dirname))).isDirectory(); + } + + async isRepository(dirname: string): Promise { + if (await this.isIntegration(dirname)) { + return false; + } + // If there is at least one integration in a directory, it's a repository. + for (const item of await this.readDir(dirname)) { + if (await this.isIntegration(item)) { + return true; + } + } return false; } + + async isIntegration(dirname: string): Promise { + if (!(await this.isDirectory(dirname))) { + return false; + } + return new Integration(this._prepare(dirname)).check(); + } } From 676591df5c6d22a05cbf0121a444ce04c8af270b Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 21 Aug 2023 11:36:02 -0700 Subject: [PATCH 03/24] Refactor validation logic with a deeper interface Signed-off-by: Simeon Widdis --- .../repository/__test__/integration.test.ts | 11 ++++-- .../integrations/repository/integration.ts | 21 +--------- server/adaptors/integrations/validators.ts | 39 ++++++++++++++++++- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index 4474fc48f..2002ad04a 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -16,8 +16,13 @@ describe('Integration', () => { name: 'sample', version: '2.0.0', license: 'Apache-2.0', - type: '', - components: [], + type: 'logs', + components: [ + { + name: 'logs', + version: '1.0.0', + }, + ], assets: { savedObjects: { name: 'sample', @@ -105,7 +110,7 @@ describe('Integration', () => { const result = await integration.getConfig(sampleIntegration.version); expect(result).toBeNull(); - expect(logValidationErrorsMock).toHaveBeenCalledWith(expect.any(String), expect.any(Array)); + expect(logValidationErrorsMock).toHaveBeenCalled(); }); it('should return null and log syntax errors if the config file has syntax errors', async () => { diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index fe9ddd0ef..1350cb653 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -6,7 +6,7 @@ import * as fs from 'fs/promises'; import path from 'path'; import { ValidateFunction } from 'ajv'; -import { templateValidator } from '../validators'; +import { validateTemplate } from '../validators'; /** * Helper function to compare version numbers. @@ -49,18 +49,6 @@ async function isDirectory(dirPath: string): Promise { } } -/** - * Helper function to log validation errors. - * Relies on the `ajv` package for validation error logs.. - * - * @param integration The name of the component that failed validation. - * @param validator A failing ajv validator. - */ -function logValidationErrors(integration: string, validator: ValidateFunction) { - const errors = validator.errors?.map((e) => e.message); - console.error(`Validation errors in ${integration}`, errors); -} - /** * The Integration class represents the data for Integration Templates. * It is backed by the repository file system. @@ -165,12 +153,7 @@ export class Integration { const config = await fs.readFile(configPath, { encoding: 'utf-8' }); const possibleTemplate = JSON.parse(config); - if (!templateValidator(possibleTemplate)) { - logValidationErrors(configFile, templateValidator); - return null; - } - - return possibleTemplate; + return validateTemplate(possibleTemplate, true) ? possibleTemplate : null; } catch (err: any) { if (err instanceof SyntaxError) { console.error(`Syntax errors in ${configFile}`, err); diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts index 3cb24212d..ab51c17cb 100644 --- a/server/adaptors/integrations/validators.ts +++ b/server/adaptors/integrations/validators.ts @@ -107,5 +107,40 @@ const instanceSchema: JSONSchemaType = { required: ['name', 'templateName', 'dataSource', 'creationDate', 'assets'], }; -export const templateValidator = ajv.compile(templateSchema); -export const instanceValidator = ajv.compile(instanceSchema); +const templateValidator = ajv.compile(templateSchema); +const instanceValidator = ajv.compile(instanceSchema); + +// AJV validators use side effects for errors, so we provide a more conventional wrapper. +// The wrapper optionally handles error logging with the `logErrors` parameter. +export const validateTemplate = (data: { name?: unknown }, logErrors?: true): boolean => { + if (!templateValidator(data)) { + if (logErrors) { + console.error( + `The integration '${data.name ?? 'config'}' is invalid:`, + ajv.errorsText(templateValidator.errors) + ); + } + return false; + } + // We assume an invariant that the type of an integration is connected with its component. + if (data.components.findIndex((x) => x.name === data.type) < 0) { + if (logErrors) { + console.error(`The integration type '${data.type}' must be included as a component`); + } + return false; + } + return true; +}; + +export const validateInstance = (data: { name?: unknown }, logErrors?: true): boolean => { + if (!instanceValidator(data)) { + if (logErrors) { + console.error( + `The integration '${data.name ?? 'instance'} is invalid:`, + ajv.errorsText(instanceValidator.errors) + ); + } + return false; + } + return true; +}; From 12c4bcf8b72a2a067827b3958da4ca24595b9985 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 21 Aug 2023 11:36:02 -0700 Subject: [PATCH 04/24] Refactor validation logic with a deeper interface Signed-off-by: Simeon Widdis --- .../repository/__test__/integration.test.ts | 11 ++++-- .../integrations/repository/integration.ts | 21 +--------- server/adaptors/integrations/validators.ts | 39 ++++++++++++++++++- 3 files changed, 47 insertions(+), 24 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index 4474fc48f..2002ad04a 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -16,8 +16,13 @@ describe('Integration', () => { name: 'sample', version: '2.0.0', license: 'Apache-2.0', - type: '', - components: [], + type: 'logs', + components: [ + { + name: 'logs', + version: '1.0.0', + }, + ], assets: { savedObjects: { name: 'sample', @@ -105,7 +110,7 @@ describe('Integration', () => { const result = await integration.getConfig(sampleIntegration.version); expect(result).toBeNull(); - expect(logValidationErrorsMock).toHaveBeenCalledWith(expect.any(String), expect.any(Array)); + expect(logValidationErrorsMock).toHaveBeenCalled(); }); it('should return null and log syntax errors if the config file has syntax errors', async () => { diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index fe9ddd0ef..1350cb653 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -6,7 +6,7 @@ import * as fs from 'fs/promises'; import path from 'path'; import { ValidateFunction } from 'ajv'; -import { templateValidator } from '../validators'; +import { validateTemplate } from '../validators'; /** * Helper function to compare version numbers. @@ -49,18 +49,6 @@ async function isDirectory(dirPath: string): Promise { } } -/** - * Helper function to log validation errors. - * Relies on the `ajv` package for validation error logs.. - * - * @param integration The name of the component that failed validation. - * @param validator A failing ajv validator. - */ -function logValidationErrors(integration: string, validator: ValidateFunction) { - const errors = validator.errors?.map((e) => e.message); - console.error(`Validation errors in ${integration}`, errors); -} - /** * The Integration class represents the data for Integration Templates. * It is backed by the repository file system. @@ -165,12 +153,7 @@ export class Integration { const config = await fs.readFile(configPath, { encoding: 'utf-8' }); const possibleTemplate = JSON.parse(config); - if (!templateValidator(possibleTemplate)) { - logValidationErrors(configFile, templateValidator); - return null; - } - - return possibleTemplate; + return validateTemplate(possibleTemplate, true) ? possibleTemplate : null; } catch (err: any) { if (err instanceof SyntaxError) { console.error(`Syntax errors in ${configFile}`, err); diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts index 3cb24212d..ab51c17cb 100644 --- a/server/adaptors/integrations/validators.ts +++ b/server/adaptors/integrations/validators.ts @@ -107,5 +107,40 @@ const instanceSchema: JSONSchemaType = { required: ['name', 'templateName', 'dataSource', 'creationDate', 'assets'], }; -export const templateValidator = ajv.compile(templateSchema); -export const instanceValidator = ajv.compile(instanceSchema); +const templateValidator = ajv.compile(templateSchema); +const instanceValidator = ajv.compile(instanceSchema); + +// AJV validators use side effects for errors, so we provide a more conventional wrapper. +// The wrapper optionally handles error logging with the `logErrors` parameter. +export const validateTemplate = (data: { name?: unknown }, logErrors?: true): boolean => { + if (!templateValidator(data)) { + if (logErrors) { + console.error( + `The integration '${data.name ?? 'config'}' is invalid:`, + ajv.errorsText(templateValidator.errors) + ); + } + return false; + } + // We assume an invariant that the type of an integration is connected with its component. + if (data.components.findIndex((x) => x.name === data.type) < 0) { + if (logErrors) { + console.error(`The integration type '${data.type}' must be included as a component`); + } + return false; + } + return true; +}; + +export const validateInstance = (data: { name?: unknown }, logErrors?: true): boolean => { + if (!instanceValidator(data)) { + if (logErrors) { + console.error( + `The integration '${data.name ?? 'instance'} is invalid:`, + ajv.errorsText(instanceValidator.errors) + ); + } + return false; + } + return true; +}; From 412480e74ab4cfbcf2cdf42a689f46d0c8eb3487 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 21 Aug 2023 12:02:27 -0700 Subject: [PATCH 05/24] Remove redundant test. This test is unneeded after 12c4bcf Signed-off-by: Simeon Widdis --- .../integrations/__test__/local_repository.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/server/adaptors/integrations/__test__/local_repository.test.ts b/server/adaptors/integrations/__test__/local_repository.test.ts index 722711710..6b0ddc72f 100644 --- a/server/adaptors/integrations/__test__/local_repository.test.ts +++ b/server/adaptors/integrations/__test__/local_repository.test.ts @@ -31,14 +31,4 @@ describe('The local repository', () => { const integrations: Integration[] = await repository.getIntegrationList(); await Promise.all(integrations.map((i) => expect(i.deepCheck()).resolves.toBeTruthy())); }); - - it('Should not have a type that is not imported in the config', async () => { - const repository: Repository = new Repository(path.join(__dirname, '../__data__/repository')); - const integrations: Integration[] = await repository.getIntegrationList(); - for (const integration of integrations) { - const config = await integration.getConfig(); - const components = config!.components.map((x) => x.name); - expect(components).toContain(config!.type); - } - }); }); From 6d2bad103cb35ec5934e3e614b022040b61a6931 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 21 Aug 2023 14:12:44 -0700 Subject: [PATCH 06/24] Add tests for new validators Signed-off-by: Simeon Widdis --- .../integrations/__test__/validators.test.ts | 79 +++++++++++++++++++ .../integrations/repository/integration.ts | 1 - 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 server/adaptors/integrations/__test__/validators.test.ts diff --git a/server/adaptors/integrations/__test__/validators.test.ts b/server/adaptors/integrations/__test__/validators.test.ts new file mode 100644 index 000000000..f998324af --- /dev/null +++ b/server/adaptors/integrations/__test__/validators.test.ts @@ -0,0 +1,79 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { validateTemplate, validateInstance } from '../validators'; + +const validTemplate: IntegrationTemplate = { + name: 'test', + version: '1.0.0', + license: 'Apache-2.0', + type: 'logs', + components: [ + { + name: 'logs', + version: '1.0.0', + }, + ], + assets: {}, +}; + +const validInstance: IntegrationInstance = { + name: 'test', + templateName: 'test', + dataSource: 'test', + creationDate: new Date(0).toISOString(), + assets: [], +}; + +describe('validateTemplate', () => { + it('Returns true for a valid Integration Template', () => { + expect(validateTemplate(validTemplate)).toBe(true); + }); + + it('Returns false if a template is missing a license', () => { + const sample: any = structuredClone(validTemplate); + sample.license = undefined; + expect(validateTemplate(sample)).toBe(false); + }); + + it('Returns false if a template has an invalid type', () => { + const sample: any = structuredClone(validTemplate); + sample.components[0].name = 'not-logs'; + expect(validateTemplate(sample)).toBe(false); + }); + + it('Respects logErrors', () => { + const logValidationErrorsMock = jest.spyOn(console, 'error'); + const sample1: any = structuredClone(validTemplate); + sample1.license = undefined; + const sample2: any = structuredClone(validTemplate); + sample2.components[0].name = 'not-logs'; + + expect(validateTemplate(sample1, true)).toBe(false); + expect(validateTemplate(sample2, true)).toBe(false); + expect(logValidationErrorsMock).toBeCalledTimes(2); + }); +}); + +describe('validateInstance', () => { + it('Returns true for a valid Integration Instance', () => { + expect(validateInstance(validInstance)).toBe(true); + }); + + it('Returns false if an instance is missing a template', () => { + const sample: any = structuredClone(validInstance); + sample.templateName = undefined; + expect(validateInstance(sample)).toBe(false); + }); + + it('Respects logErrors', () => { + const logValidationErrorsMock = jest.spyOn(console, 'error'); + const sample1: any = structuredClone(validInstance); + sample1.templateName = undefined; + + expect(validateInstance(sample1, true)).toBe(false); + expect(logValidationErrorsMock).toBeCalled(); + }); +}); diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index 1350cb653..7c20db6e4 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -5,7 +5,6 @@ import * as fs from 'fs/promises'; import path from 'path'; -import { ValidateFunction } from 'ajv'; import { validateTemplate } from '../validators'; /** From 55d649097dd89c98c8cad91d0f9640d88fe755d6 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Mon, 21 Aug 2023 14:22:35 -0700 Subject: [PATCH 07/24] Make better failure mode for invalid objects Signed-off-by: Simeon Widdis --- .../adaptors/integrations/__test__/validators.test.ts | 10 ++++++++++ server/adaptors/integrations/validators.ts | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/server/adaptors/integrations/__test__/validators.test.ts b/server/adaptors/integrations/__test__/validators.test.ts index f998324af..b52128d54 100644 --- a/server/adaptors/integrations/__test__/validators.test.ts +++ b/server/adaptors/integrations/__test__/validators.test.ts @@ -55,6 +55,11 @@ describe('validateTemplate', () => { expect(validateTemplate(sample2, true)).toBe(false); expect(logValidationErrorsMock).toBeCalledTimes(2); }); + + it("Doesn't crash if given a non-object", () => { + // May happen in some user-provided JSON parsing scenarios. + expect(validateTemplate([] as any, true)).toBe(false); + }); }); describe('validateInstance', () => { @@ -76,4 +81,9 @@ describe('validateInstance', () => { expect(validateInstance(sample1, true)).toBe(false); expect(logValidationErrorsMock).toBeCalled(); }); + + it("Doesn't crash if given a non-object", () => { + // May happen in some user-provided JSON parsing scenarios. + expect(validateInstance([] as any, true)).toBe(false); + }); }); diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts index ab51c17cb..7621cf717 100644 --- a/server/adaptors/integrations/validators.ts +++ b/server/adaptors/integrations/validators.ts @@ -116,7 +116,7 @@ export const validateTemplate = (data: { name?: unknown }, logErrors?: true): bo if (!templateValidator(data)) { if (logErrors) { console.error( - `The integration '${data.name ?? 'config'}' is invalid:`, + `The integration config '${data.name ?? data}' is invalid:`, ajv.errorsText(templateValidator.errors) ); } @@ -136,7 +136,7 @@ export const validateInstance = (data: { name?: unknown }, logErrors?: true): bo if (!instanceValidator(data)) { if (logErrors) { console.error( - `The integration '${data.name ?? 'instance'} is invalid:`, + `The integration instance '${data.name ?? data}' is invalid:`, ajv.errorsText(instanceValidator.errors) ); } From ba56fb8050385c88ab4cad41a8c1722eff1a9da1 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 14:16:54 -0700 Subject: [PATCH 08/24] Generalize Result type Signed-off-by: Simeon Widdis --- .../integrations/__test__/validators.test.ts | 12 ++++++------ server/adaptors/integrations/types.ts | 2 ++ server/adaptors/integrations/validators.ts | 10 ++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/server/adaptors/integrations/__test__/validators.test.ts b/server/adaptors/integrations/__test__/validators.test.ts index ba573c4c4..3c6e133f5 100644 --- a/server/adaptors/integrations/__test__/validators.test.ts +++ b/server/adaptors/integrations/__test__/validators.test.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { validateTemplate, validateInstance, ValidationResult } from '../validators'; +import { validateTemplate, validateInstance } from '../validators'; const validTemplate: IntegrationTemplate = { name: 'test', @@ -29,7 +29,7 @@ const validInstance: IntegrationInstance = { describe('validateTemplate', () => { it('Returns a success value for a valid Integration Template', () => { - const result: ValidationResult = validateTemplate(validTemplate); + const result: Result = validateTemplate(validTemplate); expect(result.ok).toBe(true); expect((result as any).value).toBe(validTemplate); }); @@ -38,7 +38,7 @@ describe('validateTemplate', () => { const sample: any = structuredClone(validTemplate); sample.license = undefined; - const result: ValidationResult = validateTemplate(sample); + const result: Result = validateTemplate(sample); expect(result.ok).toBe(false); expect((result as any).error).toBeInstanceOf(Error); @@ -48,7 +48,7 @@ describe('validateTemplate', () => { const sample: any = structuredClone(validTemplate); sample.components[0].name = 'not-logs'; - const result: ValidationResult = validateTemplate(sample); + const result: Result = validateTemplate(sample); expect(result.ok).toBe(false); expect((result as any).error).toBeInstanceOf(Error); @@ -62,7 +62,7 @@ describe('validateTemplate', () => { describe('validateInstance', () => { it('Returns true for a valid Integration Instance', () => { - const result: ValidationResult = validateInstance(validInstance); + const result: Result = validateInstance(validInstance); expect(result.ok).toBe(true); expect((result as any).value).toBe(validInstance); }); @@ -71,7 +71,7 @@ describe('validateInstance', () => { const sample: any = structuredClone(validInstance); sample.templateName = undefined; - const result: ValidationResult = validateInstance(sample); + const result: Result = validateInstance(sample); expect(result.ok).toBe(false); expect((result as any).error).toBeInstanceOf(Error); diff --git a/server/adaptors/integrations/types.ts b/server/adaptors/integrations/types.ts index c12476909..d84dc3c5a 100644 --- a/server/adaptors/integrations/types.ts +++ b/server/adaptors/integrations/types.ts @@ -3,6 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ +type Result = { ok: true; value: T } | { ok: false; error: E }; + interface IntegrationTemplate { name: string; version: string; diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts index b40871eef..b9c7f33f6 100644 --- a/server/adaptors/integrations/validators.ts +++ b/server/adaptors/integrations/validators.ts @@ -5,8 +5,6 @@ import Ajv, { JSONSchemaType } from 'ajv'; -export type ValidationResult = { ok: true; value: T } | { ok: false; error: E }; - const ajv = new Ajv(); const staticAsset: JSONSchemaType = { @@ -118,11 +116,11 @@ const instanceValidator = ajv.compile(instanceSchema); * this is a more conventional wrapper that simplifies calling. * * @param data The data to be validated as an IntegrationTemplate. - * @return A ValidationResult indicating whether the validation was successful or not. + * @return A Result indicating whether the validation was successful or not. * If validation succeeds, returns an object with 'ok' set to true and the validated data. * If validation fails, returns an object with 'ok' set to false and an Error object describing the validation error. */ -export const validateTemplate = (data: unknown): ValidationResult => { +export const validateTemplate = (data: unknown): Result => { if (!templateValidator(data)) { return { ok: false, error: new Error(ajv.errorsText(templateValidator.errors)) }; } @@ -143,11 +141,11 @@ export const validateTemplate = (data: unknown): ValidationResult => { +export const validateInstance = (data: unknown): Result => { if (!instanceValidator(data)) { return { ok: false, From cb29208bea86407beea76042b9238d8f994084b1 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 15:14:42 -0700 Subject: [PATCH 09/24] Convert backend to use catalog reader (unstable) Signed-off-by: Simeon Widdis --- .../integrations_kibana_backend.ts | 61 +++++- .../integrations/repository/catalog_reader.ts | 4 +- .../integrations/repository/integration.ts | 188 ++++++++---------- .../repository/local_catalog_reader.ts | 27 +-- 4 files changed, 137 insertions(+), 143 deletions(-) diff --git a/server/adaptors/integrations/integrations_kibana_backend.ts b/server/adaptors/integrations/integrations_kibana_backend.ts index f28c883ec..b503390a2 100644 --- a/server/adaptors/integrations/integrations_kibana_backend.ts +++ b/server/adaptors/integrations/integrations_kibana_backend.ts @@ -53,17 +53,33 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { return result; }; + // Internal; use getIntegrationTemplates. + _getAllIntegrationTemplates = async (): Promise => { + const integrationList = await this.repository.getIntegrationList(); + const configResults = await Promise.all(integrationList.map((x) => x.getConfig())); + const configs = configResults.filter((cfg) => cfg.ok) as Array<{ value: IntegrationTemplate }>; + return Promise.resolve({ hits: configs.map((cfg) => cfg.value) }); + }; + + // Internal; use getIntegrationTemplates. + _getIntegrationTemplatesByName = async ( + name: string + ): Promise => { + const integration = await this.repository.getIntegration(name); + const config = await integration?.getConfig(); + if (!config || !config.ok) { + return Promise.resolve({ hits: [] }); + } + return Promise.resolve({ hits: [config.value] }); + }; + getIntegrationTemplates = async ( query?: IntegrationTemplateQuery ): Promise => { if (query?.name) { - const integration = await this.repository.getIntegration(query.name); - const config = await integration?.getConfig(); - return Promise.resolve({ hits: config ? [config] : [] }); + return this._getIntegrationTemplatesByName(query.name); } - const integrationList = await this.repository.getIntegrationList(); - const configList = await Promise.all(integrationList.map((x) => x.getConfig())); - return Promise.resolve({ hits: configList.filter((x) => x !== null) as IntegrationTemplate[] }); + return this._getAllIntegrationTemplates(); }; getIntegrationInstances = async ( @@ -159,14 +175,21 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { }; getStatic = async (templateName: string, staticPath: string): Promise => { - const data = await (await this.repository.getIntegration(templateName))?.getStatic(staticPath); - if (!data) { + const integration = await this.repository.getIntegration(templateName); + if (integration === null) { + return Promise.reject({ + message: `Template ${templateName} not found`, + statusCode: 404, + }); + } + const data = await integration.getStatic(staticPath); + if (!data.ok) { return Promise.reject({ message: `Asset ${staticPath} not found`, statusCode: 404, }); } - return Promise.resolve(data); + return Promise.resolve(data.value); }; getSchemas = async (templateName: string): Promise => { @@ -188,7 +211,15 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { statusCode: 404, }); } - return Promise.resolve(integration.getAssets()); + const assets = await integration.getAssets(); + if (assets.ok) { + return assets.value; + } + const is404 = (assets.error as { code?: string }).code === 'ENOENT'; + return Promise.reject({ + message: assets.error.message, + statusCode: is404 ? 404 : 500, + }); }; getSampleData = async (templateName: string): Promise<{ sampleData: object[] | null }> => { @@ -199,6 +230,14 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { statusCode: 404, }); } - return Promise.resolve(integration.getSampleData()); + const sampleData = await integration.getSampleData(); + if (sampleData.ok) { + return sampleData.value; + } + const is404 = (sampleData.error as { code?: string }).code === 'ENOENT'; + return Promise.reject({ + message: sampleData.error.message, + statusCode: is404 ? 404 : 500, + }); }; } diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts index 9710ce7fd..b48938227 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -5,7 +5,7 @@ interface CatalogReader { readFile: (filename: string) => Promise; + readFileRaw: (filename: string) => Promise; readDir: (filename: string) => Promise; - isIntegration: (filename: string) => Promise; - isRepository: (filename: string) => Promise; + isDirectory: (filename: string) => Promise; } diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index 21f187bde..ca48c606f 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -3,9 +3,9 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'fs/promises'; import path from 'path'; import { validateTemplate } from '../validators'; +import { LocalCatalogReader } from './local_catalog_reader'; /** * Helper function to compare version numbers. @@ -33,77 +33,47 @@ function compareVersions(a: string, b: string): number { return 0; // a == b } -/** - * Helper function to check if the given path is a directory - * - * @param dirPath The directory to check. - * @returns True if the path is a directory. - */ -async function isDirectory(dirPath: string): Promise { - try { - const stats = await fs.stat(dirPath); - return stats.isDirectory(); - } catch { - return false; - } -} - /** * The Integration class represents the data for Integration Templates. * It is backed by the repository file system. * It includes accessor methods for integration configs, as well as helpers for nested components. */ export class Integration { + reader: CatalogReader; directory: string; name: string; - constructor(directory: string) { + constructor(directory: string, reader?: CatalogReader) { this.directory = directory; this.name = path.basename(directory); + this.reader = reader ?? new LocalCatalogReader(directory); } /** - * Check the integration for validity. - * This is not a deep check, but a quick check to verify that the integration is a valid directory and has a config file. - * - * @returns true if the integration is valid. - */ - async check(): Promise { - if (!(await isDirectory(this.directory))) { - return false; - } - return (await this.getConfig()) !== null; - } - - /** - * Like check(), but thoroughly checks all nested integration dependencies. + * Like getConfig(), but thoroughly checks all nested integration dependencies for validity. * - * @returns true if the integration is valid. + * @returns a Result indicating whether the integration is valid. */ - async deepCheck(): Promise { - if (!(await this.check())) { - console.error('check failed'); - return false; + async deepCheck(): Promise> { + const configResult = await this.getConfig(); + if (!configResult.ok) { + return configResult; } try { - // An integration must have at least one mapping const schemas = await this.getSchemas(); - if (Object.keys(schemas.mappings).length === 0) { - return false; + if (!schemas.ok || Object.keys(schemas.value.mappings).length === 0) { + return { ok: false, error: new Error('The integration has no schemas available') }; } - // An integration must have at least one asset const assets = await this.getAssets(); - if (Object.keys(assets).length === 0) { - return false; + if (!assets.ok || Object.keys(assets).length === 0) { + return { ok: false, error: new Error('An integration must have at least one asset') }; } } catch (err: any) { - // Any loading errors are considered invalid - console.error('Deep check failed for exception', err); - return false; + return { ok: false, error: err }; } - return true; + return configResult; } /** @@ -114,7 +84,7 @@ export class Integration { * @returns A string with the latest version, or null if no versions are available. */ async getLatestVersion(): Promise { - const files = await fs.readdir(this.directory); + const files = await this.reader.readDir(this.directory); const versions: string[] = []; for (const file of files) { @@ -138,35 +108,34 @@ export class Integration { * @param version The version of the config to retrieve. * @returns The config if a valid config matching the version is present, otherwise null. */ - async getConfig(version?: string): Promise { + async getConfig(version?: string): Promise> { + if (!this.reader.isDirectory(this.directory)) { + return { ok: false, error: new Error(`${this.directory} is not a valid directory`) }; + } + const maybeVersion: string | null = version ? version : await this.getLatestVersion(); if (maybeVersion === null) { - return null; + return { + ok: false, + error: new Error(`No valid config matching version ${version} is available`), + }; } const configFile = `${this.name}-${maybeVersion}.json`; - const configPath = path.join(this.directory, configFile); try { - const config = await fs.readFile(configPath, { encoding: 'utf-8' }); + const config = await this.reader.readFile(configFile); const possibleTemplate = JSON.parse(config); - const template = validateTemplate(possibleTemplate); - if (template.ok) { - return template.value; - } - console.error(template.error); - return null; + return validateTemplate(possibleTemplate); } catch (err: any) { if (err instanceof SyntaxError) { console.error(`Syntax errors in ${configFile}`, err); - return null; } if (err instanceof Error && (err as { code?: string }).code === 'ENOENT') { console.error(`Attempted to retrieve non-existent config ${configFile}`); - return null; } - throw new Error('Could not load integration', { cause: err }); + return { ok: false, error: err }; } } @@ -181,30 +150,33 @@ export class Integration { */ async getAssets( version?: string - ): Promise<{ - savedObjects?: object[]; - }> { - const config = await this.getConfig(version); - if (config === null) { - return Promise.reject(new Error('Attempted to get assets of invalid config')); + ): Promise< + Result<{ + savedObjects?: object[]; + }> + > { + const configResult = await this.getConfig(version); + if (!configResult.ok) { + return configResult; } - const result: { savedObjects?: object[] } = {}; + const config = configResult.value; + + const resultValue: { savedObjects?: object[] } = {}; if (config.assets.savedObjects) { const sobjPath = path.join( - this.directory, 'assets', `${config.assets.savedObjects.name}-${config.assets.savedObjects.version}.ndjson` ); try { - const ndjson = await fs.readFile(sobjPath, { encoding: 'utf-8' }); + const ndjson = await this.reader.readFile(sobjPath); const asJson = '[' + ndjson.trim().replace(/\n/g, ',') + ']'; const parsed = JSON.parse(asJson); - result.savedObjects = parsed; + resultValue.savedObjects = parsed; } catch (err: any) { - console.error("Failed to load saved object assets, proceeding as if it's absent", err); + return { ok: false, error: err }; } } - return result; + return { ok: true, value: resultValue }; } /** @@ -217,18 +189,22 @@ export class Integration { */ async getSampleData( version?: string - ): Promise<{ - sampleData: object[] | null; - }> { - const config = await this.getConfig(version); - if (config === null) { - return Promise.reject(new Error('Attempted to get assets of invalid config')); + ): Promise< + Result<{ + sampleData: object[] | null; + }> + > { + const configResult = await this.getConfig(version); + if (!configResult.ok) { + return configResult; } - const result: { sampleData: object[] | null } = { sampleData: null }; + const config = configResult.value; + + const resultValue: { sampleData: object[] | null } = { sampleData: null }; if (config.sampleData) { - const sobjPath = path.join(this.directory, 'data', config.sampleData?.path); + const sobjPath = path.join('data', config.sampleData?.path); try { - const jsonContent = await fs.readFile(sobjPath, { encoding: 'utf-8' }); + const jsonContent = await this.reader.readFile(sobjPath); const parsed = JSON.parse(jsonContent) as object[]; for (const value of parsed) { if (!('@timestamp' in value)) { @@ -243,12 +219,12 @@ export class Integration { ).toISOString(), }); } - result.sampleData = parsed; + resultValue.sampleData = parsed; } catch (err: any) { - console.error("Failed to load saved object assets, proceeding as if it's absent", err); + return { ok: false, error: err }; } } - return result; + return { ok: true, value: resultValue }; } /** @@ -263,32 +239,31 @@ export class Integration { */ async getSchemas( version?: string - ): Promise<{ - mappings: { [key: string]: any }; - }> { - const config = await this.getConfig(version); - if (config === null) { - return Promise.reject(new Error('Attempted to get assets of invalid config')); + ): Promise< + Result<{ + mappings: { [key: string]: any }; + }> + > { + const configResult = await this.getConfig(version); + if (!configResult.ok) { + return configResult; } - const result: { mappings: { [key: string]: any } } = { + const config = configResult.value; + + const resultValue: { mappings: { [key: string]: object } } = { mappings: {}, }; try { for (const component of config.components) { const schemaFile = `${component.name}-${component.version}.mapping.json`; - const rawSchema = await fs.readFile(path.join(this.directory, 'schemas', schemaFile), { - encoding: 'utf-8', - }); + const rawSchema = await this.reader.readFile(path.join('schemas', schemaFile)); const parsedSchema = JSON.parse(rawSchema); - result.mappings[component.name] = parsedSchema; + resultValue.mappings[component.name] = parsedSchema; } } catch (err: any) { - // It's not clear that an invalid schema can be recovered from. - // For integrations to function, we need schemas to be valid. - console.error('Error loading schema', err); - return Promise.reject(new Error('Could not load schema', { cause: err })); + return { ok: false, error: err }; } - return result; + return { ok: true, value: resultValue }; } /** @@ -297,16 +272,13 @@ export class Integration { * @param staticPath The path of the static to retrieve. * @returns A buffer with the static's data if present, otherwise null. */ - async getStatic(staticPath: string): Promise { - const fullStaticPath = path.join(this.directory, 'static', staticPath); + async getStatic(staticPath: string): Promise> { + const fullStaticPath = path.join('static', staticPath); try { - return await fs.readFile(fullStaticPath); + const buffer = await this.reader.readFileRaw(fullStaticPath); + return { ok: true, value: buffer }; } catch (err: any) { - if (err instanceof Error && (err as { code?: string }).code === 'ENOENT') { - console.error(`Static not found: ${staticPath}`); - return null; - } - throw err; + return { ok: false, error: err }; } } } diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts index 2634e7e62..2422ce127 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -6,13 +6,12 @@ import * as fs from 'fs/promises'; import path from 'path'; import sanitize from 'sanitize-filename'; -import { Integration } from './integration'; /** * A CatalogReader that reads from the local filesystem. * Used to read Integration information when the user uploads their own catalog. */ -class LocalCatalogReader implements CatalogReader { +export class LocalCatalogReader implements CatalogReader { directory: string; constructor(directory: string) { @@ -28,6 +27,10 @@ class LocalCatalogReader implements CatalogReader { return await fs.readFile(this._prepare(filename), { encoding: 'utf-8' }); } + async readFileRaw(filename: string): Promise { + return await fs.readFile(this._prepare(filename)); + } + async readDir(dirname: string): Promise { // TODO return empty list if not a directory return await fs.readdir(this._prepare(dirname)); @@ -36,24 +39,4 @@ class LocalCatalogReader implements CatalogReader { async isDirectory(dirname: string): Promise { return (await fs.lstat(this._prepare(dirname))).isDirectory(); } - - async isRepository(dirname: string): Promise { - if (await this.isIntegration(dirname)) { - return false; - } - // If there is at least one integration in a directory, it's a repository. - for (const item of await this.readDir(dirname)) { - if (await this.isIntegration(item)) { - return true; - } - } - return false; - } - - async isIntegration(dirname: string): Promise { - if (!(await this.isDirectory(dirname))) { - return false; - } - return new Integration(this._prepare(dirname)).check(); - } } From c9de36e20924ffd5973e33820ce3ce240cb36880 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 15:56:43 -0700 Subject: [PATCH 10/24] Repair tests for integrations class (unstable) Signed-off-by: Simeon Widdis --- .../repository/__test__/integration.test.ts | 120 +++++++----------- .../integrations/repository/catalog_reader.ts | 6 +- .../integrations/repository/integration.ts | 17 +-- .../repository/local_catalog_reader.ts | 13 +- 4 files changed, 66 insertions(+), 90 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index 2002ad04a..e6611e3a5 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -33,35 +33,7 @@ describe('Integration', () => { beforeEach(() => { integration = new Integration('./sample'); - }); - - describe('check', () => { - it('should return false if the directory does not exist', async () => { - const spy = jest.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => false } as Stats); - - const result = await integration.check(); - - expect(spy).toHaveBeenCalled(); - expect(result).toBe(false); - }); - - it('should return true if the directory exists and getConfig returns a valid template', async () => { - jest.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => true } as Stats); - integration.getConfig = jest.fn().mockResolvedValue(sampleIntegration); - - const result = await integration.check(); - - expect(result).toBe(true); - }); - - it('should return false if the directory exists but getConfig returns null', async () => { - jest.spyOn(fs, 'stat').mockResolvedValue({ isDirectory: () => true } as Stats); - integration.getConfig = jest.fn().mockResolvedValue(null); - - const result = await integration.check(); - - expect(result).toBe(false); - }); + jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); }); describe('getLatestVersion', () => { @@ -94,39 +66,45 @@ describe('Integration', () => { }); describe('getConfig', () => { + it('should return an error if the directory does not exist', async () => { + const spy = jest + .spyOn(fs, 'lstat') + .mockResolvedValueOnce({ isDirectory: () => false } as Stats); + + const result = await integration.getConfig(); + + expect(spy).toHaveBeenCalled(); + expect(result.ok).toBe(false); + }); + it('should return the parsed config template if it is valid', async () => { jest.spyOn(fs, 'readFile').mockResolvedValue(JSON.stringify(sampleIntegration)); const result = await integration.getConfig(sampleIntegration.version); - expect(result).toEqual(sampleIntegration); + expect(result).toEqual({ ok: true, value: sampleIntegration }); }); - it('should return null and log validation errors if the config template is invalid', async () => { + it('should return an error if the config template is invalid', async () => { const invalidTemplate = { ...sampleIntegration, version: 2 }; jest.spyOn(fs, 'readFile').mockResolvedValue(JSON.stringify(invalidTemplate)); - const logValidationErrorsMock = jest.spyOn(console, 'error'); const result = await integration.getConfig(sampleIntegration.version); - expect(result).toBeNull(); - expect(logValidationErrorsMock).toHaveBeenCalled(); + expect(result.ok).toBe(false); }); - it('should return null and log syntax errors if the config file has syntax errors', async () => { + it('should return an error if the config file has syntax errors', async () => { jest.spyOn(fs, 'readFile').mockResolvedValue('Invalid JSON'); - const logSyntaxErrorsMock = jest.spyOn(console, 'error'); const result = await integration.getConfig(sampleIntegration.version); - expect(result).toBeNull(); - expect(logSyntaxErrorsMock).toHaveBeenCalledWith(expect.any(String), expect.any(SyntaxError)); + expect(result.ok).toBe(false); }); - it('should return null and log errors if the integration config does not exist', async () => { - integration.directory = './non-existing-directory'; - const logErrorsMock = jest.spyOn(console, 'error'); - jest.spyOn(fs, 'readFile').mockImplementation((..._args) => { + it('should return an error if the integration config does not exist', async () => { + integration.directory = './empty-directory'; + const readFileMock = jest.spyOn(fs, 'readFile').mockImplementation((..._args) => { // Can't find any information on how to mock an actual file not found error, // But at least according to the current implementation this should be equivalent. const error: any = new Error('ENOENT: File not found'); @@ -136,37 +114,38 @@ describe('Integration', () => { const result = await integration.getConfig(sampleIntegration.version); - expect(jest.spyOn(fs, 'readFile')).toHaveBeenCalled(); - expect(logErrorsMock).toHaveBeenCalledWith(expect.any(String)); - expect(result).toBeNull(); + expect(readFileMock).toHaveBeenCalled(); + expect(result.ok).toBe(false); }); }); describe('getAssets', () => { it('should return linked saved object assets when available', async () => { - integration.getConfig = jest.fn().mockResolvedValue(sampleIntegration); + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleIntegration }); jest.spyOn(fs, 'readFile').mockResolvedValue('{"name":"asset1"}\n{"name":"asset2"}'); const result = await integration.getAssets(sampleIntegration.version); - expect(result.savedObjects).toEqual([{ name: 'asset1' }, { name: 'asset2' }]); + expect(result.ok).toBe(true); + expect((result as any).value.savedObjects).toStrictEqual([ + { name: 'asset1' }, + { name: 'asset2' }, + ]); }); - it('should reject a return if the provided version has no config', async () => { - integration.getConfig = jest.fn().mockResolvedValue(null); + it('should return an error if the provided version has no config', async () => { + integration.getConfig = jest.fn().mockResolvedValue({ ok: false, error: new Error() }); - expect(integration.getAssets()).rejects.toThrowError(); + expect(integration.getAssets()).resolves.toHaveProperty('ok', false); }); - it('should log an error if the saved object assets are invalid', async () => { - const logErrorsMock = jest.spyOn(console, 'error'); - integration.getConfig = jest.fn().mockResolvedValue(sampleIntegration); + it('should return an error if the saved object assets are invalid', async () => { + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleIntegration }); jest.spyOn(fs, 'readFile').mockResolvedValue('{"unclosed":'); const result = await integration.getAssets(sampleIntegration.version); - expect(logErrorsMock).toHaveBeenCalledWith(expect.any(String), expect.any(Error)); - expect(result.savedObjects).toBeUndefined(); + expect(result.ok).toBe(false); }); }); @@ -178,7 +157,7 @@ describe('Integration', () => { { name: 'component2', version: '2.0.0' }, ], }; - integration.getConfig = jest.fn().mockResolvedValue(sampleConfig); + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleConfig }); const mappingFile1 = 'component1-1.0.0.mapping.json'; const mappingFile2 = 'component2-2.0.0.mapping.json'; @@ -190,7 +169,8 @@ describe('Integration', () => { const result = await integration.getSchemas(); - expect(result).toEqual({ + expect(result.ok).toBe(true); + expect((result as any).value).toStrictEqual({ mappings: { component1: { mapping: 'mapping1' }, component2: { mapping: 'mapping2' }, @@ -207,22 +187,20 @@ describe('Integration', () => { ); }); - it('should reject with an error if the config is null', async () => { - integration.getConfig = jest.fn().mockResolvedValue(null); + it('should reject with an error if the config is invalid', async () => { + integration.getConfig = jest.fn().mockResolvedValue({ ok: false, error: new Error() }); - await expect(integration.getSchemas()).rejects.toThrowError( - 'Attempted to get assets of invalid config' - ); + await expect(integration.getSchemas()).resolves.toHaveProperty('ok', false); }); it('should reject with an error if a mapping file is invalid', async () => { const sampleConfig = { components: [{ name: 'component1', version: '1.0.0' }], }; - integration.getConfig = jest.fn().mockResolvedValue(sampleConfig); + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleConfig }); jest.spyOn(fs, 'readFile').mockRejectedValueOnce(new Error('Could not load schema')); - await expect(integration.getSchemas()).rejects.toThrowError('Could not load schema'); + await expect(integration.getSchemas()).resolves.toHaveProperty('ok', false); }); }); @@ -231,21 +209,21 @@ describe('Integration', () => { const readFileMock = jest .spyOn(fs, 'readFile') .mockResolvedValue(Buffer.from('logo data', 'ascii')); - expect(await integration.getStatic('/logo.png')).toStrictEqual( - Buffer.from('logo data', 'ascii') - ); + + const result = await integration.getStatic('logo.png'); + + expect(result.ok).toBe(true); + expect((result as any).value).toStrictEqual(Buffer.from('logo data', 'ascii')); expect(readFileMock).toBeCalledWith(path.join('sample', 'static', 'logo.png')); }); - it('should return null and log an error if the static file is not found', async () => { - const logErrorsMock = jest.spyOn(console, 'error'); + it('should return an error if the static file is not found', async () => { jest.spyOn(fs, 'readFile').mockImplementation((..._args) => { const error: any = new Error('ENOENT: File not found'); error.code = 'ENOENT'; return Promise.reject(error); }); - expect(await integration.getStatic('/logo.png')).toBeNull(); - expect(logErrorsMock).toBeCalledWith(expect.any(String)); + expect(integration.getStatic('/logo.png')).resolves.toHaveProperty('ok', false); }); }); }); diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts index b48938227..db6a70ee6 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -3,9 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +type IntegrationPart = 'assets' | 'data' | 'schemas' | 'static'; + interface CatalogReader { - readFile: (filename: string) => Promise; - readFileRaw: (filename: string) => Promise; + readFile: (filename: string, type?: IntegrationPart) => Promise; + readFileRaw: (filename: string, type?: IntegrationPart) => Promise; readDir: (filename: string) => Promise; isDirectory: (filename: string) => Promise; } diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index ca48c606f..e3514b1ed 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -109,7 +109,7 @@ export class Integration { * @returns The config if a valid config matching the version is present, otherwise null. */ async getConfig(version?: string): Promise> { - if (!this.reader.isDirectory(this.directory)) { + if (!(await this.reader.isDirectory(this.directory))) { return { ok: false, error: new Error(`${this.directory} is not a valid directory`) }; } @@ -163,12 +163,9 @@ export class Integration { const resultValue: { savedObjects?: object[] } = {}; if (config.assets.savedObjects) { - const sobjPath = path.join( - 'assets', - `${config.assets.savedObjects.name}-${config.assets.savedObjects.version}.ndjson` - ); + const sobjPath = `${config.assets.savedObjects.name}-${config.assets.savedObjects.version}.ndjson`; try { - const ndjson = await this.reader.readFile(sobjPath); + const ndjson = await this.reader.readFile(sobjPath, 'assets'); const asJson = '[' + ndjson.trim().replace(/\n/g, ',') + ']'; const parsed = JSON.parse(asJson); resultValue.savedObjects = parsed; @@ -202,9 +199,8 @@ export class Integration { const resultValue: { sampleData: object[] | null } = { sampleData: null }; if (config.sampleData) { - const sobjPath = path.join('data', config.sampleData?.path); try { - const jsonContent = await this.reader.readFile(sobjPath); + const jsonContent = await this.reader.readFile(config.sampleData.path, 'data'); const parsed = JSON.parse(jsonContent) as object[]; for (const value of parsed) { if (!('@timestamp' in value)) { @@ -256,7 +252,7 @@ export class Integration { try { for (const component of config.components) { const schemaFile = `${component.name}-${component.version}.mapping.json`; - const rawSchema = await this.reader.readFile(path.join('schemas', schemaFile)); + const rawSchema = await this.reader.readFile(schemaFile, 'schemas'); const parsedSchema = JSON.parse(rawSchema); resultValue.mappings[component.name] = parsedSchema; } @@ -273,9 +269,8 @@ export class Integration { * @returns A buffer with the static's data if present, otherwise null. */ async getStatic(staticPath: string): Promise> { - const fullStaticPath = path.join('static', staticPath); try { - const buffer = await this.reader.readFileRaw(fullStaticPath); + const buffer = await this.reader.readFileRaw(staticPath, 'static'); return { ok: true, value: buffer }; } catch (err: any) { return { ok: false, error: err }; diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts index 2422ce127..e04259229 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -19,16 +19,17 @@ export class LocalCatalogReader implements CatalogReader { } // Use before any call to `fs` - _prepare(filename: string): string { - return path.join(this.directory, sanitize(filename)); + // Sanitizes filenames by default, manually prepend directories with a prefix if necessary + _prepare(filename: string, prefix?: string): string { + return path.join(this.directory, prefix ?? '.', sanitize(filename)); } - async readFile(filename: string): Promise { - return await fs.readFile(this._prepare(filename), { encoding: 'utf-8' }); + async readFile(filename: string, type?: IntegrationPart): Promise { + return await fs.readFile(this._prepare(filename, type), { encoding: 'utf-8' }); } - async readFileRaw(filename: string): Promise { - return await fs.readFile(this._prepare(filename)); + async readFileRaw(filename: string, type?: IntegrationPart): Promise { + return await fs.readFile(this._prepare(filename, type)); } async readDir(dirname: string): Promise { From 37c798000d0cf9748b2a42d029943d12c6baf2fb Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 16:33:32 -0700 Subject: [PATCH 11/24] Refactor repository for new integration interface Signed-off-by: Simeon Widdis --- .../repository/__test__/repository.test.ts | 24 +++++++------ .../integrations/repository/repository.ts | 34 ++++++++++--------- 2 files changed, 32 insertions(+), 26 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/repository.test.ts b/server/adaptors/integrations/repository/__test__/repository.test.ts index 913968f49..0e08199e9 100644 --- a/server/adaptors/integrations/repository/__test__/repository.test.ts +++ b/server/adaptors/integrations/repository/__test__/repository.test.ts @@ -20,14 +20,11 @@ describe('Repository', () => { describe('getIntegrationList', () => { it('should return an array of Integration instances', async () => { - // Mock fs.readdir to return a list of folders jest.spyOn(fs, 'readdir').mockResolvedValue((['folder1', 'folder2'] as unknown) as Dirent[]); - - // Mock fs.lstat to return a directory status jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); - - // Mock Integration check method to always return true - jest.spyOn(Integration.prototype, 'check').mockResolvedValue(true); + jest + .spyOn(Integration.prototype, 'getConfig') + .mockResolvedValue({ ok: true, value: {} as any }); const integrations = await repository.getIntegrationList(); @@ -48,7 +45,9 @@ describe('Repository', () => { } }); - jest.spyOn(Integration.prototype, 'check').mockResolvedValue(true); + jest + .spyOn(Integration.prototype, 'getConfig') + .mockResolvedValue({ ok: true, value: {} as any }); const integrations = await repository.getIntegrationList(); @@ -67,15 +66,20 @@ describe('Repository', () => { describe('getIntegration', () => { it('should return an Integration instance if it exists and passes the check', async () => { - jest.spyOn(Integration.prototype, 'check').mockResolvedValue(true); + jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); + jest + .spyOn(Integration.prototype, 'getConfig') + .mockResolvedValue({ ok: true, value: {} as any }); const integration = await repository.getIntegration('integrationName'); expect(integration).toBeInstanceOf(Integration); }); - it('should return null if the integration does not exist or fails the check', async () => { - jest.spyOn(Integration.prototype, 'check').mockResolvedValue(false); + it('should return null if the integration does not exist or fails checks', async () => { + jest + .spyOn(Integration.prototype, 'getConfig') + .mockResolvedValue({ ok: false, error: new Error() }); const integration = await repository.getIntegration('invalidIntegration'); diff --git a/server/adaptors/integrations/repository/repository.ts b/server/adaptors/integrations/repository/repository.ts index 00d241327..00039851b 100644 --- a/server/adaptors/integrations/repository/repository.ts +++ b/server/adaptors/integrations/repository/repository.ts @@ -3,31 +3,24 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'fs/promises'; import * as path from 'path'; import { Integration } from './integration'; +import { LocalCatalogReader } from './local_catalog_reader'; export class Repository { + reader: CatalogReader; directory: string; - constructor(directory: string) { + constructor(directory: string, reader?: CatalogReader) { this.directory = directory; + this.reader = reader ?? new LocalCatalogReader(directory); } async getIntegrationList(): Promise { try { - const folders = await fs.readdir(this.directory); - const integrations = Promise.all( - folders.map(async (folder) => { - const integPath = path.join(this.directory, folder); - if (!(await fs.lstat(integPath)).isDirectory()) { - return null; - } - const integ = new Integration(integPath); - return (await integ.check()) ? integ : null; - }) - ); - return (await integrations).filter((x) => x !== null) as Integration[]; + const folders = await this.reader.readDir(this.directory); + const integrations = await Promise.all(folders.map((i) => this.getIntegration(i))); + return integrations.filter((x) => x !== null) as Integration[]; } catch (error) { console.error(`Error reading integration directories in: ${this.directory}`, error); return []; @@ -35,7 +28,16 @@ export class Repository { } async getIntegration(name: string): Promise { - const integ = new Integration(path.join(this.directory, name)); - return (await integ.check()) ? integ : null; + if (!(await this.reader.isDirectory(name))) { + console.error(`Requested integration '${name}' does not exist`); + return null; + } + const integ = new Integration(path.join(this.directory, name), this.reader); + const checkResult = await integ.getConfig(); + if (!checkResult.ok) { + console.error(`Integration '${name}' is invalid:`, checkResult.error); + return null; + } + return integ; } } From b331090bc3423639a2d70e4729cca74460e6c148 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 17:03:51 -0700 Subject: [PATCH 12/24] Fix outer repository and backend tests Signed-off-by: Simeon Widdis --- .../integrations/__test__/kibana_backend.test.ts | 16 +++++++++------- .../__test__/local_repository.test.ts | 12 ++++++++++-- .../integrations/repository/catalog_reader.ts | 1 + .../integrations/repository/integration.ts | 4 ++-- .../repository/local_catalog_reader.ts | 4 ++++ .../integrations/repository/repository.ts | 9 ++++++--- 6 files changed, 32 insertions(+), 14 deletions(-) diff --git a/server/adaptors/integrations/__test__/kibana_backend.test.ts b/server/adaptors/integrations/__test__/kibana_backend.test.ts index 63d62764c..c14ef6f53 100644 --- a/server/adaptors/integrations/__test__/kibana_backend.test.ts +++ b/server/adaptors/integrations/__test__/kibana_backend.test.ts @@ -147,21 +147,23 @@ describe('IntegrationsKibanaBackend', () => { describe('getIntegrationTemplates', () => { it('should get integration templates by name', async () => { const query = { name: 'template1' }; - const integration = { getConfig: jest.fn().mockResolvedValue({ name: 'template1' }) }; + const integration = { + getConfig: jest.fn().mockResolvedValue({ ok: true, value: { name: 'template1' } }), + }; mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); const result = await backend.getIntegrationTemplates(query); expect(mockRepository.getIntegration).toHaveBeenCalledWith(query.name); expect(integration.getConfig).toHaveBeenCalled(); - expect(result).toEqual({ hits: [await integration.getConfig()] }); + expect(result).toEqual({ hits: [{ name: 'template1' }] }); }); it('should get all integration templates', async () => { const integrationList = [ - { getConfig: jest.fn().mockResolvedValue({ name: 'template1' }) }, - { getConfig: jest.fn().mockResolvedValue(null) }, - { getConfig: jest.fn().mockResolvedValue({ name: 'template2' }) }, + { getConfig: jest.fn().mockResolvedValue({ ok: true, value: { name: 'template1' } }) }, + { getConfig: jest.fn().mockResolvedValue({ ok: false, error: new Error() }) }, + { getConfig: jest.fn().mockResolvedValue({ ok: true, value: { name: 'template2' } }) }, ]; mockRepository.getIntegrationList.mockResolvedValue( (integrationList as unknown) as Integration[] @@ -174,7 +176,7 @@ describe('IntegrationsKibanaBackend', () => { expect(integrationList[1].getConfig).toHaveBeenCalled(); expect(integrationList[2].getConfig).toHaveBeenCalled(); expect(result).toEqual({ - hits: [await integrationList[0].getConfig(), await integrationList[2].getConfig()], + hits: [{ name: 'template1' }, { name: 'template2' }], }); }); }); @@ -277,7 +279,7 @@ describe('IntegrationsKibanaBackend', () => { const staticPath = 'path/to/static'; const assetData = Buffer.from('asset data'); const integration = { - getStatic: jest.fn().mockResolvedValue(assetData), + getStatic: jest.fn().mockResolvedValue({ ok: true, value: assetData }), }; mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); diff --git a/server/adaptors/integrations/__test__/local_repository.test.ts b/server/adaptors/integrations/__test__/local_repository.test.ts index 6b0ddc72f..162b95414 100644 --- a/server/adaptors/integrations/__test__/local_repository.test.ts +++ b/server/adaptors/integrations/__test__/local_repository.test.ts @@ -21,7 +21,7 @@ describe('The local repository', () => { } // Otherwise, all directories must be integrations const integ = new Integration(integPath); - await expect(integ.check()).resolves.toBe(true); + expect(integ.getConfig()).resolves.toHaveProperty('ok', true); }) ); }); @@ -29,6 +29,14 @@ describe('The local repository', () => { it('Should pass deep validation for all local integrations.', async () => { const repository: Repository = new Repository(path.join(__dirname, '../__data__/repository')); const integrations: Integration[] = await repository.getIntegrationList(); - await Promise.all(integrations.map((i) => expect(i.deepCheck()).resolves.toBeTruthy())); + await Promise.all( + integrations.map(async (i) => { + const result = await i.deepCheck(); + if (!result.ok) { + console.error(result.error); + } + expect(result.ok).toBe(true); + }) + ); }); }); diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts index db6a70ee6..9683bcaf9 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -10,4 +10,5 @@ interface CatalogReader { readFileRaw: (filename: string, type?: IntegrationPart) => Promise; readDir: (filename: string) => Promise; isDirectory: (filename: string) => Promise; + join: (filename: string) => CatalogReader; } diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index e3514b1ed..ac7d3d16e 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -84,7 +84,7 @@ export class Integration { * @returns A string with the latest version, or null if no versions are available. */ async getLatestVersion(): Promise { - const files = await this.reader.readDir(this.directory); + const files = await this.reader.readDir(''); const versions: string[] = []; for (const file of files) { @@ -109,7 +109,7 @@ export class Integration { * @returns The config if a valid config matching the version is present, otherwise null. */ async getConfig(version?: string): Promise> { - if (!(await this.reader.isDirectory(this.directory))) { + if (!(await this.reader.isDirectory(''))) { return { ok: false, error: new Error(`${this.directory} is not a valid directory`) }; } diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts index e04259229..e95f3c46c 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -40,4 +40,8 @@ export class LocalCatalogReader implements CatalogReader { async isDirectory(dirname: string): Promise { return (await fs.lstat(this._prepare(dirname))).isDirectory(); } + + join(filename: string): LocalCatalogReader { + return new LocalCatalogReader(path.join(this.directory, filename)); + } } diff --git a/server/adaptors/integrations/repository/repository.ts b/server/adaptors/integrations/repository/repository.ts index 00039851b..acea2d4ed 100644 --- a/server/adaptors/integrations/repository/repository.ts +++ b/server/adaptors/integrations/repository/repository.ts @@ -18,8 +18,11 @@ export class Repository { async getIntegrationList(): Promise { try { - const folders = await this.reader.readDir(this.directory); - const integrations = await Promise.all(folders.map((i) => this.getIntegration(i))); + // TODO in the future, we want to support traversing nested directory structures. + const folders = await this.reader.readDir(''); + const integrations = await Promise.all( + folders.map((i) => this.getIntegration(path.basename(i))) + ); return integrations.filter((x) => x !== null) as Integration[]; } catch (error) { console.error(`Error reading integration directories in: ${this.directory}`, error); @@ -32,7 +35,7 @@ export class Repository { console.error(`Requested integration '${name}' does not exist`); return null; } - const integ = new Integration(path.join(this.directory, name), this.reader); + const integ = new Integration(name, this.reader.join(name)); const checkResult = await integ.getConfig(); if (!checkResult.ok) { console.error(`Integration '${name}' is invalid:`, checkResult.error); From 4b29e17c72f381e0a1277564548d4132044c6675 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Tue, 22 Aug 2023 17:16:30 -0700 Subject: [PATCH 13/24] Add tests for sample data Signed-off-by: Simeon Widdis --- .../repository/__test__/integration.test.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index e6611e3a5..105f81cb4 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -226,4 +226,39 @@ describe('Integration', () => { expect(integration.getStatic('/logo.png')).resolves.toHaveProperty('ok', false); }); }); + + describe('getSampleData', () => { + it('should return sample data', async () => { + const sampleConfig = { sampleData: { path: 'sample.json' } }; + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleConfig }); + const readFileMock = jest.spyOn(fs, 'readFile').mockResolvedValue('[{"sample": true}]'); + + const result = await integration.getSampleData(); + + expect(result.ok).toBe(true); + expect((result as any).value.sampleData).toStrictEqual([{ sample: true }]); + expect(readFileMock).toBeCalledWith(path.join('sample', 'data', 'sample.json'), { + encoding: 'utf-8', + }); + }); + + it("should return null if there's no sample data", async () => { + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: {} }); + + const result = await integration.getSampleData(); + + expect(result.ok).toBe(true); + expect((result as any).value.sampleData).toBeNull(); + }); + + it('should catch and fail gracefully on invalid sample data', async () => { + const sampleConfig = { sampleData: { path: 'sample.json' } }; + integration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: sampleConfig }); + jest.spyOn(fs, 'readFile').mockResolvedValue('[{"closingBracket": false]'); + + const result = await integration.getSampleData(); + + expect(result.ok).toBe(false); + }); + }); }); From 1bef29eacd686134f1f22a10c6c8dd31d3545cee Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 10:43:18 -0700 Subject: [PATCH 14/24] Add CatalogReader JavaDocs Signed-off-by: Simeon Widdis --- .../integrations/repository/catalog_reader.ts | 44 +++++++++++++++++-- .../repository/local_catalog_reader.ts | 19 ++++++-- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_reader.ts index 9683bcaf9..c52fb9d0f 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_reader.ts @@ -6,9 +6,47 @@ type IntegrationPart = 'assets' | 'data' | 'schemas' | 'static'; interface CatalogReader { + /** + * Reads a file from the data source. + * + * @param filename The name of the file to read. + * @param type Optional. The type of integration part to read ('assets', 'data', 'schemas', or 'static'). + * @returns A Promise that resolves with the content of the file as a string. + */ readFile: (filename: string, type?: IntegrationPart) => Promise; + + /** + * Reads a file from the data source as raw binary data. + * + * @param filename The name of the file to read. + * @param type Optional. The type of integration part to read ('assets', 'data', 'schemas', or 'static'). + * @returns A Promise that resolves with the content of the file as a Buffer. + */ readFileRaw: (filename: string, type?: IntegrationPart) => Promise; - readDir: (filename: string) => Promise; - isDirectory: (filename: string) => Promise; - join: (filename: string) => CatalogReader; + + /** + * Reads the contents of a directory from the data source. + * + * @param dirname The name of the directory to read. + * @returns A Promise that resolves with an array of filenames within the directory. + */ + readDir: (dirname: string) => Promise; + + /** + * Checks if a given path on the data source is a directory. + * + * @param dirname The path to check. + * @returns A Promise that resolves with a boolean indicating whether the path is a directory or not. + */ + isDirectory: (dirname: string) => Promise; + + /** + * Creates a new CatalogReader instance with the specified subdirectory appended to the current directory. + * Since CatalogReaders sanitize given paths by default, + * this is useful for exploring nested data. + * + * @param subdirectory The path to append to the current directory. + * @returns A new CatalogReader instance. + */ + join: (subdirectory: string) => CatalogReader; } diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/local_catalog_reader.ts index e95f3c46c..4f473022c 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/local_catalog_reader.ts @@ -14,14 +14,25 @@ import sanitize from 'sanitize-filename'; export class LocalCatalogReader implements CatalogReader { directory: string; + /** + * Creates a new LocalCatalogReader instance. + * + * @param directory The base directory from which to read files. This is not sanitized. + */ constructor(directory: string) { this.directory = directory; } - // Use before any call to `fs` - // Sanitizes filenames by default, manually prepend directories with a prefix if necessary - _prepare(filename: string, prefix?: string): string { - return path.join(this.directory, prefix ?? '.', sanitize(filename)); + /** + * Prepares a filename for use in filesystem operations by sanitizing and joining it with the base directory. + * This method is intended to be used before any filesystem-related call. + * + * @param filename The name of the file to prepare. + * @param subdir Optional. A subdirectory to prepend to the filename. Not sanitized. + * @returns The prepared path for the file, including the base directory and optional prefix. + */ + _prepare(filename: string, subdir?: string): string { + return path.join(this.directory, subdir ?? '.', sanitize(filename)); } async readFile(filename: string, type?: IntegrationPart): Promise { From a18cf47912e22f38a23cbe58f57b71272defe9a9 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:07:46 -0700 Subject: [PATCH 15/24] Repair integrations builder Signed-off-by: Simeon Widdis --- .../integrations/__test__/builder.test.ts | 65 ++++++++++++------- .../integrations/integrations_builder.ts | 24 +++++-- 2 files changed, 59 insertions(+), 30 deletions(-) diff --git a/server/adaptors/integrations/__test__/builder.test.ts b/server/adaptors/integrations/__test__/builder.test.ts index 81af1d6f6..6fa0d7918 100644 --- a/server/adaptors/integrations/__test__/builder.test.ts +++ b/server/adaptors/integrations/__test__/builder.test.ts @@ -93,13 +93,23 @@ describe('IntegrationInstanceBuilder', () => { ], }; - // Mock the implementation of the methods in the Integration class - sampleIntegration.deepCheck = jest.fn().mockResolvedValue(true); - sampleIntegration.getAssets = jest.fn().mockResolvedValue({ savedObjects: remappedAssets }); - sampleIntegration.getConfig = jest.fn().mockResolvedValue({ + const mockTemplate: Partial = { name: 'integration-template', type: 'integration-type', - }); + assets: { + savedObjects: { + name: 'assets', + version: '1.0.0', + }, + }, + }; + + // Mock the implementation of the methods in the Integration class + sampleIntegration.deepCheck = jest.fn().mockResolvedValue({ ok: true, value: mockTemplate }); + sampleIntegration.getAssets = jest + .fn() + .mockResolvedValue({ ok: true, value: { savedObjects: remappedAssets } }); + sampleIntegration.getConfig = jest.fn().mockResolvedValue({ ok: true, value: mockTemplate }); // Mock builder sub-methods const remapIDsSpy = jest.spyOn(builder, 'remapIDs'); @@ -121,22 +131,24 @@ describe('IntegrationInstanceBuilder', () => { dataSource: 'instance-datasource', name: 'instance-name', }; - sampleIntegration.deepCheck = jest.fn().mockResolvedValue(false); + sampleIntegration.deepCheck = jest + .fn() + .mockResolvedValue({ ok: false, error: new Error('Mock error') }); - await expect(builder.build(sampleIntegration, options)).rejects.toThrowError( - 'Integration is not valid' - ); + await expect(builder.build(sampleIntegration, options)).rejects.toThrowError('Mock error'); }); - it('should reject with an error if getAssets throws an error', async () => { + it('should reject with an error if getAssets rejects', async () => { const options = { dataSource: 'instance-datasource', name: 'instance-name', }; const errorMessage = 'Failed to get assets'; - sampleIntegration.deepCheck = jest.fn().mockResolvedValue(true); - sampleIntegration.getAssets = jest.fn().mockRejectedValue(new Error(errorMessage)); + sampleIntegration.deepCheck = jest.fn().mockResolvedValue({ ok: true, value: {} }); + sampleIntegration.getAssets = jest + .fn() + .mockResolvedValue({ ok: false, error: new Error(errorMessage) }); await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(errorMessage); }); @@ -153,22 +165,24 @@ describe('IntegrationInstanceBuilder', () => { }, ]; const errorMessage = 'Failed to post assets'; - sampleIntegration.deepCheck = jest.fn().mockResolvedValue(true); - sampleIntegration.getAssets = jest.fn().mockResolvedValue({ savedObjects: remappedAssets }); + sampleIntegration.deepCheck = jest.fn().mockResolvedValue({ ok: true, value: {} }); + sampleIntegration.getAssets = jest + .fn() + .mockResolvedValue({ ok: true, value: { savedObjects: remappedAssets } }); builder.postAssets = jest.fn().mockRejectedValue(new Error(errorMessage)); await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(errorMessage); }); - it('should reject with an error if getConfig returns null', async () => { - const options = { - dataSource: 'instance-datasource', - name: 'instance-name', - }; - sampleIntegration.getConfig = jest.fn().mockResolvedValue(null); + // it('should reject with an error if getConfig returns null', async () => { + // const options = { + // dataSource: 'instance-datasource', + // name: 'instance-name', + // }; + // sampleIntegration.getConfig = jest.fn().mockResolvedValue(null); - await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(); - }); + // await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(); + // }); }); describe('remapIDs', () => { @@ -264,8 +278,11 @@ describe('IntegrationInstanceBuilder', () => { it('should build an integration instance', async () => { const integration = { getConfig: jest.fn().mockResolvedValue({ - name: 'integration-template', - type: 'integration-type', + ok: true, + value: { + name: 'integration-template', + type: 'integration-type', + }, }), }; const refs = [ diff --git a/server/adaptors/integrations/integrations_builder.ts b/server/adaptors/integrations/integrations_builder.ts index b12e1a132..117816187 100644 --- a/server/adaptors/integrations/integrations_builder.ts +++ b/server/adaptors/integrations/integrations_builder.ts @@ -21,15 +21,21 @@ export class IntegrationInstanceBuilder { this.client = client; } - async build(integration: Integration, options: BuilderOptions): Promise { + build(integration: Integration, options: BuilderOptions): Promise { const instance = integration .deepCheck() .then((result) => { - if (!result) { - return Promise.reject(new Error('Integration is not valid')); + if (!result.ok) { + return Promise.reject(result.error); } + return integration.getAssets(); + }) + .then((assets) => { + if (!assets.ok) { + return Promise.reject(assets.error); + } + return assets.value; }) - .then(() => integration.getAssets()) .then((assets) => this.remapIDs(assets.savedObjects!)) .then((assets) => this.remapDataSource(assets, options.dataSource)) .then((assets) => this.postAssets(assets)) @@ -90,10 +96,16 @@ export class IntegrationInstanceBuilder { refs: AssetReference[], options: BuilderOptions ): Promise { - const config: IntegrationTemplate = (await integration.getConfig())!; + const config: Result = await integration.getConfig(); + console.error(config); + if (!config.ok) { + return Promise.reject( + new Error('Attempted to create instance with invalid template', config.error) + ); + } return Promise.resolve({ name: options.name, - templateName: config.name, + templateName: config.value.name, dataSource: options.dataSource, creationDate: new Date().toISOString(), assets: refs, From 047b14a96b71d62646cfc07e58bd2d304ff1afcd Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:09:12 -0700 Subject: [PATCH 16/24] Remove extra commented test Signed-off-by: Simeon Widdis --- server/adaptors/integrations/__test__/builder.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/server/adaptors/integrations/__test__/builder.test.ts b/server/adaptors/integrations/__test__/builder.test.ts index 6fa0d7918..84387ca8c 100644 --- a/server/adaptors/integrations/__test__/builder.test.ts +++ b/server/adaptors/integrations/__test__/builder.test.ts @@ -173,16 +173,6 @@ describe('IntegrationInstanceBuilder', () => { await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(errorMessage); }); - - // it('should reject with an error if getConfig returns null', async () => { - // const options = { - // dataSource: 'instance-datasource', - // name: 'instance-name', - // }; - // sampleIntegration.getConfig = jest.fn().mockResolvedValue(null); - - // await expect(builder.build(sampleIntegration, options)).rejects.toThrowError(); - // }); }); describe('remapIDs', () => { From e65fe8b568135f26ccef3fa0a2eaf6597a1e6572 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:11:25 -0700 Subject: [PATCH 17/24] Remove unnecessary log statement Signed-off-by: Simeon Widdis --- server/adaptors/integrations/integrations_builder.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/server/adaptors/integrations/integrations_builder.ts b/server/adaptors/integrations/integrations_builder.ts index 117816187..960912e12 100644 --- a/server/adaptors/integrations/integrations_builder.ts +++ b/server/adaptors/integrations/integrations_builder.ts @@ -97,7 +97,6 @@ export class IntegrationInstanceBuilder { options: BuilderOptions ): Promise { const config: Result = await integration.getConfig(); - console.error(config); if (!config.ok) { return Promise.reject( new Error('Attempted to create instance with invalid template', config.error) From 00ddb5ba35e15868d9966b70a05a6f1bb5ff4cfe Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:35:12 -0700 Subject: [PATCH 18/24] Repair getSchemas behavior to return correct type Let it be known at on this day, with this commit, I have truly grokked why we don't use `any` in typescript. Signed-off-by: Simeon Widdis --- server/adaptors/integrations/integrations_adaptor.ts | 4 +--- .../adaptors/integrations/integrations_kibana_backend.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/adaptors/integrations/integrations_adaptor.ts b/server/adaptors/integrations/integrations_adaptor.ts index cf7f4853e..574a4d25d 100644 --- a/server/adaptors/integrations/integrations_adaptor.ts +++ b/server/adaptors/integrations/integrations_adaptor.ts @@ -24,9 +24,7 @@ export interface IntegrationsAdaptor { getStatic: (templateName: string, path: string) => Promise; - getSchemas: ( - templateName: string - ) => Promise<{ mappings: { [key: string]: unknown }; schemas: { [key: string]: unknown } }>; + getSchemas: (templateName: string) => Promise<{ mappings: { [key: string]: unknown } }>; getAssets: (templateName: string) => Promise<{ savedObjects?: unknown }>; diff --git a/server/adaptors/integrations/integrations_kibana_backend.ts b/server/adaptors/integrations/integrations_kibana_backend.ts index b503390a2..494b6d174 100644 --- a/server/adaptors/integrations/integrations_kibana_backend.ts +++ b/server/adaptors/integrations/integrations_kibana_backend.ts @@ -192,7 +192,7 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { return Promise.resolve(data.value); }; - getSchemas = async (templateName: string): Promise => { + getSchemas = async (templateName: string): Promise<{ mappings: { [key: string]: unknown } }> => { const integration = await this.repository.getIntegration(templateName); if (integration === null) { return Promise.reject({ @@ -200,7 +200,11 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { statusCode: 404, }); } - return Promise.resolve(integration.getSchemas()); + const result = await integration.getSchemas(); + if (result.ok) { + return result.value; + } + return Promise.reject(result.error); }; getAssets = async (templateName: string): Promise<{ savedObjects?: any }> => { From 8af7f4ab824577c07e39ac7aeb96972cacb49729 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:52:50 -0700 Subject: [PATCH 19/24] Add tests for getSchemas Signed-off-by: Simeon Widdis --- .../__test__/kibana_backend.test.ts | 55 ++++++++++++++++++- .../integrations_kibana_backend.ts | 19 ++++--- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/server/adaptors/integrations/__test__/kibana_backend.test.ts b/server/adaptors/integrations/__test__/kibana_backend.test.ts index c14ef6f53..da8ade961 100644 --- a/server/adaptors/integrations/__test__/kibana_backend.test.ts +++ b/server/adaptors/integrations/__test__/kibana_backend.test.ts @@ -290,7 +290,7 @@ describe('IntegrationsKibanaBackend', () => { expect(result).toEqual(assetData); }); - it('should reject with a 404 if asset is not found', async () => { + it('should reject with a 404 if integration is not found', async () => { const templateName = 'template1'; const staticPath = 'path/to/static'; mockRepository.getIntegration.mockResolvedValue(null); @@ -300,6 +300,59 @@ describe('IntegrationsKibanaBackend', () => { 404 ); }); + + it('should reject with a 404 if static data is not found', async () => { + const templateName = 'template1'; + const staticPath = 'path/to/static'; + mockRepository.getIntegration.mockResolvedValue({ + getStatic: jest.fn().mockResolvedValue({ + ok: false, + error: { message: 'Not found', code: 'ENOENT' }, + }), + } as any); + + await expect(backend.getStatic(templateName, staticPath)).rejects.toHaveProperty( + 'statusCode', + 404 + ); + }); + }); + + describe('getSchemas', () => { + it('should get schema data', async () => { + const templateName = 'template1'; + const staticPath = 'path/to/static'; + const schemaData = { mappings: { test: {} } }; + const integration = { + getSchemas: jest.fn().mockResolvedValue({ ok: true, value: schemaData }), + }; + mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + + const result = await backend.getSchemas(templateName); + + expect(mockRepository.getIntegration).toHaveBeenCalledWith(templateName); + expect(integration.getSchemas).toHaveBeenCalled(); + expect(result).toEqual(schemaData); + }); + + it('should reject with a 404 if integration is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue(null); + + await expect(backend.getSchemas(templateName)).rejects.toHaveProperty('statusCode', 404); + }); + + it('should reject with a 404 if schema data is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue({ + getSchemas: jest.fn().mockResolvedValue({ + ok: false, + error: { message: 'Not found', code: 'ENOENT' }, + }), + } as any); + + await expect(backend.getSchemas(templateName)).rejects.toHaveProperty('statusCode', 404); + }); }); describe('getAssetStatus', () => { diff --git a/server/adaptors/integrations/integrations_kibana_backend.ts b/server/adaptors/integrations/integrations_kibana_backend.ts index 494b6d174..da99efa7d 100644 --- a/server/adaptors/integrations/integrations_kibana_backend.ts +++ b/server/adaptors/integrations/integrations_kibana_backend.ts @@ -183,13 +183,14 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { }); } const data = await integration.getStatic(staticPath); - if (!data.ok) { - return Promise.reject({ - message: `Asset ${staticPath} not found`, - statusCode: 404, - }); + if (data.ok) { + return data.value; } - return Promise.resolve(data.value); + const is404 = (data.error as { code?: string }).code === 'ENOENT'; + return Promise.reject({ + message: data.error.message, + statusCode: is404 ? 404 : 500, + }); }; getSchemas = async (templateName: string): Promise<{ mappings: { [key: string]: unknown } }> => { @@ -204,7 +205,11 @@ export class IntegrationsKibanaBackend implements IntegrationsAdaptor { if (result.ok) { return result.value; } - return Promise.reject(result.error); + const is404 = (result.error as { code?: string }).code === 'ENOENT'; + return Promise.reject({ + message: result.error.message, + statusCode: is404 ? 404 : 500, + }); }; getAssets = async (templateName: string): Promise<{ savedObjects?: any }> => { From a31963da5b631427647568b3b4a767a8199992ad Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 23 Aug 2023 11:57:32 -0700 Subject: [PATCH 20/24] Add tests for asset and sample data backend methods Signed-off-by: Simeon Widdis --- .../__test__/kibana_backend.test.ts | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/server/adaptors/integrations/__test__/kibana_backend.test.ts b/server/adaptors/integrations/__test__/kibana_backend.test.ts index da8ade961..4b333e081 100644 --- a/server/adaptors/integrations/__test__/kibana_backend.test.ts +++ b/server/adaptors/integrations/__test__/kibana_backend.test.ts @@ -321,7 +321,6 @@ describe('IntegrationsKibanaBackend', () => { describe('getSchemas', () => { it('should get schema data', async () => { const templateName = 'template1'; - const staticPath = 'path/to/static'; const schemaData = { mappings: { test: {} } }; const integration = { getSchemas: jest.fn().mockResolvedValue({ ok: true, value: schemaData }), @@ -355,6 +354,78 @@ describe('IntegrationsKibanaBackend', () => { }); }); + describe('getAssets', () => { + it('should get asset data', async () => { + const templateName = 'template1'; + const assetData = { savedObjects: [{ test: true }] }; + const integration = { + getAssets: jest.fn().mockResolvedValue({ ok: true, value: assetData }), + }; + mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + + const result = await backend.getAssets(templateName); + + expect(mockRepository.getIntegration).toHaveBeenCalledWith(templateName); + expect(integration.getAssets).toHaveBeenCalled(); + expect(result).toEqual(assetData); + }); + + it('should reject with a 404 if integration is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue(null); + + await expect(backend.getAssets(templateName)).rejects.toHaveProperty('statusCode', 404); + }); + + it('should reject with a 404 if asset data is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue({ + getAssets: jest.fn().mockResolvedValue({ + ok: false, + error: { message: 'Not found', code: 'ENOENT' }, + }), + } as any); + + await expect(backend.getAssets(templateName)).rejects.toHaveProperty('statusCode', 404); + }); + }); + + describe('getSampleData', () => { + it('should get sample data', async () => { + const templateName = 'template1'; + const sampleData = { sampleData: [{ test: true }] }; + const integration = { + getSampleData: jest.fn().mockResolvedValue({ ok: true, value: sampleData }), + }; + mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + + const result = await backend.getSampleData(templateName); + + expect(mockRepository.getIntegration).toHaveBeenCalledWith(templateName); + expect(integration.getSampleData).toHaveBeenCalled(); + expect(result).toEqual(sampleData); + }); + + it('should reject with a 404 if integration is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue(null); + + await expect(backend.getSampleData(templateName)).rejects.toHaveProperty('statusCode', 404); + }); + + it('should reject with a 404 if sample data is not found', async () => { + const templateName = 'template1'; + mockRepository.getIntegration.mockResolvedValue({ + getSampleData: jest.fn().mockResolvedValue({ + ok: false, + error: { message: 'Not found', code: 'ENOENT' }, + }), + } as any); + + await expect(backend.getSampleData(templateName)).rejects.toHaveProperty('statusCode', 404); + }); + }); + describe('getAssetStatus', () => { it('should return "available" if all assets are available', async () => { const assets = [ From 339fb0934cf598e2485765fbd7a901facfe471c8 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 13 Sep 2023 11:32:49 -0700 Subject: [PATCH 21/24] Refactor major class names Signed-off-by: Simeon Widdis --- .../{kibana_backend.test.ts => manager.test.ts} | 6 +++--- ...tions_kibana_backend.ts => integrations_manager.ts} | 4 ++-- .../{catalog_reader.ts => catalog_data_adaptor.ts} | 4 ++-- .../{local_catalog_reader.ts => fs_data_adaptor.ts} | 10 +++++----- server/adaptors/integrations/repository/integration.ts | 8 ++++---- server/adaptors/integrations/repository/repository.ts | 8 ++++---- server/routes/integrations/integrations_router.ts | 4 ++-- 7 files changed, 22 insertions(+), 22 deletions(-) rename server/adaptors/integrations/__test__/{kibana_backend.test.ts => manager.test.ts} (99%) rename server/adaptors/integrations/{integrations_kibana_backend.ts => integrations_manager.ts} (98%) rename server/adaptors/integrations/repository/{catalog_reader.ts => catalog_data_adaptor.ts} (95%) rename server/adaptors/integrations/repository/{local_catalog_reader.ts => fs_data_adaptor.ts} (83%) diff --git a/server/adaptors/integrations/__test__/kibana_backend.test.ts b/server/adaptors/integrations/__test__/manager.test.ts similarity index 99% rename from server/adaptors/integrations/__test__/kibana_backend.test.ts rename to server/adaptors/integrations/__test__/manager.test.ts index 4b333e081..7d1972cbf 100644 --- a/server/adaptors/integrations/__test__/kibana_backend.test.ts +++ b/server/adaptors/integrations/__test__/manager.test.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { IntegrationsKibanaBackend } from '../integrations_kibana_backend'; +import { IntegrationsManager } from '../integrations_manager'; import { SavedObject, SavedObjectsClientContract } from '../../../../../../src/core/server/types'; import { Repository } from '../repository/repository'; import { IntegrationInstanceBuilder } from '../integrations_builder'; @@ -13,7 +13,7 @@ import { SavedObjectsFindResponse } from '../../../../../../src/core/server'; describe('IntegrationsKibanaBackend', () => { let mockSavedObjectsClient: jest.Mocked; let mockRepository: jest.Mocked; - let backend: IntegrationsKibanaBackend; + let backend: IntegrationsManager; beforeEach(() => { mockSavedObjectsClient = { @@ -26,7 +26,7 @@ describe('IntegrationsKibanaBackend', () => { getIntegration: jest.fn(), getIntegrationList: jest.fn(), } as any; - backend = new IntegrationsKibanaBackend(mockSavedObjectsClient, mockRepository); + backend = new IntegrationsManager(mockSavedObjectsClient, mockRepository); }); describe('deleteIntegrationInstance', () => { diff --git a/server/adaptors/integrations/integrations_kibana_backend.ts b/server/adaptors/integrations/integrations_manager.ts similarity index 98% rename from server/adaptors/integrations/integrations_kibana_backend.ts rename to server/adaptors/integrations/integrations_manager.ts index da99efa7d..857e223b7 100644 --- a/server/adaptors/integrations/integrations_kibana_backend.ts +++ b/server/adaptors/integrations/integrations_manager.ts @@ -4,13 +4,13 @@ */ import path from 'path'; -import { addRequestToMetric } from '../../../server/common/metrics/metrics_helper'; +import { addRequestToMetric } from '../../common/metrics/metrics_helper'; import { IntegrationsAdaptor } from './integrations_adaptor'; import { SavedObject, SavedObjectsClientContract } from '../../../../../src/core/server/types'; import { IntegrationInstanceBuilder } from './integrations_builder'; import { Repository } from './repository/repository'; -export class IntegrationsKibanaBackend implements IntegrationsAdaptor { +export class IntegrationsManager implements IntegrationsAdaptor { client: SavedObjectsClientContract; instanceBuilder: IntegrationInstanceBuilder; repository: Repository; diff --git a/server/adaptors/integrations/repository/catalog_reader.ts b/server/adaptors/integrations/repository/catalog_data_adaptor.ts similarity index 95% rename from server/adaptors/integrations/repository/catalog_reader.ts rename to server/adaptors/integrations/repository/catalog_data_adaptor.ts index c52fb9d0f..c791909b4 100644 --- a/server/adaptors/integrations/repository/catalog_reader.ts +++ b/server/adaptors/integrations/repository/catalog_data_adaptor.ts @@ -5,7 +5,7 @@ type IntegrationPart = 'assets' | 'data' | 'schemas' | 'static'; -interface CatalogReader { +interface CatalogDataAdaptor { /** * Reads a file from the data source. * @@ -48,5 +48,5 @@ interface CatalogReader { * @param subdirectory The path to append to the current directory. * @returns A new CatalogReader instance. */ - join: (subdirectory: string) => CatalogReader; + join: (subdirectory: string) => CatalogDataAdaptor; } diff --git a/server/adaptors/integrations/repository/local_catalog_reader.ts b/server/adaptors/integrations/repository/fs_data_adaptor.ts similarity index 83% rename from server/adaptors/integrations/repository/local_catalog_reader.ts rename to server/adaptors/integrations/repository/fs_data_adaptor.ts index 4f473022c..fd6c31290 100644 --- a/server/adaptors/integrations/repository/local_catalog_reader.ts +++ b/server/adaptors/integrations/repository/fs_data_adaptor.ts @@ -8,14 +8,14 @@ import path from 'path'; import sanitize from 'sanitize-filename'; /** - * A CatalogReader that reads from the local filesystem. + * A CatalogDataAdaptor that reads from the local filesystem. * Used to read Integration information when the user uploads their own catalog. */ -export class LocalCatalogReader implements CatalogReader { +export class FileSystemCatalogDataAdaptor implements CatalogDataAdaptor { directory: string; /** - * Creates a new LocalCatalogReader instance. + * Creates a new FileSystemCatalogDataAdaptor instance. * * @param directory The base directory from which to read files. This is not sanitized. */ @@ -52,7 +52,7 @@ export class LocalCatalogReader implements CatalogReader { return (await fs.lstat(this._prepare(dirname))).isDirectory(); } - join(filename: string): LocalCatalogReader { - return new LocalCatalogReader(path.join(this.directory, filename)); + join(filename: string): FileSystemCatalogDataAdaptor { + return new FileSystemCatalogDataAdaptor(path.join(this.directory, filename)); } } diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index ac7d3d16e..3f5c63677 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -5,7 +5,7 @@ import path from 'path'; import { validateTemplate } from '../validators'; -import { LocalCatalogReader } from './local_catalog_reader'; +import { FileSystemCatalogDataAdaptor } from './fs_data_adaptor'; /** * Helper function to compare version numbers. @@ -39,14 +39,14 @@ function compareVersions(a: string, b: string): number { * It includes accessor methods for integration configs, as well as helpers for nested components. */ export class Integration { - reader: CatalogReader; + reader: CatalogDataAdaptor; directory: string; name: string; - constructor(directory: string, reader?: CatalogReader) { + constructor(directory: string, reader?: CatalogDataAdaptor) { this.directory = directory; this.name = path.basename(directory); - this.reader = reader ?? new LocalCatalogReader(directory); + this.reader = reader ?? new FileSystemCatalogDataAdaptor(directory); } /** diff --git a/server/adaptors/integrations/repository/repository.ts b/server/adaptors/integrations/repository/repository.ts index acea2d4ed..f3ff15688 100644 --- a/server/adaptors/integrations/repository/repository.ts +++ b/server/adaptors/integrations/repository/repository.ts @@ -5,15 +5,15 @@ import * as path from 'path'; import { Integration } from './integration'; -import { LocalCatalogReader } from './local_catalog_reader'; +import { FileSystemCatalogDataAdaptor } from './fs_data_adaptor'; export class Repository { - reader: CatalogReader; + reader: CatalogDataAdaptor; directory: string; - constructor(directory: string, reader?: CatalogReader) { + constructor(directory: string, reader?: CatalogDataAdaptor) { this.directory = directory; - this.reader = reader ?? new LocalCatalogReader(directory); + this.reader = reader ?? new FileSystemCatalogDataAdaptor(directory); } async getIntegrationList(): Promise { diff --git a/server/routes/integrations/integrations_router.ts b/server/routes/integrations/integrations_router.ts index 5a4813127..46fe47768 100644 --- a/server/routes/integrations/integrations_router.ts +++ b/server/routes/integrations/integrations_router.ts @@ -13,7 +13,7 @@ import { OpenSearchDashboardsRequest, OpenSearchDashboardsResponseFactory, } from '../../../../../src/core/server/http/router'; -import { IntegrationsKibanaBackend } from '../../adaptors/integrations/integrations_kibana_backend'; +import { IntegrationsManager } from '../../adaptors/integrations/integrations_manager'; /** * Handle an `OpenSearchDashboardsRequest` using the provided `callback` function. @@ -54,7 +54,7 @@ const getAdaptor = ( context: RequestHandlerContext, _request: OpenSearchDashboardsRequest ): IntegrationsAdaptor => { - return new IntegrationsKibanaBackend(context.core.savedObjects.client); + return new IntegrationsManager(context.core.savedObjects.client); }; export function registerIntegrationsRoute(router: IRouter) { From fae680f3e7eea1b9615bbcc5a3d71bcc7580cfb2 Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 13 Sep 2023 16:26:44 -0700 Subject: [PATCH 22/24] (WIP) begin refactoring functionality into adaptor Signed-off-by: Simeon Widdis --- .../repository/__test__/integration.test.ts | 2 + .../repository/catalog_data_adaptor.ts | 29 ++-- .../repository/fs_data_adaptor.ts | 133 +++++++++++++++--- .../integrations/repository/integration.ts | 133 +++++------------- .../integrations/repository/repository.ts | 19 ++- 5 files changed, 180 insertions(+), 136 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index 105f81cb4..7898f485b 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -7,6 +7,7 @@ import * as fs from 'fs/promises'; import { Integration } from '../integration'; import { Dirent, Stats } from 'fs'; import * as path from 'path'; +import { FileSystemCatalogDataAdaptor } from '../fs_data_adaptor'; jest.mock('fs/promises'); @@ -79,6 +80,7 @@ describe('Integration', () => { it('should return the parsed config template if it is valid', async () => { jest.spyOn(fs, 'readFile').mockResolvedValue(JSON.stringify(sampleIntegration)); + jest.spyOn(fs, 'lstat').mockResolvedValueOnce({ isDirectory: () => true } as Stats); const result = await integration.getConfig(sampleIntegration.version); diff --git a/server/adaptors/integrations/repository/catalog_data_adaptor.ts b/server/adaptors/integrations/repository/catalog_data_adaptor.ts index c791909b4..6373fee4d 100644 --- a/server/adaptors/integrations/repository/catalog_data_adaptor.ts +++ b/server/adaptors/integrations/repository/catalog_data_adaptor.ts @@ -7,13 +7,13 @@ type IntegrationPart = 'assets' | 'data' | 'schemas' | 'static'; interface CatalogDataAdaptor { /** - * Reads a file from the data source. + * Reads a Json or NDJson file from the data source. * * @param filename The name of the file to read. * @param type Optional. The type of integration part to read ('assets', 'data', 'schemas', or 'static'). * @returns A Promise that resolves with the content of the file as a string. */ - readFile: (filename: string, type?: IntegrationPart) => Promise; + readFile: (filename: string, type?: IntegrationPart) => Promise>; /** * Reads a file from the data source as raw binary data. @@ -22,31 +22,38 @@ interface CatalogDataAdaptor { * @param type Optional. The type of integration part to read ('assets', 'data', 'schemas', or 'static'). * @returns A Promise that resolves with the content of the file as a Buffer. */ - readFileRaw: (filename: string, type?: IntegrationPart) => Promise; + readFileRaw: (filename: string, type?: IntegrationPart) => Promise>; /** - * Reads the contents of a directory from the data source. + * Reads the contents of a repository directory from the data source to find integrations. * * @param dirname The name of the directory to read. * @returns A Promise that resolves with an array of filenames within the directory. */ - readDir: (dirname: string) => Promise; + findIntegrations: (dirname?: string) => Promise>; /** - * Checks if a given path on the data source is a directory. + * Reads the contents of an integration version to find available versions. + * + * @param dirname The name of the directory to read. + * @returns A Promise that resolves with an array of filenames within the directory. + */ + findIntegrationVersions: (dirname?: string) => Promise>; + + /** + * Determine whether a directory is an integration, repository, or otherwise. * * @param dirname The path to check. * @returns A Promise that resolves with a boolean indicating whether the path is a directory or not. */ - isDirectory: (dirname: string) => Promise; + getDirectoryType: (dirname?: string) => Promise<'integration' | 'repository' | 'unknown'>; /** - * Creates a new CatalogReader instance with the specified subdirectory appended to the current directory. - * Since CatalogReaders sanitize given paths by default, - * this is useful for exploring nested data. + * Creates a new CatalogDataAdaptor instance with the specified subdirectory appended to the current directory. + * Useful for exploring nested data without needing to know the instance type. * * @param subdirectory The path to append to the current directory. - * @returns A new CatalogReader instance. + * @returns A new CatalogDataAdaptor instance. */ join: (subdirectory: string) => CatalogDataAdaptor; } diff --git a/server/adaptors/integrations/repository/fs_data_adaptor.ts b/server/adaptors/integrations/repository/fs_data_adaptor.ts index fd6c31290..df1c6781a 100644 --- a/server/adaptors/integrations/repository/fs_data_adaptor.ts +++ b/server/adaptors/integrations/repository/fs_data_adaptor.ts @@ -5,7 +5,48 @@ import * as fs from 'fs/promises'; import path from 'path'; -import sanitize from 'sanitize-filename'; + +/** + * Helper function to compare version numbers. + * Assumes that the version numbers are valid, produces undefined behavior otherwise. + * + * @param a Left-hand number + * @param b Right-hand number + * @returns -1 if a > b, 1 if a < b, 0 otherwise. + */ +function compareVersions(a: string, b: string): number { + const aParts = a.split('.').map(Number.parseInt); + const bParts = b.split('.').map(Number.parseInt); + + for (let i = 0; i < Math.max(aParts.length, bParts.length); i++) { + const aValue = i < aParts.length ? aParts[i] : 0; + const bValue = i < bParts.length ? bParts[i] : 0; + + if (aValue > bValue) { + return -1; // a > b + } else if (aValue < bValue) { + return 1; // a < b + } + } + + return 0; // a == b +} + +function tryParseNDJson(content: string): object[] | null { + try { + const objects = []; + for (const line of content.split('\n')) { + if (line.trim() === '') { + // Other OSD ndjson parsers skip whitespace lines + continue; + } + objects.push(JSON.parse(line)); + } + return objects; + } catch (err: any) { + return null; + } +} /** * A CatalogDataAdaptor that reads from the local filesystem. @@ -23,33 +64,85 @@ export class FileSystemCatalogDataAdaptor implements CatalogDataAdaptor { this.directory = directory; } - /** - * Prepares a filename for use in filesystem operations by sanitizing and joining it with the base directory. - * This method is intended to be used before any filesystem-related call. - * - * @param filename The name of the file to prepare. - * @param subdir Optional. A subdirectory to prepend to the filename. Not sanitized. - * @returns The prepared path for the file, including the base directory and optional prefix. - */ - _prepare(filename: string, subdir?: string): string { - return path.join(this.directory, subdir ?? '.', sanitize(filename)); + async readFile(filename: string, type?: IntegrationPart): Promise> { + let content; + try { + content = await fs.readFile(path.join(this.directory, type ?? '.', filename), { + encoding: 'utf-8', + }); + } catch (err: any) { + return { ok: false, error: err }; + } + // First try to parse as JSON, then NDJSON, then fail. + try { + const parsed = JSON.parse(content); + return { ok: true, value: parsed }; + } catch (err: any) { + const parsed = tryParseNDJson(content); + if (parsed) { + return { ok: true, value: parsed }; + } + return { + ok: false, + error: new Error('Unable to parse file as JSON or NDJson', { cause: err }), + }; + } } - async readFile(filename: string, type?: IntegrationPart): Promise { - return await fs.readFile(this._prepare(filename, type), { encoding: 'utf-8' }); + async readFileRaw(filename: string, type?: IntegrationPart): Promise> { + try { + const buffer = await fs.readFile(path.join(this.directory, type ?? '.', filename)); + return { ok: true, value: buffer }; + } catch (err: any) { + return { ok: false, error: err }; + } } - async readFileRaw(filename: string, type?: IntegrationPart): Promise { - return await fs.readFile(this._prepare(filename, type)); + async findIntegrations(dirname: string = '.'): Promise> { + try { + const files = await fs.readdir(path.join(this.directory, dirname)); + return { ok: true, value: files }; + } catch (err: any) { + return { ok: false, error: err }; + } } - async readDir(dirname: string): Promise { - // TODO return empty list if not a directory - return await fs.readdir(this._prepare(dirname)); + async findIntegrationVersions(dirname: string = '.'): Promise> { + let files; + const integPath = path.join(this.directory, dirname); + try { + files = await fs.readdir(integPath); + } catch (err: any) { + return { ok: false, error: err }; + } + const versions: string[] = []; + + for (const file of files) { + // TODO handle nested repositories -- assumes integrations are 1 level deep + if (path.extname(file) === '.json' && file.startsWith(`${path.basename(integPath)}-`)) { + const version = file.substring(path.basename(integPath).length + 1, file.length - 5); + if (!version.match(/^\d+(\.\d+)*$/)) { + continue; + } + versions.push(version); + } + } + + versions.sort((a, b) => compareVersions(a, b)); + return { ok: true, value: versions }; } - async isDirectory(dirname: string): Promise { - return (await fs.lstat(this._prepare(dirname))).isDirectory(); + async getDirectoryType(dirname?: string): Promise<'integration' | 'repository' | 'unknown'> { + const isDir = (await fs.lstat(path.join(this.directory, dirname ?? '.'))).isDirectory(); + if (!isDir) { + return 'unknown'; + } + // Sloppily just check for one mandatory integration directory to distinguish. + // Improve if we need to distinguish a repository with an integration named "schemas". + const hasSchemas = ( + await fs.lstat(path.join(this.directory, dirname ?? '.', 'schemas')) + ).isDirectory(); + return hasSchemas ? 'integration' : 'repository'; } join(filename: string): FileSystemCatalogDataAdaptor { diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index 3f5c63677..2b99635ea 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -7,32 +7,6 @@ import path from 'path'; import { validateTemplate } from '../validators'; import { FileSystemCatalogDataAdaptor } from './fs_data_adaptor'; -/** - * Helper function to compare version numbers. - * Assumes that the version numbers are valid, produces undefined behavior otherwise. - * - * @param a Left-hand number - * @param b Right-hand number - * @returns -1 if a > b, 1 if a < b, 0 otherwise. - */ -function compareVersions(a: string, b: string): number { - const aParts = a.split('.').map(Number.parseInt); - const bParts = b.split('.').map(Number.parseInt); - - for (let i = 0; i < Math.max(aParts.length, bParts.length); i++) { - const aValue = i < aParts.length ? aParts[i] : 0; - const bValue = i < bParts.length ? bParts[i] : 0; - - if (aValue > bValue) { - return -1; // a > b - } else if (aValue < bValue) { - return 1; // a < b - } - } - - return 0; // a == b -} - /** * The Integration class represents the data for Integration Templates. * It is backed by the repository file system. @@ -84,22 +58,12 @@ export class Integration { * @returns A string with the latest version, or null if no versions are available. */ async getLatestVersion(): Promise { - const files = await this.reader.readDir(''); - const versions: string[] = []; - - for (const file of files) { - if (path.extname(file) === '.json' && file.startsWith(`${this.name}-`)) { - const version = file.substring(this.name.length + 1, file.length - 5); - if (!version.match(/^\d+(\.\d+)*$/)) { - continue; - } - versions.push(version); - } + const versions = await this.reader.findIntegrationVersions(); + if (!versions.ok) { + console.error(versions.error); + return null; } - - versions.sort((a, b) => compareVersions(a, b)); - - return versions.length > 0 ? versions[0] : null; + return versions.value.length > 0 ? versions.value[0] : null; } /** @@ -109,8 +73,8 @@ export class Integration { * @returns The config if a valid config matching the version is present, otherwise null. */ async getConfig(version?: string): Promise> { - if (!(await this.reader.isDirectory(''))) { - return { ok: false, error: new Error(`${this.directory} is not a valid directory`) }; + if ((await this.reader.getDirectoryType()) !== 'integration') { + return { ok: false, error: new Error(`${this.directory} is not a valid integration`) }; } const maybeVersion: string | null = version ? version : await this.getLatestVersion(); @@ -124,19 +88,8 @@ export class Integration { const configFile = `${this.name}-${maybeVersion}.json`; - try { - const config = await this.reader.readFile(configFile); - const possibleTemplate = JSON.parse(config); - return validateTemplate(possibleTemplate); - } catch (err: any) { - if (err instanceof SyntaxError) { - console.error(`Syntax errors in ${configFile}`, err); - } - if (err instanceof Error && (err as { code?: string }).code === 'ENOENT') { - console.error(`Attempted to retrieve non-existent config ${configFile}`); - } - return { ok: false, error: err }; - } + const config = await this.reader.readFile(configFile); + return validateTemplate(config); } /** @@ -164,14 +117,11 @@ export class Integration { const resultValue: { savedObjects?: object[] } = {}; if (config.assets.savedObjects) { const sobjPath = `${config.assets.savedObjects.name}-${config.assets.savedObjects.version}.ndjson`; - try { - const ndjson = await this.reader.readFile(sobjPath, 'assets'); - const asJson = '[' + ndjson.trim().replace(/\n/g, ',') + ']'; - const parsed = JSON.parse(asJson); - resultValue.savedObjects = parsed; - } catch (err: any) { - return { ok: false, error: err }; + const assets = await this.reader.readFile(sobjPath, 'assets'); + if (!assets.ok) { + return assets; } + resultValue.savedObjects = assets.value as object[]; } return { ok: true, value: resultValue }; } @@ -199,26 +149,26 @@ export class Integration { const resultValue: { sampleData: object[] | null } = { sampleData: null }; if (config.sampleData) { - try { - const jsonContent = await this.reader.readFile(config.sampleData.path, 'data'); - const parsed = JSON.parse(jsonContent) as object[]; - for (const value of parsed) { - if (!('@timestamp' in value)) { - continue; - } - // Randomly scatter timestamps across last 10 minutes - // Assume for now that the ordering of events isn't important, can change to a sequence if needed - // Also doesn't handle fields like `observedTimestamp` if present - Object.assign(value, { - '@timestamp': new Date( - Date.now() - Math.floor(Math.random() * 1000 * 60 * 10) - ).toISOString(), - }); + const jsonContent = await this.reader.readFile(config.sampleData.path, 'data'); + if (!jsonContent.ok) { + return jsonContent; + } + for (const value of jsonContent.value as object[]) { + if (!('@timestamp' in value)) { + continue; + } + // Randomly scatter timestamps across last 10 minutes + // Assume for now that the ordering of events isn't important, can change to a sequence if needed + // Also doesn't handle fields like `observedTimestamp` if present + const newTime = new Date( + Date.now() - Math.floor(Math.random() * 1000 * 60 * 10) + ).toISOString(); + Object.assign(value, { '@timestamp': newTime }); + if ('observedTimestamp' in value) { + Object.assign(value, { observedTimestamp: newTime }); } - resultValue.sampleData = parsed; - } catch (err: any) { - return { ok: false, error: err }; } + resultValue.sampleData = jsonContent.value as object[]; } return { ok: true, value: resultValue }; } @@ -249,15 +199,13 @@ export class Integration { const resultValue: { mappings: { [key: string]: object } } = { mappings: {}, }; - try { - for (const component of config.components) { - const schemaFile = `${component.name}-${component.version}.mapping.json`; - const rawSchema = await this.reader.readFile(schemaFile, 'schemas'); - const parsedSchema = JSON.parse(rawSchema); - resultValue.mappings[component.name] = parsedSchema; + for (const component of config.components) { + const schemaFile = `${component.name}-${component.version}.mapping.json`; + const schema = await this.reader.readFile(schemaFile, 'schemas'); + if (!schema.ok) { + return schema; } - } catch (err: any) { - return { ok: false, error: err }; + resultValue.mappings[component.name] = schema.value; } return { ok: true, value: resultValue }; } @@ -269,11 +217,6 @@ export class Integration { * @returns A buffer with the static's data if present, otherwise null. */ async getStatic(staticPath: string): Promise> { - try { - const buffer = await this.reader.readFileRaw(staticPath, 'static'); - return { ok: true, value: buffer }; - } catch (err: any) { - return { ok: false, error: err }; - } + return await this.reader.readFileRaw(staticPath, 'static'); } } diff --git a/server/adaptors/integrations/repository/repository.ts b/server/adaptors/integrations/repository/repository.ts index f3ff15688..cf8e0312a 100644 --- a/server/adaptors/integrations/repository/repository.ts +++ b/server/adaptors/integrations/repository/repository.ts @@ -17,21 +17,20 @@ export class Repository { } async getIntegrationList(): Promise { - try { - // TODO in the future, we want to support traversing nested directory structures. - const folders = await this.reader.readDir(''); - const integrations = await Promise.all( - folders.map((i) => this.getIntegration(path.basename(i))) - ); - return integrations.filter((x) => x !== null) as Integration[]; - } catch (error) { - console.error(`Error reading integration directories in: ${this.directory}`, error); + // TODO in the future, we want to support traversing nested directory structures. + const folders = await this.reader.findIntegrations(); + if (!folders.ok) { + console.error(`Error reading integration directories in: ${this.directory}`, folders.error); return []; } + const integrations = await Promise.all( + folders.value.map((i) => this.getIntegration(path.basename(i))) + ); + return integrations.filter((x) => x !== null) as Integration[]; } async getIntegration(name: string): Promise { - if (!(await this.reader.isDirectory(name))) { + if ((await this.reader.getDirectoryType(name)) !== 'integration') { console.error(`Requested integration '${name}' does not exist`); return null; } From 309662671c424503d1a28fcb7ae52379d8fe316c Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Wed, 13 Sep 2023 16:42:25 -0700 Subject: [PATCH 23/24] Finish migrating functionality to data adaptor Signed-off-by: Simeon Widdis --- .../integrations/repository/__test__/repository.test.ts | 2 +- server/adaptors/integrations/repository/integration.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/adaptors/integrations/repository/__test__/repository.test.ts b/server/adaptors/integrations/repository/__test__/repository.test.ts index 0e08199e9..c452acd56 100644 --- a/server/adaptors/integrations/repository/__test__/repository.test.ts +++ b/server/adaptors/integrations/repository/__test__/repository.test.ts @@ -38,7 +38,7 @@ describe('Repository', () => { // Mock fs.lstat to return a mix of directories and files jest.spyOn(fs, 'lstat').mockImplementation(async (toLstat) => { - if (toLstat === path.join('path', 'to', 'directory', 'folder1')) { + if (toLstat.toString().startsWith(path.join('path', 'to', 'directory', 'folder1'))) { return { isDirectory: () => true } as Stats; } else { return { isDirectory: () => false } as Stats; diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index 2b99635ea..d9caa9162 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -89,7 +89,10 @@ export class Integration { const configFile = `${this.name}-${maybeVersion}.json`; const config = await this.reader.readFile(configFile); - return validateTemplate(config); + if (!config.ok) { + return config; + } + return validateTemplate(config.value); } /** From 5498724b8bf384d1066512eb782b42180282cd1d Mon Sep 17 00:00:00 2001 From: Simeon Widdis Date: Thu, 14 Sep 2023 10:47:36 -0700 Subject: [PATCH 24/24] Rename integration types for more clarity Signed-off-by: Simeon Widdis --- .../integrations/__test__/builder.test.ts | 12 +++---- .../__test__/local_repository.test.ts | 12 ++++--- .../integrations/__test__/manager.test.ts | 32 ++++++++++++------- .../integrations/__test__/validators.test.ts | 8 ++--- .../integrations/integrations_builder.ts | 8 ++--- .../integrations/integrations_manager.ts | 11 ++++--- .../repository/__test__/integration.test.ts | 8 ++--- .../repository/__test__/repository.test.ts | 24 +++++++------- .../integrations/repository/integration.ts | 6 ++-- .../integrations/repository/repository.ts | 12 +++---- server/adaptors/integrations/types.ts | 4 +-- server/adaptors/integrations/validators.ts | 4 +-- 12 files changed, 77 insertions(+), 64 deletions(-) diff --git a/server/adaptors/integrations/__test__/builder.test.ts b/server/adaptors/integrations/__test__/builder.test.ts index 84387ca8c..33aff3497 100644 --- a/server/adaptors/integrations/__test__/builder.test.ts +++ b/server/adaptors/integrations/__test__/builder.test.ts @@ -5,7 +5,7 @@ import { SavedObjectsClientContract } from '../../../../../../src/core/server'; import { IntegrationInstanceBuilder } from '../integrations_builder'; -import { Integration } from '../repository/integration'; +import { IntegrationReader } from '../repository/integration'; const mockSavedObjectsClient: SavedObjectsClientContract = ({ bulkCreate: jest.fn(), @@ -16,7 +16,7 @@ const mockSavedObjectsClient: SavedObjectsClientContract = ({ update: jest.fn(), } as unknown) as SavedObjectsClientContract; -const sampleIntegration: Integration = ({ +const sampleIntegration: IntegrationReader = ({ deepCheck: jest.fn().mockResolvedValue(true), getAssets: jest.fn().mockResolvedValue({ savedObjects: [ @@ -34,7 +34,7 @@ const sampleIntegration: Integration = ({ name: 'integration-template', type: 'integration-type', }), -} as unknown) as Integration; +} as unknown) as IntegrationReader; describe('IntegrationInstanceBuilder', () => { let builder: IntegrationInstanceBuilder; @@ -93,7 +93,7 @@ describe('IntegrationInstanceBuilder', () => { ], }; - const mockTemplate: Partial = { + const mockTemplate: Partial = { name: 'integration-template', type: 'integration-type', assets: { @@ -298,7 +298,7 @@ describe('IntegrationInstanceBuilder', () => { }; const instance = await builder.buildInstance( - (integration as unknown) as Integration, + (integration as unknown) as IntegrationReader, refs, options ); @@ -326,7 +326,7 @@ describe('IntegrationInstanceBuilder', () => { }; await expect( - builder.buildInstance((integration as unknown) as Integration, refs, options) + builder.buildInstance((integration as unknown) as IntegrationReader, refs, options) ).rejects.toThrowError(); }); }); diff --git a/server/adaptors/integrations/__test__/local_repository.test.ts b/server/adaptors/integrations/__test__/local_repository.test.ts index 162b95414..f1bfeb9b2 100644 --- a/server/adaptors/integrations/__test__/local_repository.test.ts +++ b/server/adaptors/integrations/__test__/local_repository.test.ts @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Repository } from '../repository/repository'; -import { Integration } from '../repository/integration'; +import { RepositoryReader } from '../repository/repository'; +import { IntegrationReader } from '../repository/integration'; import path from 'path'; import * as fs from 'fs/promises'; @@ -20,15 +20,17 @@ describe('The local repository', () => { return Promise.resolve(null); } // Otherwise, all directories must be integrations - const integ = new Integration(integPath); + const integ = new IntegrationReader(integPath); expect(integ.getConfig()).resolves.toHaveProperty('ok', true); }) ); }); it('Should pass deep validation for all local integrations.', async () => { - const repository: Repository = new Repository(path.join(__dirname, '../__data__/repository')); - const integrations: Integration[] = await repository.getIntegrationList(); + const repository: RepositoryReader = new RepositoryReader( + path.join(__dirname, '../__data__/repository') + ); + const integrations: IntegrationReader[] = await repository.getIntegrationList(); await Promise.all( integrations.map(async (i) => { const result = await i.deepCheck(); diff --git a/server/adaptors/integrations/__test__/manager.test.ts b/server/adaptors/integrations/__test__/manager.test.ts index 7d1972cbf..75b89e520 100644 --- a/server/adaptors/integrations/__test__/manager.test.ts +++ b/server/adaptors/integrations/__test__/manager.test.ts @@ -5,14 +5,14 @@ import { IntegrationsManager } from '../integrations_manager'; import { SavedObject, SavedObjectsClientContract } from '../../../../../../src/core/server/types'; -import { Repository } from '../repository/repository'; +import { RepositoryReader } from '../repository/repository'; import { IntegrationInstanceBuilder } from '../integrations_builder'; -import { Integration } from '../repository/integration'; +import { IntegrationReader } from '../repository/integration'; import { SavedObjectsFindResponse } from '../../../../../../src/core/server'; describe('IntegrationsKibanaBackend', () => { let mockSavedObjectsClient: jest.Mocked; - let mockRepository: jest.Mocked; + let mockRepository: jest.Mocked; let backend: IntegrationsManager; beforeEach(() => { @@ -150,7 +150,9 @@ describe('IntegrationsKibanaBackend', () => { const integration = { getConfig: jest.fn().mockResolvedValue({ ok: true, value: { name: 'template1' } }), }; - mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue( + (integration as unknown) as IntegrationReader + ); const result = await backend.getIntegrationTemplates(query); @@ -166,7 +168,7 @@ describe('IntegrationsKibanaBackend', () => { { getConfig: jest.fn().mockResolvedValue({ ok: true, value: { name: 'template2' } }) }, ]; mockRepository.getIntegrationList.mockResolvedValue( - (integrationList as unknown) as Integration[] + (integrationList as unknown) as IntegrationReader[] ); const result = await backend.getIntegrationTemplates(); @@ -226,7 +228,7 @@ describe('IntegrationsKibanaBackend', () => { build: jest.fn().mockResolvedValue({ name, dataset: 'nginx', namespace: 'prod' }), }; const createdInstance = { name, dataset: 'nginx', namespace: 'prod' }; - mockRepository.getIntegration.mockResolvedValue((template as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue((template as unknown) as IntegrationReader); mockSavedObjectsClient.create.mockResolvedValue(({ result: 'created', } as unknown) as SavedObject); @@ -265,7 +267,7 @@ describe('IntegrationsKibanaBackend', () => { build: jest.fn().mockRejectedValue(new Error('Failed to build instance')), }; backend.instanceBuilder = (instanceBuilder as unknown) as IntegrationInstanceBuilder; - mockRepository.getIntegration.mockResolvedValue((template as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue((template as unknown) as IntegrationReader); await expect( backend.loadIntegrationInstance(templateName, name, 'datasource') @@ -281,7 +283,9 @@ describe('IntegrationsKibanaBackend', () => { const integration = { getStatic: jest.fn().mockResolvedValue({ ok: true, value: assetData }), }; - mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue( + (integration as unknown) as IntegrationReader + ); const result = await backend.getStatic(templateName, staticPath); @@ -325,7 +329,9 @@ describe('IntegrationsKibanaBackend', () => { const integration = { getSchemas: jest.fn().mockResolvedValue({ ok: true, value: schemaData }), }; - mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue( + (integration as unknown) as IntegrationReader + ); const result = await backend.getSchemas(templateName); @@ -361,7 +367,9 @@ describe('IntegrationsKibanaBackend', () => { const integration = { getAssets: jest.fn().mockResolvedValue({ ok: true, value: assetData }), }; - mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue( + (integration as unknown) as IntegrationReader + ); const result = await backend.getAssets(templateName); @@ -397,7 +405,9 @@ describe('IntegrationsKibanaBackend', () => { const integration = { getSampleData: jest.fn().mockResolvedValue({ ok: true, value: sampleData }), }; - mockRepository.getIntegration.mockResolvedValue((integration as unknown) as Integration); + mockRepository.getIntegration.mockResolvedValue( + (integration as unknown) as IntegrationReader + ); const result = await backend.getSampleData(templateName); diff --git a/server/adaptors/integrations/__test__/validators.test.ts b/server/adaptors/integrations/__test__/validators.test.ts index 3c6e133f5..6c09b595b 100644 --- a/server/adaptors/integrations/__test__/validators.test.ts +++ b/server/adaptors/integrations/__test__/validators.test.ts @@ -5,7 +5,7 @@ import { validateTemplate, validateInstance } from '../validators'; -const validTemplate: IntegrationTemplate = { +const validTemplate: IntegrationConfig = { name: 'test', version: '1.0.0', license: 'Apache-2.0', @@ -29,7 +29,7 @@ const validInstance: IntegrationInstance = { describe('validateTemplate', () => { it('Returns a success value for a valid Integration Template', () => { - const result: Result = validateTemplate(validTemplate); + const result: Result = validateTemplate(validTemplate); expect(result.ok).toBe(true); expect((result as any).value).toBe(validTemplate); }); @@ -38,7 +38,7 @@ describe('validateTemplate', () => { const sample: any = structuredClone(validTemplate); sample.license = undefined; - const result: Result = validateTemplate(sample); + const result: Result = validateTemplate(sample); expect(result.ok).toBe(false); expect((result as any).error).toBeInstanceOf(Error); @@ -48,7 +48,7 @@ describe('validateTemplate', () => { const sample: any = structuredClone(validTemplate); sample.components[0].name = 'not-logs'; - const result: Result = validateTemplate(sample); + const result: Result = validateTemplate(sample); expect(result.ok).toBe(false); expect((result as any).error).toBeInstanceOf(Error); diff --git a/server/adaptors/integrations/integrations_builder.ts b/server/adaptors/integrations/integrations_builder.ts index 960912e12..7a8026cea 100644 --- a/server/adaptors/integrations/integrations_builder.ts +++ b/server/adaptors/integrations/integrations_builder.ts @@ -6,7 +6,7 @@ import { v4 as uuidv4 } from 'uuid'; import { uuidRx } from 'public/components/custom_panels/redux/panel_slice'; import { SavedObjectsClientContract } from '../../../../../src/core/server'; -import { Integration } from './repository/integration'; +import { IntegrationReader } from './repository/integration'; import { SavedObjectsBulkCreateObject } from '../../../../../src/core/public'; interface BuilderOptions { @@ -21,7 +21,7 @@ export class IntegrationInstanceBuilder { this.client = client; } - build(integration: Integration, options: BuilderOptions): Promise { + build(integration: IntegrationReader, options: BuilderOptions): Promise { const instance = integration .deepCheck() .then((result) => { @@ -92,11 +92,11 @@ export class IntegrationInstanceBuilder { } async buildInstance( - integration: Integration, + integration: IntegrationReader, refs: AssetReference[], options: BuilderOptions ): Promise { - const config: Result = await integration.getConfig(); + const config: Result = await integration.getConfig(); if (!config.ok) { return Promise.reject( new Error('Attempted to create instance with invalid template', config.error) diff --git a/server/adaptors/integrations/integrations_manager.ts b/server/adaptors/integrations/integrations_manager.ts index 857e223b7..d365e48ee 100644 --- a/server/adaptors/integrations/integrations_manager.ts +++ b/server/adaptors/integrations/integrations_manager.ts @@ -8,16 +8,17 @@ import { addRequestToMetric } from '../../common/metrics/metrics_helper'; import { IntegrationsAdaptor } from './integrations_adaptor'; import { SavedObject, SavedObjectsClientContract } from '../../../../../src/core/server/types'; import { IntegrationInstanceBuilder } from './integrations_builder'; -import { Repository } from './repository/repository'; +import { RepositoryReader } from './repository/repository'; export class IntegrationsManager implements IntegrationsAdaptor { client: SavedObjectsClientContract; instanceBuilder: IntegrationInstanceBuilder; - repository: Repository; + repository: RepositoryReader; - constructor(client: SavedObjectsClientContract, repository?: Repository) { + constructor(client: SavedObjectsClientContract, repository?: RepositoryReader) { this.client = client; - this.repository = repository ?? new Repository(path.join(__dirname, '__data__/repository')); + this.repository = + repository ?? new RepositoryReader(path.join(__dirname, '__data__/repository')); this.instanceBuilder = new IntegrationInstanceBuilder(this.client); } @@ -57,7 +58,7 @@ export class IntegrationsManager implements IntegrationsAdaptor { _getAllIntegrationTemplates = async (): Promise => { const integrationList = await this.repository.getIntegrationList(); const configResults = await Promise.all(integrationList.map((x) => x.getConfig())); - const configs = configResults.filter((cfg) => cfg.ok) as Array<{ value: IntegrationTemplate }>; + const configs = configResults.filter((cfg) => cfg.ok) as Array<{ value: IntegrationConfig }>; return Promise.resolve({ hits: configs.map((cfg) => cfg.value) }); }; diff --git a/server/adaptors/integrations/repository/__test__/integration.test.ts b/server/adaptors/integrations/repository/__test__/integration.test.ts index 7898f485b..7ffbb176b 100644 --- a/server/adaptors/integrations/repository/__test__/integration.test.ts +++ b/server/adaptors/integrations/repository/__test__/integration.test.ts @@ -4,7 +4,7 @@ */ import * as fs from 'fs/promises'; -import { Integration } from '../integration'; +import { IntegrationReader } from '../integration'; import { Dirent, Stats } from 'fs'; import * as path from 'path'; import { FileSystemCatalogDataAdaptor } from '../fs_data_adaptor'; @@ -12,8 +12,8 @@ import { FileSystemCatalogDataAdaptor } from '../fs_data_adaptor'; jest.mock('fs/promises'); describe('Integration', () => { - let integration: Integration; - const sampleIntegration: IntegrationTemplate = { + let integration: IntegrationReader; + const sampleIntegration: IntegrationConfig = { name: 'sample', version: '2.0.0', license: 'Apache-2.0', @@ -33,7 +33,7 @@ describe('Integration', () => { }; beforeEach(() => { - integration = new Integration('./sample'); + integration = new IntegrationReader('./sample'); jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); }); diff --git a/server/adaptors/integrations/repository/__test__/repository.test.ts b/server/adaptors/integrations/repository/__test__/repository.test.ts index c452acd56..d66fc5e86 100644 --- a/server/adaptors/integrations/repository/__test__/repository.test.ts +++ b/server/adaptors/integrations/repository/__test__/repository.test.ts @@ -4,18 +4,18 @@ */ import * as fs from 'fs/promises'; -import { Repository } from '../repository'; -import { Integration } from '../integration'; +import { RepositoryReader } from '../repository'; +import { IntegrationReader } from '../integration'; import { Dirent, Stats } from 'fs'; import path from 'path'; jest.mock('fs/promises'); describe('Repository', () => { - let repository: Repository; + let repository: RepositoryReader; beforeEach(() => { - repository = new Repository('path/to/directory'); + repository = new RepositoryReader('path/to/directory'); }); describe('getIntegrationList', () => { @@ -23,14 +23,14 @@ describe('Repository', () => { jest.spyOn(fs, 'readdir').mockResolvedValue((['folder1', 'folder2'] as unknown) as Dirent[]); jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); jest - .spyOn(Integration.prototype, 'getConfig') + .spyOn(IntegrationReader.prototype, 'getConfig') .mockResolvedValue({ ok: true, value: {} as any }); const integrations = await repository.getIntegrationList(); expect(integrations).toHaveLength(2); - expect(integrations[0]).toBeInstanceOf(Integration); - expect(integrations[1]).toBeInstanceOf(Integration); + expect(integrations[0]).toBeInstanceOf(IntegrationReader); + expect(integrations[1]).toBeInstanceOf(IntegrationReader); }); it('should filter out null values from the integration list', async () => { @@ -46,13 +46,13 @@ describe('Repository', () => { }); jest - .spyOn(Integration.prototype, 'getConfig') + .spyOn(IntegrationReader.prototype, 'getConfig') .mockResolvedValue({ ok: true, value: {} as any }); const integrations = await repository.getIntegrationList(); expect(integrations).toHaveLength(1); - expect(integrations[0]).toBeInstanceOf(Integration); + expect(integrations[0]).toBeInstanceOf(IntegrationReader); }); it('should handle errors and return an empty array', async () => { @@ -68,17 +68,17 @@ describe('Repository', () => { it('should return an Integration instance if it exists and passes the check', async () => { jest.spyOn(fs, 'lstat').mockResolvedValue({ isDirectory: () => true } as Stats); jest - .spyOn(Integration.prototype, 'getConfig') + .spyOn(IntegrationReader.prototype, 'getConfig') .mockResolvedValue({ ok: true, value: {} as any }); const integration = await repository.getIntegration('integrationName'); - expect(integration).toBeInstanceOf(Integration); + expect(integration).toBeInstanceOf(IntegrationReader); }); it('should return null if the integration does not exist or fails checks', async () => { jest - .spyOn(Integration.prototype, 'getConfig') + .spyOn(IntegrationReader.prototype, 'getConfig') .mockResolvedValue({ ok: false, error: new Error() }); const integration = await repository.getIntegration('invalidIntegration'); diff --git a/server/adaptors/integrations/repository/integration.ts b/server/adaptors/integrations/repository/integration.ts index d9caa9162..fca1aef5c 100644 --- a/server/adaptors/integrations/repository/integration.ts +++ b/server/adaptors/integrations/repository/integration.ts @@ -12,7 +12,7 @@ import { FileSystemCatalogDataAdaptor } from './fs_data_adaptor'; * It is backed by the repository file system. * It includes accessor methods for integration configs, as well as helpers for nested components. */ -export class Integration { +export class IntegrationReader { reader: CatalogDataAdaptor; directory: string; name: string; @@ -28,7 +28,7 @@ export class Integration { * * @returns a Result indicating whether the integration is valid. */ - async deepCheck(): Promise> { + async deepCheck(): Promise> { const configResult = await this.getConfig(); if (!configResult.ok) { return configResult; @@ -72,7 +72,7 @@ export class Integration { * @param version The version of the config to retrieve. * @returns The config if a valid config matching the version is present, otherwise null. */ - async getConfig(version?: string): Promise> { + async getConfig(version?: string): Promise> { if ((await this.reader.getDirectoryType()) !== 'integration') { return { ok: false, error: new Error(`${this.directory} is not a valid integration`) }; } diff --git a/server/adaptors/integrations/repository/repository.ts b/server/adaptors/integrations/repository/repository.ts index cf8e0312a..08200d474 100644 --- a/server/adaptors/integrations/repository/repository.ts +++ b/server/adaptors/integrations/repository/repository.ts @@ -4,10 +4,10 @@ */ import * as path from 'path'; -import { Integration } from './integration'; +import { IntegrationReader } from './integration'; import { FileSystemCatalogDataAdaptor } from './fs_data_adaptor'; -export class Repository { +export class RepositoryReader { reader: CatalogDataAdaptor; directory: string; @@ -16,7 +16,7 @@ export class Repository { this.reader = reader ?? new FileSystemCatalogDataAdaptor(directory); } - async getIntegrationList(): Promise { + async getIntegrationList(): Promise { // TODO in the future, we want to support traversing nested directory structures. const folders = await this.reader.findIntegrations(); if (!folders.ok) { @@ -26,15 +26,15 @@ export class Repository { const integrations = await Promise.all( folders.value.map((i) => this.getIntegration(path.basename(i))) ); - return integrations.filter((x) => x !== null) as Integration[]; + return integrations.filter((x) => x !== null) as IntegrationReader[]; } - async getIntegration(name: string): Promise { + async getIntegration(name: string): Promise { if ((await this.reader.getDirectoryType(name)) !== 'integration') { console.error(`Requested integration '${name}' does not exist`); return null; } - const integ = new Integration(name, this.reader.join(name)); + const integ = new IntegrationReader(name, this.reader.join(name)); const checkResult = await integ.getConfig(); if (!checkResult.ok) { console.error(`Integration '${name}' is invalid:`, checkResult.error); diff --git a/server/adaptors/integrations/types.ts b/server/adaptors/integrations/types.ts index d84dc3c5a..c74829d30 100644 --- a/server/adaptors/integrations/types.ts +++ b/server/adaptors/integrations/types.ts @@ -5,7 +5,7 @@ type Result = { ok: true; value: T } | { ok: false; error: E }; -interface IntegrationTemplate { +interface IntegrationConfig { name: string; version: string; displayName?: string; @@ -48,7 +48,7 @@ interface DisplayAsset { } interface IntegrationTemplateSearchResult { - hits: IntegrationTemplate[]; + hits: IntegrationConfig[]; } interface IntegrationTemplateQuery { diff --git a/server/adaptors/integrations/validators.ts b/server/adaptors/integrations/validators.ts index b9c7f33f6..7486a38ed 100644 --- a/server/adaptors/integrations/validators.ts +++ b/server/adaptors/integrations/validators.ts @@ -17,7 +17,7 @@ const staticAsset: JSONSchemaType = { additionalProperties: false, }; -const templateSchema: JSONSchemaType = { +const templateSchema: JSONSchemaType = { type: 'object', properties: { name: { type: 'string' }, @@ -120,7 +120,7 @@ const instanceValidator = ajv.compile(instanceSchema); * If validation succeeds, returns an object with 'ok' set to true and the validated data. * If validation fails, returns an object with 'ok' set to false and an Error object describing the validation error. */ -export const validateTemplate = (data: unknown): Result => { +export const validateTemplate = (data: unknown): Result => { if (!templateValidator(data)) { return { ok: false, error: new Error(ajv.errorsText(templateValidator.errors)) }; }