Skip to content

Commit

Permalink
[Fleet] Stop loading js-yaml in main plugin bundle (#111169)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored Sep 7, 2021
1 parent 3f015e1 commit 7b97b5c
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 41 deletions.
2 changes: 1 addition & 1 deletion packages/kbn-optimizer/limits.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pageLoadAssetSize:
indexPatternManagement: 28222
indexPatternEditor: 25000
infra: 184320
fleet: 465774
fleet: 250000
ingestPipelines: 58003
inputControlVis: 172675
inspector: 148711
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { safeDump } from 'js-yaml';
import type { safeDump } from 'js-yaml';

import type { FullAgentPolicy } from '../types';

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { safeLoad } from 'js-yaml';

import { installationStatuses } from '../constants';
import type { PackageInfo, NewPackagePolicy, RegistryPolicyTemplate } from '../types';

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -423,7 +425,11 @@ describe('Fleet - validatePackagePolicy()', () => {
enabled: false,
}));
expect(
validatePackagePolicy({ ...validPackagePolicy, inputs: disabledInputs }, mockPackage)
validatePackagePolicy(
{ ...validPackagePolicy, inputs: disabledInputs },
mockPackage,
safeLoad
)
).toEqual(noErrorsValidationResults);
});

Expand All @@ -439,7 +445,8 @@ describe('Fleet - validatePackagePolicy()', () => {
expect(
validatePackagePolicy(
{ ...invalidPackagePolicy, inputs: inputsWithDisabledStreams },
mockPackage
mockPackage,
safeLoad
)
).toEqual({
name: ['Name is required'],
Expand Down Expand Up @@ -485,21 +492,29 @@ 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,
namespace: null,
inputs: null,
});
expect(
validatePackagePolicy(validPackagePolicy, {
...mockPackage,
policy_templates: [],
})
validatePackagePolicy(
validPackagePolicy,
{
...mockPackage,
policy_templates: [],
},
safeLoad
)
).toEqual({
name: null,
description: null,
Expand All @@ -510,21 +525,29 @@ 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,
namespace: null,
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,
Expand All @@ -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();
});
Expand All @@ -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);
Expand Down
26 changes: 19 additions & 7 deletions x-pack/plugins/fleet/common/services/validate_package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -139,7 +144,8 @@ export const validatePackagePolicy = (
? validatePackagePolicyConfig(
configEntry,
inputVarDefsByPolicyTemplateAndType[inputKey][name],
name
name,
safeLoadYaml
)
: null;
return results;
Expand All @@ -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;
},
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -54,7 +55,7 @@ export const AgentPolicyYamlFlyout = memo<{ policyId: string; onClose: () => voi
</EuiCallOut>
) : (
<EuiCodeBlock language="yaml" isCopyable fontSize="m" whiteSpace="pre">
{fullAgentPolicyToYaml(yamlData!.item)}
{fullAgentPolicyToYaml(yamlData!.item, safeDump)}
</EuiCodeBlock>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { safeLoad } from 'js-yaml';

import type { PackagePolicyConfigRecord, RegistryVarsEntry } from '../../../../types';

import { validatePackagePolicyConfig } from './';
Expand All @@ -25,7 +27,8 @@ export const hasInvalidButRequiredVar = (
validatePackagePolicyConfig(
packagePolicyVars[registryVar.name],
registryVar,
registryVar.name
registryVar.name,
safeLoad
)?.length)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -71,7 +72,7 @@ export const StandaloneInstructions = React.memo<Props>(({ agentPolicy, agentPol
fetchFullPolicy();
}, [selectedPolicyId, notifications.toasts]);

const yaml = useMemo(() => fullAgentPolicyToYaml(fullAgentPolicy), [fullAgentPolicy]);
const yaml = useMemo(() => fullAgentPolicyToYaml(fullAgentPolicy, safeDump), [fullAgentPolicy]);
const steps = [
DownloadStep(),
!agentPolicy
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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"`,
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit 7b97b5c

Please sign in to comment.