From 7b97b5c4439e07f4ae203137b1bb233c2d9c5a9c Mon Sep 17 00:00:00 2001 From: Josh Dover <1813008+joshdover@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:40:14 +0200 Subject: [PATCH] [Fleet] Stop loading js-yaml in main plugin bundle (#111169) --- packages/kbn-optimizer/limits.yml | 2 +- .../services/full_agent_policy_to_yaml.ts | 6 +- .../services/validate_package_policy.test.ts | 69 +++++++++++++------ .../services/validate_package_policy.ts | 26 +++++-- .../components/agent_policy_yaml_flyout.tsx | 3 +- .../create_package_policy_page/index.tsx | 4 +- .../services/has_invalid_but_required_var.ts | 5 +- .../edit_package_policy_page/index.tsx | 8 ++- .../standalone_instructions.tsx | 3 +- .../server/routes/agent_policy/handlers.ts | 3 +- .../fleet/server/services/package_policy.ts | 3 +- 11 files changed, 91 insertions(+), 41 deletions(-) diff --git a/packages/kbn-optimizer/limits.yml b/packages/kbn-optimizer/limits.yml index 7e6a1a9350d81..282abe1e1741a 100644 --- a/packages/kbn-optimizer/limits.yml +++ b/packages/kbn-optimizer/limits.yml @@ -34,7 +34,7 @@ pageLoadAssetSize: indexPatternManagement: 28222 indexPatternEditor: 25000 infra: 184320 - fleet: 465774 + fleet: 250000 ingestPipelines: 58003 inputControlVis: 172675 inspector: 148711 diff --git a/x-pack/plugins/fleet/common/services/full_agent_policy_to_yaml.ts b/x-pack/plugins/fleet/common/services/full_agent_policy_to_yaml.ts index a2c1dcd83dd20..f4193d619e168 100644 --- a/x-pack/plugins/fleet/common/services/full_agent_policy_to_yaml.ts +++ b/x-pack/plugins/fleet/common/services/full_agent_policy_to_yaml.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { safeDump } from 'js-yaml'; +import type { safeDump } from 'js-yaml'; import type { FullAgentPolicy } from '../types'; @@ -25,8 +25,8 @@ const POLICY_KEYS_ORDER = [ 'input', ]; -export const fullAgentPolicyToYaml = (policy: FullAgentPolicy): string => { - return safeDump(policy, { +export const fullAgentPolicyToYaml = (policy: FullAgentPolicy, toYaml: typeof safeDump): string => { + return toYaml(policy, { skipInvalid: true, sortKeys: (keyA: string, keyB: string) => { const indexA = POLICY_KEYS_ORDER.indexOf(keyA); diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts index 3e4f156da3379..30bd9f071feb8 100644 --- a/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts +++ b/x-pack/plugins/fleet/common/services/validate_package_policy.test.ts @@ -5,6 +5,8 @@ * 2.0. */ +import { safeLoad } from 'js-yaml'; + import { installationStatuses } from '../constants'; import type { PackageInfo, NewPackagePolicy, RegistryPolicyTemplate } from '../types'; @@ -371,13 +373,13 @@ describe('Fleet - validatePackagePolicy()', () => { }; it('returns no errors for valid package policy', () => { - expect(validatePackagePolicy(validPackagePolicy, mockPackage)).toEqual( + expect(validatePackagePolicy(validPackagePolicy, mockPackage, safeLoad)).toEqual( noErrorsValidationResults ); }); it('returns errors for invalid package policy', () => { - expect(validatePackagePolicy(invalidPackagePolicy, mockPackage)).toEqual({ + expect(validatePackagePolicy(invalidPackagePolicy, mockPackage, safeLoad)).toEqual({ name: ['Name is required'], description: null, namespace: null, @@ -423,7 +425,11 @@ describe('Fleet - validatePackagePolicy()', () => { enabled: false, })); expect( - validatePackagePolicy({ ...validPackagePolicy, inputs: disabledInputs }, mockPackage) + validatePackagePolicy( + { ...validPackagePolicy, inputs: disabledInputs }, + mockPackage, + safeLoad + ) ).toEqual(noErrorsValidationResults); }); @@ -439,7 +445,8 @@ describe('Fleet - validatePackagePolicy()', () => { expect( validatePackagePolicy( { ...invalidPackagePolicy, inputs: inputsWithDisabledStreams }, - mockPackage + mockPackage, + safeLoad ) ).toEqual({ name: ['Name is required'], @@ -485,10 +492,14 @@ describe('Fleet - validatePackagePolicy()', () => { it('returns no errors for packages with no package policies', () => { expect( - validatePackagePolicy(validPackagePolicy, { - ...mockPackage, - policy_templates: undefined, - }) + validatePackagePolicy( + validPackagePolicy, + { + ...mockPackage, + policy_templates: undefined, + }, + safeLoad + ) ).toEqual({ name: null, description: null, @@ -496,10 +507,14 @@ describe('Fleet - validatePackagePolicy()', () => { inputs: null, }); expect( - validatePackagePolicy(validPackagePolicy, { - ...mockPackage, - policy_templates: [], - }) + validatePackagePolicy( + validPackagePolicy, + { + ...mockPackage, + policy_templates: [], + }, + safeLoad + ) ).toEqual({ name: null, description: null, @@ -510,10 +525,14 @@ describe('Fleet - validatePackagePolicy()', () => { it('returns no errors for packages with no inputs', () => { expect( - validatePackagePolicy(validPackagePolicy, { - ...mockPackage, - policy_templates: [{} as RegistryPolicyTemplate], - }) + validatePackagePolicy( + validPackagePolicy, + { + ...mockPackage, + policy_templates: [{} as RegistryPolicyTemplate], + }, + safeLoad + ) ).toEqual({ name: null, description: null, @@ -521,10 +540,14 @@ describe('Fleet - validatePackagePolicy()', () => { inputs: null, }); expect( - validatePackagePolicy(validPackagePolicy, { - ...mockPackage, - policy_templates: [({ inputs: [] } as unknown) as RegistryPolicyTemplate], - }) + validatePackagePolicy( + validPackagePolicy, + { + ...mockPackage, + policy_templates: [({ inputs: [] } as unknown) as RegistryPolicyTemplate], + }, + safeLoad + ) ).toEqual({ name: null, description: null, @@ -539,7 +562,8 @@ describe('Fleet - validatePackagePolicy()', () => { expect( validatePackagePolicy( INVALID_AWS_POLICY as NewPackagePolicy, - (AWS_PACKAGE as unknown) as PackageInfo + (AWS_PACKAGE as unknown) as PackageInfo, + safeLoad ) ).toMatchSnapshot(); }); @@ -549,7 +573,8 @@ describe('Fleet - validatePackagePolicy()', () => { validationHasErrors( validatePackagePolicy( VALID_AWS_POLICY as NewPackagePolicy, - (AWS_PACKAGE as unknown) as PackageInfo + (AWS_PACKAGE as unknown) as PackageInfo, + safeLoad ) ) ).toBe(false); diff --git a/x-pack/plugins/fleet/common/services/validate_package_policy.ts b/x-pack/plugins/fleet/common/services/validate_package_policy.ts index 67df65b2f12bf..12bef2dfd7c27 100644 --- a/x-pack/plugins/fleet/common/services/validate_package_policy.ts +++ b/x-pack/plugins/fleet/common/services/validate_package_policy.ts @@ -7,7 +7,6 @@ import { getFlattenedObject } from '@kbn/std'; import { i18n } from '@kbn/i18n'; -import { safeLoad } from 'js-yaml'; import { keyBy } from 'lodash'; import type { @@ -47,7 +46,8 @@ export type PackagePolicyValidationResults = { */ export const validatePackagePolicy = ( packagePolicy: NewPackagePolicy, - packageInfo: PackageInfo + packageInfo: PackageInfo, + safeLoadYaml: (yaml: string) => any ): PackagePolicyValidationResults => { const hasIntegrations = doesPackageHaveIntegrations(packageInfo); const validationResults: PackagePolicyValidationResults = { @@ -75,7 +75,12 @@ export const validatePackagePolicy = ( const packageVars = Object.entries(packagePolicy.vars || {}); if (packageVars.length) { validationResults.vars = packageVars.reduce((results, [name, varEntry]) => { - results[name] = validatePackagePolicyConfig(varEntry, packageVarsByName[name], name); + results[name] = validatePackagePolicyConfig( + varEntry, + packageVarsByName[name], + name, + safeLoadYaml + ); return results; }, {} as ValidationEntry); } @@ -139,7 +144,8 @@ export const validatePackagePolicy = ( ? validatePackagePolicyConfig( configEntry, inputVarDefsByPolicyTemplateAndType[inputKey][name], - name + name, + safeLoadYaml ) : null; return results; @@ -162,7 +168,12 @@ export const validatePackagePolicy = ( (results, [name, configEntry]) => { results[name] = streamVarDefs && streamVarDefs[name] && input.enabled && stream.enabled - ? validatePackagePolicyConfig(configEntry, streamVarDefs[name], name) + ? validatePackagePolicyConfig( + configEntry, + streamVarDefs[name], + name, + safeLoadYaml + ) : null; return results; }, @@ -191,7 +202,8 @@ export const validatePackagePolicy = ( export const validatePackagePolicyConfig = ( configEntry: PackagePolicyConfigRecordEntry, varDef: RegistryVarsEntry, - varName: string + varName: string, + safeLoadYaml: (yaml: string) => any ): string[] | null => { const errors = []; const { value } = configEntry; @@ -223,7 +235,7 @@ export const validatePackagePolicyConfig = ( if (varDef.type === 'yaml') { try { - parsedValue = safeLoad(value); + parsedValue = safeLoadYaml(value); } catch (e) { errors.push( i18n.translate('xpack.fleet.packagePolicyValidation.invalidYamlFormatErrorMessage', { diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/agent_policy_yaml_flyout.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/agent_policy_yaml_flyout.tsx index e434347c2c367..8cc07556eee7f 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/agent_policy_yaml_flyout.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/agent_policy_yaml_flyout.tsx @@ -8,6 +8,7 @@ import React, { memo } from 'react'; import styled from 'styled-components'; import { FormattedMessage } from '@kbn/i18n/react'; +import { safeDump } from 'js-yaml'; import { EuiCodeBlock, EuiFlexGroup, @@ -54,7 +55,7 @@ export const AgentPolicyYamlFlyout = memo<{ policyId: string; onClose: () => voi ) : ( - {fullAgentPolicyToYaml(yamlData!.item)} + {fullAgentPolicyToYaml(yamlData!.item, safeDump)} ); diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx index 47ecfd6bd121c..1f7fa0ceb354b 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx @@ -23,6 +23,7 @@ import { } from '@elastic/eui'; import type { EuiStepProps } from '@elastic/eui/src/components/steps/step'; import type { ApplicationStart } from 'kibana/public'; +import { safeLoad } from 'js-yaml'; import { toMountPoint } from '../../../../../../../../../src/plugins/kibana_react/public'; import type { @@ -191,7 +192,8 @@ export const CreatePackagePolicyPage: React.FunctionComponent = () => { if (packageInfo) { const newValidationResult = validatePackagePolicy( newPackagePolicy || packagePolicy, - packageInfo + packageInfo, + safeLoad ); setValidationResults(newValidationResult); // eslint-disable-next-line no-console diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/services/has_invalid_but_required_var.ts b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/services/has_invalid_but_required_var.ts index bf75b05f41b8d..e41b98c05e41c 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/services/has_invalid_but_required_var.ts +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/services/has_invalid_but_required_var.ts @@ -5,6 +5,8 @@ * 2.0. */ +import { safeLoad } from 'js-yaml'; + import type { PackagePolicyConfigRecord, RegistryVarsEntry } from '../../../../types'; import { validatePackagePolicyConfig } from './'; @@ -25,7 +27,8 @@ export const hasInvalidButRequiredVar = ( validatePackagePolicyConfig( packagePolicyVars[registryVar.name], registryVar, - registryVar.name + registryVar.name, + safeLoad )?.length) ) ) diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx index ea027f95eb9e8..a36bc988da89f 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx @@ -9,6 +9,7 @@ import React, { useState, useEffect, useCallback, useMemo, memo } from 'react'; import { useRouteMatch } from 'react-router-dom'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; +import { safeLoad } from 'js-yaml'; import { EuiButtonEmpty, EuiButton, @@ -201,7 +202,9 @@ export const EditPackagePolicyForm = memo<{ if (packageData?.response) { setPackageInfo(packageData.response); - setValidationResults(validatePackagePolicy(newPackagePolicy, packageData.response)); + setValidationResults( + validatePackagePolicy(newPackagePolicy, packageData.response, safeLoad) + ); setFormState('VALID'); } } @@ -239,7 +242,8 @@ export const EditPackagePolicyForm = memo<{ if (packageInfo) { const newValidationResult = validatePackagePolicy( newPackagePolicy || packagePolicy, - packageInfo + packageInfo, + safeLoad ); setValidationResults(newValidationResult); // eslint-disable-next-line no-console diff --git a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/standalone_instructions.tsx b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/standalone_instructions.tsx index 59898b9190c00..d7b9ae2aef08a 100644 --- a/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/standalone_instructions.tsx +++ b/x-pack/plugins/fleet/public/components/agent_enrollment_flyout/standalone_instructions.tsx @@ -21,6 +21,7 @@ import { import type { EuiContainedStepProps } from '@elastic/eui/src/components/steps/steps'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; +import { safeDump } from 'js-yaml'; import { useStartServices, useLink, sendGetOneAgentPolicyFull } from '../../hooks'; import { fullAgentPolicyToYaml, agentPolicyRouteService } from '../../services'; @@ -71,7 +72,7 @@ export const StandaloneInstructions = React.memo(({ agentPolicy, agentPol fetchFullPolicy(); }, [selectedPolicyId, notifications.toasts]); - const yaml = useMemo(() => fullAgentPolicyToYaml(fullAgentPolicy), [fullAgentPolicy]); + const yaml = useMemo(() => fullAgentPolicyToYaml(fullAgentPolicy, safeDump), [fullAgentPolicy]); const steps = [ DownloadStep(), !agentPolicy diff --git a/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts b/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts index 57401a525b5d7..a7cf606e92c0b 100644 --- a/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts +++ b/x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts @@ -8,6 +8,7 @@ import type { TypeOf } from '@kbn/config-schema'; import type { RequestHandler, ResponseHeaders } from 'src/core/server'; import bluebird from 'bluebird'; +import { safeDump } from 'js-yaml'; import { fullAgentPolicyToYaml } from '../../../common/services'; import { appContextService, agentPolicyService, packagePolicyService } from '../../services'; @@ -269,7 +270,7 @@ export const downloadFullAgentPolicy: RequestHandler< standalone: request.query.standalone === true, }); if (fullAgentPolicy) { - const body = fullAgentPolicyToYaml(fullAgentPolicy); + const body = fullAgentPolicyToYaml(fullAgentPolicy, safeDump); const headers: ResponseHeaders = { 'content-type': 'text/x-yaml', 'content-disposition': `attachment; filename="elastic-agent.yml"`, diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 8ff3c20b7aa15..598dd16b2928e 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -16,6 +16,7 @@ import type { SavedObjectsClientContract, } from 'src/core/server'; import uuid from 'uuid'; +import { safeLoad } from 'js-yaml'; import type { AuthenticatedUser } from '../../../security/server'; import { @@ -988,7 +989,7 @@ export function overridePackageInputs( inputs, }; - const validationResults = validatePackagePolicy(resultingPackagePolicy, packageInfo); + const validationResults = validatePackagePolicy(resultingPackagePolicy, packageInfo, safeLoad); if (validationHasErrors(validationResults)) { const responseFormattedValidationErrors = Object.entries(getFlattenedObject(validationResults))