azurerm_palo_alto_next_generation_firewall_virtual_network_local_rulestack: add lock of ruleStackID in creating
#23601
Conversation
| @@ -276,6 +280,12 @@ func (r NextGenerationFirewallVHubLocalRuleStackResource) Update() sdk.ResourceF | |||
| firewall.Tags = tags.Expand(model.Tags) | |||
| } | |||
|
|
|||
| if metadata.ResourceData.HasChange("rulestack_id") { | |||
| ruleStackID, _ := localrulestacks.ParseLocalRulestackID(model.RuleStackId) | |||
There was a problem hiding this comment.
should we be checking the error here?
There was a problem hiding this comment.
The Line 252 above has checked this error already.
There was a problem hiding this comment.
it will only get checked there if there is the rulestack_id has been updated. I think we should check it here and in next_generation_firewall_vnet_local_rulestack_resource.go line 280 as well just to be on the safe side.
| @@ -276,6 +280,15 @@ func (r NextGenerationFirewallVHubLocalRuleStackResource) Update() sdk.ResourceF | |||
| firewall.Tags = tags.Expand(model.Tags) | |||
| } | |||
|
|
|||
| if metadata.ResourceData.HasChange("rulestack_id") { | |||
There was a problem hiding this comment.
should we be locking when rulestack_id has not changed too?
There was a problem hiding this comment.
TBH I'm not sure about this because it seems to be related to the API implementation. But intuitively, we only need lock when rulestack_id changes as it's only a reference to the rulestack resource.
There was a problem hiding this comment.
we could add the locks after line 251 then to avoid checking if it's changed and parsing it twice
There was a problem hiding this comment.
I have ever thought about it before, but now moved the locks after L251.
|
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. |
fixes #23472.
Firewall Creating should be sequentialized with other rule create operation, so we should add a lock for it (also extend timeout value).