`azurerm_datadog_monitor_tag_rule` - correctly handle default rule #22806

neil-yechenwei · 2023-08-03T11:46:41Z

As datadog monitor tag rule is always there, so we don't need to check if it exists.

--- PASS: TestAccDatadogMonitorTagRules_basic (221.01s)
--- PASS: TestAccDatadogMonitorTagRules_update (340.70s)
--- PASS: TestAccDatadogMonitorTagRules_requiresImport (237.15s)

tombuildsstuff

hi @neil-yechenwei

Requires Import is a core feature of the provider which works around an issue in the Azure API where the user specified name is the unique identifier - as such it needs to be present in every resource otherwise this'd mean that we'd end up unintentionally adopting configured Tag Rules. Instead, since this resource always exists - we can fix this by updating the Requires Import check to confirm if there's any Tag Rules configured (i.e. not the default) and then raise the "requires import" error here - can we update this to handle that?

Thanks!

tombuildsstuff · 2023-08-03T11:53:15Z

internal/services/datadog/datadog_monitor_tag_rule_resource.go

-	existing, err := client.TagRulesGet(ctx, id)
-	if err != nil {
-		if !response.WasNotFound(existing.HttpResponse) {
-			return fmt.Errorf("checking for an existing %s: %+v", id, err)
-		}
-	}
-	if !response.WasNotFound(existing.HttpResponse) {
-		return tf.ImportAsExistsError("azurerm_datadog_monitor_tag_rule", id.ID())
-	}


This is a core behavioural feature of the provider, so we shouldn't be requiring the requiredImports check, instead we can update it to confirm if this is configured with the default settings, and raise the requiredImports error if so - can we update this?

I updated the code to check if the "Tag Rule" exists or the "Tag Rule" is with default settings.

Service team confirmed that below behaviors are expected.

Datadog Monitor Tag Rule only allows name "default".

For the completely new Datadog Monitor, there is no existing Datadog Monitor Tag Rule under it.

For the recreated Datadog Monitor with same name, there always is an existing default Datadog Monitor Tag Rule under it.

Service API doesn't support to delete existing Datadog Monitor Tag Rule.

tombuildsstuff · 2023-08-03T11:53:25Z

internal/services/datadog/datadog_monitor_tag_rule_resource_test.go

-func TestAccDatadogMonitorTagRules_requiresImport(t *testing.T) {
-	data := acceptance.BuildTestData(t, "azurerm_datadog_monitor_tag_rule", "test")
-	r := TagRulesDatadogMonitorResource{}
-	r.populateFromEnvironment(t)
-	data.ResourceTest(t, r, []acceptance.TestStep{
-		{
-			Config: r.basic(data),
-			Check: acceptance.ComposeTestCheckFunc(
-				check.That(data.ResourceName).ExistsInAzure(r),
-			),
-		},
-		data.RequiresImportErrorStep(r.requiresImport),
-	})
-}


can we revert this?

I reverted it.

tombuildsstuff · 2023-08-03T11:53:46Z

internal/services/datadog/datadog_monitor_tag_rule_resource_test.go

-func (r TagRulesDatadogMonitorResource) requiresImport(data acceptance.TestData) string {
-	return fmt.Sprintf(`
-%s
-
-resource "azurerm_datadog_monitor_tag_rule" "import" {
-  datadog_monitor_id = azurerm_datadog_monitor_tag_rule.test.datadog_monitor_id
-  name               = azurerm_datadog_monitor_tag_rule.test.name
-  log {
-    subscription_log_enabled = true
-  }
-  metric {
-    filter {
-      name   = "Test"
-      value  = "Testing-Logs"
-      action = "Include"
-    }
-  }
-}
-`, r.basic(data))
-}
-


can we revert this?

I reverted it.

tombuildsstuff · 2023-08-03T11:53:55Z

internal/services/datadog/datadog_monitor_tag_rule_resource_test.go

 	return fmt.Sprintf(`
-provider "azurerm" {
-  features {}
-}
-


these should be specified in each test, can we revert this?

It has been defined in template func.

provider blocks should not be defined in the template function and should be specified in each test - can we revert this?

Currently, in main branch, provider block is defined in both template function and each test. So I assume this is a bug. So I removed provider block from template function otherwise test case would fail.

tombuildsstuff · 2023-09-05T11:54:51Z

internal/services/datadog/datadog_monitor_tag_rule_resource.go

+	if name := input.Name; name != nil && *input.Name == "default" {
+		if props := input.Properties; props != nil {
+			logRules := props.LogRules
+			metricRules := props.MetricRules
+
+			if logRules != nil && metricRules != nil {
+				if (logRules.SendAadLogs != nil && !*logRules.SendAadLogs) && (logRules.SendSubscriptionLogs != nil && !*logRules.SendSubscriptionLogs) && (logRules.SendResourceLogs != nil && !*logRules.SendResourceLogs) && (logRules.FilteringTags != nil && len(*logRules.FilteringTags) == 0) && (metricRules.FilteringTags != nil && len(*metricRules.FilteringTags) == 0) {
+					return true
+				}
+			}
+		}
+	}


we can simplify this by shifting left, for example:

Suggested change

if name := input.Name; name != nil && *input.Name == "default" {

if props := input.Properties; props != nil {

logRules := props.LogRules

metricRules := props.MetricRules

if logRules != nil && metricRules != nil {

if (logRules.SendAadLogs != nil && !*logRules.SendAadLogs) && (logRules.SendSubscriptionLogs != nil && !*logRules.SendSubscriptionLogs) && (logRules.SendResourceLogs != nil && !*logRules.SendResourceLogs) && (logRules.FilteringTags != nil && len(*logRules.FilteringTags) == 0) && (metricRules.FilteringTags != nil && len(*metricRules.FilteringTags) == 0) {

return true

}

}

}

}

if input.Name == nil || strings.EqualsFold(*input.Name, "default") {

return false

}

if input.Properties == nil || input.Properties.LogRules == nil || input.Properties.MetricRules == nil {

return false

}

logRules := input.Properties.LogRules

metricRules := input.Properties.MetricRules

result := logRules.SendAadLogs != nil && !*logRules.SendAadLogs) &&

(logRules.SendSubscriptionLogs != nil && !*logRules.SendSubscriptionLogs) &&

(logRules.SendResourceLogs != nil && !*logRules.SendResourceLogs) &&

(logRules.FilteringTags != nil && len(*logRules.FilteringTags) == 0) &&

(metricRules.FilteringTags != nil && len(*metricRules.FilteringTags) == 0)

return result

tombuildsstuff · 2023-09-05T11:55:29Z

internal/services/datadog/datadog_monitor_tag_rule_resource_test.go

 	return fmt.Sprintf(`
-provider "azurerm" {
-  features {}
-}
-


provider blocks should not be defined in the template function and should be specified in each test - can we revert this?

neil-yechenwei · 2023-09-06T05:30:36Z

@tombuildsstuff ,thanks for the comment. I updated PR. Please take another look. Thanks.

jackofallops

Hi @neil-yechenwei - One comment to take a look at below, and then I think this should be good to merge.

Thanks

jackofallops · 2023-11-06T10:39:20Z

internal/services/datadog/datadog_monitor_tag_rule_resource.go

+	if input.Name == nil || !strings.EqualFold(*input.Name, "default") {
+		return false
+	}


I think this should be removed? The name property defaults to default, so could have other settings that have changed thus making this an incorrect assumption?

Yes. It is removed.

neil-yechenwei · 2023-11-07T02:57:12Z

Hi @jackofallops ,thanks for the comment. I updated PR. Please take another look. Thanks.

All related test cases passed after rerun.
--- PASS: TestAccDatadogMonitorTagRules_basic (239.60s)
--- PASS: TestAccDatadogMonitorTagRules_requiresImport (185.21s)
--- PASS: TestAccDatadogMonitorTagRules_update (345.38s)

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>"hashicorp/azurerm" updated from "3.82.0" to "3.83.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.83.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.83.0
UPGRADE NOTES

* Key Vaults are now loaded using [the `ListBySubscription` API within the Key Vault Resource Provider](https://learn.microsoft.com/en-us/rest/api/keyvault/keyvault/vaults/list-by-subscription?view=rest-keyvault-keyvault-2022-07-01&tabs=HTTP) rather than [the Resources API](https://learn.microsoft.com/en-us/rest/api/keyvault/keyvault/vaults/list?view=rest-keyvault-keyvault-2022-07-01&tabs=HTTP). This change means that the Provider now caches the list of Key Vaults available within a Subscription, rather than loading these piecemeal to workaround stale data returned from the Resources API ([#24019](https://github.com/hashicorp/terraform-provider-azurerm/issues/24019))

FEATURES:

* New Data Source: `azurerm_stack_hci_cluster` ([#24032](https://github.com/hashicorp/terraform-provider-azurerm/issues/24032))

ENHANCEMENTS:

* dependencies: updating to `v0.20231129.1103252` of `github.com/hashicorp/go-azure-sdk` ([#24063](hashicorp/terraform-provider-azurerm#24063 `automation`: updating to API Version `2023-11-01` ([#24017](hashicorp/terraform-provider-azurerm#24017 `keyvault`: the cache is now populated using the `ListBySubscription` endpoint on the KeyVault Resource Provider rather than via the `Resources` API ([#24019](hashicorp/terraform-provider-azurerm#24019 `keyvault`: updating the cache to populate all Key Vaults available within the Subscription to reduce the number of API calls ([#24019](hashicorp/terraform-provider-azurerm#24019 Data Source `azurerm_private_dns_zone`: refactoring to use the `ListBySubscription` API rather than the Resources API when `resource_group_name` is omitted ([#24024](hashicorp/terraform-provider-azurerm#24024 `azurerm_dashboard_grafana` - support for `grafana_major_version` ([#24014](hashicorp/terraform-provider-azurerm#24014 `azurerm_linux_web_app` - add support for dotnet 8 ([#23893](hashicorp/terraform-provider-azurerm#23893 `azurerm_linux_web_app_slot` - add support for dotnet 8 ([#23893](hashicorp/terraform-provider-azurerm#23893 `azurerm_media_transform` - deprecate `face_detector_preset` and `video_analyzer_preset` ([#24002](hashicorp/terraform-provider-azurerm#24002 `azurerm_postgresql_database` - update the validation of `collation` to include `Norwegian_Norway.1252` ([#24070](hashicorp/terraform-provider-azurerm#24070 `azurerm_postgresql_flexible_server` - updating to API Version `2023-06-01-preview` ([#24016](hashicorp/terraform-provider-azurerm#24016 `azurerm_redis_cache` - support for the `active_directory_authentication_enabled` property ([#23976](hashicorp/terraform-provider-azurerm#23976 `azurerm_windows_web_app` - add support for dotnet 8 ([#23893](hashicorp/terraform-provider-azurerm#23893 `azurerm_windows_web_app_slot` - add support for dotnet 8 ([#23893](hashicorp/terraform-provider-azurerm#23893 `azurerm_storage_account` - add `name` validation in custom diff ([#23799](https://github.com/hashicorp/terraform-provider-azurerm/issues/23799))

BUG FIXES:

* authentication: fix a bug where auxiliary tenants were not correctly authorized ([#24063](hashicorp/terraform-provider-azurerm#24063 `azurerm_app_configuration` - normalize location in `replica` block ([#24074](hashicorp/terraform-provider-azurerm#24074 `azurerm_cosmosdb_account` - cosmosdb version and capabilities can now be updated at the same time ([#24029](hashicorp/terraform-provider-azurerm#24029 `azurerm_data_factory_flowlet_data_flow` - `source` and `sink` properties are now optional ([#23987](hashicorp/terraform-provider-azurerm#23987 `azurerm_datadog_monitor_tag_rule` - correctly handle default rule ([#22806](hashicorp/terraform-provider-azurerm#22806 `azurerm_ip_group`: fixing a crash when `firewall_ids` and `firewall_policy_ids` weren't parsed correctly from the API Response ([#24031](hashicorp/terraform-provider-azurerm#24031 `azurerm_nginx_deployment` - add default value of `20` for `capacity` ([#24033](https://github.com/hashicorp/terraform-provider-azurerm/issues/24033))


</pre> </details> <details> <summary>3.84.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.84.0
FEATURES:

* **New Data Source:** `azurerm_storage_containers` ([#24061](hashicorp/terraform-provider-azurerm#24061 **New Resource:** `azurerm_elastic_san` ([#23619](hashicorp/terraform-provider-azurerm#23619 **New Resource:** `azurerm_key_vault_managed_hardware_security_module_role_assignment` ([#22332](hashicorp/terraform-provider-azurerm#22332 **New Resource:** `azurerm_key_vault_managed_hardware_security_module_role_definition` ([#22332](https://github.com/hashicorp/terraform-provider-azurerm/issues/22332))

ENHANCEMENTS:

* dependencies: updating mssql elasticpools from `v5.0` to `2023-05-01-preview`
* dependencies: updating to `v0.20231207.1122031` of `github.com/hashicorp/go-azure-sdk` ([#24149](hashicorp/terraform-provider-azurerm#24149 Data Source: `azurerm_storage_account` - export the primary and secondary internet and microsoft hostnames for blobs, dfs, files, queues, tables and web ([#23517](hashicorp/terraform-provider-azurerm#23517 Data Source: `azurerm_cosmosdb_account` - export the `connection_strings`, `primary_sql_connection_string`, `secondary_sql_connection_string`, `primary_readonly_sql_connection_string`, `secondary_readonly_sql_connection_string`, `primary_mongodb_connection_string`, `secondary_mongodb_connection_string`, `primary_readonly_mongodb_connection_string`, and `secondary_readonly_mongodb_connection_string` attributes ([#24129](hashicorp/terraform-provider-azurerm#24129 `azurerm_bot_service_azure_bot` - support for the `public_network_access_enabled` property ([#24125](hashicorp/terraform-provider-azurerm#24125 `azurerm_container_app_environment` - support for the `workload_profile` property ([#23478](hashicorp/terraform-provider-azurerm#23478 `azurerm_cosmosdb_cassandra_datacenter` - support for the `seed_node_ip_addresses` property ([#24076](hashicorp/terraform-provider-azurerm#24076 `azurerm_firewall` - support for the `dns_proxy_enabled` property ([#20519](hashicorp/terraform-provider-azurerm#20519 `azurerm_kubernetes_cluster` - support for the `support_plan` property and the `sku_tier` `Premium` ([#23970](hashicorp/terraform-provider-azurerm#23970 `azurerm_mssql_database` - support for `enclave_type` field ([#24054](hashicorp/terraform-provider-azurerm#24054 `azurerm_mssql_elasticpool` - support for `enclave_type` field ([#24054](hashicorp/terraform-provider-azurerm#24054 `azurerm_mssql_managed_instance` - support for more `vcores`: `6`, `10`, `12`, `20`, `48`, `56`, `96`, `128` ([#24085](hashicorp/terraform-provider-azurerm#24085 `azurerm_redis_linked_server` - support for the property `geo_replicated_primary_host_name` ([#23984](hashicorp/terraform-provider-azurerm#23984 `azurerm_storage_account` - expose the primary and secondary internet and microsoft hostnames for blobs, dfs, files, queues, tables and web ([#23517](hashicorp/terraform-provider-azurerm#23517 `azurerm_synapse_role_assignment` - support for the `principal_type` property ([#24089](hashicorp/terraform-provider-azurerm#24089 `azurerm_spring_cloud_build_deployment` - support for the `application_performance_monitoring_ids` property ([#23969](hashicorp/terraform-provider-azurerm#23969 `azurerm_virtual_network_gateway` - support for the `bgp_route_translation_for_nat_enabled`, `dns_forwarding_enabled`, `ip_sec_replay_protection_enabled`, `remote_vnet_traffic_enabled`, `virtual_wan_traffic_enabled`, `radius_server`, `virtual_network_gateway_client_connection`, `policy_group`, and `ipsec_policy` property ([#23220](https://github.com/hashicorp/terraform-provider-azurerm/issues/23220))

BUG FIXES:

* `azurerm_application_insights_api_key` - prevent a bug where multiple keys couldn't be created for an Application Insights instance ([#23463](hashicorp/terraform-provider-azurerm#23463 `azurerm_container_registry` - the `network_rule_set.virtual_network` property has been deprecated ([#24140](hashicorp/terraform-provider-azurerm#24140 `azurerm_hdinsight_hadoop_cluster` - set `roles.edge_node.install_script_action.parameters` into state by retrieving the value provided in the user config since this property isn't returned by the API ([#23971](hashicorp/terraform-provider-azurerm#23971 `azurerm_kubernetes_cluster` - prevent a bug where maintenance window start date was always recalculated and sent to the API ([#23985](hashicorp/terraform-provider-azurerm#23985 `azurerm_mssql_database` - will no longer send all long retention values in payload unless set ([#24124](hashicorp/terraform-provider-azurerm#24124 `azurerm_mssql_managed_database` - will no longer send all long retention values in payload unless set ([#24124](hashicorp/terraform-provider-azurerm#24124 `azurerm_mssql_server_microsoft_support_auditing_policy` - only include storage endpoint in payload if set ([#24122](hashicorp/terraform-provider-azurerm#24122 `azurerm_mobile_network_packet_core_control_plane` - prevent a panic if the HTTP Response is nil ([#24083](hashicorp/terraform-provider-azurerm#24083 `azurerm_storage_account` - revert plan time name validation `(#23799)` ([#24142](hashicorp/terraform-provider-azurerm#24142 `azurerm_web_application_firewall_policy` - split create and update function to fix lifecycle - ignore changes ([#23412](https://github.com/hashicorp/terraform-provider-azurerm/issues/23412))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/terraform-jobs/job/azure/job/main/931/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> --------- Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]> Co-authored-by: Damien Duportal <[email protected]>

github-actions · 2024-05-05T02:11:58Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

neil-yechenwei added 2 commits August 3, 2023 16:23

update code

6aab4d4

update code

89294bf

github-actions bot added service/datadog size/S waiting-response labels Aug 3, 2023

tombuildsstuff suggested changes Aug 3, 2023

View reviewed changes

update code

c1d9203

github-actions bot added size/M and removed waiting-response size/S labels Aug 3, 2023

update code

f8b1593

github-actions bot added size/S and removed size/M labels Aug 4, 2023

neil-yechenwei added 4 commits August 4, 2023 19:00

update code

8598b5e

update code

2533a65

update code

f23be0f

update code

39e1044

neil-yechenwei requested a review from tombuildsstuff August 4, 2023 12:42

tombuildsstuff suggested changes Sep 5, 2023

View reviewed changes

github-actions bot added the waiting-response label Sep 5, 2023

update code

e9a4c83

github-actions bot removed the waiting-response label Sep 6, 2023

neil-yechenwei requested a review from tombuildsstuff September 6, 2023 05:30

jackofallops requested changes Nov 6, 2023

View reviewed changes

github-actions bot added the waiting-response label Nov 6, 2023

update code

3fc673a

github-actions bot removed the waiting-response label Nov 7, 2023

neil-yechenwei requested a review from jackofallops November 7, 2023 02:57

jackofallops changed the title ~~azurerm_datadog_monitor_tag_rule - remove nil check~~ azurerm_datadog_monitor_tag_rule - correctly handle default rule Nov 30, 2023

jackofallops merged commit 277bf51 into hashicorp:main Nov 30, 2023
21 checks passed

github-actions bot added this to the v3.83.0 milestone Nov 30, 2023

jackofallops added a commit that referenced this pull request Nov 30, 2023

Update CHANGELOG.md for #22806

9be911e

github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_datadog_monitor_tag_rule` - correctly handle default rule #22806

`azurerm_datadog_monitor_tag_rule` - correctly handle default rule #22806

neil-yechenwei commented Aug 3, 2023 •

edited

Loading

tombuildsstuff left a comment

tombuildsstuff Aug 3, 2023

neil-yechenwei Aug 4, 2023 •

edited

Loading

tombuildsstuff Aug 3, 2023

neil-yechenwei Aug 4, 2023

tombuildsstuff Aug 3, 2023

neil-yechenwei Aug 4, 2023

tombuildsstuff Aug 3, 2023

neil-yechenwei Aug 4, 2023

tombuildsstuff Sep 5, 2023

neil-yechenwei Sep 6, 2023

tombuildsstuff Sep 5, 2023

neil-yechenwei Sep 6, 2023

tombuildsstuff Sep 5, 2023

neil-yechenwei commented Sep 6, 2023

jackofallops left a comment

jackofallops Nov 6, 2023

neil-yechenwei Nov 7, 2023

neil-yechenwei commented Nov 7, 2023 •

edited

Loading

github-actions bot commented May 5, 2024

azurerm_datadog_monitor_tag_rule - correctly handle default rule #22806

azurerm_datadog_monitor_tag_rule - correctly handle default rule #22806

Conversation

neil-yechenwei commented Aug 3, 2023 • edited Loading

tombuildsstuff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei commented Sep 6, 2023

jackofallops left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-yechenwei commented Nov 7, 2023 • edited Loading

github-actions bot commented May 5, 2024

`azurerm_datadog_monitor_tag_rule` - correctly handle default rule #22806

`azurerm_datadog_monitor_tag_rule` - correctly handle default rule #22806

neil-yechenwei commented Aug 3, 2023 •

edited

Loading

neil-yechenwei Aug 4, 2023 •

edited

Loading

neil-yechenwei commented Nov 7, 2023 •

edited

Loading