[Feature] Notification Destination resource #3820

Divyansh-db · 2024-07-25T08:49:12Z

Changes

Made a new Notification Destination resource

relevant change in docs/ folder
covered with integration tests in internal/acceptance
relevant acceptance tests are passing
using Go SDK

alexott · 2024-07-25T10:56:36Z

settings/resource_notification_destination.go

+	s.SchemaPath("config", "slack").SetRequiredWith([]string{"config.0.slack.0.url"})
+	s.SchemaPath("config", "pagerduty").SetRequiredWith([]string{"config.0.pagerduty.0.integration_key"})
+	s.SchemaPath("config", "microsoft_teams").SetRequiredWith([]string{"config.0.microsoft_teams.0.url"})
+	s.SchemaPath("config", "generic_webhook").SetRequiredWith([]string{"config.0.generic_webhook.0.url"})
+	s.SchemaPath("config", "email").SetRequiredWith([]string{"config.0.email.0.addresses"})


should we talk with the corresponding teams to mark URL and some other objects as required (remove omitempty)?

settings/resource_notification_destination.go

settings/resource_notification_destination_test.go

alexott

API doesn't return sensitive values, so we need to handle it similarly to other resources (like, MLflow webhook or connections)

settings/resource_notification_destination.go

alexott · 2024-07-25T19:21:17Z

For testing, you just need to provide a base URL; I was just using "https://hooks.slack.com/services/..." and it worked. But you need to have admin permissions on a workspace

docs/resources/notification_destination.md

Co-authored-by: vuong-nguyen <44292934+nkvuong@users.noreply.github.com>

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

internal/acceptance/notification_destination_test.go

Divyansh-db · 2024-07-29T11:29:44Z

I could not figure out on how to set the values of nested fields (fields inside the structs) directly through d.Set(...). I could not find similar integration tests which test setting nested fields (There is no integration test for mlflow_webhook as well). Used a way around to get things done
Done now

internal/acceptance/notification_destination_test.go

settings/resource_notification_destination.go

alexott · 2024-07-29T18:42:24Z

settings/resource_notification_destination.go

+	if detectConfigTypeChange(d) {
+		err := Delete(ctx, d, w)
+		if err != nil {
+			return err
+		}
+		return Create(ctx, d, w)
+	}


According to API it should be possible to update it inline: https://docs.databricks.com/api/workspace/notificationdestinations/update - the main problem is that if we do delete/create, then ID of destination will change and this may break other integrations

That is true, but the update API does not allow to change the type of notification destination. I created an integration test TestAccConfigTypeChange for this exact puspose. If we do not delete and create a new resouce and try to update it instead, the update API response is bad request and states that you can't change config types using PATCH call

We need to raise an internal ticket to update the OpenAPI spec to reflect this - right now, it's not documented. But it's better first to confirm that this is intended behavior.

Instead of implicitly recreating the resource, we may need to mark these blocks as force_new, so Terraform will delete/create automatically. We need to do this because Terraform needs to know that id is changing, and this change is propagated to the dependent resources. Otherwise, they could be broken after such an update.

We also need to document that behavior.

which team should I contact for this.
~~2) I will try to make fields force new rather than explicitly doing it.~~

Also, is raising the ticket a blocking change for this PR?

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

alexott

Code looks good, also tested it myself. Just few small comments

internal/acceptance/notification_destination_test.go

docs/resources/notification_destination.md

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

### New Features and Improvements * Added support for `cloudflare_api_token` in `databricks_storage_credential` resource ([#3835](#3835)). * Add `active` attribute to `databricks_user` data source ([#3733](#3733)). * Add `workspace_path` attribute to `databricks_notebook` resource and data source ([#3885](#3885)). * Mark attributes as sensitive in `databricks_mlflow_webhook` ([#3825](#3825)). * Added notification destination resource ([#3820](#3820)). ### Bug Fixes * Automatically assign `IS_OWNER` permission to sql warehouse if not specified ([#3829](#3829)). * Corrected kms arn format in `data_aws_unity_catalog_policy` ([#3823](#3823)). * Fix crash when destroying `databricks_compliance_security_profile_workspace_setting` ([#3883](#3883)). * Fixed read method of `databricks_entitlements` resource ([#3858](#3858)). * Retry cluster update on "INVALID_STATE" ([#3890](#3890)). * Save Pipeline resource to state in addition to spec ([#3869](#3869)). * Tolerate `databricks_workspace_conf` deletion failures ([#3737](#3737)). * Update Go SDK ([#3826](#3826)). * cluster key update for `databricks_sql_table` should not force new ([#3824](#3824)). * reading `databricks_metastore_assignment` when importing resource ([#3827](#3827)). ### Documentation * Add troubleshooting instructions for `databricks OAuth is not supported for this host` error ([#3815](#3815)). * Clarify setting of permissions for workspace objects ([#3884](#3884)). * Document missing task attributes in `databricks_job` resource ([#3817](#3817)). * Fixed documentation for `databricks_schemas` data source and `databricks_metastore_assignment` resource ([#3851](#3851)). * clarified `spot_bid_max_price` option for `databricks_cluster` ([#3830](#3830)). * marked `databricks_sql_dashboard` as legacy ([#3836](#3836)). ### Internal Changes * Refactor exporter: split huge files into smaller ones ([#3870](#3870)). * Refactored `client.ClientForHost` to use Go SDK method ([#3735](#3735)). * Revert "Rewriting DLT pipelines using SDK" ([#3838](#3838)). * Rewrite DLT pipelines using SDK ([#3839](#3839)). * Rewriting DLT pipelines using SDK ([#3792](#3792)). * Update Go SDK ([#3808](#3808)). * refactored `databricks_mws_permission_assignment` to Go SDK ([#3831](#3831)). ### Dependency Updates * Bump databricks-sdk-go to 0.44.0 ([#3896](#3896)). * Bump github.com/zclconf/go-cty from 1.14.4 to 1.15.0 ([#3775](#3775)). ### Exporter * Add retry on "Operation timed out" error ([#3897](#3897)). * Add support for Vector Search assets ([#3828](#3828)). * Add support for `databricks_notification_destination` ([#3861](#3861)). * Add support for `databricks_online_table` ([#3816](#3816)). * Don't export model serving endpoints with foundational models ([#3845](#3845)). * Fix generation of `autotermination_minutes = 0` ([#3881](#3881)). * Generate `databricks_workspace_binding` instead of legacy `databricks_catalog_workspace_binding` ([#3812](#3812)). * Ignore DLT pipelines deployed via DABs ([#3857](#3857)). * Improve exporting of `databricks_model_serving` ([#3821](#3821)). * Refactoring: remove legacy code ([#3864](#3864)).

### New Features and Improvements * Added `databricks_notification_destination` resource ([#3820](#3820)). * Added support for `cloudflare_api_token` in `databricks_storage_credential` resource ([#3835](#3835)). * Add `active` attribute to `databricks_user` data source ([#3733](#3733)). * Add `workspace_path` attribute to `databricks_notebook` resource and data source ([#3885](#3885)). * Mark attributes as sensitive in `databricks_mlflow_webhook` ([#3825](#3825)). ### Bug Fixes * Automatically assign `IS_OWNER` permission to sql warehouse if not specified ([#3829](#3829)). * Corrected kms arn format in `data_aws_unity_catalog_policy` ([#3823](#3823)). * Fix crash when destroying `databricks_compliance_security_profile_workspace_setting` ([#3883](#3883)). * Fixed read method of `databricks_entitlements` resource ([#3858](#3858)). * Retry cluster update on "INVALID_STATE" ([#3890](#3890)). * Save Pipeline resource to state in addition to spec ([#3869](#3869)). * Tolerate `databricks_workspace_conf` deletion failures ([#3737](#3737)). * Update Go SDK ([#3826](#3826)). * cluster key update for `databricks_sql_table` should not force new ([#3824](#3824)). * reading `databricks_metastore_assignment` when importing resource ([#3827](#3827)). ### Documentation * Add troubleshooting instructions for `databricks OAuth is not supported for this host` error ([#3815](#3815)). * Clarify setting of permissions for workspace objects ([#3884](#3884)). * Document missing task attributes in `databricks_job` resource ([#3817](#3817)). * Fixed documentation for `databricks_schemas` data source and `databricks_metastore_assignment` resource ([#3851](#3851)). * clarified `spot_bid_max_price` option for `databricks_cluster` ([#3830](#3830)). * marked `databricks_sql_dashboard` as legacy ([#3836](#3836)). ### Internal Changes * Refactor exporter: split huge files into smaller ones ([#3870](#3870)). * Refactored `client.ClientForHost` to use Go SDK method ([#3735](#3735)). * Revert "Rewriting DLT pipelines using SDK" ([#3838](#3838)). * Rewrite DLT pipelines using SDK ([#3839](#3839)). * Rewriting DLT pipelines using SDK ([#3792](#3792)). * Update Go SDK ([#3808](#3808)). * refactored `databricks_mws_permission_assignment` to Go SDK ([#3831](#3831)). ### Dependency Updates * Bump databricks-sdk-go to 0.44.0 ([#3896](#3896)). * Bump github.com/zclconf/go-cty from 1.14.4 to 1.15.0 ([#3775](#3775)). ### Exporter * Add retry on "Operation timed out" error ([#3897](#3897)). * Add support for Vector Search assets ([#3828](#3828)). * Add support for `databricks_notification_destination` ([#3861](#3861)). * Add support for `databricks_online_table` ([#3816](#3816)). * Don't export model serving endpoints with foundational models ([#3845](#3845)). * Fix generation of `autotermination_minutes = 0` ([#3881](#3881)). * Generate `databricks_workspace_binding` instead of legacy `databricks_catalog_workspace_binding` ([#3812](#3812)). * Ignore DLT pipelines deployed via DABs ([#3857](#3857)). * Improve exporting of `databricks_model_serving` ([#3821](#3821)). * Refactoring: remove legacy code ([#3864](#3864)).

Divyansh-db added 3 commits July 24, 2024 17:33

WIP

ff7fdd1

Merge branch 'main' into divyansh_notification_destinations

5735002

Made Notification Destination resource

209487e

Divyansh-db requested a review from hectorcast-db July 25, 2024 08:49

Divyansh-db changed the title ~~[FEATURE] Notification Destination resource~~ [Feature] Notification Destination resource Jul 25, 2024

Divyansh-db requested a review from alexott July 25, 2024 08:53

Divyansh-db added 2 commits July 25, 2024 11:19

added some sensitive fields

36f845e

added documentation and improved integration test

e7b7381

Divyansh-db marked this pull request as ready for review July 25, 2024 12:48

Divyansh-db requested review from a team as code owners July 25, 2024 12:48

alexott reviewed Jul 25, 2024

View reviewed changes

alexott requested changes Jul 25, 2024

View reviewed changes

settings/resource_notification_destination.go Outdated Show resolved Hide resolved

wip

3a2efbd

nkvuong reviewed Jul 26, 2024

View reviewed changes

docs/resources/notification_destination.md Outdated Show resolved Hide resolved

Divyansh-db and others added 2 commits July 26, 2024 14:01

Update docs/resources/notification_destination.md

2a9d9f3

Co-authored-by: vuong-nguyen <44292934+nkvuong@users.noreply.github.com>

Merge branch 'Divyansh-db/divyansh_notification_destinations' of gith…

a7a5086

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

alexott reviewed Jul 26, 2024

View reviewed changes

internal/acceptance/notification_destination_test.go Show resolved Hide resolved

Divyansh-db added 2 commits July 29, 2024 09:22

wip

2eb0d15

Fixed the resource and added integration tests for all fields

7f5de75

Divyansh-db requested a review from alexott July 29, 2024 11:29

Divyansh-db and others added 3 commits July 29, 2024 14:06

fixed the resource code

17b0463

Added support for changing Destination type

726b346

Merge branch 'main' into Divyansh-db/divyansh_notification_destinations

0a97a21

Divyansh-db requested a review from nkvuong July 29, 2024 15:44

alexott requested changes Jul 29, 2024

View reviewed changes

Divyansh-db added 2 commits July 31, 2024 10:51

wip

80c1d59

modified doc for notification destination

adc6f7b

Divyansh-db and others added 5 commits July 31, 2024 13:10

merged main

1cd8f71

added force send fields

ea6ceb5

Merge branch 'main' into Divyansh-db/divyansh_notification_destinations

db3f2ea

fixed a unit test

ae1c4f1

Merge branch 'Divyansh-db/divyansh_notification_destinations' of gith…

85441c7

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

Divyansh-db requested a review from alexott July 31, 2024 12:38

Merge branch 'main' into Divyansh-db/divyansh_notification_destinations

44ce52d

alexott approved these changes Aug 1, 2024

View reviewed changes

internal/acceptance/notification_destination_test.go Outdated Show resolved Hide resolved

docs/resources/notification_destination.md Outdated Show resolved Hide resolved

Divyansh-db added 2 commits August 1, 2024 15:49

modified docs and integration tests

cc9cc4d

Merge branch 'Divyansh-db/divyansh_notification_destinations' of gith…

78f0434

…ub.com:databricks/terraform-provider-databricks into divyansh_notification_destinations

Divyansh-db added this pull request to the merge queue Aug 1, 2024

Merged via the queue into main with commit 5258611 Aug 1, 2024
6 checks passed

Divyansh-db deleted the Divyansh-db/divyansh_notification_destinations branch August 1, 2024 13:56

alexott mentioned this pull request Aug 1, 2024

[FEATURE] Support creating notification destinations #1983

Closed

mgyucht mentioned this pull request Aug 14, 2024

[Release] Release v1.50.0 #3900

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Notification Destination resource #3820

[Feature] Notification Destination resource #3820

Divyansh-db commented Jul 25, 2024 •

edited

Loading

alexott Jul 25, 2024

alexott left a comment

alexott commented Jul 25, 2024 •

edited

Loading

Divyansh-db commented Jul 29, 2024 •

edited

Loading

alexott Jul 29, 2024

Divyansh-db Jul 30, 2024

alexott Jul 30, 2024

Divyansh-db Jul 31, 2024 •

edited

Loading

Divyansh-db Jul 31, 2024

alexott left a comment

[Feature] Notification Destination resource #3820

[Feature] Notification Destination resource #3820

Conversation

Divyansh-db commented Jul 25, 2024 • edited Loading

Changes

alexott Jul 25, 2024

Choose a reason for hiding this comment

alexott left a comment

Choose a reason for hiding this comment

alexott commented Jul 25, 2024 • edited Loading

Divyansh-db commented Jul 29, 2024 • edited Loading

alexott Jul 29, 2024

Choose a reason for hiding this comment

Divyansh-db Jul 30, 2024

Choose a reason for hiding this comment

alexott Jul 30, 2024

Choose a reason for hiding this comment

Divyansh-db Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Divyansh-db Jul 31, 2024

Choose a reason for hiding this comment

alexott left a comment

Choose a reason for hiding this comment

Divyansh-db commented Jul 25, 2024 •

edited

Loading

alexott commented Jul 25, 2024 •

edited

Loading

Divyansh-db commented Jul 29, 2024 •

edited

Loading

Divyansh-db Jul 31, 2024 •

edited

Loading