azurerm_security_center_setting - Disable on Delete and RequiresImport support

internal/services/securitycenter/security_center_setting_resource.go

@@ -2,16 +2,17 @@ package securitycenter
 
 import (
 	"fmt"
-	"log"
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/services/preview/security/mgmt/v3.0/security"
+	"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/azuresdkhacks"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/parse"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/validation"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/timeouts"
+	"github.com/hashicorp/terraform-provider-azurerm/utils"
 )
 
 // TODO: this resource should be split into data_export_setting and alert_sync_setting
@@ -39,6 +40,7 @@ func resourceSecurityCenterSetting() *pluginsdk.Resource {
 			"setting_name": {
 				Type:     pluginsdk.TypeString,
 				Required: true,
+				ForceNew: true,
 				ValidateFunc: validation.StringInSlice([]string{
 					"MCAS",
 					"WDATP",
@@ -58,9 +60,21 @@ func resourceSecurityCenterSettingUpdate(d *pluginsdk.ResourceData, meta interfa
 	ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
 	defer cancel()
 
-	// TODO: requires import if it's enabled
-
 	id := parse.NewSettingID(subscriptionId, d.Get("setting_name").(string))
+
+	if d.IsNewResource() {
+		// TODO: switch back when Swagger/API bug has been fixed:
+		// https://github.com/Azure/azure-sdk-for-go/issues/12724 (`Enabled` field missing)
+		existing, err := azuresdkhacks.GetSecurityCenterSetting(ctx, client, id.Name)
+		if err != nil {
+			return fmt.Errorf("checking for presence of existing %s: %v", id, err)
+		}
+
+		if existing.DataExportSettingProperties != nil && existing.DataExportSettingProperties.Enabled != nil && *existing.DataExportSettingProperties.Enabled {
+			return tf.ImportAsExistsError("azurerm_security_center_setting", id.ID())
+		}
+	}
+
 	enabled := d.Get("enabled").(bool)
 	setting := security.DataExportSettings{
 		DataExportSettingProperties: &security.DataExportSettingProperties{
@@ -102,9 +116,26 @@ func resourceSecurityCenterSettingRead(d *pluginsdk.ResourceData, meta interface
 	return nil
 }
 
-func resourceSecurityCenterSettingDelete(_ *pluginsdk.ResourceData, _ interface{}) error {
-	// TODO: disable this
+func resourceSecurityCenterSettingDelete(d *pluginsdk.ResourceData, meta interface{}) error {
+	client := meta.(*clients.Client).SecurityCenter.SettingClient
+	ctx, cancel := timeouts.ForDelete(meta.(*clients.Client).StopContext, d)
+	defer cancel()
+
+	id, err := parse.SettingID(d.Id())
+	if err != nil {
+		return err
+	}
 
-	log.Printf("[DEBUG] Security Center deletion invocation")
-	return nil // cannot be deleted.
+	setting := security.DataExportSettings{
+		DataExportSettingProperties: &security.DataExportSettingProperties{
+			Enabled: utils.Bool(false),
+		},
+		Kind: security.KindDataExportSettings,
+	}
+
+	if _, err := client.Update(ctx, id.Name, setting); err != nil {
+		return fmt.Errorf("disabling %s: %+v", id, err)
+	}
+
+	return nil
 }

internal/services/securitycenter/security_center_setting_resource_test.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/services/preview/security/mgmt/v3.0/security"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/acceptance/check"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/clients"
+	"github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/azuresdkhacks"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/services/securitycenter/parse"
 	"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
 	"github.com/hashicorp/terraform-provider-azurerm/utils"
@@ -20,7 +22,7 @@ func TestAccSecurityCenterSetting_update(t *testing.T) {
 	r := SecurityCenterSettingResource{}
 
 	//lintignore:AT001
-	data.ResourceTestSkipCheckDestroyed(t, []acceptance.TestStep{
+	data.ResourceSequentialTest(t, r, []acceptance.TestStep{
 		{
 			Config: r.cfg("MCAS", true),
 			Check: acceptance.ComposeTestCheckFunc(
@@ -33,7 +35,6 @@ func TestAccSecurityCenterSetting_update(t *testing.T) {
 		{
 			Config: r.cfg("MCAS", false),
 			Check: acceptance.ComposeTestCheckFunc(
-				check.That(data.ResourceName).ExistsInAzure(r),
 				check.That(data.ResourceName).Key("setting_name").HasValue("MCAS"),
 				check.That(data.ResourceName).Key("enabled").HasValue("false"),
 			),
@@ -51,7 +52,6 @@ func TestAccSecurityCenterSetting_update(t *testing.T) {
 		{
 			Config: r.cfg("WDATP", false),
 			Check: acceptance.ComposeTestCheckFunc(
-				check.That(data.ResourceName).ExistsInAzure(r),
 				check.That(data.ResourceName).Key("setting_name").HasValue("WDATP"),
 				check.That(data.ResourceName).Key("enabled").HasValue("false"),
 			),
@@ -60,18 +60,67 @@ func TestAccSecurityCenterSetting_update(t *testing.T) {
 	})
 }
 
+func TestAccSecurityCenterSetting_requiresImport(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azurerm_security_center_setting", "test")
+	r := SecurityCenterSettingResource{}
+
+	data.ResourceSequentialTest(t, r, []acceptance.TestStep{
+		{
+			Config: r.cfg("MCAS", true),
+			Check: acceptance.ComposeTestCheckFunc(
+				check.That(data.ResourceName).ExistsInAzure(r),
+			),
+		},
+		data.RequiresImportErrorStep(r.requiresImport),
+	})
+}
+
 func (SecurityCenterSettingResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
 	id, err := parse.SettingID(state.ID)
 	if err != nil {
 		return nil, err
 	}
 
-	resp, err := clients.SecurityCenter.SettingClient.Get(ctx, id.Name)
+	// TODO: switch back when Swagger/API bug has been fixed:
+	// https://github.com/Azure/azure-sdk-for-go/issues/12724 (`Enabled` field missing)
+	resp, err := azuresdkhacks.GetSecurityCenterSetting(ctx, clients.SecurityCenter.SettingClient, id.Name)
+	if err != nil {
+		return nil, fmt.Errorf("checking for presence of existing %s: %v", id, err)
+	}
+
+	return utils.Bool(resp.DataExportSettingProperties != nil && resp.DataExportSettingProperties.Enabled != nil && *resp.DataExportSettingProperties.Enabled), nil
+}
+
+func (SecurityCenterSettingResource) Destroy(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
+	client := clients.SecurityCenter.SettingClient
+	id, err := parse.SettingID(state.ID)
+	if err != nil {
+		return nil, err
+	}
+
+	setting := security.DataExportSettings{
+		DataExportSettingProperties: &security.DataExportSettingProperties{
+			Enabled: utils.Bool(false),
+		},
+		Kind: security.KindDataExportSettings,
+	}
+
+	if _, err := client.Update(ctx, id.Name, setting); err != nil {
+		return nil, fmt.Errorf("disabling %s: %+v", id, err)
+	}
+
+	// TODO: switch back when Swagger/API bug has been fixed:
+	// https://github.com/Azure/azure-sdk-for-go/issues/12724 (`Enabled` field missing)
+	resp, err := azuresdkhacks.GetSecurityCenterSetting(ctx, client, id.Name)
 	if err != nil {
-		return nil, fmt.Errorf("retrieving %s: %+v", *id, err)
+		return nil, fmt.Errorf("checking for presence of existing %s: %v", id, err)
 	}
 
-	return utils.Bool(resp.Value != nil), nil
+	if resp.DataExportSettingProperties == nil || resp.DataExportSettingProperties.Enabled == nil || *resp.DataExportSettingProperties.Enabled {
+		return utils.Bool(false), nil
+	}
+
+	return utils.Bool(true), nil
 }
 
 func (SecurityCenterSettingResource) cfg(settingName string, enabled bool) string {
@@ -86,3 +135,14 @@ resource "azurerm_security_center_setting" "test" {
 }
 `, settingName, enabled)
 }
+
+func (r SecurityCenterSettingResource) requiresImport(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+%s
+
+resource "azurerm_security_center_setting" "import" {
+  setting_name = azurerm_security_center_setting.test.setting_name
+  enabled      = azurerm_security_center_setting.test.enabled
+}
+`, r.cfg("MCAS", true))
+}

website/docs/r/security_center_setting.html.markdown

@@ -12,7 +12,7 @@ Manages the Data Access Settings for Azure Security Center.
 
 ~> **NOTE:** This resource requires the `Owner` permission on the Subscription.
 
-~> **NOTE:** Deletion of this resource does not change or reset the data access settings
+~> **NOTE:** Deletion of this resource disables the setting.
 
 ## Example Usage
 
@@ -27,7 +27,7 @@ resource "azurerm_security_center_setting" "example" {
 
 The following arguments are supported:
 
-* `setting_name` - (Required) The setting to manage. Possible values are `MCAS` and `WDATP`.
+* `setting_name` - (Required) The setting to manage. Possible values are `MCAS` and `WDATP`. Changing this disables the setting.
 * `enabled` - (Required) Boolean flag to enable/disable data access.
 
 ## Attributes Reference