[Bug:] `azurerm_cosmosdb_account` - add default values for the `consistency_policy` fields and fix regression in the `backup` code block #24830

WodansSon · 2024-02-09T02:26:02Z

No description provided.

manicminer

Hey @WodansSon, thanks for the PR. This mostly LGTM, just a few minor linting/naming comments if you could take a look at those. Thanks!

manicminer · 2024-02-14T12:59:11Z

internal/services/cosmos/cosmosdb_account_resource.go

@@ -1907,28 +1909,37 @@ func expandCosmosdbAccountBackup(input []interface{}, backupHasChange bool, crea
 	switch attr["type"].(string) {
 	case string(cosmosdb.BackupPolicyTypeContinuous):
 		if v := attr["interval_in_minutes"].(int); v != 0 && !backupHasChange {
-			return nil, fmt.Errorf("`interval_in_minutes` can not be set when `type` in `backup` is `Continuous`")
+			return nil, fmt.Errorf("`interval_in_minutes` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))


We won't need to convert to string for Printf

Suggested change

return nil, fmt.Errorf("`interval_in_minutes` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))

return nil, fmt.Errorf("`interval_in_minutes` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

manicminer · 2024-02-14T12:59:33Z

internal/services/cosmos/cosmosdb_account_resource.go

 		if v := attr["retention_in_hours"].(int); v != 0 && !backupHasChange {
-			return nil, fmt.Errorf("`retention_in_hours` can not be set when `type` in `backup` is `Continuous`")
+			return nil, fmt.Errorf("`retention_in_hours` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))


Suggested change

return nil, fmt.Errorf("`retention_in_hours` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))

return nil, fmt.Errorf("`retention_in_hours` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

manicminer · 2024-02-14T12:59:46Z

internal/services/cosmos/cosmosdb_account_resource.go

 		if v := attr["storage_redundancy"].(string); v != "" && !backupHasChange {
-			return nil, fmt.Errorf("`storage_redundancy` can not be set when `type` in `backup` is `Continuous`")
+			return nil, fmt.Errorf("`storage_redundancy` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))


Suggested change

return nil, fmt.Errorf("`storage_redundancy` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))

return nil, fmt.Errorf("`storage_redundancy` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

manicminer · 2024-02-14T13:00:10Z

internal/services/cosmos/cosmosdb_account_resource.go

 		return cosmosdb.ContinuousModeBackupPolicy{}, nil

 	case string(cosmosdb.BackupPolicyTypePeriodic):
 		if createMode != "" {
-			return nil, fmt.Errorf("`create_mode` only works when `backup.type` is `Continuous`")
+			return nil, fmt.Errorf("`create_mode` can only be defined when the `backup.type` is set to %q, got %q", string(cosmosdb.BackupPolicyTypeContinuous), string(cosmosdb.BackupPolicyTypePeriodic))


Suggested change

return nil, fmt.Errorf("`create_mode` can only be defined when the `backup.type` is set to %q, got %q", string(cosmosdb.BackupPolicyTypeContinuous), string(cosmosdb.BackupPolicyTypePeriodic))

return nil, fmt.Errorf("`create_mode` can only be defined when the `backup.type` is set to %q, got %q", cosmosdb.BackupPolicyTypeContinuous, cosmosdb.BackupPolicyTypePeriodic)

manicminer · 2024-02-14T13:03:22Z

internal/services/cosmos/cosmosdb_account_resource_test.go

+func TestAccCosmosDBAccount_icmRegression(t *testing.T) {
+	// Regression test for the Microsoft IcM 466814988...


Could this test have a more descriptive name? We can also leave out the IcM number comment as these are MSFT-internal.

manicminer · 2024-02-14T13:09:41Z

website/docs/r/cosmosdb_account.html.markdown

@@ -194,7 +194,7 @@ The `consistency_policy` block Configures the database consistency and supports

 * `max_staleness_prefix` - (Optional) When used with the Bounded Staleness consistency level, this value represents the number of stale requests tolerated. The accepted range for this value is `10` – `2147483647`. Defaults to `100`. Required when `consistency_level` is set to `BoundedStaleness`.

-~> **Note:** `max_interval_in_seconds` and `max_staleness_prefix` can only be set to custom values when `consistency_level` is set to `BoundedStaleness` - otherwise they will return the default values shown above.
+~> **NOTE:** `max_interval_in_seconds` and `max_staleness_prefix` can only be set to values other than default when the `consistency_level` is set to `BoundedStaleness`.


(capitalization here is legacy pattern)

Suggested change

~> **NOTE:** `max_interval_in_seconds` and `max_staleness_prefix` can only be set to values other than default when the `consistency_level` is set to `BoundedStaleness`.

~> **Note** `max_interval_in_seconds` and `max_staleness_prefix` can only be set to values other than default when the `consistency_level` is set to `BoundedStaleness`.

manicminer · 2024-02-14T13:11:52Z

website/docs/r/cosmosdb_account.html.markdown


-* `interval_in_minutes` - (Optional) The interval in minutes between two backups. This is configurable only when `type` is `Periodic`. Possible values are between 60 and 1440.
+~> **NOTE:** Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.


Suggested change

~> **NOTE:** Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.

~> **Note** Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.

manicminer · 2024-02-14T13:15:55Z

website/docs/r/cosmosdb_account.html.markdown

+
+* `storage_redundancy` - (Optional) The storage redundancy is used to indicate the type of backup residency. Possible values are `Geo`, `Local` and `Zone`. Defaults to `Geo`.
+
+~> **NOTE:** You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.


Suggested change

~> **NOTE:** You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.

~> **Note** You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.

katbyte

LGTM 🛻

WodansSon · 2024-02-15T01:33:43Z

Failing test cases are not related to this PR:

<Actions> <action id="f410411e63aff4bb73a81c2aec1d373cf8a903e63b30dee2006b0030d8a94cc8"> <h3>Bump Terraform `azurerm` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azurerm" updated from "3.91.0" to "3.92.0" in file ".terraform.lock.hcl"</p> <details> <summary>3.92.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azurerm/releases/tag/v3.92.0
FEATURES:

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

ENHANCEMENTS:

* provider: support for the feature flag `postgresql_flexible_server.restart_server_on_configuration_value_change property` ([#23811](hashicorp/terraform-provider-azurerm#23811 dependencies: updating to v0.20240214.1142753 of `github.com/hashicorp/go-azure-sdk` ([#24889](hashicorp/terraform-provider-azurerm#24889 `automation`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24858](hashicorp/terraform-provider-azurerm#24858 `maintenance`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24819](hashicorp/terraform-provider-azurerm#24819 `containerapps`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24862](hashicorp/terraform-provider-azurerm#24862 `containerservices`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24872](hashicorp/terraform-provider-azurerm#24872 `timeseriesinsights`: updating to use the transport layer from `hashicorp/go-azure-sdk` rather than `Azure/go-autorest` ([#24889](hashicorp/terraform-provider-azurerm#24889 `azurerm_container_app_environment`: support for the `infrastructure_resource_group_name` property ([#24361](hashicorp/terraform-provider-azurerm#24361 `azurerm_cost_anomaly_alert` - support for the `subscription_id` property ([#24258](hashicorp/terraform-provider-azurerm#24258 `azurerm_cosmosdb_account` - add default values for the `consistency_policy` code block ([#24830](hashicorp/terraform-provider-azurerm#24830 `azurerm_dashboard_grafana` - support for the `smtp` block ([#24717](hashicorp/terraform-provider-azurerm#24717 `azurerm_key_vault_certificates` - support for the `tags` property ([#24857](hashicorp/terraform-provider-azurerm#24857 `azurerm_key_vault_secrets` - support for the `tags` property ([#24857](hashicorp/terraform-provider-azurerm#24857 `azurerm_orchestrated_virtual_machine_scale_set` - support for the `additional_unattend_content` block ([#24292](hashicorp/terraform-provider-azurerm#24292 `azurerm_virtual_desktop_host_pool` - support for the `vm_template` property ([#24369](https://github.com/hashicorp/terraform-provider-azurerm/issues/24369))

BUG FIXES:

* `azurerm_container_app_environment`: avoid unwanted changes when updating and using `log_analytics_workspace_id` ([#24303](hashicorp/terraform-provider-azurerm#24303 `azurerm_cosmosdb_account` - fixed regression in the `backup` code block ([#24830](hashicorp/terraform-provider-azurerm#24830 `azurerm_data_factory` - allow the `git_url` property to be blank/empty ([#24879](hashicorp/terraform-provider-azurerm#24879 `azurerm_linux_web_app_slot` - the `worker_count` property now works correctly in the `site_config` block ([#24515](hashicorp/terraform-provider-azurerm#24515 `azurerm_linux_web_app` - support `off` for the `file_system_level` property ([#24877](hashicorp/terraform-provider-azurerm#24877 `azurerm_linux_web_app_slot` - support `off` for the `file_system_level` property ([#24877](hashicorp/terraform-provider-azurerm#24877 `azurerm_private_endpoint` - fixing an issue where updating the Private Endpoint would remove any Application Security Group Association ([#24846](hashicorp/terraform-provider-azurerm#24846 `azurerm_search_service` - fixed the update function to adjust for changed API behaviour ([#24837](hashicorp/terraform-provider-azurerm#24837 `azurerm_search_service` - fixed the update function to adjust for changed API behaviour ([#24903](hashicorp/terraform-provider-azurerm#24903 `azurerm_windows_web_app` - support `off` for the `file_system_level` property ([#24877](hashicorp/terraform-provider-azurerm#24877 `azurerm_windows_web_app_slot` - support `off` for the `file_system_level` property ([#24877](https://github.com/hashicorp/terraform-provider-azurerm/issues/24877))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/3/">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]>

* Initial Check-in... * Add regression test case... * Change fix design... * Address PR comments... * Updated description of the regression test case...

github-actions · 2024-04-24T02:04:52Z

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.

Initial Check-in...

9e1f172

WodansSon added bug documentation service/cosmosdb labels Feb 9, 2024

github-actions bot added the size/M label Feb 9, 2024

Add regression test case...

7d4334a

WodansSon added this to the v3.92.0 milestone Feb 9, 2024

WodansSon marked this pull request as draft February 14, 2024 00:46

Change fix design...

c9d299a

WodansSon marked this pull request as ready for review February 14, 2024 03:30

manicminer suggested changes Feb 14, 2024

View reviewed changes

github-actions bot added the waiting-response label Feb 14, 2024

Address PR comments...

3c5b38e

github-actions bot removed the waiting-response label Feb 14, 2024

WodansSon changed the title ~~[Bug:] azurerm_cosmosdb_account - define default values for the consistency_policy and backup code blocks~~ [Bug:] azurerm_cosmosdb_account - add default values for the consistency_policy fields and fix regression in the backup code block Feb 14, 2024

Updated description of the regression test case...

f3d5f50

katbyte approved these changes Feb 14, 2024

View reviewed changes

WodansSon merged commit 446a4a0 into main Feb 15, 2024
33 checks passed

WodansSon deleted the b_cosmos_backup_storage_redundancy branch February 15, 2024 01:36

WodansSon added a commit that referenced this pull request Feb 15, 2024

Updating to include #24830

a479afa

rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 21, 2024

Updating to include hashicorp#24830

55aceb6

rizkybiz pushed a commit to rizkybiz/terraform-provider-azurerm that referenced this pull request Feb 29, 2024

Updating to include hashicorp#24830

9277166

github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Bug:] `azurerm_cosmosdb_account` - add default values for the `consistency_policy` fields and fix regression in the `backup` code block #24830

[Bug:] `azurerm_cosmosdb_account` - add default values for the `consistency_policy` fields and fix regression in the `backup` code block #24830

WodansSon commented Feb 9, 2024

manicminer left a comment

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

manicminer Feb 14, 2024

WodansSon Feb 14, 2024

katbyte left a comment

WodansSon commented Feb 15, 2024

github-actions bot commented Apr 24, 2024

	return nil, fmt.Errorf("`interval_in_minutes` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))
	return nil, fmt.Errorf("`interval_in_minutes` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

	return nil, fmt.Errorf("`retention_in_hours` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))
	return nil, fmt.Errorf("`retention_in_hours` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

	return nil, fmt.Errorf("`storage_redundancy` cannot be defined when the `backup.type` is set to %q", string(cosmosdb.BackupPolicyTypeContinuous))
	return nil, fmt.Errorf("`storage_redundancy` cannot be defined when the `backup.type` is set to %q", cosmosdb.BackupPolicyTypeContinuous)

	return nil, fmt.Errorf("`create_mode` can only be defined when the `backup.type` is set to %q, got %q", string(cosmosdb.BackupPolicyTypeContinuous), string(cosmosdb.BackupPolicyTypePeriodic))
	return nil, fmt.Errorf("`create_mode` can only be defined when the `backup.type` is set to %q, got %q", cosmosdb.BackupPolicyTypeContinuous, cosmosdb.BackupPolicyTypePeriodic)

		func TestAccCosmosDBAccount_icmRegression(t *testing.T) {
		// Regression test for the Microsoft IcM 466814988...

	~> NOTE: `max_interval_in_seconds` and `max_staleness_prefix` can only be set to values other than default when the `consistency_level` is set to `BoundedStaleness`.
	~> Note `max_interval_in_seconds` and `max_staleness_prefix` can only be set to values other than default when the `consistency_level` is set to `BoundedStaleness`.


		* `interval_in_minutes` - (Optional) The interval in minutes between two backups. This is configurable only when `type` is `Periodic`. Possible values are between 60 and 1440.
		~> NOTE: Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.

	~> NOTE: Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.
	~> Note Migration of `Periodic` to `Continuous` is one-way, changing `Continuous` to `Periodic` forces a new resource to be created.


		* `storage_redundancy` - (Optional) The storage redundancy is used to indicate the type of backup residency. Possible values are `Geo`, `Local` and `Zone`. Defaults to `Geo`.

		~> NOTE: You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.

	~> NOTE: You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.
	~> Note You can only configure `interval_in_minutes`, `retention_in_hours` and `storage_redundancy` when the `type` field is set to `Periodic`.

[Bug:] azurerm_cosmosdb_account - add default values for the consistency_policy fields and fix regression in the backup code block #24830

[Bug:] azurerm_cosmosdb_account - add default values for the consistency_policy fields and fix regression in the backup code block #24830

Conversation

WodansSon commented Feb 9, 2024

manicminer left a 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

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

katbyte left a comment

Choose a reason for hiding this comment

WodansSon commented Feb 15, 2024

github-actions bot commented Apr 24, 2024

[Bug:] `azurerm_cosmosdb_account` - add default values for the `consistency_policy` fields and fix regression in the `backup` code block #24830

[Bug:] `azurerm_cosmosdb_account` - add default values for the `consistency_policy` fields and fix regression in the `backup` code block #24830