azurerm_network_manager - support management group register provider
#19853
Conversation
There was a problem hiding this comment.
hey @teowa
Thanks for this PR.
Unfortunately this PR contains a number of design issues which are going to need to be addressed before we can proceed here - namely registering Resource Providers at the Management Group level generally requires elevated permissions which may not be available, in addition the approach used here is problematic (for example, what happens if the RP isn't registered in 10s [since this can take up to 15 minutes] - what happens if we attempt to register multiple in parallel etc).
Once those issues are addressed we can take another look here, but I think it'd be helpful to have some additional context on what the problem is that we're looking to solve here.
Thanks!
| if len(state.Scope[0].ManagementGroups) > 0 { | ||
| metadata.Logger.Info("try to register 'Microsoft.Network' provider to the Management Group scope") | ||
| resourceProvidersClient := metadata.Client.Resource.ResourceProvidersClient | ||
| for _, group := range state.Scope[0].ManagementGroups { | ||
| groupId, err := managementGroupParse.ManagementGroupID(group) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| _, err = resourceProvidersClient.RegisterAtManagementGroupScope(ctx, "Microsoft.Network", groupId.Name) | ||
| if err != nil { | ||
| return fmt.Errorf("registering the 'Microsoft.Network' provider with Management Group %s: %+v", groupId, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
can you give some more context on the issue here? Terraform shouldn't be silently registering resource providers at management group scopes which generally require extra-permissions
| } | ||
| } | ||
| // https://github.com/Azure/azure-rest-api-specs/issues/22041 | ||
| time.Sleep(10 * time.Second) |
There was a problem hiding this comment.
Resource Provider registration takes up to 15 minutes, and unfortunately time.Sleep's aren't something we can ship/rely on - as such I think we can remove all of this RP auto-registration logic here?
|
Provisioning network manager using MG as scope need the 'Microsoft.Network' provider be registered at MG level, and using MG to provision network manager can parallelize the acc tests, see #19593 (comment). But seems we have to close this PR due to lack of related method, Azure/azure-rest-api-specs#22041. A good implementation should be a independent resource like |
|
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. |
Microsoft.Networkprovider at Management Group scope if the network manager use MG as scope.