Skip to content

Commit b4bca75

Browse files
authored
fix(rds): enablePerformanceInsights false is ignored when other performance insight properties are set (#37287)
### Issue # (if applicable) Closes #37051 ### Reason for this change `DatabaseInstanceFromSnapshot` (and other `DatabaseInstanceNew` subclasses) ignores an explicit `enablePerformanceInsights: false` when other Performance Insights properties are also set. ### Description of changes Replace `||` with `??` in the PI enablement logic so that an explicit `false` is not overridden. Remove the redundant fallback on the CFn property assignment. ### Description of how you validated changes Added a unit test for `DatabaseInstanceFromSnapshot` with `enablePerformanceInsights: false`. All existing tests pass. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)
1 parent 5104256 commit b4bca75

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

packages/aws-cdk-lib/aws-rds/lib/instance.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -976,9 +976,10 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData
976976
this.enableIamAuthentication = props.iamAuthentication;
977977

978978
const enablePerformanceInsights = props.enablePerformanceInsights
979-
|| props.performanceInsightRetention !== undefined
980-
|| props.performanceInsightEncryptionKey !== undefined
981-
|| props.databaseInsightsMode === DatabaseInsightsMode.ADVANCED;
979+
?? (props.performanceInsightRetention !== undefined
980+
|| props.performanceInsightEncryptionKey !== undefined
981+
|| props.databaseInsightsMode === DatabaseInsightsMode.ADVANCED
982+
|| undefined);
982983

983984
if (props.domain) {
984985
this.domainId = props.domain;
@@ -1018,7 +1019,7 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData
10181019
deletionProtection: defaultDeletionProtection(props.deletionProtection, props.removalPolicy),
10191020
enableCloudwatchLogsExports: this.cloudwatchLogsExports,
10201021
enableIamDatabaseAuthentication: Lazy.any({ produce: () => this.enableIamAuthentication }),
1021-
enablePerformanceInsights: enablePerformanceInsights || props.enablePerformanceInsights, // fall back to undefined if not set,
1022+
enablePerformanceInsights: enablePerformanceInsights,
10221023
iops,
10231024
monitoringInterval: props.monitoringInterval?.toSeconds(),
10241025
monitoringRoleArn: monitoringRole?.roleRef.roleArn,

packages/aws-cdk-lib/aws-rds/test/instance.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,6 +1748,19 @@ describe('instance', () => {
17481748
});
17491749
});
17501750

1751+
test('explicitly disabling performance insights is respected', () => {
1752+
new rds.DatabaseInstanceFromSnapshot(stack, 'Instance', {
1753+
engine: rds.DatabaseInstanceEngine.mysql({ version: rds.MysqlEngineVersion.VER_8_0_19 }),
1754+
vpc,
1755+
snapshotIdentifier: 'my-snapshot',
1756+
enablePerformanceInsights: false,
1757+
});
1758+
1759+
Template.fromStack(stack).hasResourceProperties('AWS::RDS::DBInstance', {
1760+
EnablePerformanceInsights: false,
1761+
});
1762+
});
1763+
17511764
test('throws if performance insights fields are set but performance insights is disabled', () => {
17521765
expect(() => {
17531766
new rds.DatabaseInstance(stack, 'Instance', {

0 commit comments

Comments
 (0)