azurerm_eventhub - fix perpetual diff with message_retention#30169
azurerm_eventhub - fix perpetual diff with message_retention#30169mbfrahry merged 4 commits intohashicorp:mainfrom
azurerm_eventhub - fix perpetual diff with message_retention#30169Conversation
4f757c1 to
cf9bfc9
Compare
eventhub_resource: fix conflict state drift on retention descriptioneventhub_resource: fix state drift on retention description
mbfrahry
left a comment
There was a problem hiding this comment.
Hey @jiaweitao001, this looks good except I think we can one step further by opening an issue on the rest api specs as returning a number that causes a panic seems wrong
| return err | ||
| } | ||
|
|
||
| if props.RetentionDescription == nil || props.RetentionDescription.TombstoneRetentionTimeInHours == nil { |
There was a problem hiding this comment.
If TombstoneRetentionTimeInHours is returning a bogus number that causes a panic, that seems like an api issue. Would it make sense to open an issue on the rest api specs and leave a comment for that here?
There was a problem hiding this comment.
Hi @mbfrahry , the issue is created Azure/azure-rest-api-specs#36018
mbfrahry
left a comment
There was a problem hiding this comment.
Just a quick change for that message and we're there
| return err | ||
| } | ||
|
|
||
| // API returns a bogus MessageRetentionInDays when TombstoneRetentionTimeInHours is set. Tracking issue: https://github.com/Azure/azure-rest-api-specs/issues/36018 |
There was a problem hiding this comment.
| // API returns a bogus MessageRetentionInDays when TombstoneRetentionTimeInHours is set. Tracking issue: https://github.com/Azure/azure-rest-api-specs/issues/36018 | |
| // TODO - the `props.RetentionDescription.TombstoneRetentionTimeInHours == nil` check can be removed when https://github.com/Azure/azure-rest-api-specs/issues/36018 is fixed |
mbfrahry
left a comment
There was a problem hiding this comment.
LGTM! Thanks for those changes
eventhub_resource: fix state drift on retention descriptioneventhub_resource: fix perpetual diff with message_retention
eventhub_resource: fix perpetual diff with message_retentionazurerm_eventhub - fix perpetual diff with message_retention
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Community Note
Description
The previous PR will break acc tests because the
retention_descriptionis acomputedattribute. The API returns a value even it's not defined by user.message_retentionhas the same behavior. Whentombstone_retention_time_in_hourspresents, the API returns an outrange number which will cause a panic.PR Checklist
For example: “
resource_name_here- description of change e.g. adding propertynew_property_name_here”Changes to existing Resource / Data Source
Testing
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_resource- support for thething1property [GH-00000]This is a (please select all that apply):
Related Issue(s)
Fixes #29427
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the provider.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
Note
If this PR changes meaningfully during the course of review please update the title and description as required.