From 5b02aedc16642f22cf6d5d4b3477253ec4da3e7a Mon Sep 17 00:00:00 2001 From: John Schulz Date: Fri, 28 Aug 2020 16:11:07 -0400 Subject: [PATCH 1/5] Add logic spec'd in issue comments. Tests pass. --- .../server/services/agents/enroll.ts | 39 +++++++++++++------ 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts index 606d5c4dcbb90..84bb94ed93f34 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts @@ -94,19 +94,36 @@ export function validateAgentVersion(metadata?: { local: any; userProvided: any if (!kibanaVersion) { throw Boom.badRequest('Kibana version is not set'); } - const version = semver.parse(metadata?.local?.elastic?.agent?.version); - if (!version) { + + const agentVersion = semver.parse(metadata?.local?.elastic?.agent?.version); + if (!agentVersion) { throw Boom.badRequest('Agent version not provided in metadata.'); } - if (!version || !semver.lte(formatVersion(version), formatVersion(kibanaVersion))) { - throw Boom.badRequest('Agent version is not compatible with kibana version'); + const diff = semver.diff(agentVersion.raw, kibanaVersion.raw); + switch (diff) { + // section 1) very close versions, only patch release differences - all combos should work + // Agent a.b.1 < Kibana a.b.2 + // Agent a.b.2 > Kibana a.b.1 + case null: + case 'prerelease': + case 'prepatch': + case 'patch': + return; // OK + + // section 2) somewhat close versions, Agent minor release is 1 or 2 versions back and is older than the stack: + // Agent a.9.x < Kibana a.10.x + // Agent a.9.x < Kibana a.11.x + case 'preminor': + case 'minor': + if (agentVersion.minor < kibanaVersion.minor && kibanaVersion.minor - agentVersion.minor <= 2) + return; + + // section 3) versions where Agent is a minor version or major version greater (newer) than the stack should not work: + // Agent 7.10.x > Kibana 7.9.x + // Agent 8.0.x > Kibana 7.9.x + default: + if (semver.lte(agentVersion, kibanaVersion)) return; + else throw Boom.badRequest('Agent version is not compatible with kibana version'); } } - -/** - * used to remove prelease from version as includePrerelease in not working as expected - */ -function formatVersion(version: semver.SemVer) { - return `${version.major}.${version.minor}.${version.patch}`; -} From 92b0d0ba28688b844be1bc64ee7918077dfae155 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Fri, 28 Aug 2020 16:31:25 -0400 Subject: [PATCH 2/5] Change fn to accept 1 (opt 2) string vs object --- .../server/services/agents/enroll.test.ts | 48 ++++--------------- .../server/services/agents/enroll.ts | 29 ++++++----- 2 files changed, 26 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts index 764564cfa49f5..1ee9e284eb1cb 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts @@ -5,62 +5,30 @@ */ import { validateAgentVersion } from './enroll'; -import { appContextService } from '../app_context'; -import { IngestManagerAppContext } from '../../plugin'; describe('validateAgentVersion', () => { it('should throw with agent > kibana version', () => { - appContextService.start(({ - kibanaVersion: '8.0.0', - } as unknown) as IngestManagerAppContext); - expect(() => - validateAgentVersion({ - local: { elastic: { agent: { version: '8.8.0' } } }, - userProvided: {}, - }) - ).toThrowError(/Agent version is not compatible with kibana version/); + expect(() => validateAgentVersion('8.8.0', '8.0.0')).toThrowError( + /Agent version is not compatible with kibana version/ + ); }); it('should work with agent < kibana version', () => { - appContextService.start(({ - kibanaVersion: '8.0.0', - } as unknown) as IngestManagerAppContext); - validateAgentVersion({ local: { elastic: { agent: { version: '7.8.0' } } }, userProvided: {} }); + validateAgentVersion('7.8.0', '8.0.0'); }); it('should work with agent = kibana version', () => { - appContextService.start(({ - kibanaVersion: '8.0.0', - } as unknown) as IngestManagerAppContext); - validateAgentVersion({ local: { elastic: { agent: { version: '8.0.0' } } }, userProvided: {} }); + validateAgentVersion('8.0.0', '8.0.0'); }); it('should work with SNAPSHOT version', () => { - appContextService.start(({ - kibanaVersion: '8.0.0-SNAPSHOT', - } as unknown) as IngestManagerAppContext); - validateAgentVersion({ - local: { elastic: { agent: { version: '8.0.0-SNAPSHOT' } } }, - userProvided: {}, - }); + validateAgentVersion('8.0.0-SNAPSHOT', '8.0.0-SNAPSHOT'); }); it('should work with a agent using SNAPSHOT version', () => { - appContextService.start(({ - kibanaVersion: '7.8.0', - } as unknown) as IngestManagerAppContext); - validateAgentVersion({ - local: { elastic: { agent: { version: '7.8.0-SNAPSHOT' } } }, - userProvided: {}, - }); + validateAgentVersion('7.8.0-SNAPSHOT', '7.8.0'); }); it('should work with a kibana using SNAPSHOT version', () => { - appContextService.start(({ - kibanaVersion: '7.8.0-SNAPSHOT', - } as unknown) as IngestManagerAppContext); - validateAgentVersion({ - local: { elastic: { agent: { version: '7.8.0' } } }, - userProvided: {}, - }); + validateAgentVersion('7.8.0', '7.8.0-SNAPSHOT'); }); }); diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts index 84bb94ed93f34..db9abf886099b 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts @@ -20,7 +20,8 @@ export async function enroll( metadata?: { local: any; userProvided: any }, sharedId?: string ): Promise { - validateAgentVersion(metadata); + const agentVersion = metadata?.local?.elastic?.agent?.version; + validateAgentVersion(agentVersion); const existingAgent = sharedId ? await getAgentBySharedId(soClient, sharedId) : null; @@ -89,18 +90,21 @@ async function getAgentBySharedId(soClient: SavedObjectsClientContract, sharedId return null; } -export function validateAgentVersion(metadata?: { local: any; userProvided: any }) { - const kibanaVersion = semver.parse(appContextService.getKibanaVersion()); - if (!kibanaVersion) { - throw Boom.badRequest('Kibana version is not set'); +export function validateAgentVersion( + agentVersion: string, + kibanaVersion = appContextService.getKibanaVersion() +) { + const agentVersionParsed = semver.parse(agentVersion); + if (!agentVersionParsed) { + throw Boom.badRequest('Agent version not provided'); } - const agentVersion = semver.parse(metadata?.local?.elastic?.agent?.version); - if (!agentVersion) { - throw Boom.badRequest('Agent version not provided in metadata.'); + const kibanaVersionParsed = semver.parse(kibanaVersion); + if (!kibanaVersionParsed) { + throw Boom.badRequest('Kibana version is not set or provided'); } - const diff = semver.diff(agentVersion.raw, kibanaVersion.raw); + const diff = semver.diff(agentVersion, kibanaVersion); switch (diff) { // section 1) very close versions, only patch release differences - all combos should work // Agent a.b.1 < Kibana a.b.2 @@ -116,14 +120,17 @@ export function validateAgentVersion(metadata?: { local: any; userProvided: any // Agent a.9.x < Kibana a.11.x case 'preminor': case 'minor': - if (agentVersion.minor < kibanaVersion.minor && kibanaVersion.minor - agentVersion.minor <= 2) + if ( + agentVersionParsed.minor < kibanaVersionParsed.minor && + kibanaVersionParsed.minor - agentVersionParsed.minor <= 2 + ) return; // section 3) versions where Agent is a minor version or major version greater (newer) than the stack should not work: // Agent 7.10.x > Kibana 7.9.x // Agent 8.0.x > Kibana 7.9.x default: - if (semver.lte(agentVersion, kibanaVersion)) return; + if (semver.lte(agentVersionParsed, kibanaVersionParsed)) return; else throw Boom.badRequest('Agent version is not compatible with kibana version'); } } From d4f30af06f80f43b0ed9c958dec422e5f6478634 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Fri, 28 Aug 2020 17:00:58 -0400 Subject: [PATCH 3/5] Add tests based on issue comments --- .../server/services/agents/enroll.test.ts | 28 +++++++++++++++++-- .../server/services/agents/enroll.ts | 5 +++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts index 1ee9e284eb1cb..44473bf2d8d79 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.test.ts @@ -8,9 +8,7 @@ import { validateAgentVersion } from './enroll'; describe('validateAgentVersion', () => { it('should throw with agent > kibana version', () => { - expect(() => validateAgentVersion('8.8.0', '8.0.0')).toThrowError( - /Agent version is not compatible with kibana version/ - ); + expect(() => validateAgentVersion('8.8.0', '8.0.0')).toThrowError('not compatible'); }); it('should work with agent < kibana version', () => { validateAgentVersion('7.8.0', '8.0.0'); @@ -31,4 +29,28 @@ describe('validateAgentVersion', () => { it('should work with a kibana using SNAPSHOT version', () => { validateAgentVersion('7.8.0', '7.8.0-SNAPSHOT'); }); + + it('very close versions, e.g. patch/prerelease - all combos should work', () => { + validateAgentVersion('7.9.1', '7.9.2'); + validateAgentVersion('7.8.1', '7.8.2'); + validateAgentVersion('7.6.99', '7.6.2'); + validateAgentVersion('7.6.2', '7.6.99'); + validateAgentVersion('5.94.3', '5.94.1234-SNAPSHOT'); + validateAgentVersion('5.94.3-SNAPSHOT', '5.94.1'); + }); + + it('somewhat close versions, minor release is 1 or 2 versions back and is older than the stack', () => { + validateAgentVersion('7.9.1', '7.10.2'); + validateAgentVersion('7.9.9', '7.11.1'); + validateAgentVersion('7.6.99', '7.6.2'); + validateAgentVersion('7.6.2', '7.6.99'); + expect(() => validateAgentVersion('5.94.3-SNAPSHOT', '5.93.1')).toThrowError('not compatible'); + expect(() => validateAgentVersion('5.94.3', '5.92.99-SNAPSHOT')).toThrowError('not compatible'); + }); + + it('versions where Agent is a minor version or major version greater (newer) than the stack should not work', () => { + expect(() => validateAgentVersion('7.10.1', '7.9.99')).toThrowError('not compatible'); + expect(() => validateAgentVersion('7.9.9', '6.11.1')).toThrowError('not compatible'); + expect(() => validateAgentVersion('5.94.3', '5.92.99-SNAPSHOT')).toThrowError('not compatible'); + }); }); diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts index db9abf886099b..959451f383fc6 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts @@ -131,6 +131,9 @@ export function validateAgentVersion( // Agent 8.0.x > Kibana 7.9.x default: if (semver.lte(agentVersionParsed, kibanaVersionParsed)) return; - else throw Boom.badRequest('Agent version is not compatible with kibana version'); + else + throw Boom.badRequest( + `Agent version ${agentVersion} is not compatible with kibana version ${kibanaVersion}` + ); } } From ea2e3d79939a0e2f2aa3bbcf376c3eed44688ae4 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Sat, 29 Aug 2020 06:31:45 -0400 Subject: [PATCH 4/5] Change expected error message in test --- .../ingest_manager_api_integration/apis/fleet/agents/enroll.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/enroll.ts b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/enroll.ts index ce356dbd081c8..93f656ea952d2 100644 --- a/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/enroll.ts +++ b/x-pack/test/ingest_manager_api_integration/apis/fleet/agents/enroll.ts @@ -115,7 +115,7 @@ export default function (providerContext: FtrProviderContext) { }, }) .expect(400); - expect(apiResponse.message).to.match(/Agent version is not compatible with kibana/); + expect(apiResponse.message).to.match(/is not compatible/); }); it('should allow to enroll an agent with a valid enrollment token', async () => { From 93997679b0cdbabb397b3c3e75360b131d652f45 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Sat, 29 Aug 2020 06:34:57 -0400 Subject: [PATCH 5/5] Capitalize Kibana in error message --- x-pack/plugins/ingest_manager/server/services/agents/enroll.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts index 959451f383fc6..3c5850c722e97 100644 --- a/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts +++ b/x-pack/plugins/ingest_manager/server/services/agents/enroll.ts @@ -133,7 +133,7 @@ export function validateAgentVersion( if (semver.lte(agentVersionParsed, kibanaVersionParsed)) return; else throw Boom.badRequest( - `Agent version ${agentVersion} is not compatible with kibana version ${kibanaVersion}` + `Agent version ${agentVersion} is not compatible with Kibana version ${kibanaVersion}` ); } }