feat(cloudformation): CDK Provider-framework custom resources and changeset/SSM param fidelity - #2688
Merged
pgermosen merged 22 commits intoAug 29, 2026
Conversation
CDK's deploy loop reads all three: DescribeChangeSet now returns computed Add/Modify/Remove resource changes instead of an empty list (an empty list makes CDK skip ExecuteChangeSet on updates), DescribeStacks echoes the stack's merged parameters, and AWS::SSM::Parameter resources expose Value/Type/Name attributes so Fn::GetAtt resolves the bootstrap version (cherry picked from commit 80ac800)
AWS::SSM::Parameter::Value<String> template parameters resolve through SSM for the stack's account and region; missing parameters fail with the real ValidationError message. Landing Zone Accelerator's security and logging stacks depend on this typed-parameter resolution. The IAM password-policy and S3 replication-config changes from the same source commit ship separately in the iam and s3 branches. (cherry picked from commit 10dbabb)
The CDK Provider framework defers its ResponseURL callback to a Step Functions waiter that polls framework.isComplete on a Retry cadence until the resource is ready. The custom-resource provisioner only waited the synchronous single-lambda budget for a ResponseURL PUT, so the waiter-driven PUT never landed. A framework.onEvent ServiceToken is now awaited on a bounded async budget so that PUT is captured; onEvent-only and single-lambda resources are unchanged. The engine-side half of the original commit -- a Task-only Retry implementation for the ASL executor (ErrorEquals/IntervalSeconds/ MaxAttempts/BackoffRate/MaxDelaySeconds) -- is dropped here: upstream floci-io#2455 ("feat(sfn): support Retry policies in Task, Parallel, and Map states") landed a broader Retry implementation independently (Task, Parallel, and Map states; JitterStrategy; $$.State.RetryCount; Retry-before-Catch ordering), a strict superset of what this commit needed. The full stepfunctions suite (411 tests) is green against it on this branch, and CustomResourceProviderFrameworkTest (a hermetic test of the provisioner's own detect-and-wait logic, no Step Functions involved) is unaffected by the drop. (cherry picked from commit 5b3502d)
Documents the CDK Provider-framework custom-resource support and folds its cross-reference into upstream's existing "Retry policies" section in step-functions.md rather than duplicating it with a separate "State Retry" section (upstream floci-io#2455 already documents the Retry engine itself).
…otal time The async custom-resource wait was a fixed 3-minute total budget. That is a property of the emulator, but the time a CDK provider-framework resource needs is a property of the work: Custom::CreateOrganizationAccounts creates exactly one account per 15s waiter poll, so a 3-account org finishes in ~45s while a 15-account org needs ~195s. Past the budget the Lambda is guillotined mid- success -- the accounts all exist, but the resource reports CREATE_FAILED, the stack rolls back, and CDK's readback then surfaces the misleading "NoStack: CloudFormationStack object does not hold a stack". Measure idleness instead. The provisioner already embeds its callback token in the ResponseURL it puts on every event, and the framework echoes that event into each framework.isComplete poll, so a poll arriving at LambdaService is proof of progress for a specific pending resource. Each poll resets the budget, which makes the wait proportional to the work without CloudFormation needing to know what the work is -- and a genuinely hung resource still fails after 3 idle minutes exactly as before. CustomResourceLiveness lives in core.common so the Lambda service can report progress without depending on CloudFormation; the dependency already runs the other way. (cherry picked from commit 14087c992ebb75d109999153a9e8af7e3c2d79f4) (cherry picked from commit 51349e4e017ed08d90e6e71212414e2f306c826b)
…that exists The changeset-diff commit re-derived onto main carried a call to a three-arg getStackOrThrow(name, region, accountId) overload that only existed on the account-scoping commit deliberately skipped as superseded by upstream. Upstream's resolveStack already scopes to the caller's account, so the two-arg form is equivalent and compiles.
…its budget A synchronous custom-resource handler PUTs to the ResponseURL from inside the invoke the provisioner is blocked on, so its response can already be delivered by the time await() is entered. Two independent flaws then threw it away. The idle clock started at register(), before the invoke. A provider-framework onEvent-only handler that took 75s to return -- two nested cold container starts -- had spent its 10s CR_RESPONSE_TIMEOUT six times over before await was reached, so the very first deadline check was already negative. Idleness of the wait cannot include time the handler spent working; the clock now starts when the caller starts waiting. The deadline guard also threw without ever consulting the future, so a response that had completed it was discarded as a timeout. Observed on AccessAnalyzerServiceLinkedRoleCreateServiceLinkedRoleResource: the handler logged its response at 15:52:11.768 and the resource was failed 24ms later. An arrived response outranks the clock. Both are pinned by red-first tests. The existing idle-timeout guarantees are unchanged: nothing arriving still times out, and a resource that stops reporting liveness still dies on schedule from its last touch. (cherry picked from commit 7dc8bbca7d47b2a029b5656a89421a556b8296be) (cherry picked from commit 77b71b31a634ae27d7460a078b52dac250d67dc7)
…he provisioner The CDK Provider-framework `framework.onEvent` detection and the longer idle budget it earns lived in CloudFormationResourceProvisioner, adding ~55 lines to a class that is being dismantled rather than grown. Move the concern to ProviderFrameworkDetector: it owns the two waiter marker env vars, the best-effort environment probe, the 3-minute async budget and the test seam that shortens it. CustomResourceResponseStore — which already owns the idle-timeout concept via touch/await — gains an await overload that resolves the budget for a ServiceToken and delegates to the existing one, so the provisioner only passes the token through and appends the store's timeout detail to its error. Behaviour is unchanged. (cherry picked from commit 5f93439f409a32012a01b996c5edd0c29d23b260)
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationService.java | Adds normalized change-set computation, SSM parameter resolution, condition-aware previews, and rollback-safe parameter persistence without a remaining blocking issue. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationQueryHandler.java | Serializes computed change-set entries and stack parameters while resolving UsePreviousValue for update and StackSet requests. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CustomResourceResponseStore.java | Replaces fixed callback waiting with an idle timeout that is safely extended by token-specific liveness signals. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/ProviderFrameworkDetector.java | Detects asynchronous CDK Provider-framework handlers from their Lambda environment and selects the appropriate response timeout. |
| src/main/java/io/github/hectorvent/floci/core/common/CustomResourceLiveness.java | Introduces lightweight callback-token extraction and a shared liveness interface for Lambda and Step Functions paths. |
| src/main/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutor.java | Reports custom-resource liveness before both direct and optimized Lambda Task invocations. |
| src/main/java/io/github/hectorvent/floci/services/lambda/LambdaService.java | Reports callback-token activity on the Lambda service invocation path used by custom-resource handlers. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/model/Stack.java | Persists resolved parameter values so previews can detect live SSM drift and rollback can restore successful state. |
Sequence Diagram
sequenceDiagram
participant CFN as CloudFormation
participant Store as Response Store
participant OnEvent as framework.onEvent
participant SFN as Waiter State Machine
participant IsComplete as framework.isComplete
CFN->>Store: Register callback token
CFN->>OnEvent: Invoke with ResponseURL
OnEvent->>SFN: Start waiter workflow
OnEvent-->>CFN: Return without callback
CFN->>Store: Await response using idle budget
loop Until complete
SFN->>IsComplete: Poll with original event
IsComplete->>Store: Report token liveness
end
SFN->>Store: PUT final callback response
Store-->>CFN: Complete pending resource
Reviews (11): Last reviewed commit: "fix(stepfunctions): pass EventBridgeHand..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CloudFormation fidelity features needed by CDK-style workflows, spanning change set previews, SSM-typed parameter resolution, and CDK Provider Framework custom-resource completion (async waiter-driven callbacks).
Changes:
- Compute and return real
DescribeChangeSetresource diffs (Add/Modify/Remove) instead of an empty<Changes/>. - Resolve
AWS::SSM::Parameter::Value<String>stack parameters from live SSM Parameter Store values during template execution, and exposeAWS::SSM::Parameterattributes (Name/Type/Value) forFn::GetAtt. - Support CDK Provider Framework two-phase custom resources by extending the custom-resource callback wait logic to an idle-based async budget and introducing Provider Framework detection.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/io/github/hectorvent/floci/services/lambda/LambdaArnInvocationAccountTest.java | Updates LambdaService constructor usage for new dependency. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/ProviderFrameworkDetectorTest.java | Unit tests for Provider Framework detection and timeout selection. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CustomResourceResponseStoreTest.java | Unit tests for idle-based custom-resource callback waiting. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CustomResourceProvisionerTest.java | Updates response store construction to include ProviderFrameworkDetector. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CustomResourceProviderFrameworkTest.java | Hermetic tests for Provider Framework async completion behavior. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationSsmParameterIntegrationTest.java | Integration tests for SSM-typed stack parameter resolution and validation failure. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationServiceRollbackTest.java | Updates CloudFormationService constructor usage for new SsmService dependency. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationCdkDeployLoopIntegrationTest.java | End-to-end tests for CDK deploy loop dependencies (change set diffs, SSM GetAtt, parameter echo). |
| src/test/java/io/github/hectorvent/floci/core/common/CustomResourceLivenessTest.java | Tests token extraction from observed Provider Framework payload shapes. |
| src/main/java/io/github/hectorvent/floci/services/lambda/LambdaService.java | Adds custom-resource liveness reporting hook during Lambda invokes. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/ProviderFrameworkDetector.java | New detector for identifying Provider Framework framework.onEvent and selecting async idle budget. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CustomResourceResponseStore.java | Reworks callback waiting into idle-timeout logic and exposes touch() liveness sink. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationService.java | Adds change set diff computation and SSM-typed parameter substitution during execution. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationResourceProvisioner.java | Exposes SSM Parameter resource attributes and uses new store await(timeout, token, region) API. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationQueryHandler.java | Emits populated <Changes> in DescribeChangeSet and includes stack parameters in DescribeStacks. |
| src/main/java/io/github/hectorvent/floci/core/common/CustomResourceLiveness.java | New interface + token extraction helper for liveness signaling. |
| docs/services/step-functions.md | Documents Retry’s role in Provider Framework convergence. |
| docs/services/cloudformation.md | Updates supported behavior docs for parameters, change set diffs, and Provider Framework custom resources. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A CDK provider-framework waiter's framework.isComplete poll is itself a Lambda Task the state machine invokes, so it goes through AslExecutor.invokeResource -> LambdaExecutorService directly, bypassing LambdaService.invoke() and the liveness hook it carries. A long-but- progressing custom resource's idle budget in CustomResourceResponseStore ticked down as if no polls were arriving, even while they kept landing. AslExecutor now reports liveness the same way LambdaService does: extract the callback token from the invoke payload and touch() it before invoking.
CustomResourceLiveness.tokenIn() ran on every Lambda invoke once liveness reporting is wired in, decoding the whole invoke payload as a UTF-8 String even though the overwhelming majority of invokes carry no callback URL at all. Scan the raw bytes for the ASCII "/cfn-response/" marker instead, and only allocate a String for the small token substring when one is found.
Open
5 tasks
computeChangeSetChanges never read cs.getParameters(), so an update that only changed parameter values reported an empty diff and CDK skipped execution, leaving the old parameter-derived config deployed. It also diffed a SAM change set's raw template against the stack's expanded deployed template, reporting spurious Modify/Remove on every SAM-generated resource for a no-op update, and hardcoded Replacement=False even when a resource's Type changed. Now the diff expands a SAM change set's template before comparing, marks a resource Modify when it references a changed parameter, and reports Replacement=True when a resource's Type changes.
…lback executeTemplate overwrote the stack's live parameter map with an update's attempted values before resolving SSM-typed parameters, which can fail. A failed resolution rolled the stack back, but the update snapshot never captured parameters, so DescribeStacks reported the failed update's attempted values instead of the last successfully deployed ones. StackUpdateSnapshot now captures the stack's parameters alongside its template body, and rollbackFailedUpdate restores them together.
…urce poll loop Each slice of the custom-resource poll loop below the outer idle budget hit an intentionally-tolerated TimeoutException that was swallowed with only a comment, against the repo's convention of logging deliberately caught exceptions.
extractParameters never read UsePreviousValue, so submitting it without a ParameterValue stored an empty string for that key. The new parameter-diff logic in computeChangeSetChanges (9fb02c6) then treated that empty string as different from the deployed value, reporting a spurious Modify for every resource referencing an otherwise-unchanged parameter. extractParameters now takes the stack's current parameters and resolves UsePreviousValue against them; CloudFormationService.currentParameters looks those up (empty map if the stack doesn't exist yet, e.g. a CREATE change set).
…et parameters UpdateStackSet extracted parameters with the previous-value-unaware extractParameters() overload, so a member with UsePreviousValue=true and no ParameterValue stored an empty string and clobbered the StackSet's deployed value instead of preserving it.
… in changeset diff computeChangeSetChanges compared only explicitly submitted parameters against the deployed stack, so an update that omits a parameter (falling back to its template Default) was never flagged as a change even when that default differs from the deployed value. ExecuteChangeSet applies the default when it actually runs, so the preview under-reported what would change. Resolve defaults for the change set's parameters the same way execution does before diffing.
…ions in changeset diff computeChangeSetChanges compared raw submitted parameter values against the deployed stack, so it missed two cases ExecuteChangeSet actually handles differently: an omitted parameter with no template Default disappears entirely from the resolved map (previously undetected since the diff only walked the new parameter set), and AWS::SSM::Parameter::Value<String> parameters are resolved against the live SSM store at execution time but compared as raw parameter names in the preview, masking a changed SSM value behind an unchanged parameter name. Stack now tracks resolvedParameters (the SSM-resolved map executeTemplate last applied, restored on rollback like parameters already was) so the diff can compare resolved values on both sides. The diff falls back to unresolved values if SSM resolution fails, since a preview must not fail harder than the operation it previews - ExecuteChangeSet still raises the real ValidationError when it resolves for real.
…ion-cdk-provider-framework
…or-termination test Upstream's issue floci-io#2666 regression test predates this branch's addition of CustomResourceLiveness to AslExecutor's constructor. The merge from upstream/main brought the file in as new content with no line overlap, so it auto-merged cleanly but left the constructor call one argument short.
pgermosen
reviewed
Aug 29, 2026
pgermosen
left a comment
pgermosen
left a comment
Contributor
There was a problem hiding this comment.
Very thorough work overall, and I checked the notable claim about dropping this branch's own Retry implementation in favor of an already-merged upstream one rather than taking it on faith — the cited PR exists, is merged, and matches the description exactly.
Two things are still open with no response yet, and I verified both directly in the current code rather than just relaying the earlier findings. Condition-gated resources are genuinely invisible to the changeset preview: the diff logic only inspects a resource's own JSON definition for a literal Ref/Fn::Sub reference to a changed parameter, and never looks at the template's Conditions block at all. A resource gated by a parameter-driven condition would have its Add or Remove computed correctly when the change set actually executes, but nothing in the preview logic would ever flag it as changing.
Second, a rollback that itself partially fails leaves the failed update's parameters in place. The block that restores the template body and both parameter maps back to the pre-update snapshot only runs when every resource rolled back successfully — so a stack that lands in UPDATE_ROLLBACK_FAILED because even one resource couldn't roll back keeps reporting the failed update's attempted parameter values afterward, not the last successfully deployed ones.
Both read as real, current gaps rather than edge cases that don't matter in practice — worth addressing or explicitly scoping out the same way the other findings on this PR already were before it merges.
…store params on rollback computeChangeSetChanges never evaluated template Conditions, so a resource whose Condition depends on a changed parameter but whose own definition text is unchanged was invisible to the diff even though ExecuteChangeSet would Add or Remove it once the condition flips (see deleteRemovedOrConditionFalseResources). The diff now resolves Conditions against the new resolved parameters and compares against whether the resource is actually present in stack.getResources() - the same ground truth execution already uses to decide whether a resource was created - falling back to the existing text-based Modify detection when the condition doesn't change. rollbackFailedUpdate only restored the last-successful parameter maps when resource rollback itself also succeeded, so a stack that lands in UPDATE_ROLLBACK_FAILED (resource rollback failed too) kept serving the failed update's attempted parameter values via DescribeStacks and later change-set previews. Parameters are independent of resource-rollback outcome and are now restored unconditionally; templateBody restoration stays gated on rollback success since a partially-rolled-back resource set doesn't correspond to either template.
pgermosen
approved these changes
Aug 29, 2026
pgermosen
left a comment
pgermosen
left a comment
Contributor
There was a problem hiding this comment.
Both items from the earlier round are closed, and I verified both directly in the current code rather than trusting the resolved status. The condition-gated resources fix uses a well-reasoned decision tree: it evaluates the new template's conditions and checks whether each resource was actually present in the stack before, using the stack's real resource registry as ground truth rather than re-evaluating old conditions separately — the same ground truth execution already uses when it deletes condition-excluded resources. It correctly distinguishes a brand-new resource, a condition flipping either direction, and the normal in-place modify path, including the case where a formerly-inactive resource becomes active for the first time (correctly reported as an Add, not a Modify, since it was never actually deployed under the old template).
The rollback-parameter fix makes parameter restoration unconditional while correctly keeping template-body restoration gated on full resource-rollback success. That's a deliberate, correct distinction rather than a blanket fix — parameters are plain data values independent of what actually got provisioned, so they can always safely revert to the last-known-good values, but if resource rollback itself failed, the actual deployed resources are in a genuinely mixed state, and restoring the template body to fully match "old" in that situation would misrepresent what's really running.
…ion-cdk-provider-framework # Conflicts: # src/test/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutorArrayParamsTest.java # src/test/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutorIntrinsicContextTest.java
7 tasks
…rController mocks in AslExecutor test Merging upstream/main (55acdec) added these three constructor parameters to AslExecutor without updating this test's call site, breaking test-compile.
This was referenced Aug 29, 2026
This was referenced Aug 29, 2026
This was referenced Aug 29, 2026
This was referenced Aug 29, 2026
7 tasks
hectorvent
pushed a commit
that referenced
this pull request
Aug 29, 2026
…st endpoint) (#2726) * fix(stepfunctions): repair test compilation after the AslExecutor constructor gained CustomResourceLiveness #2703 added AslExecutorTaskHistoryEventsTest and #2688 added a CustomResourceLiveness parameter to the AslExecutor constructor; each was green alone, but merged together main no longer compiles the test sources. Pass a mock for the new parameter. * chore: retrigger CI * fix(redshift): resolve the compat test endpoint from TestFixtures instead of hardcoding localhost The sdk-test-java RedshiftTest built its own client against a hardcoded http://localhost:4566 and dialed the DescribeClusters endpoint address directly, but the CI harness serves Floci at FLOCI_ENDPOINT (a non-localhost host), so both API calls failed with connection refused on every run. Use the suite's TestFixtures.redshiftClient() (added alongside the test but unused) and proxyHost() for the JDBC URL, the same pattern RdsJdbcCompatTest uses. * fix(redshift): drop the compat test's JDBC hop, the endpoint isn't harness-reachable DescribeClusters returns the backing container's own host and port (Redshift has no RDS-style auth proxy), so the JDBC connection can't succeed when Floci runs in a container: confirmed on CI, where the API assertions now pass and only the JDBC hop fails with connection refused. Keep the API coverage and leave a note; a follow-up proxy would let the connection check return.
hectorvent
added a commit
that referenced
this pull request
Aug 29, 2026
Groundwork for moving the remaining resource types out of CloudFormationResourceProvisioner into per-service provisioners. No behaviour change: every type is provisioned and deleted as before. Make a dropped type fail loudly. The provision switch's default arm stubs an unknown type with a fake ARN and reports CREATE_COMPLETE, so a type that fell out of both the switch and the registry would pass every test while provisioning nothing. Adds registeredTypes() on the registry, a LEGACY_SWITCH_TYPES constant, a throwing default arm for a declared type with no arm, and a checked-in inventory of all 112 types and their owner. The inventory test runs under QuarkusTest so it compares against the CDI-resolved registry, catching a provisioner written without @ApplicationScoped. Fix delete precedence. The registry lookup in delete(StackResource, region) sat after a Custom:: prefix branch and nine attribute-aware special cases, so migrating one of those types would leave its provisioner's delete unreachable. The lookup now runs first, the nine branches sit behind a DELETE_NEEDS_STACK_RESOURCE set that gates them, and a duplicated AWS::Events::Rule block is removed. Replace 13 positional constructions with one fixture. Every test built the provisioner with 30-plus positional nulls, so each constructor change edited all of them. CfnProvisionerFixture gives one named setter per argument; removing the already-dead sqsService argument now touches only that file. Add the helpers the slices will use: ProvisionContext.isUpdate() and resolveTags, CfnDeletes.safeDelete with explicit tolerated error codes, and CfnDynamicReferences, which extracts 189 lines of dynamic-reference handling so a future RDS provisioner injects one collaborator instead of two unrelated services. resolveTags migrates no call sites: the seven existing copies disagree on real behaviour and each adopts it in its own slice. Also fixes AslExecutorTaskHistoryEventsTest, which does not compile on main: #2703 added it and #2688 then added a CustomResourceLiveness parameter to AslExecutor without updating the call.
FrameWorkDesign1
added a commit
to FrameWorkDesign1/floci_pratik
that referenced
this pull request
Aug 30, 2026
…st endpoint) (floci-io#2726) * fix(stepfunctions): repair test compilation after the AslExecutor constructor gained CustomResourceLiveness floci-io#2703 added AslExecutorTaskHistoryEventsTest and floci-io#2688 added a CustomResourceLiveness parameter to the AslExecutor constructor; each was green alone, but merged together main no longer compiles the test sources. Pass a mock for the new parameter. * chore: retrigger CI * fix(redshift): resolve the compat test endpoint from TestFixtures instead of hardcoding localhost The sdk-test-java RedshiftTest built its own client against a hardcoded http://localhost:4566 and dialed the DescribeClusters endpoint address directly, but the CI harness serves Floci at FLOCI_ENDPOINT (a non-localhost host), so both API calls failed with connection refused on every run. Use the suite's TestFixtures.redshiftClient() (added alongside the test but unused) and proxyHost() for the JDBC URL, the same pattern RdsJdbcCompatTest uses. * fix(redshift): drop the compat test's JDBC hop, the endpoint isn't harness-reachable DescribeClusters returns the backing container's own host and port (Redshift has no RDS-style auth proxy), so the JDBC connection can't succeed when Floci runs in a container: confirmed on CI, where the API assertions now pass and only the JDBC hop fails with connection refused. Keep the API coverage and leave a note; a follow-up proxy would let the connection check return. ## Summary <!-- What does this PR do? Link any related issues with "Closes #N" --> ## Type of change - [ ] Bug fix (`fix:`) - [ ] New feature (`feat:`) - [ ] Breaking change (`feat!:` or `fix!:`) - [ ] Docs / chore ## AWS Compatibility <!-- For new actions: which SDK version and AWS CLI version were used to verify the wire protocol? --> <!-- For bug fixes: what was the incorrect behavior? --> ## Checklist - [ ] `./mvnw test` passes locally - [ ] New or updated integration test added - [ ] Commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) <!-- First PR here? Your CI checks wait for a maintainer to approve them before they run — that's GitHub's gate on first-time contributors, not a problem with your PR. --> <!-- Questions, or want feedback on an approach before going further? Join us on Slack: https://join.slack.com/t/floci/shared_invite/zt-3tjn02s3q-A00kEjJ1cZxsg_imTfy6Cw -->
Closed
7 tasks
pgermosen
pushed a commit
that referenced
this pull request
Aug 30, 2026
Groundwork for moving the remaining resource types out of CloudFormationResourceProvisioner into per-service provisioners. No behaviour change: every type is provisioned and deleted as before. Make a dropped type fail loudly. The provision switch's default arm stubs an unknown type with a fake ARN and reports CREATE_COMPLETE, so a type that fell out of both the switch and the registry would pass every test while provisioning nothing. Adds registeredTypes() on the registry, a LEGACY_SWITCH_TYPES constant, a throwing default arm for a declared type with no arm, and a checked-in inventory of all 112 types and their owner. The inventory test runs under QuarkusTest so it compares against the CDI-resolved registry, catching a provisioner written without @ApplicationScoped. Fix delete precedence. The registry lookup in delete(StackResource, region) sat after a Custom:: prefix branch and nine attribute-aware special cases, so migrating one of those types would leave its provisioner's delete unreachable. The lookup now runs first, the nine branches sit behind a DELETE_NEEDS_STACK_RESOURCE set that gates them, and a duplicated AWS::Events::Rule block is removed. Replace 13 positional constructions with one fixture. Every test built the provisioner with 30-plus positional nulls, so each constructor change edited all of them. CfnProvisionerFixture gives one named setter per argument; removing the already-dead sqsService argument now touches only that file. Add the helpers the slices will use: ProvisionContext.isUpdate() and resolveTags, CfnDeletes.safeDelete with explicit tolerated error codes, and CfnDynamicReferences, which extracts 189 lines of dynamic-reference handling so a future RDS provisioner injects one collaborator instead of two unrelated services. resolveTags migrates no call sites: the seven existing copies disagree on real behaviour and each adopts it in its own slice. Also fixes AslExecutorTaskHistoryEventsTest, which does not compile on main: #2703 added it and #2688 then added a CustomResourceLiveness parameter to AslExecutor without updating the call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Split B of the
feature/cloudformation-on-mainwork (sibling to #2685, which covers CodeBuild/CodePipeline CFN provisioning). This split covers three related gaps in CloudFormation's stack/changeset/custom-resource surface:CreateChangeSet/DescribeChangeSetcompute a real diff of the proposed template against the current stack instead of a stub, andSSMparameter types (AWS::SSM::Parameter::Value<...>) resolve against the live SSM parameter store rather than passing the parameter name through unresolved.framework.onEventLambda returns without PUTting toResponseURLand instead defers completion to aframework.isCompletewaiter driven by a Step Functions state machine. The custom-resource provisioner previously only waited the synchronous single-Lambda budget for that PUT, so waiter-driven (async, longer-running) custom resources always timed out. A bounded async budget is now awaited for aframework.onEventServiceToken, keyed by idleness (time since the last liveness signal) rather than total elapsed time, so a resource that's still actively being worked isn't killed just because the wall-clock budget expired.ProviderFrameworkDetectorclass, separating "is this a Provider-framework custom resource" from the provisioning/waiting logic itself.Provenance: same as #2685 — this is a diverged-fork branch re-derived onto current
upstream/main. A prior reassessment (2026-08-28) confirmed the specific regions this branch touches inCloudFormationService.java/CustomResourceResponseStore.javaare still byte-identical to this branch's merge-base, and upstream's own code comment still states the async Provider framework "is not emulated," so this fills a real gap rather than duplicating existing coverage. (Adjacent Step Functions surface did move upstream in that window — see the Retry note below — the reassessment's "untouched" finding was scoped to the CloudFormation files specifically, not the Step Functions files this branch's original commit also reached into.)A note on what changed during rebase: SFN Retry is upstream's, not ours
The original commit ("drive CDK Provider framework custom resources") bundled two things: a Task-only Step Functions
Retryimplementation (needed because the waiter'sframework.isCompletepoll relies on ASLRetryto keep polling until done) and the provisioner-side async budget described above. While rebasing onto currentupstream/main, we found upstream had independently landedRetrysupport via #2455 (feat(sfn): support Retry policies in Task, Parallel, and Map states) — broader than what our commit needed: Task/Parallel/Map coverage (ours was Task-only),JitterStrategy,$$.State.RetryCounttracking, and explicit Retry-before-Catch ordering, none of which our version had. A strict superset, so we dropped ourAslExecutor.javaRetry implementation and its test in favor of upstream's.Verified the drop: the full Step Functions suite (411 tests) is green on this branch against upstream's Retry implementation, and
CustomResourceProviderFrameworkTest— which is hermetic to the provisioner's own detect-and-wait logic and never exercisesAslExecutor— is unaffected either way. The commit message and the CDK-provisioning docs were reworded to reflect this — the docs now cross-reference upstream's "Retry policies" section instep-functions.mdinstead of duplicating it with a second "State Retry" section.Wire-fidelity
Ran the
floci-reviewextractor against the branch's changed CloudFormation source (services/cloudformation/) against the current botocore CloudFormation model. All 59 emitted packets (constrained-member reads: enums, patterns, length bounds) trace toCloudFormationQueryHandler.java(wire request-parsing, untouched by this branch) or to pre-existingTags/RoleARNreads in files this branch didn't modify. None trace to the files this branch actually changed (CloudFormationResourceProvisioner.java's new async/liveness logic,CloudFormationService.java's changeset/SSM logic,CustomResourceResponseStore.java,ProviderFrameworkDetector.java) — this surface is orchestration/timing behavior, not wire-shape parsing, so the extractor's constrained-member methodology doesn't have anything new to flag here. No findings to fix.Deliberate scope notes
ServiceTokenpattern (onEvent+isCompletepolled via a waiter state machine). It does not add support for any custom-resource pattern beyond that.CloudFormationServiceRollbackTest,LambdaArnInvocationAccountTest) and one (CustomResourceProviderFrameworkTest) needed constructor-arity padding during the rebase —CloudFormationServiceandLambdaServicegrew unrelated constructor parameters (SsmService,Ec2Service) from other upstream/branch work between when these commits were originally written and now. Padded withnull/mocks consistent with the existing pattern in each test; folded into the commit that introduced the arity mismatch rather than left as a separate fixup.LambdaService.javaand addscore/common/CustomResourceLiveness.java— the async budget's idleness signal has to be recorded at the point custom-resource Lambdas are actually invoked, which is Lambda-side, not CloudFormation-side. Checked for collision against the open Lambda PR (fix(lambda): resolve launched-container placeholder creds to the owning account #2657): zero file overlap.Type of change
AWS compatibility
Changeset-diff and SSM stack-parameter behavior, and the CDK Provider-framework's
onEvent/isComplete/ResponseURLasync pattern, follow documented AWS/CDK behavior for these features.Checklist
clean test-compile)make docs-checkpasses