diff --git a/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/eventbridge-stepfunctions.test.ts b/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/eventbridge-stepfunctions.test.ts index 31e00a159..6c6eb9c2c 100644 --- a/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/eventbridge-stepfunctions.test.ts +++ b/source/patterns/@aws-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/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.expected.json b/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.expected.json index 0c108fa64..bb8778950 100644 --- a/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.expected.json +++ b/source/patterns/@aws-solutions-constructs/aws-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/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.ts b/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.ts index cd487eaa8..9d43105b1 100644 --- a/source/patterns/@aws-solutions-constructs/aws-eventbridge-stepfunctions/test/integ.eventbridge-stepfunctions-no-argument.ts +++ b/source/patterns/@aws-solutions-constructs/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/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/fargate-stepfunctions.test.ts b/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/fargate-stepfunctions.test.ts index e33e2461b..c265b78ec 100644 --- a/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/fargate-stepfunctions.test.ts +++ b/source/patterns/@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/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.expected.json b/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.expected.json index 2c7a36d6d..f8b2fa26e 100644 --- a/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.expected.json +++ b/source/patterns/@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/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.ts b/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.ts index 45619476f..3543ba05f 100644 --- a/source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/test/integ.new-resources.ts +++ b/source/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/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.expected.json b/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.expected.json index cf8519009..5d32965b4 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.expected.json +++ b/source/patterns/@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/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.ts index 7bf44dcb1..d1578b19e 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/integ.deploy-lambda.ts +++ b/source/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/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/lambda-stepfunctions.test.ts b/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/lambda-stepfunctions.test.ts index 040fdc55c..cf9fbc33b 100644 --- a/source/patterns/@aws-solutions-constructs/aws-lambda-stepfunctions/test/lambda-stepfunctions.test.ts +++ b/source/patterns/@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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json index 456f580d9..c2d6d28e0 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.expected.json +++ b/source/patterns/@aws-solutions-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/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts b/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts index 184941c9d..e79e25c07 100644 --- a/source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/test/integ.s3-stepfunctions-no-argument.ts +++ b/source/patterns/@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 index c4cd1a914..927f09213 100644 --- 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 index ea62e10a8..8ae409523 100644 --- 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 index 2b13ad0ed..6789d9cbb 100644 --- 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. */ diff --git a/source/patterns/@aws-solutions-constructs/core/test/step-function-helper.test.ts b/source/patterns/@aws-solutions-constructs/core/test/step-function-helper.test.ts index 4de810912..001858ac2 100644 --- a/source/patterns/@aws-solutions-constructs/core/test/step-function-helper.test.ts +++ b/source/patterns/@aws-solutions-constructs/core/test/step-function-helper.test.ts @@ -194,7 +194,7 @@ test('Test deployment with custom role', () => { test('Confirm format of name', () => { // Stack - const stack = new Stack(); + const stack = new Stack(undefined, 'teststack'); // Step function definition const startState = new sfn.Pass(stack, 'StartState'); // Build state machine @@ -211,19 +211,10 @@ test('Confirm format of name', () => { }); // Perform some fancy stuff to examine the specifics of the LogGroupName - const expectedPrefix = '/aws/vendedlogs/states/constructs/'; - const lengthOfDatetimeSuffix = 13; - const LogGroup = template.findResources("AWS::Logs::LogGroup"); const logName = LogGroup.StateMachineLogGroup15B91BCB.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); + expect(logName['Fn::Join']).toBeDefined(); + expect(logName['Fn::Join'].length).toEqual(2); + expect(logName['Fn::Join'][1][1]['Fn::Select'][1]['Fn::Split'][1].Ref).toEqual("AWS::StackId"); }); - -function IsWholeNumber(target: string): boolean { - const numberPattern = /[0-9]{13}/; - return target.match(numberPattern) !== null; -} diff --git a/source/patterns/@aws-solutions-constructs/core/test/utils.test.ts b/source/patterns/@aws-solutions-constructs/core/test/utils.test.ts index 72de0c7eb..6597b2154 100644 --- a/source/patterns/@aws-solutions-constructs/core/test/utils.test.ts +++ b/source/patterns/@aws-solutions-constructs/core/test/utils.test.ts @@ -64,6 +64,26 @@ test('Test generateResourceName with randomized extension', () => { }); +test('Test generatePhysicalName', () => { + const result = defaults.generatePhysicalName('/aws/vendedlogs/states/constructs/', parts, 255); + + // The token number is not constant, so need to be flexible checking this value + const regex = /\/aws\/vendedlogs\/states\/constructs\/firstportionislongsecondsection-\${Token\[TOKEN\.[0-9]+\]}/; + expect(result).toMatch(regex); +}); + +test('Test truncation of generatePhysicalName', () => { + const longParts = [ ...parts, ...parts, ...parts, ...parts, ...parts ]; + const prefix = '/aws/vendedlogs/states/constructs/'; + const lengthOfGuid = 36; + const maxNameLength = 125; + + const result = defaults.generatePhysicalName(prefix, longParts, maxNameLength); + + const fixedPortion = result.split('$')[0]; + expect(fixedPortion.length).toEqual(maxNameLength - lengthOfGuid); +}); + test('Test generateIntegStackName', () => { const result = defaults.generateIntegStackName('integ.apigateway-dynamodb-CRUD.js'); expect(result).toContain('apigateway-dynamodb-CRUD');