feat(cloudformation): support UpdateTerminationProtection#1692
Conversation
CDK bootstrap calls UpdateTerminationProtection on the CDKToolkit stack; Floci returned UnknownAction. Add it: a boolean enableTerminationProtection on Stack, a CloudFormationService.updateTerminationProtection(stackName, enabled, region) that sets and persists it, a handler returning the UpdateTerminationProtectionResult/StackId envelope, and the flag echoed as <EnableTerminationProtection> in DescribeStacks. Register the action in AwsQueryController's CloudFormation action set so the Query-protocol POST routes to CloudFormation instead of falling through. Adds an integration test toggling the flag and asserting it round-trips through DescribeStacks.
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationService.java | Adds updateTerminationProtection which stores the flag correctly, but deleteStack never checks the flag, so protection is stored but never enforced. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationQueryHandler.java | Adds UpdateTerminationProtection handler and echoes EnableTerminationProtection in DescribeStacks XML; handler is missing the catch (AwsException e) try-wrap (already flagged in prior thread), and the happy-path XML structure is correct. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/model/Stack.java | Adds enableTerminationProtection field with correct default false, getter/setter. Clean change. |
| src/main/java/io/github/hectorvent/floci/core/common/AwsQueryController.java | Adds UpdateTerminationProtection to the CloudFormation action set so Query-protocol routing reaches the correct handler. Correct and minimal change. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationIntegrationTest.java | Adds updateTerminationProtection_togglesFlagAndReflectsInDescribeStacks covering the happy path end-to-end. No test for enforced deletion rejection when protection is enabled. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant CDK as CDK Client
participant QC as AwsQueryController
participant QH as CloudFormationQueryHandler
participant SVC as CloudFormationService
participant Store as StorageBackend
CDK->>QC: "POST Action=UpdateTerminationProtection"
QC->>QH: handle("UpdateTerminationProtection", params, region)
QH->>SVC: updateTerminationProtection(stackName, enabled, region)
SVC->>SVC: getStackOrThrow(stackName, region)
SVC->>SVC: stack.setEnableTerminationProtection(enabled)
SVC->>Store: persistStack(stack)
SVC-->>QH: Stack
QH-->>CDK: 200 XML UpdateTerminationProtectionResult(StackId)
CDK->>QC: "POST Action=DescribeStacks"
QC->>QH: handle("DescribeStacks", params, region)
QH->>SVC: describeStacks(stackName, region)
SVC-->>QH: List[Stack]
QH-->>CDK: 200 XML (includes EnableTerminationProtection)
CDK->>QC: "POST Action=DeleteStack"
QC->>QH: handle("DeleteStack", params, region)
QH->>SVC: deleteStack(stackName, region)
Note over SVC: Does NOT check enableTerminationProtection
SVC-->>CDK: Stack deleted (should be ValidationError)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant CDK as CDK Client
participant QC as AwsQueryController
participant QH as CloudFormationQueryHandler
participant SVC as CloudFormationService
participant Store as StorageBackend
CDK->>QC: "POST Action=UpdateTerminationProtection"
QC->>QH: handle("UpdateTerminationProtection", params, region)
QH->>SVC: updateTerminationProtection(stackName, enabled, region)
SVC->>SVC: getStackOrThrow(stackName, region)
SVC->>SVC: stack.setEnableTerminationProtection(enabled)
SVC->>Store: persistStack(stack)
SVC-->>QH: Stack
QH-->>CDK: 200 XML UpdateTerminationProtectionResult(StackId)
CDK->>QC: "POST Action=DescribeStacks"
QC->>QH: handle("DescribeStacks", params, region)
QH->>SVC: describeStacks(stackName, region)
SVC-->>QH: List[Stack]
QH-->>CDK: 200 XML (includes EnableTerminationProtection)
CDK->>QC: "POST Action=DeleteStack"
QC->>QH: handle("DeleteStack", params, region)
QH->>SVC: deleteStack(stackName, region)
Note over SVC: Does NOT check enableTerminationProtection
SVC-->>CDK: Stack deleted (should be ValidationError)
Comments Outside Diff (1)
-
src/main/java/io/github/hectorvent/floci/services/cloudformation/CloudFormationService.java, line 298-308 (link)deleteStackdoes not enforce termination protectionupdateTerminationProtectionpersists the flag butdeleteStacknever reads it. In real AWS, callingDeleteStackon a protection-enabled stack returns aValidationError: Stack [...] cannot be deleted while TerminationProtection is enabled. Without this guard, the flag is stored but has no behavioral effect — a client that enables protection (e.g.cdk bootstrap) will not be guarded against accidental deletion, which is the entire purpose of the feature.
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/cloudforma..." | Re-trigger Greptile
| private Response updateTerminationProtection(MultivaluedMap<String, String> params, String region) { | ||
| String stackName = params.getFirst("StackName"); | ||
| boolean enabled = Boolean.parseBoolean(params.getFirst("EnableTerminationProtection")); | ||
| Stack stack = cfnService.updateTerminationProtection(stackName, enabled, region); | ||
| String xml = new XmlBuilder() | ||
| .start("UpdateTerminationProtectionResponse", CF_NS) | ||
| .start("UpdateTerminationProtectionResult") | ||
| .elem("StackId", stack.getStackId()) | ||
| .end("UpdateTerminationProtectionResult") | ||
| .raw(AwsQueryResponse.responseMetadata()) | ||
| .end("UpdateTerminationProtectionResponse") | ||
| .build(); | ||
| return Response.ok(xml).type("text/xml").build(); | ||
| } |
There was a problem hiding this comment.
The handler does not catch
AwsException, so calling UpdateTerminationProtection with a non-existent stack name lets the exception propagate to AwsExceptionMapper, which always returns application/json. For a CloudFormation Query-protocol endpoint the error must be XML; the AWS SDK will fail to deserialize a JSON error body. Every other handler that calls a service method guarded by getStackOrThrow (describeStacks, describeChangeSet, executeChangeSet, deleteChangeSet) wraps the call in catch (AwsException e) { return xmlError(...); } for exactly this reason.
| private Response updateTerminationProtection(MultivaluedMap<String, String> params, String region) { | |
| String stackName = params.getFirst("StackName"); | |
| boolean enabled = Boolean.parseBoolean(params.getFirst("EnableTerminationProtection")); | |
| Stack stack = cfnService.updateTerminationProtection(stackName, enabled, region); | |
| String xml = new XmlBuilder() | |
| .start("UpdateTerminationProtectionResponse", CF_NS) | |
| .start("UpdateTerminationProtectionResult") | |
| .elem("StackId", stack.getStackId()) | |
| .end("UpdateTerminationProtectionResult") | |
| .raw(AwsQueryResponse.responseMetadata()) | |
| .end("UpdateTerminationProtectionResponse") | |
| .build(); | |
| return Response.ok(xml).type("text/xml").build(); | |
| } | |
| private Response updateTerminationProtection(MultivaluedMap<String, String> params, String region) { | |
| String stackName = params.getFirst("StackName"); | |
| boolean enabled = Boolean.parseBoolean(params.getFirst("EnableTerminationProtection")); | |
| try { | |
| Stack stack = cfnService.updateTerminationProtection(stackName, enabled, region); | |
| String xml = new XmlBuilder() | |
| .start("UpdateTerminationProtectionResponse", CF_NS) | |
| .start("UpdateTerminationProtectionResult") | |
| .elem("StackId", stack.getStackId()) | |
| .end("UpdateTerminationProtectionResult") | |
| .raw(AwsQueryResponse.responseMetadata()) | |
| .end("UpdateTerminationProtectionResponse") | |
| .build(); | |
| return Response.ok(xml).type("text/xml").build(); | |
| } catch (AwsException e) { | |
| return xmlError(e.getErrorCode(), e.getMessage(), e.getHttpStatus()); | |
| } | |
| } |
CDK
cdk bootstrapcallsUpdateTerminationProtectionon the CDKToolkit stack; Floci returnedUnknownAction, so bootstrap couldn't complete.Change
Stack: adds aenableTerminationProtectionflag (default false) + accessors.CloudFormationService.updateTerminationProtection(stackName, enabled, region): resolves the stack (ValidationErrorif it doesn't exist), sets the flag, and persists.CloudFormationQueryHandler: dispatchesUpdateTerminationProtection→ returns theUpdateTerminationProtectionResultenvelope with theStackId; and echoes<EnableTerminationProtection>in theDescribeStacksstack XML.AwsQueryController: registers the action in the CloudFormation action set so the Query-protocol POST routes to CloudFormation (without this it fell through to another service).Test
updateTerminationProtection_togglesFlagAndReflectsInDescribeStacks— DescribeStacks reportsfalseby default,UpdateTerminationProtectionreturns the StackId, and DescribeStacks then reflectstrue.Built + tested in the temurin container; the full
CloudFormationIntegrationTestpasses (the one unrelated failure is anAWS::ECR::Repositorytest that needs a Docker socket the build container doesn't have).