Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_cosmosdb_account: Remove the invalid validation and update TF doc for cosmos DB account resource #14301

Closed

Conversation

sinbai
Copy link
Contributor

@sinbai sinbai commented Nov 23, 2021

The purposes of this PR:

  • The validation of "capability EnableMongo must be enabled if MongoDBv3.4 is also enabled." was added to Terraform In PR #13757. We received a request from service team to remove this validation because this is an incorrect validation. In fact it’s the exact opposite, MongoDBv3.4 is meaningless if EnableMongo is also specified in API. Therefore, remove the invalid validation in Terraform.

  • Update the TF doc:

    1. Remove capabilities in existing TF doc because the kind is GlobalDocumentDB . The ca

    pabilities should not be specified in this kind.

    1. Add new example to the TF doc to introduce the kinds of Parse and MongoDB.

Related issues : #14039, #14116

Test Results (The failed testcase report the same error before modification):
PASS: TestAccCosmosDBAccount_mongoVersion32 (1177.29s)
PASS: TestAccCosmosDBAccount_mongoVersion36 (1240.95s)
PASS: TestAccCosmosDBAccount_failover_boundedStaleness (1317.57s)
PASS: TestAccCosmosDBAccount_capabilities_EnableServerless (1453.88s)
PASS: TestAccCosmosDBAccount_localAuthenticationDisabled (1463.08s)
PASS: TestAccCosmosDBAccount_backupContinuous (1520.82s)
PASS: TestAccCosmosDBAccount_mongoVersion40 (1594.67s)
PASS: TestAccCosmosDBAccount_backupPeriodicToContinuous (1808.14s)
PASS: TestAccCosmosDBAccount_networkBypass (1969.15s)
PASS: TestAccCosmosDBAccount_capabilities_EnableTable (1459.64s)
PASS: TestAccCosmosDBAccount_capabilities_EnableAggregationPipeline (1343.42s)
PASS: TestAccCosmosDBAccount_capabilities_EnableCassandra (1360.13s)
PASS: TestAccCosmosDBAccount_capabilities_EnableGremlin (1561.95s)
PASS: TestAccCosmosDBAccount_mongoVersionUpdate (2900.83s)
PASS: TestAccCosmosDBAccount_basic_mongo_session (1630.26s)
PASS: TestAccCosmosDBAccount_basic_parse_strong (1204.16s)
PASS: TestAccCosmosDBAccount_requiresImport (1346.28s)
PASS: TestAccCosmosDBAccount_basic_parse_session (1317.86s)
PASS: TestAccCosmosDBAccount_basic_parse_eventual (1373.08s)
PASS: TestAccCosmosDBAccount_complete_mongo (4368.53s)
PASS: TestAccCosmosDBAccount_basic_parse_consistentPrefix (1276.77s)
PASS: TestAccCosmosDBAccount_updateConsistency_mongo (2758.33s)
PASS: TestAccCosmosDBAccount_updateConsistency_global (2476.24s)
PASS: TestAccCosmosDBAccount_basic_parse_boundedStaleness (1206.19s)
PASS: TestAccCosmosDBAccount_basic_mongo_strong (1337.05s)
PASS: TestAccCosmosDBAccount_basic_mongo_strong_without_capability (1520.42s)
PASS: TestAccCosmosDBAccount_update_global (5465.89s)
PASS: TestAccCosmosDBAccount_capabilities_AllowSelfServeUpgradeToMongo36 (1532.30s)
PASS: TestAccCosmosDBAccount_backup (1905.93s)
PASS: TestAccCosmosDBAccount_capabilities_mongoEnableDocLevelTTL (1351.58s)
PASS: TestAccCosmosDBAccount_update_parse (5594.33s)
PASS: TestAccCosmosDBAccount_capabilities_DisableRateLimitingResponses (1500.52s)
PASS: TestAccCosmosDBAccount_capabilitiesUpdate (2778.28s)
PASS: TestAccCosmosDBAccount_update_mongo (6527.73s)
PASS: TestAccCosmosDBAccount_vNetFilters (1647.01s)
PASS: TestAccCosmosDBAccount_capabilitiesAdd (2824.56s)
PASS: TestAccCosmosDBAccount_analyticalStorage (1216.96s)
PASS: TestAccCosmosDBAccount_identity (2196.70s)
PASS: TestAccCosmosDBAccount_capabilities_EnableMongo (1334.12s)
PASS: TestAccCosmosDBAccount_capabilities_MongoDBv34 (1543.99s)
PASS: TestAccCosmosDBAccount_basic_global_consistentPrefix (1452.07s)
PASS: TestAccCosmosDBAccount_basic_mongo_boundedStaleness (1381.83s)
PASS: TestAccCosmosDBAccount_basic_mongo_eventual (1371.73s)
PASS: TestAccCosmosDBAccount_geoLocationsUpdate (2583.37s)
PASS: TestAccCosmosDBAccount_basic_global_strong (1472.82s)
PASS: TestAccCosmosDBAccount_failover_session (1369.97s)
PASS: TestAccCosmosDBAccount_basic_mongo_consistentPrefix (1516.84s)
PASS: TestAccCosmosDBAccount_basic_global_session (1334.89s)
PASS: TestAccCosmosDBAccount_basic_global_boundedStaleness (1191.61s)
PASS: TestAccCosmosDBAccount_complete_parse (3210.30s)
PASS: TestAccCosmosDBAccount_basic_global_eventual (1472.35s)
PASS: TestAccCosmosDBAccount_failover_strong (1174.84s)
PASS: TestAccCosmosDBAccount_complete_global (3593.57s)
PASS: TestAccCosmosDBAccount_failover_eventualConsistency (1388.50s)
PASS: TestAccCosmosDBAccount_failover_mongoDB (1308.17s)
PASS: TestAccCosmosDBAccount_failover_boundedStalenessComplete (1469.10s)
PASS: TestAccCosmosDBAccount_failover_geoReplicated (2234.54s)
PASS: TestAccCosmosDBAccount_public_network_access_enabled (1654.77s)

FAIL: TestAccCosmosDBAccount_keyVaultUriUpdateConsistancy (15.01s)
FAIL: TestAccCosmosDBAccount_keyVaultUri (13.69s)
FAIL: TestAccCosmosDBAccount_completeZoneRedundant_global (1463.84s)
FAIL: TestAccCosmosDBAccount_zoneRedundant_update_mongo (1725.12s)
FAIL: TestAccCosmosDBAccount_completeZoneRedundant_parse (1461.12s)
FAIL: TestAccCosmosDBAccount_freeTier (143.65s)
FAIL: TestAccCosmosDBAccount_completeZoneRedundant_mongo (1612.80s)

@tombuildsstuff
Copy link
Contributor

@sinbai

The validation of "capability EnableMongo must be enabled if MongoDBv3.4 is also enabled." was added In PR #13757. Per the feedback from service team, this is an incorrect validation:

In fact it’s the exact opposite, MongoDBv3.4 is meaningless if EnableMongo is also specified.

Is this in the API, or in Terraform? Whilst you may not need to specify this at creation it could be defaulted in the API here

@sinbai
Copy link
Contributor Author

sinbai commented Nov 24, 2021

Is this in the API, or in Terraform? Whilst you may not need to specify this at creation it could be defaulted in the API here

@tombuildsstuff The validation of "capability EnableMongo must be enabled if MongoDBv3.4 is also enabled." was added to Terraform In PR #13757, the contributor is koikonom (Kyriakos Oikonomakos). We received a request from service team to remove this validation because this is an incorrect validation. In fact it’s the exact opposite, MongoDBv3.4 is meaningless if EnableMongo is also specified in API. So, I removed the "capability EnableMongo must be enabled if MongoDBv3.4 is also enabled." validation in Terraform in current PR.

@koikonom
Copy link
Contributor

Hi @sinbai! I took a look at what you said and I found the following:

After building a provider without the validation I previously added, I created a new cosmosdb_account resource. I had debugging enabled while applying the changes and I captured the HTTP requests.

This is the create request:

PUT /subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123?api-version=2021-10-15 HTTP/1.1
Host: management.azure.com
User-Agent: Go/go1.17.2 (amd64-darwin) go-autorest/v14.2.1 Azure-SDK-For-Go/v59.2.0 documentdb/2021-10-15 HashiCorp Terraform/1.0.11 (+https://www.terraform.io) Terraform Plugin SDK/2.8.0 terraform-provider-azurerm/dev pid-222c6c49-1b0a-5959-a213-6608f9eb8820
Content-Length: 766
Content-Type: application/json; charset=utf-8
X-Ms-Correlation-Request-Id: 80759c3b-cdb1-cd9d-3243-ec7df5be6acc
Accept-Encoding: gzip

{"identity":{"type":"None"},"kind":"MongoDB","location":"westeurope","properties":{"consistencyPolicy":{"defaultConsistencyLevel":"Strong","maxStalenessPrefix":100,"maxIntervalInSeconds":5},"locations":[{"failoverPriority":0,"isZoneRedundant":false,"locationName":"westeurope"}],"databaseAccountOfferType":"Standard","ipRules":[],"isVirtualNetworkFilterEnabled":false,"enableAutomaticFailover":false,"capabilities":[{"name":"MongoDBv3.4"}],"virtualNetworkRules":[],"enableMultipleWriteLocations":false,"disableKeyBasedMetadataWriteAccess":false,"defaultIdentity":"FirstPartyIdentity","publicNetworkAccess":"Enabled","enableFreeTier":false,"enableAnalyticalStorage":false,"networkAclBypass":"None","networkAclBypassResourceIds":[],"disableLocalAuth":false},"tags":{}}: timestamp=2021-11-29T22:37:30.447Z

As you can see the capabilities list has only one item: "capabilities":[{"name":"MongoDBv3.4"}] .

Unfortunately the response we get from the Azure API is not what I'd expect to see based on the description above:

2021-11-29T22:37:35.976Z [DEBUG] provider.terraform-provider-azurerm: AzureRM Response for https://management.azure.com/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123?api-version=2021-10-15:
HTTP/2.0 200 OK
Azure-Asyncoperation: https://management.azure.com/subscriptions/<mysubscription>/providers/Microsoft.DocumentDB/locations/westeurope/operationsStatus/248a7d8b-74db-407f-9e8c-6e68ce335a74?api-version=2021-10-15
Cache-Control: no-store, no-cache
Content-Type: application/json
Date: Mon, 29 Nov 2021 22:37:35 GMT
Location: https://management.azure.com/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123/operationResults/248a7d8b-74db-407f-9e8c-6e68ce335a74?api-version=2021-10-15
Pragma: no-cache
Server: Microsoft-HTTPAPI/2.0
Strict-Transport-Security: max-age=31536000; includeSubDomains
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
X-Ms-Correlation-Request-Id: 80759c3b-cdb1-cd9d-3243-ec7df5be6acc
X-Ms-Gatewayversion: version=2.14.0
X-Ms-Ratelimit-Remaining-Subscription-Writes: 1199
X-Ms-Request-Id: 248a7d8b-74db-407f-9e8c-6e68ce335a74
X-Ms-Routing-Request-Id: UKSOUTH:20211129T223735Z:22b68822-acea-4aa4-9c00-92f58623af14

{"id":"/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123","name":"acctest-ca-123","location":"West Europe","type":"Microsoft.DocumentDB/databaseAccounts","kind":"MongoDB","tags":{},"systemData":{"createdAt":"2021-11-29T22:37:32.879834Z"},"properties":{"provisioningState":"Creating","publicNetworkAccess":"Enabled","enableAutomaticFailover":false,"enableMultipleWriteLocations":false,"enablePartitionKeyMonitor":false,"isVirtualNetworkFilterEnabled":false,"virtualNetworkRules":[],"EnabledApiTypes":"MongoDB","disableKeyBasedMetadataWriteAccess":false,"enableFreeTier":false,"enableAnalyticalStorage":false,"analyticalStorageConfiguration":{"schemaType":"FullFidelity"},"instanceId":"57c26af0-8077-422f-8375-cdf57fd7b9b6","databaseAccountOfferType":"Standard","defaultIdentity":"FirstPartyIdentity","networkAclBypass":"None","disableLocalAuth":false,"consistencyPolicy":{"defaultConsistencyLevel":"Strong","maxIntervalInSeconds":5,"maxStalenessPrefix":100},"apiProperties":{"serverVersion":"3.6"},"configurationOverrides":{},"writeLocations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"readLocations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"locations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"failoverPolicies":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","failoverPriority":0}],"cors":[],"capabilities":[{"name":"MongoDBv3.4"},{"name":"EnableMongo"}],"ipRules":[],"backupPolicy":{"type":"Periodic","periodicModeProperties":{"backupIntervalInMinutes":240,"backupRetentionIntervalInHours":8,"backupStorageRedundancy":"Invalid"}},"networkAclBypassResourceIds":[]},"identity":{"type":"None"}}: timestamp=2021-11-29T22:37:35.975Z

Specifically the capabilities returned by Azure now have two items in the list: "capabilities":[{"name":"MongoDBv3.4"},{"name":"EnableMongo"}],.

As such I am afraid the validation needs to stay in place otherwise users are stuck with one of the two options:
a) add capabilities to ignore_changes, which can cause drift in the future
b) add the EnableMongo after the fact, which is poor user experience.

I'm also interested in hearing from the service team because what they told you contradicts what the API is doing, so I'd love to know if there is something in our PUT request that should be different, or if what they described apply in a different API Version than the one we're using (2021-10-15).

@wmengmsft
Copy link

Hi @sinbai! I took a look at what you said and I found the following:

After building a provider without the validation I previously added, I created a new cosmosdb_account resource. I had debugging enabled while applying the changes and I captured the HTTP requests.

This is the create request:

PUT /subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123?api-version=2021-10-15 HTTP/1.1
Host: management.azure.com
User-Agent: Go/go1.17.2 (amd64-darwin) go-autorest/v14.2.1 Azure-SDK-For-Go/v59.2.0 documentdb/2021-10-15 HashiCorp Terraform/1.0.11 (+https://www.terraform.io) Terraform Plugin SDK/2.8.0 terraform-provider-azurerm/dev pid-222c6c49-1b0a-5959-a213-6608f9eb8820
Content-Length: 766
Content-Type: application/json; charset=utf-8
X-Ms-Correlation-Request-Id: 80759c3b-cdb1-cd9d-3243-ec7df5be6acc
Accept-Encoding: gzip

{"identity":{"type":"None"},"kind":"MongoDB","location":"westeurope","properties":{"consistencyPolicy":{"defaultConsistencyLevel":"Strong","maxStalenessPrefix":100,"maxIntervalInSeconds":5},"locations":[{"failoverPriority":0,"isZoneRedundant":false,"locationName":"westeurope"}],"databaseAccountOfferType":"Standard","ipRules":[],"isVirtualNetworkFilterEnabled":false,"enableAutomaticFailover":false,"capabilities":[{"name":"MongoDBv3.4"}],"virtualNetworkRules":[],"enableMultipleWriteLocations":false,"disableKeyBasedMetadataWriteAccess":false,"defaultIdentity":"FirstPartyIdentity","publicNetworkAccess":"Enabled","enableFreeTier":false,"enableAnalyticalStorage":false,"networkAclBypass":"None","networkAclBypassResourceIds":[],"disableLocalAuth":false},"tags":{}}: timestamp=2021-11-29T22:37:30.447Z

As you can see the capabilities list has only one item: "capabilities":[{"name":"MongoDBv3.4"}] .

Unfortunately the response we get from the Azure API is not what I'd expect to see based on the description above:

2021-11-29T22:37:35.976Z [DEBUG] provider.terraform-provider-azurerm: AzureRM Response for https://management.azure.com/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123?api-version=2021-10-15:
HTTP/2.0 200 OK
Azure-Asyncoperation: https://management.azure.com/subscriptions/<mysubscription>/providers/Microsoft.DocumentDB/locations/westeurope/operationsStatus/248a7d8b-74db-407f-9e8c-6e68ce335a74?api-version=2021-10-15
Cache-Control: no-store, no-cache
Content-Type: application/json
Date: Mon, 29 Nov 2021 22:37:35 GMT
Location: https://management.azure.com/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123/operationResults/248a7d8b-74db-407f-9e8c-6e68ce335a74?api-version=2021-10-15
Pragma: no-cache
Server: Microsoft-HTTPAPI/2.0
Strict-Transport-Security: max-age=31536000; includeSubDomains
Vary: Accept-Encoding
X-Content-Type-Options: nosniff
X-Ms-Correlation-Request-Id: 80759c3b-cdb1-cd9d-3243-ec7df5be6acc
X-Ms-Gatewayversion: version=2.14.0
X-Ms-Ratelimit-Remaining-Subscription-Writes: 1199
X-Ms-Request-Id: 248a7d8b-74db-407f-9e8c-6e68ce335a74
X-Ms-Routing-Request-Id: UKSOUTH:20211129T223735Z:22b68822-acea-4aa4-9c00-92f58623af14

{"id":"/subscriptions/<mysubscription>/resourceGroups/acctestRG-cosmos-123/providers/Microsoft.DocumentDB/databaseAccounts/acctest-ca-123","name":"acctest-ca-123","location":"West Europe","type":"Microsoft.DocumentDB/databaseAccounts","kind":"MongoDB","tags":{},"systemData":{"createdAt":"2021-11-29T22:37:32.879834Z"},"properties":{"provisioningState":"Creating","publicNetworkAccess":"Enabled","enableAutomaticFailover":false,"enableMultipleWriteLocations":false,"enablePartitionKeyMonitor":false,"isVirtualNetworkFilterEnabled":false,"virtualNetworkRules":[],"EnabledApiTypes":"MongoDB","disableKeyBasedMetadataWriteAccess":false,"enableFreeTier":false,"enableAnalyticalStorage":false,"analyticalStorageConfiguration":{"schemaType":"FullFidelity"},"instanceId":"57c26af0-8077-422f-8375-cdf57fd7b9b6","databaseAccountOfferType":"Standard","defaultIdentity":"FirstPartyIdentity","networkAclBypass":"None","disableLocalAuth":false,"consistencyPolicy":{"defaultConsistencyLevel":"Strong","maxIntervalInSeconds":5,"maxStalenessPrefix":100},"apiProperties":{"serverVersion":"3.6"},"configurationOverrides":{},"writeLocations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"readLocations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"locations":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","provisioningState":"Creating","failoverPriority":0,"isZoneRedundant":false}],"failoverPolicies":[{"id":"acctest-ca-123-westeurope","locationName":"West Europe","failoverPriority":0}],"cors":[],"capabilities":[{"name":"MongoDBv3.4"},{"name":"EnableMongo"}],"ipRules":[],"backupPolicy":{"type":"Periodic","periodicModeProperties":{"backupIntervalInMinutes":240,"backupRetentionIntervalInHours":8,"backupStorageRedundancy":"Invalid"}},"networkAclBypassResourceIds":[]},"identity":{"type":"None"}}: timestamp=2021-11-29T22:37:35.975Z

Specifically the capabilities returned by Azure now have two items in the list: "capabilities":[{"name":"MongoDBv3.4"},{"name":"EnableMongo"}],.

As such I am afraid the validation needs to stay in place otherwise users are stuck with one of the two options: a) add capabilities to ignore_changes, which can cause drift in the future b) add the EnableMongo after the fact, which is poor user experience.

I'm also interested in hearing from the service team because what they told you contradicts what the API is doing, so I'd love to know if there is something in our PUT request that should be different, or if what they described apply in a different API Version than the one we're using (2021-10-15).

Hi @koikonom, I think there is a bit of confusion here. The reason why "EnableMongo" is returned is unrelated to the "MongoDBv3.4" capability. For example, if you repeat your experiment without specifying "MongoDBv3.4", you will still see "EnableMongo" capability automatically being added. As a result, this validation should be removed.

In terms of why EnableMongo is automatically added to the capabilities: starting with Mongo server version 3.6 (which is the default server version for new accounts), Cosmos DB service will automatically append this capability if missing. We recommend all Terraform clients specify "EnableMongo" capability upfront in their configs, except for server version 3.2 accounts, so that there will be no discrepancy.

@sinbai
Copy link
Contributor Author

sinbai commented Dec 15, 2021

@koikonom @wmengmsft Thank you for your time. Per the information provided from you all, I want to confirm that the validation of "capability EnableMongo must be enabled if MongoDBv3.4 is also enabled." is valid if the value of kind property is MongoDB, correct? Since the EnableMongo capability is not allowed with kind GlobalDocumentDB.

@koikonom
Copy link
Contributor

Hi @wmengmsft, thanks for providing more information. I understand that the API for its own purposes will inject a new value in the response when it thinks it has to.

The problem here is that it breaks what users expect from terrraform.

Unless they set both capabilities in the configuration, next time terraform runs it will notice the new field being returned and since the capabilties attribute is set to be ForceNew it will plan to destroy the resource and created again.

Whilst sending both capabilities is "meaningless" for the service, it prevents our users experiencing a breaking change in their configurations, are there any negative effects of the service receiving this additional value?

@tombuildsstuff
Copy link
Contributor

hey @sinbai

Since I believe this has been addressed in #14039 (and we've not heard back from you otherwise here) I'm going to close this out - however if there's still work needed on this one then let us know and we can take another look.

Thanks!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants