r/aws_cognito_log_delivery_configuration: fix log_configurations order inconsistency - #49220
Open
PanosKard wants to merge 4 commits into
Open
r/aws_cognito_log_delivery_configuration: fix log_configurations order inconsistency#49220PanosKard wants to merge 4 commits into
PanosKard wants to merge 4 commits into
Conversation
Contributor
|
✅ Thank you for correcting the previously detected issues! The maintainers appreciate your efforts to make the review process as smooth as possible. |
Contributor
Community GuidelinesThis comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀 Voting for Prioritization
Pull Request Authors
|
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.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Changes to Security Controls
No. This is purely a fix for a Terraform plan/apply consistency error — a schema type change from a list to a set. It doesn't change what gets logged, where it's delivered, the log levels, or anything to do with access, encryption, or IAM.
Description
Right now, if you write the
log_configurationsblocks in a different order than Cognito hands them back,terraform applyfails withError: Provider produced inconsistent result after apply.Here's why: Cognito's
GetLogDeliveryConfigurationalways returns the blocks in its own fixed order (userNotificationbeforeuserAuthEvents), no matter what order you sent. The resource storedlog_configurationsas an ordered list, so after applyflex.Flattenwrites AWS's order into state — and if that doesn't match your config, the block at index 0 isn't the one Terraform planned for index 0, so it throws the inconsistent-result error.The fix is to stop treating it as ordered: make
log_configurationsaSetNestedBlockinstead of aListNestedBlock(and switch the model field to the set type). Each block is keyed byevent_sourceand the order means nothing, so a set is what this should have been. Sets don't care about order, so the error is gone for create, update, and read — no sorting code needed. The inner blocks (cloud_watch_logs_configuration,firehose_configuration,s3_configuration) stay as they are. Same fix that already landed foraws_cloudfront_multitenant_distribution(#46045).Tests. With a set, the old index-based checks (
log_configurations.0.…) don't work anymore, so I moved them toTestCheckTypeSetElemNestedAttrs/TestCheckTypeSetElemAttrPair. I also addedTestAccCognitoIDPLogDeliveryConfiguration_orderSwap— it applies two blocks, swaps their order, and expects an empty plan. That's exactly the case that used to break.Getting the
userAuthEventstests to actually pass against real AWS meant fixing a few unrelated things the existing fixtures got wrong:ESSENTIALStier, butuserAuthEventsneeds Threat Protection, which needsPLUS. Addeduser_pool_tier = "PLUS"+advanced_security_mode = "AUDIT".userAuthEventsonly sendsINFOlogs, solog_level = "ERROR"gets rejected. Changed those toINFO.LogDeliveryEnabledtag to the Firehose stream on its own, which leaves a dirty plan. Addedignore_changes = [tags, tags_all]on the stream in the test.One test I left alone.
TestAccCognitoIDPLogDeliveryConfiguration_disappearsfails withexpected Create, got update. That's not from this change — delete just empties the config, butReadstill returns the empty config, so the plan is an update instead of a recreate (same behavior whether it's a list or a set). Fixing it means touching the read/delete logic, so I kept it out of this PR to stay focused. Happy to do it separately.Heads-up on compatibility. List → set is technically breaking if someone references these blocks by index (
log_configurations[0]). But the resource is new and today it just errors whenever the order doesn't match AWS, so I think this is the right trade. Glad to switch to a non-breaking reorder approach if you'd rather.AI disclaimer: The acceptance tests were amended with AI help. I've reviewed everything and confirmed it with real acceptance-test runs against AWS.
Relations
Closes #49202
References
Same bug and the same fix on another Plugin Framework resource: #46045 (
aws_cloudfront_multitenant_distributionorigin ordering).Output from Acceptance Testing
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_basic (51.86s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_logLevelUpdate (60.28s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_orderSwap (61.51s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_Identity_regionOverride (61.91s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_Identity_basic (62.18s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_firehose (161.33s)
--- PASS: TestAccCognitoIDPLogDeliveryConfiguration_update (224.89s)
Pre-existing, unrelated to this change (see Description):
--- FAIL: TestAccCognitoIDPLogDeliveryConfiguration_disappears
AWS_DEFAULT_REGION=us-east-1 make testacc TESTS=TestAccCognitoIDPLogDeliveryConfiguration_ PKG=cognitoidp