fix: allowing empty log group prefixes

aws · Aug 27, 2020 · 0810ea1 · 0810ea1
1 parent a69054a
commit 0810ea1
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
   }));
 });