Azure-rest-api-specs: [Redis] Updating Tags is broken 😢

[tombuildsstuff]

:wave: hey folks!

I realise this may not be the best place to file this issue (sorry!), but I'm unsure where I should be filing this kind of issue.

An issue's been opened on the Terraform repo about being unable to update Tags for a Redis instance.

This is supported in both the API and the Azure SDK for Go - however when making changes to the Redis instance, as shown below - the value for Tags isn't updated. Note that the provisioningState is succeeded, so it's not in an updating state either:

PATCH /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-dev/providers/Microsoft.Cache/Redis/tharvey-redis1?api-version=2016-04-01 HTTP/1.1
{"properties":{"enableNonSslPort":false,"sku":{"name":"Standard","family":"C","capacity":1},"tags":{"example":"Test"}}}

...

HTTP/1.1 200 OK
{"id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-dev/providers/Microsoft.Cache/Redis/tharvey-redis1","location":"West Europe","name":"tharvey-redis1","type":"Microsoft.Cache/Redis","tags":{},"properties":{"provisioningState":"Succeeded","redisVersion":"3.2","sku":{"name":"Standard","family":"C","capacity":1},"enableNonSslPort":false,"redisConfiguration":{"maxmemory-policy":"volatile-lru","maxclients":"1000","maxmemory-reserved":"50","maxfragmentationmemory-reserved":"50","maxmemory-delta":"50"},"accessKeys":null,"hostName":"tharvey-redis1.redis.cache.windows.net","port":6379,"sslPort":6380}}

When subsequently doing a GET Request to retrieve the status, the Tags aren't set. This also happens when requesting it ~15m later, so it's not an eventual consistency issue either.

GET /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-dev/providers/Microsoft.Cache/Redis/tharvey-redis1?api-version=2016-04-01 HTTP/1.1
{"id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-dev/providers/Microsoft.Cache/Redis/tharvey-redis1","location":"West Europe","name":"tharvey-redis1","type":"Microsoft.Cache/Redis","tags":{},"properties":{"provisioningState":"Succeeded","redisVersion":"3.2","sku":{"name":"Standard","family":"C","capacity":1},"enableNonSslPort":false,"redisConfiguration":{"maxmemory-policy":"volatile-lru","maxclients":"1000","maxmemory-reserved":"50","maxfragmentationmemory-reserved":"50","maxmemory-delta":"50"},"accessKeys":null,"hostName":"tharvey-redis1.redis.cache.windows.net","port":6379,"sslPort":6380}}

All of this leads me to think that the API is broken :(
Would it be possible to tag someone from the Redis team to look at this?

Out of interest, is there a repository that's best to file these issues on? I'm aware this probably isn't the best place to raise this kind of issue (sorry!), but I'm lost for where else would be best to raise it :(

Thanks!

[tombuildsstuff]

@StephenWeatherford / @whiskeyjay / @marstr / @colemickens sorry to ask, but would one of you mind pointing a member of the Redis team at this issue? Thanks! :)

[amarzavery]

@TimLovellSmith from the Redis Cache team and @ravbhatnagar from the ARM team should be able to help us in this issue.

[tombuildsstuff]

Thanks @amarzavery :)

[tombuildsstuff]

@TimLovellSmith 👋 sorry to pester, any idea of a rough timeframe for this?

[sarangan12]

Update: I am trying to contact Tim offline. I will get the details and provide the update soon.

[TimLovellSmith]

@tombuildsstuff @amarzavery Hi, thanks for reporting the issue, there's indeed a bug here. We will fix this in our next server update.

[tombuildsstuff]

👋 hey @TimLovellSmith - so that I can manage expectations on our end - any idea when the next server update will be? Thanks! :)

[TimLovellSmith]

@tombuildsstuff Hey Tom, we just finished rolling the fix for the Tags issue end of last week. I hope it works for you now!

[TimLovellSmith]

@kirthik Hi, the issue should be fixed now could you resolve/close this please as I don't have the permissions!

[kirthik]

@TimLovellSmith - Could you please link to the commit id for tracking purpose?

[TimLovellSmith]

@kirthik Its fixed server side, so its not part of this repo!

[tombuildsstuff]

Hey @TimLovellSmith @kirthik

Thanks for looking into this - unfortunately this still appears to be broken for me, would it be possible to take another look?

Thanks! :)

[TimLovellSmith]

@tombuildsstuff @kirthik
Are you reproing this issue with the client now? It is working fine for me with Fiddler composed requests. Maybe there is also a client bug?

(Sample response for GET after doing PATCH:
{"id":"/subscriptions/.../resourceGroups/timallalerts/providers/Microsoft.Cache/Redis/timallalertsause","location":"Australia East","name":"timallalertsause","type":"Microsoft.Cache/Redis","tags":{"A":"1","B":"2"},"properties":{"provisioningState":"Succeeded","redisVersion":"3.2.7","sku":{"name":"Premium","family":"P","capacity":1},"enableNonSslPort":false,"redisConfiguration":{"maxclients":"7500","maxmemory-reserved":"200","maxmemory-delta":"200"},"accessKeys":null,"hostName":"timallalertsause.redis.cache.windows.net","port":6379,"sslPort":6380,"shardCount":2,"linkedServers":[]}}

[tombuildsstuff]

👋 @TimLovellSmith out of interest which API version are you using?

This is the PATCH request I'm doing through the Go SDK:

PATCH /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-redisdev/providers/Microsoft.Cache/Redis/tharveyredis2?api-version=2016-04-01 HTTP/1.1
Host: management.azure.com
User-Agent: HashiCorp-Terraform-v0.9.7-dev
Content-Length: 111
Content-Type: application/json
Accept-Encoding: gzip

{"properties":{"enableNonSslPort":false,"sku":{"name":"Basic","family":"C","capacity":0},"tags":{"Foo":"Bar"}}}


HTTP/1.1 200 OK
Transfer-Encoding: chunked
Cache-Control: no-cache
Content-Type: application/json; charset=utf-8
Date: Thu, 08 Jun 2017 07:30:05 GMT
Expires: -1
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: e24714ad-31d9-47d8-838a-7619e47f41a1
X-Ms-Ratelimit-Remaining-Subscription-Writes: 1198
X-Ms-Request-Id: c280b5ff-e028-44c0-b15d-d2192d6f593a
X-Ms-Routing-Request-Id: WESTEUROPE:20170608T073006Z:e24714ad-31d9-47d8-838a-7619e47f41a1
X-Rp-Server-Mvid: 7f0277b4-61aa-49ae-91a1-ae0d09db3cd1

271
{"id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-redisdev/providers/Microsoft.Cache/Redis/tharveyredis2","location":"West US","name":"tharveyredis2","type":"Microsoft.Cache/Redis","tags":{},"properties":{"provisioningState":"Succeeded","redisVersion":"3.2.7","sku":{"name":"Basic","family":"C","capacity":0},"enableNonSslPort":false,"redisConfiguration":{"maxmemory-policy":"volatile-lru","maxclients":"256","maxmemory-reserved":"2","maxfragmentationmemory-reserved":"12","maxmemory-delta":"2"},"accessKeys":null,"hostName":"tharveyredis2.redis.cache.windows.net","port":6379,"sslPort":6380}}

and the GET response afterwards:

GET /subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-redisdev/providers/Microsoft.Cache/Redis/tharveyredis2?api-version=2016-04-01 HTTP/1.1
Host: management.azure.com
User-Agent: HashiCorp-Terraform-v0.9.7-dev
Accept-Encoding: gzip

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Cache-Control: no-cache
Content-Type: application/json; charset=utf-8
Date: Thu, 08 Jun 2017 07:30:06 GMT
Expires: -1
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: 46d83366-7ec5-478a-8126-57ff9b9b8188
X-Ms-Ratelimit-Remaining-Subscription-Reads: 14993
X-Ms-Request-Id: 2cb02ce6-7e98-4602-ba8f-2c73028256c2
X-Ms-Routing-Request-Id: WESTEUROPE:20170608T073007Z:46d83366-7ec5-478a-8126-57ff9b9b8188
X-Rp-Server-Mvid: 7f0277b4-61aa-49ae-91a1-ae0d09db3cd1

271
{"id":"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/tharvey-redisdev/providers/Microsoft.Cache/Redis/tharveyredis2","location":"West US","name":"tharveyredis2","type":"Microsoft.Cache/Redis","tags":{},"properties":{"provisioningState":"Succeeded","redisVersion":"3.2.7","sku":{"name":"Basic","family":"C","capacity":0},"enableNonSslPort":false,"redisConfiguration":{"maxmemory-policy":"volatile-lru","maxclients":"256","maxmemory-reserved":"2","maxfragmentationmemory-reserved":"12","maxmemory-delta":"2"},"accessKeys":null,"hostName":"tharveyredis2.redis.cache.windows.net","port":6379,"sslPort":6380}}

Would you mind sharing the PATCH request you're doing to compare if there's a Swagger bug here too?

Thanks! :)

[TimLovellSmith]

@tombuildsstuff @kirthik
I see the problem with the request now - tags is supposed to be at the top level, not inside of 'properties'.

This is specified by inheritance relationship in the swagger https://raw.githubusercontent.com/Azure/azure-rest-api-specs/master/arm-redis/2017-02-01/swagger/redis.json

 "RedisResource": {
      "properties": {
        "properties": {
          "x-ms-client-flatten": true,
          "$ref": "#/definitions/RedisResourceProperties",
          "description": "Redis cache properties."
        }
      },
      "allOf": [
        {
          "$ref": "#/definitions/Resource"
        }
      ],
      "description": "A single Redis item in List or Get Operation."
    },

    "Resource": {
      "description": "The Resource definition.",
      "properties": {
        "id": {
          "readOnly": true,
          "type": "string",
          "description": "Resource ID."
        },
        "name": {
          "readOnly": true,
          "type": "string",
          "description": "Resource name."
        },
        "type": {
          "readOnly": true,
          "type": "string",
          "description": "Resource type."
        },
        "location": {
          "type": "string",
          "description": "Resource location."
        },
        "tags": {
          "type": "object",
          "additionalProperties": {
            "type": "string"
          },
          "description": "Resource tags."
        }
      },
      "required": [
        "location"
      ],
      "x-ms-azure-resource": true
    },

(It should be the same for all api-versions.)
Tim

[tombuildsstuff]

@TimLovellSmith so does that need a Swagger change to resolve this issue? Thanks!

[tombuildsstuff]

👋 @amarzavery @TimLovellSmith given this isn't fixed and requires a Swagger update, can we re-open this issue until this is resolved please?

_related_: I thought I'd have permission to re-open an issue I created? apparently not :/

Thanks!