generic: path-aware attribute name mapping for duplicate CloudFormation property names - #3226
Draft
arditti wants to merge 2 commits into
Draft
generic: path-aware attribute name mapping for duplicate CloudFormation property names#3226arditti wants to merge 2 commits into
arditti wants to merge 2 commits into
Conversation
…on property names The generated Terraform attribute name map is flat: every attribute name maps to a CloudFormation property name regardless of where the property appears in the resource document. When a resource has a top-level property named Id, the generator renames its Terraform attribute to <type>_id (Terraform reserves id for the resource's primary identifier). If the same resource also has a nested property named Id (mapped to id), the map ends up with two attribute names for the same property name and inverting it for the CloudFormation-to-Terraform direction failed with 'duplicate attribute name mapping for CloudFormation property Id', so such resources had to be suppressed from generation. Resolve the collision by making the name translation aware of whether a property sits at the top level of the resource document: - invertAttributeNameMap builds the nested (naming-convention) reverse map plus small top-level override maps for both translation directions. - toTerraform.valueFromRaw and propertyPathToAttributePath use the top-level override when translating root document properties, so a top-level Id maps to <type>_id while a nested Id maps to id. - toCloudControl.rawFromValue maps the reserved top-level id attribute to the synthetic ID property so it can never collide with the real Id property in DesiredState documents. Un-suppress AWS::CloudFront::Distribution and AWS::CloudFront::AnycastIpList and generate their resources, singular data sources, and docs. The other seven resources suppressed for the same reason can be un-suppressed as a follow-up. Relates hashicorp#2311.
…ning entries The seven resources still suppressed for the duplicate-Id collision kept their one-line comment, which stops being accurate once the path-aware name mapping lands (the generator no longer errors on them). Expand each comment to state the mechanism, link the tracking issue, and note they remain suppressed only pending regeneration and validation.
Author
|
Pushed a comment-only follow-up commit: the seven resources that stay suppressed for the duplicate-Id collision now document the mechanism and link #2311 in |
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.
Community Note
Relates #2311
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Changes to Security Controls
No changes to security controls.
Description
Fixes the
duplicate attribute name mapping for CloudFormation property Idstartup error that currently forcessuppress_resource_generationon 9 resources, and un-suppresses two of them:awscc_cloudfront_distributionandawscc_cloudfront_anycast_ip_list.Root cause
The generator renames a top-level CloudFormation
Idproperty to<type>_id(e.g.distribution_id) because Terraform reservesid. When the same schema also has a nested property namedId(e.g.AWS::CloudFront::Distributionhas top-levelIdplusDistributionConfig/Origins/*/Id), the generated flattfToCfNameMapcontains bothdistribution_id → Idandid → Id. Inverting that map ininternal/generic/resource.go/data_source.gothen fails, because the reverse lookup is path-insensitive while the collision only exists across different JSON paths.Fix
invertAttributeNameMap(new, ininternal/generic/translate.go) splits the mapping into:cfToTfNameMap— the naming-convention reverse map (Id → id), used at nested paths as before;rootCfToTfNameMap— top-level overrides (Id → distribution_id), consulted only when translating properties at the document root;rootTfToCfNameMap— the TF→CFN top-level override, so the synthetic top-levelid(the Cloud Control identifier, Computed-only) can never leak into DesiredState/patch documents as the resource's ownIdproperty.Consumers are made root-aware at the three places with path context:
toTerraform.valueFromRaw,toCloudControl.rawFromValue, andpropertyPathToAttributePath. Resources without a top-level/nested collision produce empty override maps and behave exactly as before; irreconcilable duplicates (two same-named properties at the same level) still error exactly as before.Scope
Only
aws_cloudfront_distributionandaws_cloudfront_anycast_ip_listare un-suppressed here to keep the review surface small. The same fix unlocks the other 7 resources suppressed for this reason (aws_amazonmq_broker,aws_eks_nodegroup,aws_emrcontainers_security_configuration,aws_emrcontainers_virtual_cluster,aws_mediapackage_channel,aws_mediapackage_origin_endpoint,aws_vpclattice_target_group); happy to follow up.Test evidence
go build ./...,gofmt,go vet,golangci-lint run(pinned version): clean.internal/generic/translate_test.go:TestInvertAttributeNameMapandTestTranslateRoundTripWithDuplicateIdprove full round-trip translation (state → Cloud Control document → state) with a colliding nestedId.go test ./internal/generic/... ./internal/aws/cloudfront/... ./internal/provider/...— all pass, including a factory smoke test instantiating every registered resource/data source schema._NonExistentacctests forawscc_cloudfront_distributionpass. End-to-end against a real AWS account: created a disabled CloudFront distribution via Cloud Control,terraform import+ refresh through this provider round-tripped correctly (distribution_id= distribution ID, syntheticid= Cloud Control identifier, nestedorigins[0].idpreserved); the singular data source returns the same. AnycastIpList acceptance was skipped (the resource carries real cost); maintainers run acceptance tests pre-merge.Known pre-existing issue (not introduced here)
The current CloudFront Distribution schema declares
default: ""on several optional strings (e.g.RealtimeLogConfigArn), which the emitter turns intostringdefault.StaticString(""); CloudFront rejects the empty string on create. Verified via direct Cloud Control calls that this is independent of the name-mapping change (import/read/update are unaffected). Worth tracking separately as a schema-defaults issue.