From f433566e440545a8c2d0deed8325b2cb1d0d57d0 Mon Sep 17 00:00:00 2001 From: magodo Date: Thu, 21 Apr 2022 10:17:51 +0800 Subject: [PATCH 1/2] `azurerm_resource_policy_remediation` - Fix deletion failure due to canceling a completed remediation task Remediation task can only be canceled when it is in "Evaluating" status; Canceling a completed remediation task will cause 400 bad request. --- .../services/policy/remediation_resource.go | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/internal/services/policy/remediation_resource.go b/internal/services/policy/remediation_resource.go index 3ac88bd9a1db..810673e1726a 100644 --- a/internal/services/policy/remediation_resource.go +++ b/internal/services/policy/remediation_resource.go @@ -196,24 +196,27 @@ func resourceArmResourcePolicyRemediationDelete(d *pluginsdk.ResourceData, meta } if existing.RemediationProperties != nil && existing.RemediationProperties.ResourceDiscoveryMode == policyinsights.ReEvaluateCompliance { - log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") - if _, err := client.CancelAtResource(ctx, id.ResourceId, id.Name); err != nil { - return fmt.Errorf("cancelling %s: %+v", id.ID(), err) - } + // Remediation can only be canceld when it is in "Evaluating" status, otherwise, API might raise error (e.g. canceling a "Completed" remediation returns 400). + if existing.RemediationProperties.ProvisioningState != nil && *existing.RemediationProperties.ProvisioningState == "Evaluating" { + log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") + if _, err := client.CancelAtResource(ctx, id.ResourceId, id.Name); err != nil { + return fmt.Errorf("cancelling %s: %+v", id.ID(), err) + } - log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Cancelling"}, - Target: []string{ - "Succeeded", "Canceled", "Failed", - }, - Refresh: resourcePolicyRemediationCancellationRefreshFunc(ctx, client, *id), - MinTimeout: 10 * time.Second, - Timeout: d.Timeout(pluginsdk.TimeoutDelete), - } + log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Cancelling"}, + Target: []string{ + "Succeeded", "Canceled", "Failed", + }, + Refresh: resourcePolicyRemediationCancellationRefreshFunc(ctx, client, *id), + MinTimeout: 10 * time.Second, + Timeout: d.Timeout(pluginsdk.TimeoutDelete), + } - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + } } } From 239eee7b5005f5184bba1b4f6c4c8a55182854ee Mon Sep 17 00:00:00 2001 From: magodo Date: Thu, 21 Apr 2022 10:30:52 +0800 Subject: [PATCH 2/2] Replicate the fix for other remediation resources --- .../policy/remediation_management_group.go | 35 ++++++++++--------- .../policy/remediation_resource_group.go | 35 ++++++++++--------- .../policy/remediation_subscription.go | 35 ++++++++++--------- 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/internal/services/policy/remediation_management_group.go b/internal/services/policy/remediation_management_group.go index f19bab858c78..0c9f14bd0bfc 100644 --- a/internal/services/policy/remediation_management_group.go +++ b/internal/services/policy/remediation_management_group.go @@ -210,24 +210,27 @@ func resourceArmManagementGroupPolicyRemediationDelete(d *pluginsdk.ResourceData } if existing.RemediationProperties != nil && existing.RemediationProperties.ResourceDiscoveryMode == policyinsights.ReEvaluateCompliance { - log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") - if _, err := client.CancelAtManagementGroup(ctx, managementGroupId.Name, id.Name); err != nil { - return fmt.Errorf("cancelling %s: %+v", id.ID(), err) - } + // Remediation can only be canceld when it is in "Evaluating" status, otherwise, API might raise error (e.g. canceling a "Completed" remediation returns 400). + if existing.RemediationProperties.ProvisioningState != nil && *existing.RemediationProperties.ProvisioningState == "Evaluating" { + log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") + if _, err := client.CancelAtManagementGroup(ctx, managementGroupId.Name, id.Name); err != nil { + return fmt.Errorf("cancelling %s: %+v", id.ID(), err) + } - log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Cancelling"}, - Target: []string{ - "Succeeded", "Canceled", "Failed", - }, - Refresh: managementGroupPolicyRemediationCancellationRefreshFunc(ctx, client, *id, *managementGroupId), - MinTimeout: 10 * time.Second, - Timeout: d.Timeout(pluginsdk.TimeoutDelete), - } + log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Cancelling"}, + Target: []string{ + "Succeeded", "Canceled", "Failed", + }, + Refresh: managementGroupPolicyRemediationCancellationRefreshFunc(ctx, client, *id, *managementGroupId), + MinTimeout: 10 * time.Second, + Timeout: d.Timeout(pluginsdk.TimeoutDelete), + } - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + } } } diff --git a/internal/services/policy/remediation_resource_group.go b/internal/services/policy/remediation_resource_group.go index 50ea776eff41..e2d7646a78cc 100644 --- a/internal/services/policy/remediation_resource_group.go +++ b/internal/services/policy/remediation_resource_group.go @@ -201,24 +201,27 @@ func resourceArmResourceGroupPolicyRemediationDelete(d *pluginsdk.ResourceData, } if existing.RemediationProperties != nil && existing.RemediationProperties.ResourceDiscoveryMode == policyinsights.ReEvaluateCompliance { - log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") - if _, err := client.CancelAtResourceGroup(ctx, id.SubscriptionId, id.ResourceGroup, id.RemediationName); err != nil { - return fmt.Errorf("cancelling %s: %+v", id.ID(), err) - } + // Remediation can only be canceld when it is in "Evaluating" status, otherwise, API might raise error (e.g. canceling a "Completed" remediation returns 400). + if existing.RemediationProperties.ProvisioningState != nil && *existing.RemediationProperties.ProvisioningState == "Evaluating" { + log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") + if _, err := client.CancelAtResourceGroup(ctx, id.SubscriptionId, id.ResourceGroup, id.RemediationName); err != nil { + return fmt.Errorf("cancelling %s: %+v", id.ID(), err) + } - log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Cancelling"}, - Target: []string{ - "Succeeded", "Canceled", "Failed", - }, - Refresh: resourceGroupPolicyRemediationCancellationRefreshFunc(ctx, client, *id), - MinTimeout: 10 * time.Second, - Timeout: d.Timeout(pluginsdk.TimeoutDelete), - } + log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Cancelling"}, + Target: []string{ + "Succeeded", "Canceled", "Failed", + }, + Refresh: resourceGroupPolicyRemediationCancellationRefreshFunc(ctx, client, *id), + MinTimeout: 10 * time.Second, + Timeout: d.Timeout(pluginsdk.TimeoutDelete), + } - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + } } } diff --git a/internal/services/policy/remediation_subscription.go b/internal/services/policy/remediation_subscription.go index 1c9eb6dd665e..c6ff1726c13c 100644 --- a/internal/services/policy/remediation_subscription.go +++ b/internal/services/policy/remediation_subscription.go @@ -201,24 +201,27 @@ func resourceArmSubscriptionPolicyRemediationDelete(d *pluginsdk.ResourceData, m } if existing.RemediationProperties != nil && existing.RemediationProperties.ResourceDiscoveryMode == policyinsights.ReEvaluateCompliance { - log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") - if _, err := client.CancelAtSubscription(ctx, id.SubscriptionId, id.RemediationName); err != nil { - return fmt.Errorf("cancelling %s: %+v", id.ID(), err) - } + // Remediation can only be canceld when it is in "Evaluating" status, otherwise, API might raise error (e.g. canceling a "Completed" remediation returns 400). + if existing.RemediationProperties.ProvisioningState != nil && *existing.RemediationProperties.ProvisioningState == "Evaluating" { + log.Printf("[DEBUG] cancelling the remediation first before deleting it when `resource_discovery_mode` is set to `ReEvaluateCompliance`") + if _, err := client.CancelAtSubscription(ctx, id.SubscriptionId, id.RemediationName); err != nil { + return fmt.Errorf("cancelling %s: %+v", id.ID(), err) + } - log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) - stateConf := &pluginsdk.StateChangeConf{ - Pending: []string{"Cancelling"}, - Target: []string{ - "Succeeded", "Canceled", "Failed", - }, - Refresh: subscriptionPolicyRemediationCancellationRefreshFunc(ctx, client, *id), - MinTimeout: 10 * time.Second, - Timeout: d.Timeout(pluginsdk.TimeoutDelete), - } + log.Printf("[DEBUG] waiting for the %s to be canceled", id.ID()) + stateConf := &pluginsdk.StateChangeConf{ + Pending: []string{"Cancelling"}, + Target: []string{ + "Succeeded", "Canceled", "Failed", + }, + Refresh: subscriptionPolicyRemediationCancellationRefreshFunc(ctx, client, *id), + MinTimeout: 10 * time.Second, + Timeout: d.Timeout(pluginsdk.TimeoutDelete), + } - if _, err := stateConf.WaitForStateContext(ctx); err != nil { - return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + if _, err := stateConf.WaitForStateContext(ctx); err != nil { + return fmt.Errorf("waiting for %s to be canceled: %+v", id.ID(), err) + } } }