feat(acm): provision AWS::CertificateManager::Certificate - #3003
Conversation
Adds AcmCfnProvisioner over the existing AcmService. Ref and Fn::GetAtt CertificateArn both resolve to the certificate ARN instead of the stub's literal "LogicalId.CertificateArn". Maps DomainName, SubjectAlternativeNames, ValidationMethod (default DNS), KeyAlgorithm and Tags; DomainValidationOptions is accepted and ignored because the certificate is ISSUED at once, there is nothing to validate locally. UpdateStack: a change to a createOnly property (DomainName, SubjectAlternativeNames, KeyAlgorithm) requests a new certificate and deletes the previous one; otherwise tags are reconciled in place. Delete tolerates an already removed certificate and lets ResourceInUseException propagate. Tests: AcmCfnProvisionerTest (attribute keys, update paths, delete), AcmCfnIntegrationTest (stack output is an ARN DescribeCertificate finds, gone after DeleteStack). Inventory TSV, fixture wiring and the generated resource-type table updated.
|
🎉 Thanks for your first pull request to Floci! Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime. While you wait, a couple of things that make review faster:
Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it. |
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/AcmCfnProvisioner.java | Implements ACM certificate creation, replacement, tag reconciliation, deletion, rollback cleanup, and ARN attributes without a blocking issue. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/AcmCfnProvisionerTest.java | Covers property forwarding, defaults, update paths, replacement cleanup, missing resources, and deletion behavior. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/AcmCfnIntegrationTest.java | Verifies that stack references expose a real, describable certificate ARN and that stack deletion removes it. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java | Adds ACM service wiring for provisioner fixture discovery. |
| src/test/resources/cloudformation/supported-resource-types.tsv | Registers the certificate resource type to the new provisioner. |
| tools/docs/cfn_resource_types.yaml | Adds ACM ordering and labeling metadata for generated documentation. |
| docs/services/cloudformation.md | Documents ACM certificate support in the CloudFormation resource table. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
CFN[CloudFormation stack operation] --> Provisioner[AcmCfnProvisioner]
Provisioner -->|Create or replacement| Request[AcmService.requestCertificate]
Provisioner -->|In-place update| Tags[Reconcile certificate tags]
Request --> Certificate[Issued ACM certificate]
Certificate --> ARN[Physical ID and CertificateArn attribute]
Provisioner -->|Delete| Delete[AcmService.deleteCertificate]
Reviews (2): Last reviewed commit: "fix(acm): remove the replacement certifi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (existing != null && sameCreateOnlyProperties(existing, domainName, sans, keyAlgorithm)) { | ||
| arn = existing.getArn(); | ||
| reconcileTags(arn, tags, ctx.region()); |
There was a problem hiding this comment.
When UpdateStack changes only ValidationMethod, sameCreateOnlyProperties still selects this branch and only reconciles tags, causing the stack to report success while the existing certificate retains its previous validation method.
Knowledge Base Used: CloudFormation provisioning
There was a problem hiding this comment.
Not a defect: keeping the certificate is what AWS does here.
- The us-east-1 registry schema (
aws-certificatemanager-certificate.json) listsValidationMethodunderwriteOnlyProperties, notcreateOnlyProperties. The createOnly set isDomainName,SubjectAlternativeNames,DomainValidationOptions,KeyAlgorithm,CertificateAuthorityArn,CertificateExport, which is what the replacement check compares. - The AWS resource docs say
ValidationMethod: "Update requires: No interruption", so a change never replaces the certificate. ValidationMethodonly matters while a certificate is being issued. ACM has no API to change it on an issued certificate, so on AWS the update is a no-op for the existing resource too.
Treating it as a replacement would change the ARN on an update where AWS keeps it, which would break every consumer of Ref. Pinned the behaviour with updateChangingOnlyValidationMethodKeepsTheCertificate in AcmCfnProvisionerTest.
There was a problem hiding this comment.
🟡 Changes recommended
The replacement-update path can orphan a newly created certificate if deleting the prior certificate fails after creation, because failed update tracking restoration will not include cleanup for the new ARN.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds first-class CloudFormation support for AWS::CertificateManager::Certificate by introducing an extracted provisioner wired into the CloudFormation provisioner fixture, plus inventory and documentation updates, and both unit and Quarkus integration tests to validate Ref and Fn::GetAtt CertificateArn resolve to a real ACM ARN.
Changes:
- Introduce
AcmCfnProvisionerto map CFNAWS::CertificateManager::Certificateproperties ontoAcmService.requestCertificate, and implement update and delete semantics. - Add targeted tests (
AcmCfnProvisionerTest,AcmCfnIntegrationTest) verifying physical id andCertificateArnattribute behavior across create, update, and delete flows. - Register the resource type in the inventory TSV, test fixtures, and CloudFormation service labeling/docs.
File summaries
| File | Description |
|---|---|
| tools/docs/cfn_resource_types.yaml | Adds the AWS::CertificateManager service label and ordering for generated docs. |
| src/test/resources/cloudformation/supported-resource-types.tsv | Registers AWS::CertificateManager::Certificate as supported by AcmCfnProvisioner. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/provisioners/AcmCfnProvisionerTest.java | Unit tests for physical id / GetAtt keys, replacement logic, tag reconciliation, and delete behavior. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/CfnProvisionerFixture.java | Wires ACM into the test provisioner discovery so CFN tests can run with an AcmService. |
| src/test/java/io/github/hectorvent/floci/services/cloudformation/AcmCfnIntegrationTest.java | End-to-end CFN stack test asserting the returned ARN is describable in ACM and removed after stack deletion. |
| src/main/java/io/github/hectorvent/floci/services/cloudformation/provisioners/AcmCfnProvisioner.java | New CFN provisioner implementing create, update (replace vs in-place), and delete behavior for ACM certificates. |
| docs/services/cloudformation.md | Updates the supported-resource table to include ACM Certificate. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } else { | ||
| arn = acmService.requestCertificate(domainName, sans, validationMethod, null, | ||
| keyAlgorithm, null, null, tags, ctx.region()).getArn(); | ||
| if (existing != null) { | ||
| delete(r.getResourceType(), existing.getArn(), ctx.region()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixed in 7636611. If deleting the prior certificate throws, the provisioner now deletes the certificate this attempt created, sets CfnRollback.UPDATE_ROLLBACK_RESTORED_ATTR so the rollback walker knows the prior one is intact, and rethrows, the same shape Route53CfnProvisioner uses for a hosted zone. Covered by replacementThatCannotDeleteThePriorCertificateRemovesTheNewOneAndFails.
…t be deleted On a replacing update, a failure deleting the previous certificate (ResourceInUseException, for one) made the update fail after the new certificate existed. CloudFormationService restores the previous StackResource on a failed update and never learns the new ARN, which orphaned that certificate. Delete it and mark the resource restored for the rollback walker, as Route53CfnProvisioner does for zones. Also pins that a ValidationMethod-only update keeps the certificate: the property is writeOnly in the registry schema and "No interruption" in the AWS docs, and ACM has no call to change it on an issued certificate.
pgermosen
left a comment
pgermosen
left a comment
There was a problem hiding this comment.
Approving. Checked both review threads independently rather than taking either side at face value. Pulled AWS's own current CloudFormation reference for this resource: ValidationMethod really is "No interruption", and the replacement-triggering set matches sameCreateOnlyProperties exactly, so greptile's finding there was wrong and you were right to push back rather than change working code. The orphan-certificate fix for Copilot's finding checks out too - deletePriorOrUnwind deletes the just-created replacement and sets UPDATE_ROLLBACK_RESTORED_ATTR on failure, and the test genuinely proves it rather than just asserting it. Good, honest scope table on what's not implemented too.
pgermosen
added
acm
Summary
CloudFormation can now create, update and delete
AWS::CertificateManager::Certificate.Before this change the type was a stub. The stack turned green, but
RefandFn::GetAtt CertificateArnreturned the textLogicalId.CertificateArninstead of a real certificate ARN. Anything that used the certificate got a broken value.Now the provisioner calls the existing ACM
RequestCertificate, and bothRefandFn::GetAtt CertificateArnreturn the ARN of a certificate thatDescribeCertificatefinds.One difference from AWS, on purpose: the certificate is
ISSUEDright away. AWS waits for DNS or email validation. The emulator has nothing to validate.What is covered
Legend: ✅ full, 🟡 partial, ❌ not implemented
DomainName,SubjectAlternativeNamesorKeyAlgorithmreplaces the certificate, as on AWS. If the old certificate cannot be deleted, the new one is removed again and the update fails. Other changes update in place. Only tags are applied in place.DomainNameSubjectAlternativeNamesValidationMethodDNS. No validation happens. Changing it later does not touch an issued certificate, as on AWS (writeOnly in the schema, "No interruption" in the docs).KeyAlgorithmRSA_2048TagsDomainValidationOptionsCertificateTransparencyLoggingPreferenceCertificateAuthorityArnCertificateExportRefFn::GetAtt CertificateArnFiles
AcmCfnProvisioner(new), plus its row insupported-resource-types.tsvand theCfnProvisionerFixturewiring.AcmCfnProvisionerTest(unit) andAcmCfnIntegrationTest(stack output is a real ARN, gone afterDeleteStack).docs/services/cloudformation.mdtable regenerated withmake docs-sync.Type of change
Checklist
./mvnw testpasses locally. Ran per class instead:CfnResourceInventoryTest,CfnProvisionerFixtureTest,CfnSchemaCoverageTest(with the us-east-1 schema corpus: no unset attribute for this type),AcmCfnProvisionerTest,AcmCfnIntegrationTest, and every test undercloudformation.provisionersandacm.make docs-checkpasses.