Azure-rest-api-specs: [Search] Resource Group is lower-cased

Created on 22 Nov 2018 · 21Comments · Source: Azure/azure-rest-api-specs

👋

We've got acceptance tests which spin up a Search Service every night using version 2015-08-19 of the API. From 2018-11-20 our tests for the Search Services started failing as the casing of the Resource Group in the id returned from the Get API call is now returned in lower-case (but other values are not) - for example when using the Resource Group name acctestRG-3269366889119765244 the following ID returned from the Get API call/method in the Go SDK returns the following:

/subscriptions/*/resourceGroups/acctestrg-3269366889119765244/providers/Microsoft.Search/searchServices/acctestsearchservice3269366889119765244

when previously it would have been returned as:

/subscriptions/*/resourceGroups/acctestRG-3269366889119765244/providers/Microsoft.Search/searchServices/acctestsearchservice3269366889119765244

This leads to the following test failure on our end:

------- Stdout: -------
=== RUN   TestAccAzureRMSearchService_basic
=== PAUSE TestAccAzureRMSearchService_basic
=== CONT  TestAccAzureRMSearchService_basic
--- FAIL: TestAccAzureRMSearchService_basic (85.33s)
    testing.go:538: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        DESTROY/CREATE: azurerm_search_service.test
          location:            "westeurope" => "westeurope"
          name:                "acctestsearchservice3269366889119765244" => "acctestsearchservice3269366889119765244"
          partition_count:     "1" => "<computed>"
          primary_key:         "8EB8A12F388792A9FADDE2AA208A8D5F" => "<computed>"
          replica_count:       "1" => "<computed>"
          resource_group_name: "acctestrg-3269366889119765244" => "acctestRG-3269366889119765244" (forces new resource)
          secondary_key:       "13DF57C5F46E9146C0DBA4A518AFC823" => "<computed>"
          sku:                 "standard" => "standard"
          tags.%:              "1" => "1"
          tags.environment:    "staging" => "staging"

Would it be possible to fix the behaviour in this API so that the Resource Group name is returned in the case it's stored in, as they were prior to the 20th? Whilst /technically/ Resource Group names are supposed to be case-insensitive - there's a sufficient number of API's in Azure where they are case sensitive that we're forced to treat them as case sensitive for consistency.

Thanks!

Search Service Attention

Source

tombuildsstuff

All 21 comments

@tombuildsstuff Hi Tom, thanks for reporting this. I'll pass this along to the team who owns the management API for Azure Search and one of them will respond.

brjohnstmsft on 26 Nov 2018

👍1

@tombuildsstuff Hi Tom, our recent change is to make resource naming to be case insensitive for Azure Search, and all newly created search services will be stored in lower case. We don't plan to revert it back. Please refactor your test case to case insensitive too.

ms-xiaoy on 27 Nov 2018

hi @viki1987

Thanks for getting back to me here :)

Whilst I can understand the intention here - unfortunately this behaviour is different to almost every other Azure API which return the ID of the resource in the casing provided in the URI (even where Resource Groups are treated case insensitively, or stored case-insensitively in an internal data store) - as such I believe this is a bug in the API and should be reverted (cc @devigned).

From our side unfortunately this is prompting customers to delete & recreate their Search Services, so unfortunately this is more than just a test suite issue. On that note, given this is a breaking change to the API's behaviour, shouldn't this be rolled out in a new API version?

Thanks!

tombuildsstuff on 27 Nov 2018

hi @tombuildsstuff,

Actually, this is a bug fix applied to all API versions. Previously, search services is case sensitive and caused lots of failure when user querying existing services with unmatched parameters. It's a quick fix and didn't touch data store. So, you can see, we would not take a risk to revert back. Sorry for breaking you user case. Could you find a way to tolerate this case?

Thanks

ms-xiaoy on 27 Nov 2018

hi @viki1987

Actually, this is a bug fix applied to all API versions. Previously, search services is case sensitive and caused lots of failure when user querying existing services with unmatched parameters. It's a quick fix and didn't touch data store

From what I've understood of this (please correct me if I'm wrong 😄) - the issue that's attempting to be solved here was customers submitting values which conflicted with values in a different case (e.g. a customer attempts to submit Hello when hello already exists)? If so - it should be possible to work around this bug internally within the API (e.g. by doing a case-insensitive lookup as necessary) - without exposing this lower-cased value in the response / making this breaking behavioural change, which would solve both issues? 😄

Could you find a way to tolerate this case?

Whilst technically possible (we could release a patch for this) - some customers have environments which require that an older version is used for an extended period of time, such that they will still encounter this issue - as such whilst we could patch this in the client (and ask users to upgrade) - I believe this needs to be fixed in the API.

Thanks!

tombuildsstuff on 27 Nov 2018

@viki1987 I believe this behavioral change to the API would be considered a breaking change to an existing API version and fall outside of the guidelines for ARM APIs. Please apply the change only to future API versions and leave the existing API versions functioning as they have.

I'll be starting an internal thread with the API review board to review these changes.

Please review: https://github.com/Microsoft/api-guidelines/blob/vNext/Guidelines.md#123-definition-of-a-breaking-change

devigned on 27 Nov 2018

@devigned thanks for confirming 👍

Given that - @viki1987 is there a rough/estimated timeline for fixing this so that the API response is the same as it was previously/reverting this change?

Thanks!

tombuildsstuff on 29 Nov 2018

👍1

ping @viki1987 :)

tombuildsstuff on 5 Dec 2018

@tombuildsstuff thanks for you suggestion.

We made an incremental fix to support case preserving, which means we revert the API behavior and don't need to upgrade API version. Unfortunately, the resource group name of services created during this period of time will keep in lower case.

It's important to know that we are still on the way to bringing this fix to production. Please give us more time.

Thanks!

ms-xiaoy on 7 Dec 2018

hey @viki1987

It's important to know that we are still on the way to bringing this fix to production. Please give us more time.

No worries - just looking for a rough timeframe for this more than anything, be it days or weeks :)

We made an incremental fix to support case preserving, which means we revert the API behavior and don't need to upgrade API version. Unfortunately, the resource group name of services created during this period of time will keep in lower case.

I'm a little confused as to why Search Services created during this time period would remain lower-cased since in this comment (It's a quick fix and didn't touch data store) I thought they were stored in the same case in the data store? - or did you mean that only new ones are lower-cased? If so - I'm assuming that any Search Services created during this time period would also need to be updated to the correct casing, since otherwise this'll cause a (hard to diagnose) issue for customers who've created Search Services during this time period?

Thanks!

tombuildsstuff on 12 Dec 2018

ping @viki1987 is there an update on when this'll be fixed? :)

tombuildsstuff on 16 Jan 2019

Hi @tombuildsstuff,

I'm really sorry we made you confused and reply late. You are right, I meant only newly created Search Services were lower-cased.

To completely fix this issue, we need 2 steps:

Fix bug to preserve casing of resource group name.
Update all Search Services affected before #1 from lower case to original case.

We finished #1 after 2018-12-10, so new Search Services created after that should be in original case.

We are evaluating #2, meanwhile if you have urgent needs for #2 fix, let us know and see if we can coordinate to fix it.

Thanks!

ms-xiaoy on 17 Jan 2019

Ping @tombuildsstuff, please let us know if you have further concern. If not, could you please close this issue?

ms-xiaoy on 23 Jan 2019

@viki1987 thanks for patching the API so the original bug is now resolved, however there's still the issue of the 20 days of broken data (from 2018-11-20 to 2018-12-10), since any customers using a Search Service created between these two dates will get a different response from the API (where the resource group name in the ID will be lower-cased) - which could lead to subtle/hard to reproduce bugs in applications which consume the API.

As such I believe it's still necessary to update the ID field to use the correct casing for Search Services created between 2018-11-20 and 2018-12-10 before we can close this issue?

Thanks!

tombuildsstuff on 23 Jan 2019

ping @ms-xiaoy

tombuildsstuff on 19 Mar 2019

ping @ms-xiaoy

tombuildsstuff on 12 Jun 2019

FYI @miwelsh

brjohnstmsft on 12 Jun 2019

👍1

Ack, thanks Bruce. Hi @tombuildsstuff. @ms-xiaoy handed off this component to me a few months ago so I'll be taking over from here. My team will be repairing the 20 days of broken data (from 2018-11-20 to 2018-12-10) in the next few weeks. I'll update this thread when we're done. Thanks, and sorry for the delays.

miwelsh on 25 Jun 2019

👍1

Hi @tombuildsstuff, we now have the means to repair the casing of the data from the 20 day window where it was broken. Due to the time that has passed, we are concerned that by making a change now for all customers, we could break customers who have already created a dependency on the data from that time window. As a result, we will only change data for customers that escalate to us that they need their data fixed in this time window. If you email us the subscriptions that you would like fixed in this time window, we will quickly repair the data for you. Thanks for your patience, we would love to get this issue closed.

tjacobhi on 3 Jul 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @brjohnstmsft, @bleroy, @tjacobhi, @markheff, @miwelsh.

Issue Details

👋

/subscriptions/*/resourceGroups/acctestrg-3269366889119765244/providers/Microsoft.Search/searchServices/acctestsearchservice3269366889119765244

when previously it would have been returned as:

/subscriptions/*/resourceGroups/acctestRG-3269366889119765244/providers/Microsoft.Search/searchServices/acctestsearchservice3269366889119765244

This leads to the following test failure on our end:

------- Stdout: -------
=== RUN   TestAccAzureRMSearchService_basic
=== PAUSE TestAccAzureRMSearchService_basic
=== CONT  TestAccAzureRMSearchService_basic
--- FAIL: TestAccAzureRMSearchService_basic (85.33s)
    testing.go:538: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        DESTROY/CREATE: azurerm_search_service.test
          location:            "westeurope" => "westeurope"
          name:                "acctestsearchservice3269366889119765244" => "acctestsearchservice3269366889119765244"
          partition_count:     "1" => "<computed>"
          primary_key:         "8EB8A12F388792A9FADDE2AA208A8D5F" => "<computed>"
          replica_count:       "1" => "<computed>"
          resource_group_name: "acctestrg-3269366889119765244" => "acctestRG-3269366889119765244" (forces new resource)
          secondary_key:       "13DF57C5F46E9146C0DBA4A518AFC823" => "<computed>"
          sku:                 "standard" => "standard"
          tags.%:              "1" => "1"
          tags.environment:    "staging" => "staging"

Thanks!