Implement GeneratePhysicalName (#954)

awslabs · Apr 23, 2023 · 3e46579 · 3e46579
1 parent 44f5d37
commit 3e46579
Show file tree

Hide file tree

Showing 16 changed files with 159 additions and 139 deletions.
diff --git a/...solutions-constructs/aws-eventbridge-stepfunctions/test/eventbridge-stepfunctions.test.ts b/...solutions-constructs/aws-eventbridge-stepfunctions/test/eventbridge-stepfunctions.test.ts
@@ -197,28 +197,3 @@ test('check custom event bus resource with props when deploy:true', () => {
     Name: 'testcustomeventbus'
   });
 });
-
-test('check LogGroup name', () => {
-  const stack = new cdk.Stack();
-
-  deployNewStateMachine(stack);
-
-  // Perform some fancy stuff to examine the specifics of the LogGroupName
-  const expectedPrefix = '/aws/vendedlogs/states/constructs/';
-  const lengthOfDatetimeSuffix = 13;
-
-  const template = Template.fromStack(stack);
-  const LogGroup = template.findResources("AWS::Logs::LogGroup");
-
-  const logName = LogGroup.testeventbridgestepfunctionsStateMachineLogGroup826A5B74.Properties.LogGroupName;
-  const suffix = logName.slice(-lengthOfDatetimeSuffix);
-
-  // Look for the expected Prefix and the 13 digit time suffix
-  expect(logName.slice(0, expectedPrefix.length)).toEqual(expectedPrefix);
-  expect(IsWholeNumber(suffix)).not.toBe(false);
-});
-
-function IsWholeNumber(target: string): boolean {
-  const numberPattern = /[0-9]{13}/;
-  return target.match(numberPattern) !== null;
-}
diff --git a/...-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.expected.json b/...-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.expected.json
@@ -3,7 +3,27 @@
     "testeventbridgestepfunctionsconstructStateMachineLogGroup3098B32C": {
       "Type": "AWS::Logs::LogGroup",
       "Properties": {
-        "LogGroupName": "integ-test-no-arguments"
+        "LogGroupName": {
+          "Fn::Join": [
+            "",
+            [
+              "/aws/vendedlogs/states/constructs/eventbridge-stepfunctions-no-argumenttest-eventbridge-stepfunctions-constructStateMachineLog-",
+              {
+                "Fn::Select": [
+                  2,
+                  {
+                    "Fn::Split": [
+                      "/",
+                      {
+                        "Ref": "AWS::StackId"
+                      }
+                    ]
+                  }
+                ]
+              }
+            ]
+          ]
+        }
       },
       "UpdateReplacePolicy": "Delete",
       "DeletionPolicy": "Delete",

diff --git a/...structs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.ts b/...structs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.ts
@@ -33,7 +33,6 @@ const props: EventbridgeToStepfunctionsProps = {
   },
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY,
-    logGroupName: "integ-test-no-arguments"
   },
 };
 

diff --git a/...ns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/fargate-stepfunctions.test.ts b/...ns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/fargate-stepfunctions.test.ts
@@ -321,29 +321,3 @@ function testStateMachineProps(stack: cdk.Stack, userProps?: stepfunctions.State
 
   return defaults.consolidateProps(defaultTestProp, userProps);
 }
-
-test('check LogGroup name', () => {
-  const stack = new cdk.Stack();
-  const publicApi = true;
-
-  createFargateConstructWithNewResources(stack, publicApi);
-
-  // Perform some fancy stuff to examine the specifics of the LogGroupName
-  const expectedPrefix = '/aws/vendedlogs/states/constructs/';
-  const lengthOfDatetimeSuffix = 13;
-
-  const template = Template.fromStack(stack);
-  const LogGroup = template.findResources("AWS::Logs::LogGroup");
-
-  const logName = LogGroup.testconstructStateMachineLogGroup2EB4F48B.Properties.LogGroupName;
-  const suffix = logName.slice(-lengthOfDatetimeSuffix);
-
-  // Look for the expected Prefix and the 13 digit time suffix
-  expect(logName.slice(0, expectedPrefix.length)).toEqual(expectedPrefix);
-  expect(IsWholeNumber(suffix)).not.toBe(false);
-});
-
-function IsWholeNumber(target: string): boolean {
-  const numberPattern = /[0-9]{13}/;
-  return target.match(numberPattern) !== null;
-}
diff --git a/...aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.expected.json b/...aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.expected.json
@@ -1076,7 +1076,27 @@
     "testconstructStateMachineLogGroup2EB4F48B": {
       "Type": "AWS::Logs::LogGroup",
       "Properties": {
-        "LogGroupName": "with-lambda"
+        "LogGroupName": {
+          "Fn::Join": [
+            "",
+            [
+              "/aws/vendedlogs/states/constructs/new-resourcestest-constructStateMachineLog-",
+              {
+                "Fn::Select": [
+                  2,
+                  {
+                    "Fn::Split": [
+                      "/",
+                      {
+                        "Ref": "AWS::StackId"
+                      }
+                    ]
+                  }
+                ]
+              }
+            ]
+          ]
+        }
       },
       "UpdateReplacePolicy": "Delete",
       "DeletionPolicy": "Delete",

diff --git a/.../patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.ts b/.../patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.ts
@@ -50,7 +50,6 @@ const constructProps: FargateToStepfunctionsProps = {
   stateMachineEnvironmentVariableName: 'CUSTOM_NAME',
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY,
-    logGroupName: "with-lambda"
   }
 };
 

diff --git a/...@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.expected.json b/...@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.expected.json
@@ -3,7 +3,27 @@
     "testlambdastepfunctionsconstructStateMachineLogGroup1FD4C0D4": {
       "Type": "AWS::Logs::LogGroup",
       "Properties": {
-        "LogGroupName": "with-lambda"
+        "LogGroupName": {
+          "Fn::Join": [
+            "",
+            [
+              "/aws/vendedlogs/states/constructs/deploy-lambdatest-lambda-stepfunctions-constructStateMachineLog-",
+              {
+                "Fn::Select": [
+                  2,
+                  {
+                    "Fn::Split": [
+                      "/",
+                      {
+                        "Ref": "AWS::StackId"
+                      }
+                    ]
+                  }
+                ]
+              }
+            ]
+          ]
+        }
       },
       "UpdateReplacePolicy": "Delete",
       "DeletionPolicy": "Delete",

diff --git a/...e/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.ts b/...e/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.ts
@@ -37,7 +37,6 @@ const props: LambdaToStepfunctionsProps = {
   },
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY,
-    logGroupName: "with-lambda"
   }
 };
 

diff --git a/...erns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/lambda-stepfunctions.test.ts b/...erns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/lambda-stepfunctions.test.ts
@@ -469,41 +469,3 @@ test("Test bad call with existingVpc and deployVpc", () => {
   // Assertion
   expect(app).toThrowError();
 });
-
-test('check LogGroup name', () => {
-  // Stack
-  const stack = new Stack();
-  // Helper declaration
-  const startState = new stepfunctions.Pass(stack, 'StartState');
-  new LambdaToStepfunctions(stack, 'lambda-to-step-function-stack', {
-    lambdaFunctionProps: {
-      runtime: lambda.Runtime.NODEJS_16_X,
-      handler: 'index.handler',
-      code: lambda.Code.fromAsset(`${__dirname}/lambda`),
-      environment: {
-        LAMBDA_NAME: 'existing-function'
-      }
-    },
-    stateMachineProps: {
-      definition: startState
-    }
-  });
-  // Perform some fancy stuff to examine the specifics of the LogGroupName
-  const expectedPrefix = '/aws/vendedlogs/states/constructs/';
-  const lengthOfDatetimeSuffix = 13;
-
-  const template = Template.fromStack(stack);
-  const LogGroup = template.findResources("AWS::Logs::LogGroup");
-
-  const logName = LogGroup.lambdatostepfunctionstackStateMachineLogGroupEAD4854E.Properties.LogGroupName;
-  const suffix = logName.slice(-lengthOfDatetimeSuffix);
-
-  // Look for the expected Prefix and the 13 digit time suffix
-  expect(logName.slice(0, expectedPrefix.length)).toEqual(expectedPrefix);
-  expect(IsWholeNumber(suffix)).not.toBe(false);
-});
-
-function IsWholeNumber(target: string): boolean {
-  const numberPattern = /[0-9]{13}/;
-  return target.match(numberPattern) !== null;
-}
diff --git a/...ons-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json b/...ons-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json
@@ -116,7 +116,27 @@
     "tests3stepfunctionsconstructtests3stepfunctionsconstructeventrulestepfunctionconstructStateMachineLogGroupE86C2CF5": {
       "Type": "AWS::Logs::LogGroup",
       "Properties": {
-        "LogGroupName": "with-lambda"
+        "LogGroupName": {
+          "Fn::Join": [
+            "",
+            [
+              "/aws/vendedlogs/states/constructs/s3-stepfunctions-no-argumenttest-s3-stepfunctions-construct-event-rule-step-function-constructStateMachineLog-",
+              {
+                "Fn::Select": [
+                  2,
+                  {
+                    "Fn::Split": [
+                      "/",
+                      {
+                        "Ref": "AWS::StackId"
+                      }
+                    ]
+                  }
+                ]
+              }
+            ]
+          ]
+        }
       },
       "UpdateReplacePolicy": "Delete",
       "DeletionPolicy": "Delete",

diff --git a/...@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts b/...@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts
@@ -33,7 +33,6 @@ const props: S3ToStepfunctionsProps = {
   },
   logGroupProps: {
     removalPolicy: RemovalPolicy.DESTROY,
-    logGroupName: "with-lambda"
   },
   logS3AccessLogs: false
 };

diff --git a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/s3-stepfunctions.test.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/s3-stepfunctions.test.ts
@@ -191,28 +191,3 @@ test('s3 bucket with no logging bucket', () => {
   template.hasResourceProperties("Custom::S3BucketNotifications", {});
   expect(construct.s3LoggingBucket).toEqual(undefined);
 });
-
-test('check LogGroup name', () => {
-  const stack = new cdk.Stack();
-
-  deployNewStateMachine(stack);
-
-  // Perform some fancy stuff to examine the specifics of the LogGroupName
-  const expectedPrefix = '/aws/vendedlogs/states/constructs/';
-  const lengthOfDatetimeSuffix = 13;
-
-  const template = Template.fromStack(stack);
-  const LogGroup = template.findResources("AWS::Logs::LogGroup");
-
-  const logName = LogGroup.tests3stepfunctionstests3stepfunctionseventrulestepfunctionconstructStateMachineLogGroupB4555776.Properties.LogGroupName;
-  const suffix = logName.slice(-lengthOfDatetimeSuffix);
-
-  // Look for the expected Prefix and the 13 digit time suffix
-  expect(logName.slice(0, expectedPrefix.length)).toEqual(expectedPrefix);
-  expect(IsWholeNumber(suffix)).not.toBe(false);
-});
-
-function IsWholeNumber(target: string): boolean {
-  const numberPattern = /[0-9]{13}/;
-  return target.match(numberPattern) !== null;
-}
diff --git a/source/patterns/@aws-solutions-constructs/core/lib/step-function-helper.ts b/source/patterns/@aws-solutions-constructs/core/lib/step-function-helper.ts
@@ -21,7 +21,7 @@ import * as logs from 'aws-cdk-lib/aws-logs';
 import * as cdk from 'aws-cdk-lib';
 import * as smDefaults from './step-function-defaults';
 import * as sfn from 'aws-cdk-lib/aws-stepfunctions';
-import { overrideProps, generateResourceName, addCfnSuppressRules } from './utils';
+import { overrideProps, addCfnSuppressRules, generatePhysicalName } from './utils';
 import * as iam from 'aws-cdk-lib/aws-iam';
 import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch';
 import { buildLogGroup } from './cloudwatch-log-group-helper';
@@ -59,16 +59,18 @@ export function buildStateMachine(scope: Construct, stateMachineProps: sfn.State
     if (!consolidatedLogGroupProps) {
       consolidatedLogGroupProps = {};
     }
+
+    const maxLogGroupNameLength = 255;
     if (!consolidatedLogGroupProps?.logGroupName) {
       const logGroupPrefix = '/aws/vendedlogs/states/constructs/';
-      const maxResourceNameLength = 255 - logGroupPrefix.length;
+      const maxGeneratedNameLength = maxLogGroupNameLength - logGroupPrefix.length;
       const nameParts: string[] = [
         cdk.Stack.of(scope).stackName, // Name of the stack
         scope.node.id,                 // Construct ID
         'StateMachineLog'              // Literal string for log group name portion
       ];
 
-      const logGroupName = logGroupPrefix + generateResourceName(nameParts, maxResourceNameLength, true);
+      const logGroupName = generatePhysicalName(logGroupPrefix, nameParts, maxGeneratedNameLength);
       consolidatedLogGroupProps = overrideProps(consolidatedLogGroupProps, { logGroupName });
     }
 

diff --git a/source/patterns/@aws-solutions-constructs/core/lib/utils.ts b/source/patterns/@aws-solutions-constructs/core/lib/utils.ts
@@ -104,7 +104,9 @@ export function printWarning(message: string) {
 /**
  * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
  *
- * @summary Creates a resource name in the style of the CDK (string+hash)
+ * @summary Creates a resource name in the style of the CDK (string+hash) - this value should be used for logical IDs, but
+ * not Physical Names, as it will not be static within a single stack instance lifetime, or it will not be different in
+ * different stack instances
  * @param {string[]} parts - the various string components of the name (eg - stackName, solutions construct ID, L2 construct ID)
  * @param {number} maxLength - the longest string that can be returned
  * @returns {string} - a string with concatenated parts (truncated if neccessary) + a hash of the full concatenated parts
@@ -135,6 +137,49 @@ export function generateResourceName(
   return finalName.toLowerCase();
 }
 
+/**
+ * @internal This is an internal core function and should not be called directly by Solutions Constructs clients.
+ *
+ * @summary Creates a physical resource name in the style of the CDK (string+hash) - this value incorporates Stack ID,
+ * so it will remain static in multiple updates of a single stack, but will be different in a separate stack instance
+ * @param {string[]} parts - the various string components of the name (eg - stackName, solutions construct ID, L2 construct ID)
+ * @param {number} maxLength - the longest string that can be returned
+ * @returns {string} - a string with concatenated parts (truncated if neccessary) + a hash of the full concatenated parts
+ *
+ */
+export function generatePhysicalName(
+  prefix: string,
+  parts: string[],
+  maxLength: number,
+): string {
+  // The result will consist of:
+  //    -The prefix - unaltered
+  //    -The parts concatenated, but reduced in size to meet the maxLength limit for the overall name
+  //    -A hyphen delimiter
+  //    -The GUID portion of the stack arn
+
+  const stackIdGuidLength = 36;
+  const prefixLength = prefix.length;
+  const maxPartsLength = maxLength - prefixLength - 1 - stackIdGuidLength; // 1 is the hyphen
+
+  // Extract the Stack ID Guid
+  const uniqueStackIdPart = cdk.Fn.select(2, cdk.Fn.split('/', `${cdk.Aws.STACK_ID}`));
+
+  let allParts: string = '';
+
+  parts.forEach((part) => {
+    allParts += part;
+  });
+
+  if (allParts.length > maxPartsLength) {
+    const subStringLength = maxPartsLength / 2;
+    allParts = allParts.substring(0, subStringLength) + allParts.substring(allParts.length - subStringLength);
+  }
+
+  const finalName  = prefix.toLowerCase() + allParts + '-' + uniqueStackIdPart;
+  return finalName;
+}
+
 /**
  * Removes all non-alphanumeric characters in a string.
  */