Skip to content

Commit

Permalink
fix(core): Add stage prefix to stack name shortening process (#25359)
Browse files Browse the repository at this point in the history
There is an issue in the stack name generation process where the prefix generated from assembly's stage name is not taken into account when shortening a stack name to meet the requirement of being equal or less than 128 characters longs.

This can lead to generating an invalid stack name greater than 128 characters: since the stack name is shortened to 128 characters, when the prefix is added, the limit is exceeded.

Current solution:
- Adding a feature flag
  - With the feature flag on, the prefix is processed within the generateUniqueName function.
  - With the feature flag off, stack name generation is not changed

Fixes #23628


NOTE: This PR was previously opened, but it was merged before I was able to add a feature flag, which ended up adding breaking changes and the changes of the PR were rolled back. Old PR: #24443
  • Loading branch information
QustodioPablo authored Jun 12, 2023
1 parent a7ae5a4 commit 79c58ed
Show file tree
Hide file tree
Showing 6 changed files with 153 additions and 9 deletions.
12 changes: 10 additions & 2 deletions packages/aws-cdk-lib/core/lib/private/unique-resource-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ interface MakeUniqueResourceNameOptions {
* @default - none
*/
readonly allowedSpecialCharacters?: string;

/**
* Prefix to be added into the stack name
*
* @default - none
*/
readonly prefix?: string;
}

/**
Expand All @@ -49,6 +56,7 @@ const HASH_LEN = 8;
export function makeUniqueResourceName(components: string[], options: MakeUniqueResourceNameOptions) {
const maxLength = options.maxLength ?? 256;
const separator = options.separator ?? '';
const prefix = options.prefix ?? '';
components = components.filter(x => x !== HIDDEN_ID);

if (components.length === 0) {
Expand All @@ -59,7 +67,7 @@ export function makeUniqueResourceName(components: string[], options: MakeUnique
// in order to support transparent migration of cloudformation templates to the CDK without the
// need to rename all resources.
if (components.length === 1) {
const topLevelResource = removeNonAllowedSpecialCharacters(components[0], separator, options.allowedSpecialCharacters);
const topLevelResource = prefix + removeNonAllowedSpecialCharacters(components[0], separator, options.allowedSpecialCharacters);

if (topLevelResource.length <= maxLength) {
return topLevelResource;
Expand All @@ -68,7 +76,7 @@ export function makeUniqueResourceName(components: string[], options: MakeUnique

// Calculate the hash from the full path, included unresolved tokens so the hash value is always unique
const hash = pathHash(components);
const human = removeDupes(components)
const human = prefix + removeDupes(components)
.filter(pathElement => pathElement !== HIDDEN_FROM_HUMAN_ID)
.map(pathElement => removeNonAllowedSpecialCharacters(pathElement, separator, options.allowedSpecialCharacters))
.filter(pathElement => pathElement)
Expand Down
22 changes: 16 additions & 6 deletions packages/aws-cdk-lib/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { LogicalIDs } from './private/logical-id';
import { resolve } from './private/resolve';
import { makeUniqueId } from './private/uniqueid';
import * as cxschema from '../../cloud-assembly-schema';
import { INCLUDE_PREFIX_IN_UNIQUE_NAME_GENERATION } from '../../cx-api';
import * as cxapi from '../../cx-api';

const STACK_SYMBOL = Symbol.for('@aws-cdk/core.Stack');
Expand Down Expand Up @@ -1432,7 +1433,11 @@ export class Stack extends Construct implements ITaggable {
private generateStackName() {
const assembly = Stage.of(this);
const prefix = (assembly && assembly.stageName) ? `${assembly.stageName}-` : '';
return `${prefix}${this.generateStackId(assembly)}`;
if (FeatureFlags.of(this).isEnabled(INCLUDE_PREFIX_IN_UNIQUE_NAME_GENERATION)) {
return `${this.generateStackId(assembly, prefix)}`;
} else {
return `${prefix}${this.generateStackId(assembly)}`;
}
}

/**
Expand All @@ -1447,7 +1452,7 @@ export class Stack extends Construct implements ITaggable {
/**
* Generate an ID with respect to the given container construct.
*/
private generateStackId(container: IConstruct | undefined) {
private generateStackId(container: IConstruct | undefined, prefix: string='') {
const rootPath = rootPathTo(this, container);
const ids = rootPath.map(c => Node.of(c).id);

Expand All @@ -1457,7 +1462,7 @@ export class Stack extends Construct implements ITaggable {
throw new Error('unexpected: stack id must always be defined');
}

return makeStackName(ids);
return makeStackName(ids, prefix);
}

private resolveExportedValue(exportedValue: any): ResolvedExport {
Expand Down Expand Up @@ -1643,9 +1648,14 @@ export function rootPathTo(construct: IConstruct, ancestor?: IConstruct): IConst
* has only one component. Otherwise we fall back to the regular "makeUniqueId"
* behavior.
*/
function makeStackName(components: string[]) {
if (components.length === 1) { return components[0]; }
return makeUniqueResourceName(components, { maxLength: 128 });
function makeStackName(components: string[], prefix: string='') {
if (components.length === 1) {
const stack_name = prefix + components[0];
if (stack_name.length <= 128) {
return stack_name;
}
}
return makeUniqueResourceName(components, { maxLength: 128, prefix: prefix });
}

function getCreateExportsScope(stack: Stack) {
Expand Down
62 changes: 62 additions & 0 deletions packages/aws-cdk-lib/core/test/stage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,68 @@ describe('stage', () => {
expect(stack.stackName).toEqual('MyStage-MyStack');
});

test('FF include prefix: Prefix and stack names not exceeding 128 characters are not shortened', () => {
// WHEN
const app = new App({
context: {
'@aws-cdk/core:includePrefixInUniqueNameGeneration': true,
},
});
const stage = new Stage(app, 'ShortPrefix');
const stack = new BogusStack(stage, 'Short-Stack-Name');

// THEN
expect(stack.stackName.length).toEqual(28);
expect(stack.stackName).toEqual('ShortPrefix-Short-Stack-Name');
});

test('FF include prefix: Stacks with more than one component and a prefix hashed even if short enough', () => {
// WHEN
const app = new App({
context: {
'@aws-cdk/core:includePrefixInUniqueNameGeneration': true,
},
});
const stage = new Stage(app, 'ThePrefix');
const rootStack = new Stack(stage, 'Prod');
const stack = new Stack(rootStack, 'MyStack');

// THEN
expect(stack.stackName.length).toEqual(29);
expect(stack.stackName).toEqual('ThePrefix-ProdMyStackFEA60919');
});

test('FF include prefix: Stacks with more than one component and a prefix shortened if too big', () => {
// WHEN
const app = new App({
context: {
'@aws-cdk/core:includePrefixInUniqueNameGeneration': true,
},
});
const stage = new Stage(app, 'ThePrefixIsLongEnoughToExceedTheMaxLenght');
const construct = new Construct(stage, 'ReallyReallyLoooooooongConstructName');
const stack = new BogusStack(construct, 'ThisStageNameIsVeryLongButWillOnlyBeTooLongWhenCombinedWithTheStackName');

// THEN
expect(stack.stackName.length).toEqual(128);
expect(stack.stackName).toEqual('ThePrefixIsLongEnoughToExceedTheMaxLenght-ReallyReallyLooooomeIsVeryLongButWillOnlyBeTooLongWhenCombinedWithTheStackName1E474FCA');
});

test('generated stack names will not exceed 128 characters when using prefixes', () => {
// WHEN
const app = new App({
context: {
'@aws-cdk/core:includePrefixInUniqueNameGeneration': true,
},
});
const stage = new Stage(app, 'ThisStageNameIsVeryLongButWillOnlyBeTooLongWhenCombinedWithTheStackName');
const stack = new BogusStack(stage, 'ThisStackNameIsVeryLongButItWillOnlyBeTooLongWhenCombinedWithTheLongPrefix');

// THEN
expect(stack.stackName.length).toEqual(128);
expect(stack.stackName).toEqual('ThisStageNameIsVeryLongButWillOnlyBeTooLongWhenCombinedWithTsVeryLongButItWillOnlyBeTooLongWhenCombinedWithTheLongPrefix4CA9F65B');
});

test('Can not have dependencies to stacks outside the nested asm', () => {
// GIVEN
const app = new App();
Expand Down
24 changes: 23 additions & 1 deletion packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Flags come in three types:
| [@aws-cdk/aws-secretsmanager:useAttachedSecretResourcePolicyForSecretTargetAttachments](#aws-cdkaws-secretsmanageruseattachedsecretresourcepolicyforsecrettargetattachments) | SecretTargetAttachments uses the ResourcePolicy of the attached Secret. | 2.67.0 | (fix) |
| [@aws-cdk/aws-redshift:columnId](#aws-cdkaws-redshiftcolumnid) | Whether to use an ID to track Redshift column changes | 2.68.0 | (fix) |
| [@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2](#aws-cdkaws-stepfunctions-tasksenableemrservicepolicyv2) | Enable AmazonEMRServicePolicy_v2 managed policies | 2.72.0 | (fix) |
| [@aws-cdk/core:includePrefixInUniqueNameGeneration](#aws-cdkcoreincludeprefixinuniquenamegeneration) | Include the stack prefix in the stack name generation process | V2NEXT | (fix) |

<!-- END table -->

Expand Down Expand Up @@ -96,7 +97,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
"@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2": true,
"@aws-cdk/aws-ec2:restrictDefaultSecurityGroup": true,
"@aws-cdk/aws-apigateway:requestValidatorUniqueId": true,
"@aws-cdk/aws-kms:aliasNameRef": true
"@aws-cdk/aws-kms:aliasNameRef": true,
"@aws-cdk/core:includePrefixInUniqueNameGeneration": true
}
}
```
Expand Down Expand Up @@ -986,4 +988,24 @@ intervention since they might not have the appropriate tags propagated automatic
| 2.72.0 | `false` | `true` |


### @aws-cdk/core:includePrefixInUniqueNameGeneration

*Include the stack prefix in the stack name generation process* (fix)

This flag prevents the prefix of a stack from making the stack's name longer than the 128 character limit.

If the flag is set, the prefix is included in the stack name generation process.
If the flag is not set, then the prefix of the stack is prepended to the generated stack name.

**NOTE** - Enabling this flag comes at a **risk**. If you have already deployed stacks, changing the status of this
feature flag can lead to a change in stacks' name. Changing a stack name mean recreating the whole stack, which
is not viable in some productive setups.


| Since | Default | Recommended |
| ----- | ----- | ----- |
| (not in v1) | | |
| V2NEXT | `false` | `true` |


<!-- END details -->
22 changes: 22 additions & 0 deletions packages/aws-cdk-lib/cx-api/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,25 @@ _cdk.json_
}
}
```

* `@aws-cdk/core:includePrefixInUniqueNameGeneration`

Enable this feature flag to include the stack's prefixes to the name generation process.

Not doing so can cause the name of stack to exceed 128 characters:
- The name generation ensures it doesn't exceed 128 characters
- Without this feature flag, the prefix is prepended to the generated name, which result can exceed 128 characters

This is a feature flag as it changes the name generated for stacks. Any CDK application deployed prior this fix will
most likely be generated with a new name, causing the stack to be recreated with the new name, and then deleting the old one.
For applications running on production environments this can be unmanageable.

_cdk.json_

```json
{
"context": {
"@aws-cdk/core:includePrefixInUniqueNameGeneration": true
}
}
```
20 changes: 20 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const REDSHIFT_COLUMN_ID = '@aws-cdk/aws-redshift:columnId';
export const ENABLE_EMR_SERVICE_POLICY_V2 = '@aws-cdk/aws-stepfunctions-tasks:enableEmrServicePolicyV2';
export const EC2_RESTRICT_DEFAULT_SECURITY_GROUP = '@aws-cdk/aws-ec2:restrictDefaultSecurityGroup';
export const APIGATEWAY_REQUEST_VALIDATOR_UNIQUE_ID = '@aws-cdk/aws-apigateway:requestValidatorUniqueId';
export const INCLUDE_PREFIX_IN_UNIQUE_NAME_GENERATION = '@aws-cdk/core:includePrefixInUniqueNameGeneration';
export const KMS_ALIAS_NAME_REF = '@aws-cdk/aws-kms:aliasNameRef';

export const FLAGS: Record<string, FlagInfo> = {
Expand Down Expand Up @@ -804,6 +805,25 @@ export const FLAGS: Record<string, FlagInfo> = {
introducedIn: { v2: 'V2·NEXT' },
recommendedValue: true,
},

//////////////////////////////////////////////////////////////////////
[INCLUDE_PREFIX_IN_UNIQUE_NAME_GENERATION]: {
type: FlagType.BugFix,
summary: 'Include the stack prefix in the stack name generation process',
detailsMd: `
This flag prevents the prefix of a stack from making the stack's name longer than the 128 character limit.
If the flag is set, the prefix is included in the stack name generation process.
If the flag is not set, then the prefix of the stack is prepended to the generated stack name.
**NOTE** - Enabling this flag comes at a **risk**. If you have already deployed stacks, changing the status of this
feature flag can lead to a change in stacks' name. Changing a stack name mean recreating the whole stack, which
is not viable in some productive setups.
`,
introducedIn: { v2: 'V2NEXT' },
recommendedValue: true,
},

};

const CURRENT_MV = 'v2';
Expand Down

0 comments on commit 79c58ed

Please sign in to comment.