Skip to content

fix(providers): declare the two NatGateway / LogGroup wiring gaps as silent drops so they auto-route via Cloud Control - #1429

Merged
go-to-k merged 4 commits into
mainfrom
fix/1411-1412-handled-wiring-gaps
Aug 9, 2026
Merged

fix(providers): declare the two NatGateway / LogGroup wiring gaps as silent drops so they auto-route via Cloud Control#1429
go-to-k merged 4 commits into
mainfrom
fix/1411-1412-handled-wiring-gaps

Conversation

@go-to-k

@go-to-k go-to-k commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Both properties were declared in handledProperties while nothing read them, so a template setting either deployed "successfully" with the value discarded. Found by the first real-tree run of the handled-property-wiring critic (#1404), which seeded them as tracked KNOWN GAP allow-list entries rather than fixing them in the critic's own PR.

Both issues proposed wiring the property into create(). For #1411 that turned out to be impossible, and re-deriving it changed the fix.

#1411AWS::EC2::NatGateway.MaxDrainDurationSeconds

Verified against the installed @aws-sdk/client-ec2:

  • CreateNatGatewayRequest (models/models_1.d.ts) has no MaxDrainDurationSeconds member.
  • EC2 ships no ModifyNatGateway* operation at all (ls dist-types/commands/ | grep -i natgateway).
  • The only two SDK inputs carrying the field are DisassociateNatGatewayAddressRequest (models_5) and UnassignPrivateNatGatewayAddressRequest (models_7) — it is a per-call drain timeout for RELEASING secondary addresses, which is why the CFn registry schema lists it under writeOnlyProperties.
  • cdkd's updateNatGateway rejects every NatGateway property change, so it never issues those two calls and has nowhere to deliver the value.

Modelling it as a replacement trigger — the issue's fallback — was rejected too: the registry schema does not list it under createOnlyProperties, so recreating a gateway on a drain-timeout change would diverge from CloudFormation and needlessly break the data plane.

#1412AWS::Logs::LogGroup.ResourcePolicyDocument

Takes option 2 of the issue, for the reason the issue itself gives: the value maps to the separate AWS::Logs::ResourcePolicy type, whose logs:PutResourcePolicy is account-scoped, not per-log-group. Owning it from a log group's lifecycle would mean inventing an ownership answer cdkd has no basis for — which policy name to claim, what to do on delete when the account-wide policy may be shared, how to resolve two log groups declaring conflicting documents. Managing the sibling resource remains the real feature.

Both are now unhandledByDesign, which is better than either alternative

Correcting a premise carried in both issues: neither provider sets disableCcApiFallback, so declaring the drop does NOT hard-reject these templates via the #614 viability guard. It fires the #614 auto-route — the resource is provisioned through Cloud Control API, where AWS's own resource handler applies the value.

That is strictly better than both the silent drop and a reject.

Note on existing stacks: routing stickiness is cc-api -> cc-api only (provider-registry.ts). A resource already recorded as provisionedBy: 'sdk' whose template sets one of these properties WILL re-evaluate and flip to Cloud Control on its next UPDATE — deploy-engine.ts does this deliberately. That is the intended outcome (the value starts being applied instead of dropped) and carries no physical-ID churn, since Cloud Control updates in place.

Both HANDLED_WIRING_ALLOW_LIST entries are removed, so the critic now verifies the fix instead of excusing it: 0 gaps, 2 allow-listed (down from 4).

Test plan

  • 10 new unit tests across two files. Each asserts the property is absent from handledProperties, present in unhandledByDesign with a rationale, reported by findSilentDropProperties, and that ProviderRegistry.getProviderFor routes to CC for a template setting it and to SDK for one that does not. Plus a guard that CreateNatGateway never carries the field, and a fence that neither retired property may return to handledProperties.

  • Revert-proofed: re-adding both properties to handledProperties fails 6 of 10 and makes audit:handled-property-wiring:check fail naming both.

  • tests/unit/scripts/gen-handled-property-wiring.test.ts re-pointed its real-code stale-entry probes onto a still-live allow-list entry (IAMAccessKeyProvider#Serial), since the two they were bound to are now gone.

  • Real AWS integ (/run-integ vpc-nat-gateway, us-east-1, 265s, PASS). The fixture gains a SECOND, L1-only private NAT gateway setting the property, so the L2 gateway stays on the SDK path and the two together assert heterogeneous routing in one stack. Live output confirms the route is chosen for the stated reason:

    DrainNatGateway (AWS::EC2::NatGateway): routing via Cloud Control API
    (cdkd's SDK Provider does not yet wire MaxDrainDurationSeconds — CC API
    will forward the full property map.)
    

    Assertions: template/state carry MaxDrainDurationSeconds=120; drain gateway provisionedBy == 'cc-api'; plain gateway provisionedBy == 'sdk'; both live on AWS with the drain one available and ConnectivityType=private (proof CC forwarded the full property map); both gone after destroy; state gone. Destroy: 22 deleted, 0 errors, 0 orphans.

    The value is writeOnlyProperties and no EC2 API returns it, so a read-back-and-compare assertion is structurally impossible — the fixture asserts the routing consequence plus a vacuity guard that greps the literal out of the stack file so the two cannot drift.

  • Full suite: 524 files / 8949 tests pass.

Deliberately not done

  • A live integ for AWS::Logs::LogGroup: ResourcePolicyDocument declared handled but never wired #1412's routing. Setting ResourcePolicyDocument makes AWS create an account-wide AWS::Logs::ResourcePolicy, which would collide with other fixtures sharing the integ account and is awkward to clean up. The mechanism it would exercise is the same Add Cloud Control API greenfield fallback for unhandled top-level properties #614 auto-route the NAT-gateway fixture now proves end-to-end against real AWS, so the marginal value does not justify the account-wide side effect. Recorded in the unhandledByDesign JSDoc.
  • NAT-gateway secondary-address updates — the only way to genuinely deliver MaxDrainDurationSeconds from the SDK path. That means lifting updateNatGateway's blanket rejection and adding associate/disassociate/assign/unassign handling: a substantial feature, out of scope for a wiring-gap fix, and the CC route already delivers the property correctly today. Recorded in the code comment.
  • Managing the sibling AWS::Logs::ResourcePolicy (option 1 of AWS::Logs::LogGroup: ResourcePolicyDocument declared handled but never wired #1412). The issue calls this "the real feature"; option 2 was the maintainer's stated preference. Recorded in the unhandledByDesign JSDoc.

Closes #1411
Closes #1412

go-to-k added 4 commits August 9, 2026 23:36
…silent drops so they auto-route via Cloud Control

Both properties were declared in handledProperties while nothing read them, so a template setting either deployed successfully with the value discarded. Both issues proposed wiring them into create; for MaxDrainDurationSeconds that is impossible. It is not a CreateNatGatewayRequest member and EC2 ships no ModifyNatGateway operation at all, the only two SDK inputs carrying it being DisassociateNatGatewayAddress and UnassignPrivateNatGatewayAddress, which is why the registry schema lists it under writeOnlyProperties. cdkd rejects every NatGateway property change, so it has nowhere to deliver the value.

Modelling it as a replacement trigger was rejected too: the registry schema does not list it under createOnlyProperties, so recreating a gateway on a drain-timeout change would diverge from CloudFormation and needlessly break the data plane.

ResourcePolicyDocument takes option 2 of its issue for the reason the issue gives: logs:PutResourcePolicy is account-scoped, not per-log-group, so owning it from a log group lifecycle would require inventing an ownership answer. Managing the sibling AWS::Logs::ResourcePolicy resource remains the real feature.

Neither provider sets disableCcApiFallback, so declaring the drop does not hard-reject these templates. It fires the auto-route and provisions through Cloud Control, where AWS's own handler applies the value. Routing is sticky from state, so already-deployed resources do not flip layer mid-life.

Closes #1411

Closes #1412
…ng-stickiness claim

Review findings on the first cut.

The cleanup trap wiped the state file unconditionally. A classic NAT stuck in deleting makes the VPC delete fail with DependencyViolation, and dropping the state anyway would orphan a per-hour-billed NAT and VPC with nothing left to retry from. The removal is now gated on the state destroy exit status, and the skip is announced.

Two smaller fixture defects: the missing-constant diagnostic was unreachable because set -e aborts at the grep assignment, and the post-destroy assertion accepted deleting as success even though both delete paths wait for settle, so a deleting readback means the wait did not hold.

The changelog claimed routing stickiness prevents an existing SDK-provisioned resource from flipping layer. That is wrong: stickiness is cc-api to cc-api only, so such a resource does re-evaluate and move to Cloud Control on its next update, which is the intended outcome.
@go-to-k
go-to-k force-pushed the fix/1411-1412-handled-wiring-gaps branch from 223c0a5 to 90d5fa7 Compare August 9, 2026 14:37
@go-to-k
go-to-k merged commit a39e201 into main Aug 9, 2026
5 checks passed
@go-to-k
go-to-k deleted the fix/1411-1412-handled-wiring-gaps branch August 9, 2026 14:42
github-actions Bot pushed a commit that referenced this pull request Aug 9, 2026
## [0.278.16](v0.278.15...v0.278.16) (2026-08-09)

### Bug Fixes

* **providers:** declare the two NatGateway / LogGroup wiring gaps as silent drops so they auto-route via Cloud Control ([#1429](#1429)) ([a39e201](a39e201))
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

🎉 This PR is included in version 0.278.16 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

1 participant