Added support for databricks_mws_log_delivery update (#1439)

nkvuong · web-flow · commit 15bcffc4be84 · 2022-07-12T20:26:49.000+02:00
diff --git a/docs/resources/mws_log_delivery.md b/docs/resources/mws_log_delivery.md
@@ -5,7 +5,9 @@ subcategory: "Log Delivery"
 
 -> **Note** This resource has an evolving API, which will change in the upcoming versions of the provider in order to simplify user experience.
 
-Make sure you have authenticated with [username and password for Accounts Console](../guides/aws-workspace.md). This resource configures the delivery of the two supported log types from Databricks workspaces: [billable usage logs](https://docs.databricks.com/administration-guide/account-settings/billable-usage-delivery.html) and [audit logs](https://docs.databricks.com/administration-guide/account-settings/audit-logs.html). You cannot delete a log delivery configuration, but you can disable it when you no longer need it. This fact is important because there is a limit to the number of enabled log delivery configurations that you can create for an account. There is a limit on the number of log delivery configurations that you can create for an account. You can create a maximum of two enabled configurations that use the account level (no workspace filter) and two enabled configurations for every specific workspace (a workspaceId can occur in the workspace filter for two configurations). You cannot delete a log delivery configuration, but you can disable it. You can re-enable a disabled configuration, but the request fails if it violates the limits previously described.
+Make sure you have authenticated with [username and password for Accounts Console](../guides/aws-workspace.md). This resource configures the delivery of the two supported log types from Databricks workspaces: [billable usage logs](https://docs.databricks.com/administration-guide/account-settings/billable-usage-delivery.html) and [audit logs](https://docs.databricks.com/administration-guide/account-settings/audit-logs.html). 
+
+You cannot delete a log delivery configuration, but you can disable it when you no longer need it. This fact is important because there is a limit to the number of enabled log delivery configurations that you can create for an account. You can create a maximum of two enabled configurations that use the account level (no workspace filter) and two enabled configurations for every specific workspace (a workspaceId can occur in the workspace filter for two configurations). You can re-enable a disabled configuration, but the request fails if it violates the limits previously described.
 
 ## Example Usage
 
@@ -130,6 +132,7 @@ resource "databricks_mws_log_delivery" "audit_logs" {
 * `output_format` - The file type of log delivery. Currently `CSV` (for `BILLABLE_USAGE`) and `JSON` (for `AUDIT_LOGS`) are supported.
 * `credentials_id` - The ID for a Databricks [credential configuration](mws_credentials.md) that represents the AWS IAM role [with policy](../data-sources/aws_assume_role_policy.md) and [trust relationship](../data-sources/aws_assume_role_policy.md) as described in the main billable usage documentation page.
 * `storage_configuration_id` - The ID for a Databricks [storage configuration](mws_storage_configurations.md) that represents the S3 bucket with [bucket policy](../data-sources/aws_bucket_policy.md) as described in the main billable usage documentation page.
+* `status` - Status of log delivery configuration. Set to ENABLED or DISABLED. Defaults to ENABLED. This is the only field you can update.
 * `workspace_ids_filter` - (Optional) By default, this log configuration applies to all workspaces associated with your account ID. If your account is on the E2 version of the platform or on a select custom plan that allows multiple workspaces per account, you may have multiple workspaces associated with your account ID. You can optionally set the field as mentioned earlier to an array of workspace IDs. If you plan to use different log delivery configurations for several workspaces, set this explicitly rather than leaving it blank. If you leave this blank and your account ID gets additional workspaces in the future, this configuration will also apply to the new workspaces.
 * `delivery_path_prefix` - (Optional) Defaults to empty, which means that logs are delivered to the root of the bucket. The value must be a valid S3 object key. It must not start or end with a slash character.
 * `delivery_start_time` - (Optional) The optional start month and year for delivery, specified in YYYY-MM format. Defaults to current year and month. Usage is not available before 2019-03.
diff --git a/mws/resource_mws_log_delivery.go b/mws/resource_mws_log_delivery.go
@@ -3,7 +3,6 @@ package mws
 import (
 	"context"
 	"fmt"
-	"log"
 
 	"github.com/databricks/terraform-provider-databricks/common"
 
@@ -18,16 +17,16 @@ type LogDelivery struct {
 
 // LogDeliveryConfiguration describes log delivery
 type LogDeliveryConfiguration struct {
-	AccountID              string  `json:"account_id"`
-	ConfigID               string  `json:"config_id,omitempty" tf:"computed"`
-	CredentialsID          string  `json:"credentials_id"`
-	StorageConfigurationID string  `json:"storage_configuration_id"`
-	WorkspaceIdsFilter     []int64 `json:"workspace_ids_filter,omitempty"`
-	ConfigName             string  `json:"config_name,omitempty"`
+	AccountID              string  `json:"account_id" tf:"force_new"`
+	ConfigID               string  `json:"config_id,omitempty" tf:"computed,force_new"`
+	CredentialsID          string  `json:"credentials_id" tf:"force_new"`
+	StorageConfigurationID string  `json:"storage_configuration_id" tf:"force_new"`
+	WorkspaceIdsFilter     []int64 `json:"workspace_ids_filter,omitempty" tf:"force_new"`
+	ConfigName             string  `json:"config_name,omitempty" tf:"force_new"`
 	Status                 string  `json:"status,omitempty" tf:"computed"`
-	LogType                string  `json:"log_type"`
-	OutputFormat           string  `json:"output_format"`
-	DeliveryPathPrefix     string  `json:"delivery_path_prefix,omitempty"`
+	LogType                string  `json:"log_type" tf:"force_new"`
+	OutputFormat           string  `json:"output_format" tf:"force_new"`
+	DeliveryPathPrefix     string  `json:"delivery_path_prefix,omitempty" tf:"force_new"`
 	DeliveryStartTime      string  `json:"delivery_start_time,omitempty" tf:"computed,force_new"`
 }
 
@@ -59,10 +58,10 @@ func (a LogDeliveryAPI) Create(ldc LogDeliveryConfiguration) (string, error) {
 	return ld.LogDeliveryConfiguration.ConfigID, err
 }
 
-// Disable log delivery configuration - e.g. delete it
-func (a LogDeliveryAPI) Disable(accountID, configID string) error {
+// patch log delivery configuration - i.e. can only enable or disable it
+func (a LogDeliveryAPI) Patch(accountID, configID string, status string) error {
 	return a.client.Patch(a.context, fmt.Sprintf("/accounts/%s/log-delivery/%s", accountID, configID), map[string]string{
-		"status": "DISABLED",
+		"status": status,
 	})
 }
 
@@ -93,6 +92,15 @@ func ResourceMwsLogDelivery() *schema.Resource {
 			p.Pack(d)
 			return nil
 		},
+		Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
+			var ldc LogDeliveryConfiguration
+			common.DataToStructPointer(d, s, &ldc)
+			err := NewLogDeliveryAPI(ctx, c).Patch(ldc.AccountID, ldc.ConfigID, ldc.Status)
+			if err != nil {
+				return err
+			}
+			return common.StructToData(ldc, s, d)
+		},
 		Read: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			accountID, configID, err := p.Unpack(d)
 			if err != nil {
@@ -102,19 +110,14 @@ func ResourceMwsLogDelivery() *schema.Resource {
 			if err != nil {
 				return err
 			}
-			if ldc.Status == "DISABLED" {
-				log.Printf("[DEBUG] Log delivery configuration %s was disabled. Removing from state.", configID)
-				d.SetId("")
-				return nil
-			}
 			return common.StructToData(ldc, s, d)
 		},
 		Delete: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {
 			accountID, configID, err := p.Unpack(d)
 			if err != nil {
 				return err
 			}
-			return NewLogDeliveryAPI(ctx, c).Disable(accountID, configID)
+			return NewLogDeliveryAPI(ctx, c).Patch(accountID, configID, "DISABLED")
 		},
 	}.ToResource()
 }
diff --git a/mws/resource_mws_log_delivery_test.go b/mws/resource_mws_log_delivery_test.go
@@ -47,7 +47,7 @@ func TestMwsAccLogDelivery(t *testing.T) {
 	})
 	require.NoError(t, err)
 	defer func() {
-		assert.NoError(t, logDeliveryAPI.Disable(acctID, configID))
+		assert.NoError(t, logDeliveryAPI.Patch(acctID, configID, "DISABLED"))
 	}()
 
 	ldc, err := logDeliveryAPI.Read(acctID, configID)
@@ -117,6 +117,71 @@ func TestResourceLogDeliveryCreate(t *testing.T) {
 	assert.Equal(t, "nid", d.Get("config_id"))
 }
 
+func TestResourceLogDeliveryCreateDisabled(t *testing.T) {
+	d, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "POST",
+				Resource: "/api/2.0/accounts/abc/log-delivery",
+				ExpectedRequest: LogDelivery{
+					LogDeliveryConfiguration: LogDeliveryConfiguration{
+						AccountID:              "abc",
+						ConfigName:             "Audit logs",
+						CredentialsID:          "bcd",
+						DeliveryPathPrefix:     "/a/b",
+						LogType:                "AUDIT_LOGS",
+						OutputFormat:           "JSON",
+						StorageConfigurationID: "def",
+						DeliveryStartTime:      "2020-10",
+						Status:                 "DISABLED",
+						WorkspaceIdsFilter:     []int64{1111111111111111, 222222222222222},
+					},
+				},
+				Response: LogDelivery{
+					LogDeliveryConfiguration: LogDeliveryConfiguration{
+						ConfigID: "nid",
+					},
+				},
+			},
+			{
+				Method:   "GET",
+				Resource: "/api/2.0/accounts/abc/log-delivery/nid",
+				Response: LogDelivery{
+					LogDeliveryConfiguration: LogDeliveryConfiguration{
+						ConfigID:               "nid",
+						AccountID:              "abc",
+						ConfigName:             "Audit logs",
+						CredentialsID:          "bcd",
+						DeliveryPathPrefix:     "/a/b",
+						LogType:                "AUDIT_LOGS",
+						OutputFormat:           "JSON",
+						StorageConfigurationID: "def",
+						DeliveryStartTime:      "2020-10",
+						Status:                 "DISABLED",
+						WorkspaceIdsFilter:     []int64{1111111111111111, 222222222222222},
+					},
+				},
+			},
+		},
+		Resource: ResourceMwsLogDelivery(),
+		HCL: `
+		account_id = "abc"
+		credentials_id = "bcd"
+		storage_configuration_id = "def"
+		config_name = "Audit logs"
+		log_type = "AUDIT_LOGS"
+		output_format = "JSON"
+		delivery_path_prefix = "/a/b"
+		workspace_ids_filter = [1111111111111111, 222222222222222]
+		delivery_start_time = "2020-10"
+		status = "DISABLED"`,
+		Create: true,
+	}.Apply(t)
+	assert.NoError(t, err, err)
+	assert.Equal(t, "abc|nid", d.Id())
+	assert.Equal(t, "nid", d.Get("config_id"))
+}
+
 func TestResourceLogDeliveryCreate_Error(t *testing.T) {
 	d, err := qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{
@@ -222,8 +287,113 @@ func TestResourceLogDeliveryRead_Error(t *testing.T) {
 	assert.Equal(t, "abc|nid", d.Id(), "Id should not be empty for error reads")
 }
 
+func TestUpdateLogDelivery(t *testing.T) {
+	qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "PATCH",
+				Resource: "/api/2.0/accounts/abc/log-delivery/nid",
+				ExpectedRequest: map[string]interface{}{
+					"status": "ENABLED",
+				},
+			},
+			{
+				Method:   "GET",
+				Resource: "/api/2.0/accounts/abc/log-delivery/nid",
+				Response: LogDelivery{
+					LogDeliveryConfiguration: LogDeliveryConfiguration{
+						ConfigID:               "nid",
+						Status:                 "ENABLED",
+						AccountID:              "abc",
+						ConfigName:             "Audit logs",
+						CredentialsID:          "bcd",
+						DeliveryPathPrefix:     "/a/b",
+						LogType:                "AUDIT_LOGS",
+						OutputFormat:           "JSON",
+						StorageConfigurationID: "def",
+						DeliveryStartTime:      "2020-10",
+					},
+				},
+			},
+		},
+		Resource:    ResourceMwsLogDelivery(),
+		ID:          "abc|nid",
+		Update:      true,
+		RequiresNew: true,
+		InstanceState: map[string]string{
+			"account_id":               "abc",
+			"config_id":                "nid",
+			"config_name":              "Audit logs",
+			"credentials_id":           "bcd",
+			"delivery_path_prefix":     "/a/b",
+			"delivery_start_time":      "2020-10",
+			"id":                       "abc|nid",
+			"log_type":                 "AUDIT_LOGS",
+			"output_format":            "JSON",
+			"status":                   "DISABLED",
+			"storage_configuration_id": "def",
+		},
+		HCL: `
+		account_id = "abc"
+		credentials_id = "bcd"
+		storage_configuration_id = "def"
+		config_name = "Audit logs"
+		log_type = "AUDIT_LOGS"
+		output_format = "JSON"
+		delivery_path_prefix = "/a/b"
+		delivery_start_time = "2020-10"
+		status = "ENABLED"
+		`,
+	}.ApplyNoError(t)
+}
+
+func TestUpdateLogDeliveryError(t *testing.T) {
+	_, err := qa.ResourceFixture{
+		Fixtures: []qa.HTTPFixture{
+			{
+				Method:   "PATCH",
+				Resource: "/api/2.0/accounts/abc/log-delivery/nid",
+				Response: common.APIErrorBody{
+					ErrorCode: "INVALID_REQUEST",
+					Message:   "Internal error happened",
+				},
+				Status: 400,
+			},
+		},
+		Resource:    ResourceMwsLogDelivery(),
+		ID:          "abc|nid",
+		Update:      true,
+		RequiresNew: true,
+		InstanceState: map[string]string{
+			"account_id":               "abc",
+			"config_id":                "nid",
+			"config_name":              "Audit logs",
+			"credentials_id":           "bcd",
+			"delivery_path_prefix":     "/a/b",
+			"delivery_start_time":      "2020-10",
+			"id":                       "abc|nid",
+			"log_type":                 "AUDIT_LOGS",
+			"output_format":            "JSON",
+			"status":                   "DISABLED",
+			"storage_configuration_id": "def",
+		},
+		HCL: `
+		account_id = "abc"
+		credentials_id = "bcd"
+		storage_configuration_id = "def"
+		config_name = "Audit logs"
+		log_type = "AUDIT_LOGS"
+		output_format = "JSON"
+		delivery_path_prefix = "/a/b"
+		delivery_start_time = "2020-10"
+		status = "ENABLED"
+		`,
+	}.Apply(t)
+	qa.AssertErrorStartsWith(t, err, "Internal error happened")
+}
+
 func TestResourceLogDeliveryDelete(t *testing.T) {
-	d, err := qa.ResourceFixture{
+	qa.ResourceFixture{
 		Fixtures: []qa.HTTPFixture{
 			{
 				Method:   "PATCH",
@@ -236,9 +406,7 @@ func TestResourceLogDeliveryDelete(t *testing.T) {
 		Resource: ResourceMwsLogDelivery(),
 		Delete:   true,
 		ID:       "abc|nid",
-	}.Apply(t)
-	assert.NoError(t, err, err)
-	assert.Equal(t, "abc|nid", d.Id())
+	}.ApplyNoError(t)
 }
 
 func TestResourceLogDeliveryDelete_Error(t *testing.T) {
