-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
azurerm_network_watcher_flow_log
: the name
property can now be configured for new resources.
#15016
Conversation
id := parse.NewFlowLogID(subscriptionId, resourceGroupName, networkWatcherName, *nsgId) | ||
|
||
locks.ByID(nsgId.ID()) | ||
defer locks.UnlockByID(nsgId.ID()) | ||
id := parse.NewFlowLogIDShim(subscriptionId, resourceGroupName, networkWatcherName, d.Get("name").(string), *nsgId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come we are removing the lock?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for that, I've added them back.
Besides, do you think we should support the way how to import the resource backward compatibly? Or I should just add a ID migration and use the new way to import only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we'll need to do a state migration, could you update this PR with the new 3.0 beta "this is for good" flag so this can be included in the beta?
@katbyte Please see whether it makes sense, thank you! 💤 TF_ACC=1 go test -v -timeout=20h ./internal/services/network -run='TestAccNetworkWatcherFlowLog_'
=== RUN TestAccNetworkWatcherFlowLog_basic
--- PASS: TestAccNetworkWatcherFlowLog_basic (189.14s)
=== RUN TestAccNetworkWatcherFlowLog_requiresImport
--- PASS: TestAccNetworkWatcherFlowLog_requiresImport (190.40s)
=== RUN TestAccNetworkWatcherFlowLog_disabled
--- PASS: TestAccNetworkWatcherFlowLog_disabled (127.55s)
=== RUN TestAccNetworkWatcherFlowLog_reenabled
--- PASS: TestAccNetworkWatcherFlowLog_reenabled (210.11s)
=== RUN TestAccNetworkWatcherFlowLog_retentionPolicy
--- PASS: TestAccNetworkWatcherFlowLog_retentionPolicy (151.02s)
=== RUN TestAccNetworkWatcherFlowLog_updateStorageAccount
--- PASS: TestAccNetworkWatcherFlowLog_updateStorageAccount (259.72s)
=== RUN TestAccNetworkWatcherFlowLog_trafficAnalytics
--- PASS: TestAccNetworkWatcherFlowLog_trafficAnalytics (326.26s)
=== RUN TestAccNetworkWatcherFlowLog_version
--- PASS: TestAccNetworkWatcherFlowLog_version (839.23s)
=== RUN TestAccNetworkWatcherFlowLog_location
--- PASS: TestAccNetworkWatcherFlowLog_location (223.67s)
=== RUN TestAccNetworkWatcherFlowLog_tags
--- PASS: TestAccNetworkWatcherFlowLog_tags (227.80s)
PASS
ok github.com/hashicorp/terraform-provider-azurerm/internal/services/network 2744.944s |
azurerm_network_watcher_flow_log
: add supports of name
azurerm_network_watcher_flow_log
: the name
property can now be configured for new resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @magodo - much more elegant solution! LGTM 🔥
This functionality has been released in v2.96.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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. |
This PR expose the
name
for theazurerm_network_watcher_flow_log
, as an enhancement after #12533. It is implemented in a backward compatible manner.I've tested locally that I can import a flow log resource that is created without setting the name via either the old import format and the new import format, both has no plan diff.
Test Result
The only failed test is due to that the NSG API now treat a name with an ending dash an invalid name, therefore, this PR has removed that test.