From 428817ade68ff9a8b23c0c354ee6887089664685 Mon Sep 17 00:00:00 2001 From: Zijie He Date: Wed, 29 Sep 2021 14:38:44 +0800 Subject: [PATCH 1/3] add nil check to fix #12163 --- .../managementgroup/management_group_resource.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/services/managementgroup/management_group_resource.go b/internal/services/managementgroup/management_group_resource.go index 301f1c8e79b0..e5a04957f047 100644 --- a/internal/services/managementgroup/management_group_resource.go +++ b/internal/services/managementgroup/management_group_resource.go @@ -292,6 +292,9 @@ func resourceManagementGroupDelete(d *pluginsdk.ResourceData, meta interface{}) } subscriptionId, err := parseManagementGroupSubscriptionID(*v.ID) + if subscriptionId == nil { + continue + } if err != nil { return fmt.Errorf("unable to parse child Subscription ID %+v", err) } @@ -364,8 +367,8 @@ func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) { // this is either: // /subscriptions/00000000-0000-0000-0000-000000000000 - // we skip out the managementGroup ID's - if strings.HasPrefix(input, "/providers/Microsoft.Management/managementGroups/") { + // we skip out the child managementGroup ID's + if isChildMgrGroupId(input) { return nil, nil } @@ -385,6 +388,10 @@ func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) { return &id, nil } +func isChildMgrGroupId(input string) bool { + return strings.HasPrefix(input, "/providers/Microsoft.Management/managementGroups/") +} + func determineManagementGroupSubscriptionsIdsToRemove(existing *[]managementgroups.ChildInfo, updated []string) (*[]string, error) { subscriptionIdsToRemove := make([]string, 0) if existing == nil { From ba5fbda0d4e6fe2c314d3e28bfc620b058a199bd Mon Sep 17 00:00:00 2001 From: Zijie He Date: Wed, 29 Sep 2021 14:49:04 +0800 Subject: [PATCH 2/3] adjust errcheck and nil check's order --- .../services/managementgroup/management_group_resource.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/services/managementgroup/management_group_resource.go b/internal/services/managementgroup/management_group_resource.go index e5a04957f047..d2009f5869ba 100644 --- a/internal/services/managementgroup/management_group_resource.go +++ b/internal/services/managementgroup/management_group_resource.go @@ -292,12 +292,12 @@ func resourceManagementGroupDelete(d *pluginsdk.ResourceData, meta interface{}) } subscriptionId, err := parseManagementGroupSubscriptionID(*v.ID) - if subscriptionId == nil { - continue - } if err != nil { return fmt.Errorf("unable to parse child Subscription ID %+v", err) } + if subscriptionId == nil { + continue + } log.Printf("[DEBUG] De-associating Subscription %q from Management Group %q..", subscriptionId, id.Name) // NOTE: whilst this says `Delete` it's actually `Deassociate` - which is /really/ helpful deleteResp, err2 := subscriptionsClient.Delete(ctx, id.Name, subscriptionId.subscriptionId, managementGroupCacheControl) From 852b8478b6161bd3804772b79c4e319ecb23d087 Mon Sep 17 00:00:00 2001 From: Zijie He Date: Fri, 8 Oct 2021 11:18:53 +0800 Subject: [PATCH 3/3] inline function due to @stephybun code review --- .../services/managementgroup/management_group_resource.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/internal/services/managementgroup/management_group_resource.go b/internal/services/managementgroup/management_group_resource.go index d2009f5869ba..297b44931f66 100644 --- a/internal/services/managementgroup/management_group_resource.go +++ b/internal/services/managementgroup/management_group_resource.go @@ -368,7 +368,7 @@ func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) { // /subscriptions/00000000-0000-0000-0000-000000000000 // we skip out the child managementGroup ID's - if isChildMgrGroupId(input) { + if strings.HasPrefix(input, "/providers/Microsoft.Management/managementGroups/") { return nil, nil } @@ -388,10 +388,6 @@ func parseManagementGroupSubscriptionID(input string) (*subscriptionId, error) { return &id, nil } -func isChildMgrGroupId(input string) bool { - return strings.HasPrefix(input, "/providers/Microsoft.Management/managementGroups/") -} - func determineManagementGroupSubscriptionsIdsToRemove(existing *[]managementgroups.ChildInfo, updated []string) (*[]string, error) { subscriptionIdsToRemove := make([]string, 0) if existing == nil {