From 5fc6aa8970f0904e8210857cc220ab5144e69616 Mon Sep 17 00:00:00 2001 From: Jen Huang Date: Tue, 6 Apr 2021 15:45:48 -0700 Subject: [PATCH] Make `packageToPackagePolicy` params an object, fix types --- .../package_to_package_policy.test.ts | 52 ++++++++++++------- .../services/package_to_package_policy.ts | 29 +++++++---- .../step_define_package_policy.tsx | 20 ++++--- .../routes/package_policy/handlers.test.ts | 4 +- .../fleet/server/services/agent_policy.ts | 14 ++--- .../ingest_pipeline/ingest_pipelines.test.ts | 2 +- .../fleet/server/services/package_policy.ts | 10 ++-- 7 files changed, 79 insertions(+), 52 deletions(-) diff --git a/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts b/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts index 59768e6ac8c85..549192413527b 100644 --- a/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts +++ b/x-pack/plugins/fleet/common/services/package_to_package_policy.test.ts @@ -325,9 +325,11 @@ describe('Fleet - packageToPackagePolicy', () => { describe('packageToPackagePolicy', () => { it('returns package policy with default name', () => { - expect(packageToPackagePolicy(mockPackage, '1', '2')).toEqual({ + expect( + packageToPackagePolicy({ packageInfo: mockPackage, agentPolicyId: '1', outputId: '2' }) + ).toEqual({ policy_id: '1', - namespace: '', + namespace: 'default', enabled: true, inputs: [], name: 'mock-package-1', @@ -341,7 +343,14 @@ describe('Fleet - packageToPackagePolicy', () => { }); it('returns package policy with custom name', () => { - expect(packageToPackagePolicy(mockPackage, '1', '2', 'default', 'pkgPolicy-1')).toEqual({ + expect( + packageToPackagePolicy({ + name: 'pkgPolicy-1', + packageInfo: mockPackage, + agentPolicyId: '1', + outputId: '2', + }) + ).toEqual({ policy_id: '1', namespace: 'default', enabled: true, @@ -358,14 +367,14 @@ describe('Fleet - packageToPackagePolicy', () => { it('returns package policy with namespace and description', () => { expect( - packageToPackagePolicy( - mockPackage, - '1', - '2', - 'mock-namespace', - 'pkgPolicy-1', - 'Test description' - ) + packageToPackagePolicy({ + name: 'pkgPolicy-1', + namespace: 'mock-namespace', + description: 'Test description', + packageInfo: mockPackage, + agentPolicyId: '1', + outputId: '2', + }) ).toEqual({ policy_id: '1', enabled: true, @@ -383,14 +392,19 @@ describe('Fleet - packageToPackagePolicy', () => { }); it('returns package policy with inputs and package-level vars', () => { - const mockPackageWithPolicyTemplates = ({ + const mockPackageWithPackageVars = ({ ...mockPackage, policy_templates: [{ inputs: [{ type: 'foo' }] }], vars: [{ default: 'foo-var-value', name: 'var-name', type: 'text' }], } as unknown) as PackageInfo; expect( - packageToPackagePolicy(mockPackageWithPolicyTemplates, '1', '2', 'default', 'pkgPolicy-1') + packageToPackagePolicy({ + name: 'pkgPolicy-1', + packageInfo: mockPackageWithPackageVars, + agentPolicyId: '1', + outputId: '2', + }) ).toEqual({ policy_id: '1', namespace: 'default', @@ -409,12 +423,12 @@ describe('Fleet - packageToPackagePolicy', () => { it('returns package policy with multiple policy templates correctly', () => { expect( - packageToPackagePolicy( - packageWithPolicyTemplates, - 'some-policy-id', - 'some-output-id', - 'default' - ) + packageToPackagePolicy({ + name: 'aws-1', + packageInfo: packageWithPolicyTemplates, + agentPolicyId: 'some-policy-id', + outputId: 'some-output-id', + }) ).toMatchSnapshot(); }); }); diff --git a/x-pack/plugins/fleet/common/services/package_to_package_policy.ts b/x-pack/plugins/fleet/common/services/package_to_package_policy.ts index 88bfb77238133..386c4a61e7de1 100644 --- a/x-pack/plugins/fleet/common/services/package_to_package_policy.ts +++ b/x-pack/plugins/fleet/common/services/package_to_package_policy.ts @@ -16,7 +16,7 @@ import type { PackagePolicyConfigRecordEntry, } from '../types'; -import { doesPackageHaveIntegrations, groupInputs } from './'; +import { groupInputs } from './'; export const findDataStreamsByNames = ( names: string[] = [], @@ -112,16 +112,25 @@ export const packageToPackagePolicyInputs = ( * @param outputId * @param packagePolicyName */ -export const packageToPackagePolicy = ( - packageInfo: PackageInfo, - agentPolicyId: string, - outputId: string, - namespace: string = '', - packagePolicyName?: string, - description?: string -): NewPackagePolicy => { +export const packageToPackagePolicy = (options: { + name?: string; + description?: string; + namespace?: string; + packageInfo: PackageInfo; + agentPolicyId: string; + outputId: string; +}): NewPackagePolicy => { + const { + name, + packageInfo, + agentPolicyId, + outputId, + namespace = 'default', + description, + } = options; + const packagePolicy: NewPackagePolicy = { - name: packagePolicyName || `${packageInfo.name}-1`, + name: name || `${packageInfo.name}-1`, namespace, description, package: { diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx index 4e18950fc55de..090f4a86dcfc2 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx @@ -5,13 +5,12 @@ * 2.0. */ -import React, { memo, useEffect, useState, useMemo } from 'react'; +import React, { memo, useEffect, useState } from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; import { EuiFormRow, EuiFieldText, EuiButtonEmpty, - EuiSpacer, EuiText, EuiComboBox, EuiDescribedFormGroup, @@ -74,19 +73,18 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{ .sort((a, b) => a - b); updatePackagePolicy( - packageToPackagePolicy( - packageInfo, - agentPolicy.id, - packagePolicy.output_id, - packagePolicy.namespace, - // FIXME: Improve package policies name uniqueness - https://github.com/elastic/kibana/issues/72948 - `${packageInfo.name}-${ + packageToPackagePolicy({ + name: `${packageInfo.name}-${ pkgPoliciesWithMatchingNames.length ? pkgPoliciesWithMatchingNames[pkgPoliciesWithMatchingNames.length - 1] + 1 : 1 }`, - packagePolicy.description - ) + description: packagePolicy.description, + namespace: packagePolicy.namespace, + packageInfo, + agentPolicyId: agentPolicy.id, + outputId: packagePolicy.output_id, + }) ); } diff --git a/x-pack/plugins/fleet/server/routes/package_policy/handlers.test.ts b/x-pack/plugins/fleet/server/routes/package_policy/handlers.test.ts index 7f2b9d93e2df7..5aa400d6443e6 100644 --- a/x-pack/plugins/fleet/server/routes/package_policy/handlers.test.ts +++ b/x-pack/plugins/fleet/server/routes/package_policy/handlers.test.ts @@ -24,7 +24,9 @@ jest.mock('../../services/package_policy', (): { } => { return { packagePolicyService: { - compilePackagePolicyInputs: jest.fn((packageInfo, dataInputs) => Promise.resolve(dataInputs)), + compilePackagePolicyInputs: jest.fn((packageInfo, vars, dataInputs) => + Promise.resolve(dataInputs) + ), buildPackagePolicyFromPackage: jest.fn(), bulkCreate: jest.fn(), create: jest.fn((soClient, esClient, newData) => diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index be61a70154b11..a197eeeec37f3 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -802,14 +802,14 @@ export async function addPackageToAgentPolicy( pkgVersion: packageToInstall.version, }); - const basePackagePolicy = packageToPackagePolicy( + const basePackagePolicy = packageToPackagePolicy({ + name: packagePolicyName, + description: packagePolicyDescription, + namespace: agentPolicy.namespace ?? 'default', packageInfo, - agentPolicy.id, - defaultOutput.id, - agentPolicy.namespace ?? 'default', - packagePolicyName, - packagePolicyDescription - ); + agentPolicyId: agentPolicy.id, + outputId: defaultOutput.id, + }); const newPackagePolicy = transformPackagePolicy ? transformPackagePolicy(basePackagePolicy) diff --git a/x-pack/plugins/fleet/server/services/epm/elasticsearch/ingest_pipeline/ingest_pipelines.test.ts b/x-pack/plugins/fleet/server/services/epm/elasticsearch/ingest_pipeline/ingest_pipelines.test.ts index 097f9ce28c7d1..f915e871b98f7 100644 --- a/x-pack/plugins/fleet/server/services/epm/elasticsearch/ingest_pipeline/ingest_pipelines.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/elasticsearch/ingest_pipeline/ingest_pipelines.test.ts @@ -121,7 +121,7 @@ test('getPipelineNameForInstallation gets correct name', () => { const packageVersion = '1.0.1'; const pipelineRefName = 'pipeline-json'; const pipelineEntryNameForInstallation = getPipelineNameForInstallation({ - pipelineName: dataStream.ingest_pipeline, + pipelineName: dataStream.ingest_pipeline!, dataStream, packageVersion, }); diff --git a/x-pack/plugins/fleet/server/services/package_policy.ts b/x-pack/plugins/fleet/server/services/package_policy.ts index 92b3ec68dcae0..de21452a7fd59 100644 --- a/x-pack/plugins/fleet/server/services/package_policy.ts +++ b/x-pack/plugins/fleet/server/services/package_policy.ts @@ -416,7 +416,7 @@ class PackagePolicyService { ): Promise { const pkgInstall = await getInstallation({ savedObjectsClient: soClient, pkgName }); if (pkgInstall) { - const [pkgInfo, defaultOutputId] = await Promise.all([ + const [packageInfo, defaultOutputId] = await Promise.all([ getPackageInfo({ savedObjectsClient: soClient, pkgName: pkgInstall.name, @@ -424,11 +424,15 @@ class PackagePolicyService { }), outputService.getDefaultOutputId(soClient), ]); - if (pkgInfo) { + if (packageInfo) { if (!defaultOutputId) { throw new Error('Default output is not set'); } - return packageToPackagePolicy(pkgInfo, '', defaultOutputId); + return packageToPackagePolicy({ + packageInfo, + agentPolicyId: '', + outputId: defaultOutputId, + }); } } }