Repository navigation
Conversation
kddejong
left a comment
There was a problem hiding this comment.
The two fixes are right and the change is well scoped — the good-fixtures corpus is unchanged, the conditional Fn::If cases behave, and no template that is clean today starts failing.
One thing to add before merge. The rule still requires Iops unconditionally, but RDS only requires it for io1 and io2. A cluster with StorageType: gp3 and no Iops, or with no StorageType and no Iops, is still reported on this branch. The second case is the same shape as #4962 — once StorageType is optional, nothing requires Iops. Please gate Iops on StorageType being io1 or io2 rather than leaving it in the unconditional required list.
Also, could you confirm the AllocatedStorage-on-a-replica rejection is reproducible? It's the only part of this change that adds a finding, so it's worth pinning down.
|
Thanks for the review. I've pushed a follow-up commit:
The message for the The |
E3692 treated ReplicationSourceIdentifier as a property that is not allowed on a Multi-AZ DB cluster and required AllocatedStorage, so every Multi-AZ read replica was flagged. RDS rejects AllocatedStorage on a Multi-AZ DB cluster read replica, because the replica uses the source's allocated storage. The rule also required StorageType, although RDS defaults it to io1 for Multi-AZ DB clusters. AllocatedStorage is now required only when ReplicationSourceIdentifier is absent and is not allowed when it is set, with a replica-specific message. StorageType is optional but still limited to io1, io2 and gp3. Fixes aws-cloudformation#4957 Fixes aws-cloudformation#4962 Signed-off-by: Brenden Hogan <hoganbr@amazon.com>
A Multi-AZ DB cluster with StorageType gp3 needs no Iops; RDS sets a baseline of 3000 IOPS. Iops is now required only when StorageType is io1 or io2, or when StorageType is omitted. With neither StorageType nor Iops, RDS uses gp2, which Multi-AZ DB clusters do not support, so that case gets its own message. The same rules apply to read replicas. Signed-off-by: Brenden Hogan <hoganbr@amazon.com>
ec8a28d to
5ce4b02
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4963 +/- ##
=======================================
Coverage 94.81% 94.81%
=======================================
Files 441 441
Lines 16179 16188 +9
Branches 3137 3140 +3
=======================================
+ Hits 15340 15349 +9
Misses 463 463
Partials 376 376
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Issue #, if available: Fixes #4957, fixes #4962
Description of changes:
E3692 flagged two valid Multi-AZ DB cluster templates:
ReplicationSourceIdentifiernot allowed,AllocatedStoragerequired) #4957: every Multi-AZ read replica, because it treatedReplicationSourceIdentifieras a property that is not allowed and requiredAllocatedStorage. The Multi-AZ read replica migration guide documents creating a Multi-AZ DB cluster as a read replica of a DB instance.StorageType, because it required the property. The CreateDBCluster API reference marksStorageTypeas not required. WithIopsset, RDS creates the cluster withio1. With neitherStorageTypenorIops, RDS usesgp2, which Multi-AZ DB clusters do not support.It also required
Iopson every Multi-AZ DB cluster, although RDS acceptsgp3without it.The rule now applies these requirements:
ReplicationSourceIdentifierset)DBClusterInstanceClassIopsStorageTypeisio1,io2or omittedStorageTypeisio1,io2or omittedAllocatedStorageStorageTypeio1,io2,gp3)io1,io2,gp3)Evidence, from us-east-1 in a personal AWS account. Clusters were PostgreSQL 17.10 on
db.c6gd.mediumwith 100 GB, and replica sources were PostgreSQL 17.10db.t4g.microDB instances.New Multi-AZ DB clusters:
StorageType: gp3withoutIops(CloudFormation stack, 2026-11-04) succeeds, with 3000 IOPS.StorageType: io1orio2withoutIops(CreateDBClusterAPI, 2026-11-04) fails with "The storage type io1 requires iops to be specified." (the same for io2).StorageTypenorIops(CloudFormation stack, andCreateDBClusterfor PostgreSQL 17.10 and MySQL 8.4.11, 2026-11-04) fails with "You can't use storage type gp2".StorageType-omitted results withIopsset are in E3692 requires StorageType on a Multi-AZ DB cluster, but RDS defaults it to io1 #4962.Read replicas:
AllocatedStorage(CloudFormation stack, 2026-10-24, reproduced 2026-11-04) fails with "You can't specify allocated storage for a Multi-AZ DB cluster read replica. The Multi-AZ DB cluster read replica uses the same allocated storage value as the source DB instance." The same replica withoutAllocatedStorage, from the same source, reachesCREATE_COMPLETE.StorageTypewithIopsset (CreateDBClusterAPI, 2026-10-30) succeeds, and the cluster defaults toio1.DBClusterInstanceClass(CreateDBClusterAPI, 2026-10-30) fails with "DBClusterInstanceClass is required."StorageType: io1orio2withoutIops(CreateDBClusterAPI, 2026-10-30 for io1, 2026-11-04 for both) fails with "The storage type io1 requires iops to be specified." (the same for io2).StorageType: gp3withoutIops(CloudFormation stack, 2026-11-04) succeeds, with 3000 IOPS.StorageTypenorIops(CloudFormation stack, 2026-11-04) fails with "You can't use storage type gp2".Three cases get their own message:
AllocatedStorageon a read replica:'AllocatedStorage' is not allowed on a Multi-AZ DB cluster read replica (ReplicationSourceIdentifier is set).io1orio2withoutIops:'Iops' is a required property when 'StorageType' is 'io1'.StorageTypenorIops:Set 'Iops' or set 'StorageType' to 'gp3'; without either, RDS uses gp2 storage, which Multi-AZ DB clusters do not support.The other properties that are not allowed keep the existing message.
Tests:
gp3withoutIops,io1andio2withoutIops, an unresolvedRefforStorageType, noStorageType, an invalidStorageType,AllocatedStorageon a replica, and missingDBClusterInstanceClassandIops.Fn::If: one switches between a primary and a replica withAWS::NoValue, settingAllocatedStorageonly on the primary branch (clean) and on both branches (the replica error). The other switchesStorageTypebetweenio1andgp3withoutIops, which reports only theio1branch.Verification:
ReplicationSourceIdentifiernot allowed,AllocatedStoragerequired) #4957 and E3692 requires StorageType on a Multi-AZ DB cluster, but RDS defaults it to io1 #4962 reproduction templates report E3692 onmainand are clean on this branch.CreateDBClustercases are covered by the unit tests. Onmain, thegp3cases and the replicas that deployed also report E3692.cfn-lint -t "test/fixtures/templates/good/**/*.yaml"gives identical output onmainand on this branch.pytest test/unitandpytest test/integration(including-m data) pass, apart from failures that also occur onmain.pre-commit run --all-filespasses.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.