From f50d186b079a3e01f1384344e5a073835527da33 Mon Sep 17 00:00:00 2001 From: criamico Date: Thu, 4 Jan 2024 14:51:44 +0100 Subject: [PATCH 1/3] [Fleet] Fix secrets exception when installing CSPM or other integrations --- .../fleet/server/services/secrets.test.ts | 223 +++++++++++++++++- .../plugins/fleet/server/services/secrets.ts | 15 +- 2 files changed, 230 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/secrets.test.ts b/x-pack/plugins/fleet/server/services/secrets.test.ts index 8e92836e3fbf4..01975561b0634 100644 --- a/x-pack/plugins/fleet/server/services/secrets.test.ts +++ b/x-pack/plugins/fleet/server/services/secrets.test.ts @@ -18,7 +18,14 @@ import { elasticsearchServiceMock } from '@kbn/core-elasticsearch-server-mocks'; import { createAppContextStartContractMock } from '../mocks'; -import type { NewPackagePolicy, PackageInfo, PackagePolicy, UpdatePackagePolicy } from '../types'; +import type { + NewPackagePolicy, + PackageInfo, + PackagePolicy, + UpdatePackagePolicy, + SecretPath, + Secret, +} from '../types'; import { appContextService } from './app_context'; import { @@ -27,6 +34,7 @@ import { diffOutputSecretPaths, extractAndWriteSecrets, extractAndUpdateSecrets, + getPolicyWithSecretReferences, } from './secrets'; describe('secrets', () => { @@ -1537,4 +1545,217 @@ describe('diffOutputSecretPaths', () => { noChange: [paths1[0]], }); }); + + describe.only('getPolicyWithSecretReferences', () => { + const paths1: SecretPath[] = [ + { + path: ['somepath1'], + value: { + type: 'secret_access_key', + value: 'test-secret-1', + }, + }, + ]; + const packagePolicy = { + vars: { + 'pkg-secret-1': { + value: 'pkg-secret-1-val', + }, + 'pkg-secret-2': { + value: 'pkg-secret-2-val', + }, + }, + inputs: [ + { + streams: [ + { + vars: [ + { + access_key_id: { + type: 'text', + }, + }, + { + secret_access_key: { + type: 'text', + }, + }, + ], + }, + ], + }, + { + streams: [ + { + vars: [ + { + access_key_id: { + type: 'text', + }, + }, + { + secret_access_key: { + type: 'text', + }, + }, + ], + }, + ], + }, + ], + } as unknown as NewPackagePolicy; + + const secrets: Secret[] = [{ id: 'secret-1' }]; + + it('gets the policy with the secret references', () => { + expect(getPolicyWithSecretReferences(paths1, secrets, packagePolicy)).toEqual({ + inputs: [ + { + streams: [ + { + vars: [ + { + access_key_id: { + type: 'text', + }, + }, + { + secret_access_key: { + type: 'text', + }, + }, + ], + }, + ], + }, + { + streams: [ + { + vars: [ + { + access_key_id: { + type: 'text', + }, + }, + { + secret_access_key: { + type: 'text', + }, + }, + ], + }, + ], + }, + ], + somepath1: { + value: { + id: 'secret-1', + isSecretRef: true, + }, + }, + vars: { + 'pkg-secret-1': { + value: 'pkg-secret-1-val', + }, + 'pkg-secret-2': { + value: 'pkg-secret-2-val', + }, + }, + }); + }); + + it('does not fail when paths has more elements', () => { + const secretPaths: SecretPath[] = [ + { + path: ['inputs', '1', 'streams', '0', 'vars', 'secret_access_key'], + value: { type: 'text' }, + }, + { + path: ['inputs', '2', 'streams', '0', 'vars', 'secret_access_key'], + value: { value: 'xfgdgh' }, + }, + ]; + // Jest doesn't like `toEqual` here and fails with `serializes to the same string` + expect( + JSON.stringify(getPolicyWithSecretReferences(secretPaths, secrets, packagePolicy)) + ).toEqual( + JSON.stringify({ + vars: { + 'pkg-secret-1': { value: 'pkg-secret-1-val' }, + 'pkg-secret-2': { value: 'pkg-secret-2-val' }, + }, + inputs: [ + { + streams: [ + { + vars: [ + { access_key_id: { type: 'text' } }, + { secret_access_key: { type: 'text' } }, + ], + }, + ], + }, + { + streams: [ + { + vars: [ + { access_key_id: { type: 'text' } }, + { secret_access_key: { type: 'text' } }, + ], + }, + ], + }, + { + streams: { + '0': { + vars: { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, + }, + }, + }, + ], + }) + ); + }); + it.only('does not fail when paths has multiple secrets', () => { + const manySecretPaths: SecretPath[] = [ + { + path: ['inputs', '0', 'streams', '0', 'vars', 'secret_access_key'], + value: { value: 'ksdfgn' }, + }, + { + path: ['inputs', '1', 'streams', '0', 'vars', 'secret_access_key'], + value: { value: 'xfgdgh' }, + }, + ]; + const manySecrets: Secret[] = [{ id: 'secret-1' }, { id: 'secret-2' }]; + expect(getPolicyWithSecretReferences(manySecretPaths, manySecrets, packagePolicy)).toEqual({ + vars: { + 'pkg-secret-1': { value: 'pkg-secret-1-val' }, + 'pkg-secret-2': { value: 'pkg-secret-2-val' }, + }, + inputs: [ + { + streams: [ + { + vars: [ + { access_key_id: { type: 'text' } }, + { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, + ], + }, + ], + }, + { + streams: [ + { + vars: [ + { access_key_id: { type: 'text' } }, + { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, + ], + }, + ], + }, + ], + }); + }); + }); }); diff --git a/x-pack/plugins/fleet/server/services/secrets.ts b/x-pack/plugins/fleet/server/services/secrets.ts index 6f50b2cd8b73b..3ac03bf92bae3 100644 --- a/x-pack/plugins/fleet/server/services/secrets.ts +++ b/x-pack/plugins/fleet/server/services/secrets.ts @@ -771,23 +771,24 @@ function _getInputSecretVarDefsByPolicyTemplateAndType(packageInfo: PackageInfo) * new package policy object that includes resolved secret reference values at each * provided path. */ -function getPolicyWithSecretReferences( +export function getPolicyWithSecretReferences( secretPaths: SecretPath[], secrets: Secret[], packagePolicy: NewPackagePolicy ) { const result = JSON.parse(JSON.stringify(packagePolicy)); - secretPaths.forEach((secretPath, secretPathIndex) => { + let secretIndex = 0; + secretPaths.forEach((secretPath) => { secretPath.path.reduce((acc, val, secretPathComponentIndex) => { if (!acc[val]) { acc[val] = {}; } - - const isLast = secretPathComponentIndex === secretPath.path.length - 1; - - if (isLast) { - acc[val].value = toVarSecretRef(secrets[secretPathIndex].id); + const isSecretPath = + secretPathComponentIndex === secretPath.path.length - 1 && !!secretPath.value.value; + if (isSecretPath) { + acc[val].value = toVarSecretRef(secrets[secretIndex].id); + secretIndex = secretIndex + 1; } return acc[val]; From 64b955a130bda460091078490f5959e71052381c Mon Sep 17 00:00:00 2001 From: Kyle Pollich Date: Thu, 4 Jan 2024 14:38:46 -0500 Subject: [PATCH 2/3] Update test case + fix incorrect argument --- .../fleet/server/services/secrets.test.ts | 234 +----------------- .../plugins/fleet/server/services/secrets.ts | 21 +- 2 files changed, 20 insertions(+), 235 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/secrets.test.ts b/x-pack/plugins/fleet/server/services/secrets.test.ts index 01975561b0634..9a29cd618bb7d 100644 --- a/x-pack/plugins/fleet/server/services/secrets.test.ts +++ b/x-pack/plugins/fleet/server/services/secrets.test.ts @@ -18,14 +18,7 @@ import { elasticsearchServiceMock } from '@kbn/core-elasticsearch-server-mocks'; import { createAppContextStartContractMock } from '../mocks'; -import type { - NewPackagePolicy, - PackageInfo, - PackagePolicy, - UpdatePackagePolicy, - SecretPath, - Secret, -} from '../types'; +import type { NewPackagePolicy, PackageInfo, PackagePolicy, UpdatePackagePolicy } from '../types'; import { appContextService } from './app_context'; import { @@ -34,7 +27,6 @@ import { diffOutputSecretPaths, extractAndWriteSecrets, extractAndUpdateSecrets, - getPolicyWithSecretReferences, } from './secrets'; describe('secrets', () => { @@ -976,8 +968,8 @@ describe('secrets', () => { type: 'integration', status: 'not_installed', vars: [ - { name: 'pkg-secret-1', type: 'text', secret: true, required: true }, - { name: 'pkg-secret-2', type: 'text', secret: true, required: false }, + { name: 'pkg-secret-1', type: 'text', secret: true, required: false }, + { name: 'pkg-secret-2', type: 'text', secret: true, required: true }, { name: 'dot-notation.pkg-secret-3', type: 'text', secret: true, required: false }, ], data_streams: [ @@ -1026,9 +1018,11 @@ describe('secrets', () => { it('returns single secret reference for required secret', async () => { const mockPackagePolicy = { vars: { - 'pkg-secret-1': { - value: 'pkg-secret-1-val', + 'pkg-secret-1': {}, + 'pkg-secret-2': { + value: 'pkg-secret-2-val', }, + 'dot-notation.stream.pkg-secret-3': {}, }, inputs: [], } as unknown as NewPackagePolicy; @@ -1206,6 +1200,7 @@ describe('secrets', () => { value: 'pkg-secret-1-val', }, 'pkg-secret-2': {}, + 'dot-notation.pkg-secret-3': {}, }, inputs: [], } as unknown as PackagePolicy; @@ -1545,217 +1540,4 @@ describe('diffOutputSecretPaths', () => { noChange: [paths1[0]], }); }); - - describe.only('getPolicyWithSecretReferences', () => { - const paths1: SecretPath[] = [ - { - path: ['somepath1'], - value: { - type: 'secret_access_key', - value: 'test-secret-1', - }, - }, - ]; - const packagePolicy = { - vars: { - 'pkg-secret-1': { - value: 'pkg-secret-1-val', - }, - 'pkg-secret-2': { - value: 'pkg-secret-2-val', - }, - }, - inputs: [ - { - streams: [ - { - vars: [ - { - access_key_id: { - type: 'text', - }, - }, - { - secret_access_key: { - type: 'text', - }, - }, - ], - }, - ], - }, - { - streams: [ - { - vars: [ - { - access_key_id: { - type: 'text', - }, - }, - { - secret_access_key: { - type: 'text', - }, - }, - ], - }, - ], - }, - ], - } as unknown as NewPackagePolicy; - - const secrets: Secret[] = [{ id: 'secret-1' }]; - - it('gets the policy with the secret references', () => { - expect(getPolicyWithSecretReferences(paths1, secrets, packagePolicy)).toEqual({ - inputs: [ - { - streams: [ - { - vars: [ - { - access_key_id: { - type: 'text', - }, - }, - { - secret_access_key: { - type: 'text', - }, - }, - ], - }, - ], - }, - { - streams: [ - { - vars: [ - { - access_key_id: { - type: 'text', - }, - }, - { - secret_access_key: { - type: 'text', - }, - }, - ], - }, - ], - }, - ], - somepath1: { - value: { - id: 'secret-1', - isSecretRef: true, - }, - }, - vars: { - 'pkg-secret-1': { - value: 'pkg-secret-1-val', - }, - 'pkg-secret-2': { - value: 'pkg-secret-2-val', - }, - }, - }); - }); - - it('does not fail when paths has more elements', () => { - const secretPaths: SecretPath[] = [ - { - path: ['inputs', '1', 'streams', '0', 'vars', 'secret_access_key'], - value: { type: 'text' }, - }, - { - path: ['inputs', '2', 'streams', '0', 'vars', 'secret_access_key'], - value: { value: 'xfgdgh' }, - }, - ]; - // Jest doesn't like `toEqual` here and fails with `serializes to the same string` - expect( - JSON.stringify(getPolicyWithSecretReferences(secretPaths, secrets, packagePolicy)) - ).toEqual( - JSON.stringify({ - vars: { - 'pkg-secret-1': { value: 'pkg-secret-1-val' }, - 'pkg-secret-2': { value: 'pkg-secret-2-val' }, - }, - inputs: [ - { - streams: [ - { - vars: [ - { access_key_id: { type: 'text' } }, - { secret_access_key: { type: 'text' } }, - ], - }, - ], - }, - { - streams: [ - { - vars: [ - { access_key_id: { type: 'text' } }, - { secret_access_key: { type: 'text' } }, - ], - }, - ], - }, - { - streams: { - '0': { - vars: { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, - }, - }, - }, - ], - }) - ); - }); - it.only('does not fail when paths has multiple secrets', () => { - const manySecretPaths: SecretPath[] = [ - { - path: ['inputs', '0', 'streams', '0', 'vars', 'secret_access_key'], - value: { value: 'ksdfgn' }, - }, - { - path: ['inputs', '1', 'streams', '0', 'vars', 'secret_access_key'], - value: { value: 'xfgdgh' }, - }, - ]; - const manySecrets: Secret[] = [{ id: 'secret-1' }, { id: 'secret-2' }]; - expect(getPolicyWithSecretReferences(manySecretPaths, manySecrets, packagePolicy)).toEqual({ - vars: { - 'pkg-secret-1': { value: 'pkg-secret-1-val' }, - 'pkg-secret-2': { value: 'pkg-secret-2-val' }, - }, - inputs: [ - { - streams: [ - { - vars: [ - { access_key_id: { type: 'text' } }, - { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, - ], - }, - ], - }, - { - streams: [ - { - vars: [ - { access_key_id: { type: 'text' } }, - { secret_access_key: { value: { id: 'secret-1', isSecretRef: true } } }, - ], - }, - ], - }, - ], - }); - }); - }); }); diff --git a/x-pack/plugins/fleet/server/services/secrets.ts b/x-pack/plugins/fleet/server/services/secrets.ts index 3ac03bf92bae3..c01c64e37e59e 100644 --- a/x-pack/plugins/fleet/server/services/secrets.ts +++ b/x-pack/plugins/fleet/server/services/secrets.ts @@ -241,7 +241,11 @@ export async function extractAndWriteSecrets(opts: { values: secretsToCreate.map((secretPath) => secretPath.value.value), }); - const policyWithSecretRefs = getPolicyWithSecretReferences(secretPaths, secrets, packagePolicy); + const policyWithSecretRefs = getPolicyWithSecretReferences( + secretsToCreate, + secrets, + packagePolicy + ); return { packagePolicy: policyWithSecretRefs, @@ -771,24 +775,23 @@ function _getInputSecretVarDefsByPolicyTemplateAndType(packageInfo: PackageInfo) * new package policy object that includes resolved secret reference values at each * provided path. */ -export function getPolicyWithSecretReferences( +function getPolicyWithSecretReferences( secretPaths: SecretPath[], secrets: Secret[], packagePolicy: NewPackagePolicy ) { const result = JSON.parse(JSON.stringify(packagePolicy)); - let secretIndex = 0; - secretPaths.forEach((secretPath) => { + secretPaths.forEach((secretPath, secretPathIndex) => { secretPath.path.reduce((acc, val, secretPathComponentIndex) => { if (!acc[val]) { acc[val] = {}; } - const isSecretPath = - secretPathComponentIndex === secretPath.path.length - 1 && !!secretPath.value.value; - if (isSecretPath) { - acc[val].value = toVarSecretRef(secrets[secretIndex].id); - secretIndex = secretIndex + 1; + + const isLast = secretPathComponentIndex === secretPath.path.length - 1; + + if (isLast) { + acc[val].value = toVarSecretRef(secrets[secretPathIndex].id); } return acc[val]; From c6880dde4281f9390a76b8338c7526e6e0802191 Mon Sep 17 00:00:00 2001 From: criamico Date: Fri, 5 Jan 2024 17:17:23 +0100 Subject: [PATCH 3/3] Revert change in test --- x-pack/plugins/fleet/server/services/secrets.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/secrets.test.ts b/x-pack/plugins/fleet/server/services/secrets.test.ts index 9a29cd618bb7d..729c74a3ddec0 100644 --- a/x-pack/plugins/fleet/server/services/secrets.test.ts +++ b/x-pack/plugins/fleet/server/services/secrets.test.ts @@ -968,8 +968,8 @@ describe('secrets', () => { type: 'integration', status: 'not_installed', vars: [ - { name: 'pkg-secret-1', type: 'text', secret: true, required: false }, - { name: 'pkg-secret-2', type: 'text', secret: true, required: true }, + { name: 'pkg-secret-1', type: 'text', secret: true, required: true }, + { name: 'pkg-secret-2', type: 'text', secret: true, required: false }, { name: 'dot-notation.pkg-secret-3', type: 'text', secret: true, required: false }, ], data_streams: [