feat(azure): support disk update and App Service plan update (PATCH)

[aryanmehrotra]

What failed

SDK call	Route	Before
armcompute/v5 DisksClient.BeginUpdate	PATCH .../Microsoft.Compute/disks/{name}	501 NotImplemented
armappservice/v3 PlansClient.Update	PATCH .../Microsoft.Web/serverfarms/{name}	405 MethodNotAllowed (only PUT/GET/DELETE were routed)

What changed

Disks (server/azure/disks)

• PATCH applies a DiskUpdate body as a partial update. It answers 202 + Azure-AsyncOperation, the same way CreateOrUpdate does, so BeginUpdate(...).PollUntilDone completes and its final GET returns the updated disk.
• tags: when present they replace the tag set wholesale (ARM Compute PATCH semantics, same as virtualMachines). The internal bookkeeping tags are kept.
• sku / tier: switched in place. Moving off UltraSSD_LRS / PremiumV2_LRS drops the provisioned IOPS/MBps.
• diskSizeGB: can only grow. A shrink returns 400 BadRequest ("Disk size can only be increased...").
• diskIOPSReadWrite / diskMBpsReadWrite: accepted only on UltraSSD_LRS / PremiumV2_LRS. Any other SKU returns 400 InvalidParameter.
• The update goes through the existing in-place AzureDiskUpdater.UpdateVolume, so id, uniqueId, timeCreated and any attachment are kept. Resource Graph reads the same volume, so it reports the new size and tags.
• coveragegen wire-op list gains Update for azure/disks. docs/coverage has been regenerated.

App Service plans (server/azure/functions, providers/azure/functions)

• PATCH merges the body into the stored plan and answers 200 with the updated plan, the same as the sites PATCH.
• Supported fields: kind, sku.name/tier/capacity (a new SKU name re-derives the tier, as create does), tags (replaced wholesale when present), and properties.reserved, perSiteScaling, zoneRedundant, maximumElasticWorkerCount.
• Those four properties are now modeled on PUT too, and rendered on every read.
• A PATCH for a missing plan, or for a plan in another resource group, returns 404.
• The provider gains PatchAppServicePlan, a read-modify-write done under the memstore lock (Store.Update).
• Note: armappservice.PlanPatchResource has no sku or tags fields, so the SDK cannot send them. The handler still accepts them from a raw ARM PATCH, and a raw-HTTP test covers that path.

docs/sdk-server.md Azure rows are updated.

How it was tested

Real SDK clients point at an httptest TLS server:

• server/azure/disks/update_test.go:
  • BeginUpdate + PollUntilDone: grow, SKU change to PremiumV2 with IOPS/MBps, tag replace, and identity preserved
  • omitted fields are kept, and an empty PATCH is a no-op
  • a shrink returns 400, IOPS on Standard_LRS returns 400, a missing disk returns 404, and a rejected PATCH does not mutate the disk
  • a SKU downgrade drops IOPS
  • a Resource Graph query (armresourcegraph) shows the patched diskSizeGB and tags
• server/azure/functions/plan_patch_sdk_test.go:
  • PlansClient.Update sets maximumElasticWorkerCount/reserved/zoneRedundant and keeps sku/kind/tags
  • a raw PATCH of sku and tags re-derives the tier and replaces the tags
  • a missing plan returns 404
• providers/azure/functions/app_service_plan_test.go: TestPatchAppServicePlan.
• With the new PATCH case removed from each dispatcher, every new server test fails, so the tests do exercise the new routes.

go test -count=1 ./server/azure/... ./providers/azure/... ./internal/coveragegen/... .   # 149 packages ok
go generate ./... && go run ./internal/compatgen                                         # compat/{aws,azure,gcp} ok, no docs/compat diff
golangci-lint run --timeout=9m ./server/azure/disks/... ./server/azure/functions/... ./providers/azure/functions/... ./internal/coveragegen/...

Lint reports 15 goconst findings, and origin/development reports the same 15 for these packages. This PR adds no new findings.

[NitinKumar004]

Disks PATCH and serverfarms PATCH both route, and BeginUpdate/PollUntilDone completes. The findings below need to be addressed before merge.

High

1. Disk PATCH loses concurrent writes and races with attach/detach. server/azure/disks/update.go:40-58, providers/azure/virtualmachines/vm.go:1158-1184

• The handler builds a full VolumeConfig from a DescribeVolumes snapshot and then calls UpdateVolume. Two concurrent PATCHes (one for size, one for tags) overwrite each other.
• UpdateVolume does Get, mutates the stored *VolumeInfo in place with no lock, then calls Set. That is a data race with describeResources. It also writes back a stale pointer over a concurrent AttachVolume copy-on-write.
• Repro: AttachVolume, UpdateVolume and DescribeVolumes in parallel, 300 rounds with -race. Result: 10 DATA RACE reports at vm.go:1165/1169/1589/1226, and 14/300 disks lost their attachment (state back to available while the VM still lists the disk).
• The plan side already does this correctly with Store.Update.
• Fix: add a provider method such as PatchVolume(ctx, id, VolumeUpdate) that merges and validates inside m.volumes.Update(...) with copy-on-write, the same way AttachVolume works.

2. Azure's disk rules live only in the wire PATCH handler. update.go:117-152

• The grow-only and IOPS/MBps-SKU checks run only on PATCH. A re-PUT on an existing disk still shrinks it: PUT diskSizeGB:16 on a 128 GB disk returned 202, and GET reported 16.
• The Go library path (UpdateVolume) has no checks at all.
• Fix: move the validation into the provider (item 1) so PUT, PATCH, the library and serve share it.

Medium

3. Wrong error code for a shrink. update.go:18,122, test at update_test.go:213

• Real Azure returns InvalidParameter (internal InvalidResizeWithName); see Microsoft Learn, "Troubleshoot Azure disk resize failures", error code table.
• The PR returns BadRequest, and the test pins that code.
• Fix: return cerrors.InvalidArgument from the provider. WriteCErr already maps it to 400 InvalidParameter.

4. Attached-disk and SAS restrictions are not enforced.

• Resizing a disk attached to a running VM succeeds. Real Azure returns 409 OperationNotAllowed (internal ChangeDiskSizeWhileAttachedNotAllowed): "Cannot resize disk X while it is attached to running VM Y. ... requires the virtual machine to be deallocated". The same applies to OS disks and to SKU changes on attached disks.
• With an active SAS (beginGetAccess succeeded, tracked in diskAccess), a PATCH grew the disk from 16 to 256 GB. Real Azure returns ChangeDiskSizeWhileActiveSasNotAllowed.
• Fix: check vol.State, the attaching instance's power state and diskAccess in the provider, and return OperationNotAllowed with status 409.

5. The App Service plan OS can be flipped. providers/azure/functions/app_service_plan.go:274-276, PUT at server/azure/functions/handler.go:742

• PATCH {"properties":{"reserved":false}} on a Linux plan returned 200 with reserved:false. PATCH {"kind":"app"} was also accepted.
• A re-PUT without reserved silently turns a Linux plan into a Windows one (kind:"linux", reserved:false).
• Real Azure rejects this: "You cannot change the OS hosting your app at this time. Please recreate your app with the desired OS." (Azure/bicep#5724). azurerm treats os_type as ForceNew.
• TestPatchAppServicePlan (app_service_plan_test.go:233) asserts that the flip succeeds.
• Fix: reject a change to reserved, or to the Linux/Windows kind, on an existing plan in both the PATCH and PUT paths, and change the test to expect the error.

Low

• Plan SKU/capacity are not validated. sku.capacity:-5 and sku.name:"ZZ9" both return 200, and ZZ9 keeps the stale tier "Standard". Capacity 0 drops capacity from the response. Real Azure rejects capacity below 1, capacity above the SKU maximum, unknown SKUs, and moves between Dynamic/ElasticPremium and dedicated tiers. Setting tier explicitly next to a new name can also produce a mismatched pair.
• Disk tier handling. update.go:107 copies sku.tier (read-only, Standard/Premium) into the performance tier. properties.tier:"P50" was accepted on an UltraSSD disk and read back as sku.tier:"P50". Performance tiers only apply to Premium_LRS. A SKU change should also clear or re-derive the tier. Converting to or from UltraSSD_LRS is accepted without any check.
• Missing tests. Nothing covers attached disks, active SAS, a PUT-after-PATCH shrink, concurrent PATCH, or rejecting a plan OS flip.
• Test helpers. providers/azure/functions/app_service_plan_test.go uses testify; provider tests use hand-rolled helpers.
• Merge conflicts with current development in docs/coverage/README.md and docs/coverage/azure/README.md. Rebase and run go generate ./... again.

Verified

• Build, vet and go test -race pass for server/azure/{disks,functions}, providers/azure/{functions,virtualmachines}, internal/coveragegen and persist. golangci-lint reports 0 new issues. go run ./internal/coveragegen shows no drift on this branch. The new plan fields round-trip through the plans snapshot.
• Tags: a PATCH with tags replaces the whole set and omitted tags are kept, for both disks and plans, matching ARM resource-level semantics.
• serve over HTTPS with curl: disk grow returns 202 with Azure-AsyncOperation; a missing disk or wrong resource group returns 404; malformed JSON returns 400 InvalidRequestContent; plan sku/capacity/tags PATCH returns 200 and re-derives the tier.
• Terraform (azurerm v4.81.0): azurerm_managed_disk resized 32, 64, then 128 and moved from Standard_LRS to Premium_LRS through PATCH, with "No changes" on the follow-up plan. azurerm_service_plan B1 to S1 and worker_count 1 to 3 update through PUT (azurerm does not use PATCH for plans), with "No changes" on the follow-up plan. Destroy is clean.

[aryanmehrotra]

Thanks for the review, and for the race repro. The fixes are in 3de8b772 (disks), f0c5f2d9 (plans) and 6710f0e7 (regenerated coverage docs), rebased onto development with the README conflicts resolved.

Finding	Fix	Proof
High 1: PATCH loses concurrent writes and races attach	New optional capability AzureDiskPatcher.PatchVolume merges and validates inside m.volumes.Update with copy-on-write, the same way AttachVolume does. The handler no longer builds a full VolumeConfig from a snapshot.	TestConcurrentAttachAndPatchVolumeKeepsAttachment / …UpdateVolume… (-race). On the old code it gave 5 DATA RACE reports and 269 of 300 disks lost their attachment. TestSDKDiskConcurrentPatchesBothLand
High 2: rules only in the wire PATCH	UpdateVolume (re-PUT) goes through the same path, so PUT, PATCH, the library and serve share the rules.	TestSDKDiskRePutCannotShrink, TestPatchVolumeRules
Medium 3: shrink error code	InvalidParameter. The test that pinned BadRequest is updated.	TestSDKDiskUpdateRejections
Medium 4: attached disk / SAS	See below	TestSDKDiskUpdateAttachedToRunningVM, TestSDKDiskUpdateWithActiveSAS, TestPatchVolumeOSDiskRules, TestPatchVolumeSASExpires
Medium 5: plan OS flip	A change to reserved or to the Linux/Windows kind is refused on PATCH and PUT with Azure's message (Azure/bicep#5724). TestPatchAppServicePlan no longer asserts the flip.	TestPatchAppServicePlanRefusesOSChange, TestPutAppServicePlanRefusesOSChange, TestSDKAzureAppServicePlanOSChangeRefused
Low: plan SKU/capacity	Capacity must be at least 1 and at most the SKU's maximum. Unknown SKUs and Dynamic/ElasticPremium ↔ dedicated moves are refused, and the tier is derived from the name.	TestPatchAppServicePlanSKUAndCapacityRules
Low: disk tier	A performance tier is only accepted on Premium LRS/ZRS, must be a valid P-name at or above the size baseline, and P60–P80 only above 4 TiB. A SKU change re-derives or clears it, and sku.tier on input is ignored. Ultra can't convert either way, Premium SSD v2 can't convert away, and an OS disk can't become Premium SSD v2.	TestSDKDiskUpdatePerformanceTier, TestSDKDiskUpdatePremiumV2CannotConvertBack (which replaces the old downgrade test)
Low: tests / helpers	The missing tests above are added. The new plan provider tests use hand-rolled helpers in their own file.

Medium 4: one departure, with sources. Microsoft Learn says data disks can be expanded without deallocating the VM (expand-disks). Online resize is unsupported only for OS disks, shared disks, and Standard/Premium disks of 4 TiB or less growing past 4 TiB (troubleshoot-disk-resize). So:

• OS disk resized while its VM is still allocated: 409 OperationNotAllowed.
• SKU change on an attached disk whose VM isn't deallocated: 409 OperationNotAllowed.
• ≤4 TiB → >4 TiB while attached: 409 InvalidResizeForLargeDisks.
• Active SAS: a resize gets ChangeDiskSizeWhileActiveSasNotAllowed, and SKU, tier or IOPS changes get OperationNotAllowed.
• A data disk still grows online.

Was your live 409 on an OS disk, a shared disk, or one crossing 4 TiB? If it was a plain data disk under 4 TiB, I'd like to see it.

⚠️ Unverified:

• The HTTP status (409) for the SAS and large-disk codes. The doc names the codes but not the status.
• Allowing tag-only updates while a SAS is active.
• The per-SKU plan capacity maximums: Basic 3, Standard 10, Premium 30, Isolated 100.
• P*mv3, *v4, I*mv2 and FC1 are refused on a SKU change until deriveSKUTier knows their tiers.

Not in this PR, suggest follow-ups:

• Create/GET still fill sku.tier from the stored performance tier, and TestSDKDiskCostFields plus resource discovery depend on that.
• The 12-hour tier-downgrade limit and the twice-a-day conversion limit.
• transitionInstances writes PowerState in place without a lock. This is pre-existing, and every m.instances.Get reader shares it.
• internal/coveragegen/wireops.go already had 11 goconst hits. The new "Update" entry carries a //nolint:goconst. Turning that table into constants would be the cleaner fix.

Gates on 6710f0e7:

• go build / go vet
• go test -race across azure providers/servers, compute, resourcediscovery, persist and coveragegen: 152 packages ok
• golangci-lint --new-from-rev origin/development ./... reports 0 issues
• no generate, compat or tidy drift

[NitinKumar004]

The disk and plan fixes hold up: the races are gone, the rules now live in the provider, and the error codes match Azure. Two new issues need fixing before merge. A plan re-PUT is refused as an OS change even when nothing changed, and there is one lint failure.

Earlier findings

High 1. Disk PATCH lost writes and raced attach/detach: fixed. PatchVolume merges inside m.volumes.Update with copy-on-write, and the handler no longer rebuilds a full config from a snapshot. I ran PatchVolume(size), PatchVolume(tags), AttachVolume and DescribeVolumes concurrently over 60 disks with -race -count=50. There were no races, and every disk kept its size, tags and attachment. The lock order is volumes, then instances and diskAccess read locks. No instances callback takes the volumes lock, so there is no inversion.

High 2. Rules only on the wire PATCH: fixed. UpdateVolume goes through patchVolume. Over serve, a re-PUT of diskSizeGB:16 on a 128 GB disk now returns 400 InvalidParameter, and GET still reports 128.

Medium 3. Shrink error code: fixed. Shrinking through PATCH or re-PUT returns 400 InvalidParameter.

Medium 4. Attached disk and SAS: fixed. The data-disk departure is fine, because Microsoft documents online expansion for data disks. Verified over serve:

• A data disk grows online on a running VM (202).
• A SKU change on a running VM returns 409 OperationNotAllowed.
• An OS disk resize returns 409 OperationNotAllowed both while the VM is running and while it is stopped but still allocated.
• Crossing 4 TiB while attached returns 409 InvalidResizeForLargeDisks.
• After deallocate, the OS disk resize and the SKU change both succeed.
• With an active SAS, a resize returns 409 ChangeDiskSizeWhileActiveSasNotAllowed on both PATCH and re-PUT. After endGetAccess it goes through.

Medium 5. Plan OS flip: fixed. PATCH reserved:false and PATCH kind:"app" on a Linux plan both return 400 BadRequest with the Azure message. A re-PUT that omits reserved is refused the same way. New finding 1 below is a false positive on this same check.

Low, plan SKU/capacity: fixed. Out-of-range capacity is refused, and so is an unknown SKU. So are moves between the dedicated, Elastic Premium and Consumption families, and a mismatched sku.tier. The tier is re-derived from the name.

Low, disk tier: fixed.

Low, tests: fixed. This is moot anyway, because the other tests in both packages already use testify.

Low, merge conflicts: fixed. The coverage docs have been regenerated.

New findings

1. A Linux plan with no stored kind can't be re-PUT as Linux (providers/azure/functions/app_service_plan_rules.go:81, app_service_plan.go:322).
checkPlanOS compares linuxKind(cur.Kind) with linuxKind(next.Kind) whenever next.Kind is set, so an empty stored kind reads as Windows.

• Repro: PUT {"sku":{"name":"B1"},"properties":{"reserved":true}}, then PUT the same body with "kind":"linux" added. The second call returns 400 BadRequest "You cannot change the OS hosting your app", even though nothing changed.
• PATCH {"kind":""} is accepted and blanks the kind on a Linux plan. From then on, every re-PUT with kind:"linux" and reserved:true fails the same way.
• A plan restored from a snapshot taken before this PR has Reserved=false, even when its kind is linux.

Suggested fix:

• Treat reserved as the single source of truth for the OS, since Azure derives kind from it.
• Skip the kind comparison when either side is empty.
• Ignore an empty kind in PATCH.
• Optionally, derive Reserved from a Linux kind on create and on restore.

2. Lint gate (internal/coveragegen/wireops.go:67). golangci-lint run --new-from-rev=origin/development reports nolintlint: the //nolint:goconst on "Update" is no longer used. Drop the directive.

3. (Low) Disk errors name internal IDs (providers/azure/virtualmachines/disk_update.go:353,371,381). The messages read Cannot change the SKU of disk "/subscriptions/sub/resourceGroups/rg/.../disks/disk-1" while it is attached to running VM "vm-00000001". Those are the driver's placeholder IDs, not the caller's disk (d1 in rg1) or VM (vm1). Format the message with the ARM name tag instead, or leave the ID out and let the handler add the resource name.

4. (Low) vmAllocated adds a reader to an existing PowerState race (disk_update.go:421). If you add Stop, Deallocate and Start to the concurrent test, you get DATA RACE reports at vm.go:709/714 (transitionInstances) against vmAllocated, and also against DescribeInstances, which predates this PR. A follow-up is fine, but this race is now on the disk update path too.

5. (Low, pre-existing, fine as follow-ups)

• diskState only reports Unattached and Attached. Azure also reports ActiveSAS during a grant and Reserved for a disk on a deallocated VM, and these new rules depend on those states.
• A re-PUT that changes location silently moves a disk or plan.
• sku.tier echoes the performance tier (P15) after a PATCH.
• Plan SKU names keep the caller's casing.
• The create path doesn't apply the per-tier capacity maximum (B1 with capacity 50 is accepted).

Checked

• go build ./....
• go vet and go test -race on providers/azure/{virtualmachines,functions}, server/azure/{disks,functions}, services/compute, internal/coveragegen and persist.
• Concurrent disk and plan PATCH tests with -race -count=50.
• Over serve with curl:
  • Disk PATCH and re-PUT rules.
  • Attached and deallocated OS and data disks.
  • SAS grant and revoke.
  • 404 and malformed JSON.
  • Tags REPLACE on PATCH, and tags absent or null keeping the existing set, for both disks and plans.
  • Plan PATCH for OS, SKU, capacity, tier and property fields.
  • Snapshot round-trip of the new plan fields.