-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
: Property changes
#26015
azurerm_network_watcher_flow_log
: Property changes
#26015
Conversation
azurerm_network_watcher_flow_log
: Property changesazurerm_network_watcher_flow_log
: Property changes
azurerm_network_watcher_flow_log
: Property changesazurerm_network_watcher_flow_log
: Property changes
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 for this PR @favoretti. Overall this looks good. Would you mind fixing up the comments I left in-line? Once that's done I can run the tests.
@stephybun Thanks, will try to fix it up asap. I can run tests myself as well, but since the run takes 6 hours, I'm fairly slow in fixing all this :) I'll poke again once I get TC to green up :) |
Drafting this, due to lack of time, will try to get to it again asap (this/next week's timeframe). |
@favoretti any updates on that request? |
@favoretti just as a heads up we're targeting the major release for mid to end August. Just something to keep in mind if you want these deprecations/renames to make it in. |
Oops, completely forgot about this one, so sorry. I'll pick this up over the weekend |
Bump? |
@favoretti Will this request be delivered in August? Favoretti can you give us any news related to this request? |
Folks, I'm very sorry, but I'm swamped at work atm. If anyone has bandwidth - feel free to finish this one. I can't give any deadlines right now :( @aristosvo mind giving a hand here maybe? I know it might not be super relevant for you, but apparently a very wanted feature. |
@stephybun can we give a code review another pass please? I'll watch TC in the meantime. Thanks! |
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 @favoretti. I had another look over and left some comments. Could you also update the 5.0 upgrade guide with those changes in this PR?
FiveOh passes too.
|
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 @favoretti LGTM 🚀
Thanks a lot @favoretti , best timing with v4.11.0 , you rock ! 🔥🔥 |
@gguibert you are most welcome. Apologies it took me a year to finish, but work takes precedence unfortunately :) |
Community Note
Description
Allow for VNet ID to be passed as a resource to watch.
Rename
network_security_group_id
totarget_resource_id
.Deprecate
network_security_group_id
behind a feature switch.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
-network_security_group_id
is superseded bytarget_resource_id
, allowing for VNet ID to be passed along with NSG ID [azurerm_network_watcher_flow_log
: Property changes #26015]This is a (please select all that apply):
Related Issue(s)
Fixes #25982
Note
If this PR changes meaningfully during the course of review please update the title and description as required.