From 0810ea1e6e718738bb28cfd3d5847685c004e4f4 Mon Sep 17 00:00:00 2001 From: Roman Yakobenchuk Date: Thu, 27 Aug 2020 17:50:59 +0000 Subject: [PATCH] fix: allowing empty log group prefixes --- .../lib/core/test/mongodb-instance.test.ts | 46 ++++--------------- .../aws-rfdk/lib/deadline/lib/repository.ts | 2 +- .../lib/deadline/lib/usage-based-licensing.ts | 2 +- .../aws-rfdk/lib/deadline/lib/worker-fleet.ts | 2 +- .../lib/deadline/test/repository.test.ts | 16 +++++-- .../test/usage-based-licensing.test.ts | 27 +++++++++++ .../lib/deadline/test/worker-fleet.test.ts | 16 +++++-- 7 files changed, 62 insertions(+), 49 deletions(-) diff --git a/packages/aws-rfdk/lib/core/test/mongodb-instance.test.ts b/packages/aws-rfdk/lib/core/test/mongodb-instance.test.ts index d1986c757..978611fbc 100644 --- a/packages/aws-rfdk/lib/core/test/mongodb-instance.test.ts +++ b/packages/aws-rfdk/lib/core/test/mongodb-instance.test.ts @@ -671,13 +671,15 @@ describe('Test MongoDbInstance', () => { })); }); - test('is created with correct LogGroup prefix', () => { + test.each([ + 'test-prefix/', + '', + ])('is created with correct LogGroup prefix %s', (testPrefix: string) => { // GIVEN - const logGroupPrefix = 'test-prefix/'; - const uniqueId = 'MongoDbInstance'; + const id = 'MongoDbInstance'; // WHEN - new MongoDbInstance(stack, uniqueId, { + new MongoDbInstance(stack, id, { mongoDb: { version, dnsZone, @@ -687,45 +689,13 @@ describe('Test MongoDbInstance', () => { }, vpc, logGroupProps: { - logGroupPrefix, + logGroupPrefix: testPrefix, }, }); // THEN cdkExpect(stack).to(haveResource('Custom::LogRetention', { - LogGroupName: logGroupPrefix + uniqueId, - })); - }); - - test('not using default LogGroup prefix if prefix is empty', () => { - // GIVEN - const logGroupPrefix = ''; - const uniqueId = 'MongoDbInstance'; - const expectedLogGroupName = logGroupPrefix + uniqueId; - const defaultLogGroupName = '/renderfarm/' + uniqueId; - - // WHEN - new MongoDbInstance(stack, uniqueId, { - mongoDb: { - version, - dnsZone, - hostname, - serverCertificate: serverCert, - userSsplAcceptance, - }, - vpc, - logGroupProps: { - logGroupPrefix, - }, - }); - - // THEN - cdkExpect(stack).to(haveResource('Custom::LogRetention', { - LogGroupName: expectedLogGroupName, - })); - - cdkExpect(stack).notTo(haveResource('Custom::LogRetention', { - LogGroupName: defaultLogGroupName, + LogGroupName: testPrefix + id, })); }); diff --git a/packages/aws-rfdk/lib/deadline/lib/repository.ts b/packages/aws-rfdk/lib/deadline/lib/repository.ts index 22bdc9161..eb3b744e2 100644 --- a/packages/aws-rfdk/lib/deadline/lib/repository.ts +++ b/packages/aws-rfdk/lib/deadline/lib/repository.ts @@ -697,7 +697,7 @@ export class Repository extends Construct implements IRepository { * @param logGroupProps */ private configureCloudWatchLogStream(installerGroup: AutoScalingGroup, groupName: string, logGroupProps?: LogGroupFactoryProps) { - const prefix = logGroupProps?.logGroupPrefix ? logGroupProps.logGroupPrefix : Repository.DEFAULT_LOG_GROUP_PREFIX; + const prefix = logGroupProps?.logGroupPrefix ?? Repository.DEFAULT_LOG_GROUP_PREFIX; const defaultedLogGroupProps = { ...logGroupProps, logGroupPrefix: prefix, diff --git a/packages/aws-rfdk/lib/deadline/lib/usage-based-licensing.ts b/packages/aws-rfdk/lib/deadline/lib/usage-based-licensing.ts index 80e1f53b6..32962ac02 100644 --- a/packages/aws-rfdk/lib/deadline/lib/usage-based-licensing.ts +++ b/packages/aws-rfdk/lib/deadline/lib/usage-based-licensing.ts @@ -523,7 +523,7 @@ export class UsageBasedLicensing extends Construct implements IGrantable { containerEnv.UBL_CERTIFICATES_URI = props.certificateSecret.secretArn; props.certificateSecret.grantRead(taskDefinition.taskRole); - const prefix = props.logGroupProps?.logGroupPrefix ? props.logGroupProps.logGroupPrefix : UsageBasedLicensing.DEFAULT_LOG_GROUP_PREFIX; + const prefix = props.logGroupProps?.logGroupPrefix ?? UsageBasedLicensing.DEFAULT_LOG_GROUP_PREFIX; const defaultedLogGroupProps: LogGroupFactoryProps = { ...props.logGroupProps, logGroupPrefix: prefix, diff --git a/packages/aws-rfdk/lib/deadline/lib/worker-fleet.ts b/packages/aws-rfdk/lib/deadline/lib/worker-fleet.ts index 635634249..aac36bdf3 100644 --- a/packages/aws-rfdk/lib/deadline/lib/worker-fleet.ts +++ b/packages/aws-rfdk/lib/deadline/lib/worker-fleet.ts @@ -463,7 +463,7 @@ export class WorkerInstanceFleet extends WorkerInstanceFleetBase { } private configureCloudWatchLogStream(fleetInstance: AutoScalingGroup, id: string, logGroupProps?: LogGroupFactoryProps) { - const prefix = logGroupProps?.logGroupPrefix ? logGroupProps.logGroupPrefix : WorkerInstanceFleet.DEFAULT_LOG_GROUP_PREFIX; + const prefix = logGroupProps?.logGroupPrefix ?? WorkerInstanceFleet.DEFAULT_LOG_GROUP_PREFIX; const defaultedLogGroupProps = { ...logGroupProps, logGroupPrefix: prefix, diff --git a/packages/aws-rfdk/lib/deadline/test/repository.test.ts b/packages/aws-rfdk/lib/deadline/test/repository.test.ts index 9107abe4b..25b2d05cf 100644 --- a/packages/aws-rfdk/lib/deadline/test/repository.test.ts +++ b/packages/aws-rfdk/lib/deadline/test/repository.test.ts @@ -825,17 +825,25 @@ test('repository instance is created with correct installer path version', () => expect(script.render()).toMatch(/10\.1\.9\.2/); }); -test('repository instance is created with correct LogGroup prefix', () => { - new Repository(stack, 'repositoryInstaller', { +test.each([ + 'test-prefix/', + '', +])('repository instance is created with correct LogGroup prefix %s', (testPrefix: string) => { + // GIVEN + const id = 'repositoryInstaller'; + + // WHEN + new Repository(stack, id, { vpc, version: deadlineVersion, logGroupProps: { - logGroupPrefix: 'test-prefix/', + logGroupPrefix: testPrefix, }, }); + // THEN expectCDK(stack).to(haveResource('Custom::LogRetention', { - LogGroupName: 'test-prefix/repositoryInstaller', + LogGroupName: testPrefix + id, })); }); diff --git a/packages/aws-rfdk/lib/deadline/test/usage-based-licensing.test.ts b/packages/aws-rfdk/lib/deadline/test/usage-based-licensing.test.ts index 0569f2914..3710a1e6c 100644 --- a/packages/aws-rfdk/lib/deadline/test/usage-based-licensing.test.ts +++ b/packages/aws-rfdk/lib/deadline/test/usage-based-licensing.test.ts @@ -6,6 +6,7 @@ import { arrayWith, expect as expectCDK, + haveResource, haveResourceLike, stringLike, } from '@aws-cdk/assert'; @@ -424,6 +425,32 @@ describe('UsageBasedLicensing', () => { })); }); + test.each([ + 'test-prefix/', + '', + ])('License Forwarder is created with correct LogGroup prefix %s', (testPrefix: string) => { + // GIVEN + stack = new Stack(app, 'IsolatedStack', { env }); + const id = 'licenseForwarder'; + + // WHEN + new UsageBasedLicensing(stack, id, { + certificateSecret, + images, + licenses, + renderQueue, + vpc, + logGroupProps: { + logGroupPrefix: testPrefix, + }, + }); + + // THEN + expectCDK(stack).to(haveResource('Custom::LogRetention', { + LogGroupName: testPrefix + id, + })); + }); + describe('license limits', () => { test('multiple licenses with limits', () => { // GIVEN diff --git a/packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts b/packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts index 5ddcdcaf9..08494476e 100644 --- a/packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts +++ b/packages/aws-rfdk/lib/deadline/test/worker-fleet.test.ts @@ -255,20 +255,28 @@ test('default worker fleet is created correctly custom Instance type', () => { })); }); -test('default worker fleet is created correctly with custom LogGroup prefix', () => { +test.each([ + 'test-prefix/', + '', +])('default worker fleet is created correctly with custom LogGroup prefix %s', (testPrefix: string) => { + // GIVEN + const id = 'workerFleet'; + // WHEN - new WorkerInstanceFleet(stack, 'workerFleet', { + new WorkerInstanceFleet(stack, id, { vpc, workerMachineImage: new GenericLinuxImage({ 'us-east-1': '123', }), renderQueue, - logGroupProps: {logGroupPrefix: 'test-prefix'}, + logGroupProps: { + logGroupPrefix: testPrefix, + }, }); expectCDK(stack).to(haveResource('Custom::LogRetention', { RetentionInDays: 3, - LogGroupName: 'test-prefixworkerFleet', + LogGroupName: testPrefix + id, })); });