diff --git a/docs/coverage/README.md b/docs/coverage/README.md index 96c4e64b3..07c6a30a6 100644 --- a/docs/coverage/README.md +++ b/docs/coverage/README.md @@ -76,7 +76,7 @@ code does not implement. Machine-readable: [`coverage.json`](./coverage.json). | `datastream` | - | - | [Datastream](./gcp/datastream.md) | - | 11 | | `devcenter` | - | [DevCenter](./azure/devcenter.md) | - | - | 8 | | `digitaltwins` | - | [DigitalTwins](./azure/digitaltwins.md) | - | - | 8 | -| `disks` | - | [Disks](./azure/disks.md) | - | - | 7 | +| `disks` | - | [Disks](./azure/disks.md) | - | - | 8 | | `dns` | [Route53](./aws/route53.md) | [DNS](./azure/dns.md) | [CloudDNS](./gcp/clouddns.md) | - | 16 | | `ecs` | [ECS](./aws/ecs.md) | - | - | - | 37 | | `efs` | [EFS](./aws/efs.md) | - | - | - | 27 | diff --git a/docs/coverage/azure/README.md b/docs/coverage/azure/README.md index 8f0b55897..1f64e6dc5 100644 --- a/docs/coverage/azure/README.md +++ b/docs/coverage/azure/README.md @@ -29,7 +29,7 @@ Services cloudemu emulates for Azure, by native name. Back to the [cross-provide | [Databricks](./databricks.md) | `databricks` | 46 | | [DevCenter](./devcenter.md) | (provider-native) | 8 | | [DigitalTwins](./digitaltwins.md) | (provider-native) | 8 | -| [Disks](./disks.md) | (provider-native) | 7 | +| [Disks](./disks.md) | (provider-native) | 8 | | [ElasticSan](./elasticsan.md) | (provider-native) | 8 | | [EventGrid](./eventgrid.md) | `eventbus` | 16 | | [Eventhub](./eventhub.md) | (provider-native) | 14 | diff --git a/docs/coverage/azure/disks.md b/docs/coverage/azure/disks.md index b45d3e747..ca932c387 100644 --- a/docs/coverage/azure/disks.md +++ b/docs/coverage/azure/disks.md @@ -3,7 +3,7 @@ provider-native `disks` wire service (Azure-only) · no portable driver · [Azure index](./README.md) -## Operations (7) +## Operations (8) | Operation | Description | | --- | --- | @@ -14,6 +14,7 @@ provider-native `disks` wire service (Azure-only) · no portable driver · [Azur | `List` | | | `ListByResourceGroup` | | | `RevokeAccess` | | +| `Update` | | ## Not in scope diff --git a/docs/coverage/azure/virtualmachines.md b/docs/coverage/azure/virtualmachines.md index c7ffd2e07..74fc999ff 100644 --- a/docs/coverage/azure/virtualmachines.md +++ b/docs/coverage/azure/virtualmachines.md @@ -66,6 +66,14 @@ AzureDiskDeleteOptioner is an optional Azure-only capability that records a | --- | --- | | `SetDiskDeleteOnTermination` | SetDiskDeleteOnTermination records whether the volume is deleted (true) or | +### AzureDiskPatcher + +AzureDiskPatcher is an optional Azure-only capability for a partial managed + +| Operation | Description | +| --- | --- | +| `PatchVolume` | PatchVolume applies patch to the volume id and returns the stored | + ### AzureDiskUpdater AzureDiskUpdater is an optional Azure-only capability for an in-place managed diff --git a/docs/coverage/coverage.json b/docs/coverage/coverage.json index 8d520510c..604715f78 100644 --- a/docs/coverage/coverage.json +++ b/docs/coverage/coverage.json @@ -4018,6 +4018,16 @@ } ] }, + { + "name": "AzureDiskPatcher", + "doc": "AzureDiskPatcher is an optional Azure-only capability for a partial managed", + "operations": [ + { + "name": "PatchVolume", + "doc": "PatchVolume applies patch to the volume id and returns the stored" + } + ] + }, { "name": "AzureDiskUpdater", "doc": "AzureDiskUpdater is an optional Azure-only capability for an in-place managed", @@ -5906,6 +5916,9 @@ }, { "name": "RevokeAccess" + }, + { + "name": "Update" } ], "providers": { diff --git a/docs/sdk-server.md b/docs/sdk-server.md index 346aa102b..002c01df8 100644 --- a/docs/sdk-server.md +++ b/docs/sdk-server.md @@ -191,13 +191,13 @@ All handlers speak ARM JSON over HTTPS unless noted. | Service | ARM provider / operations | |---------|--------------------------| | **Virtual Machines** | `Microsoft.Compute/virtualMachines`: CreateOrUpdate, Get, List, Delete, start, powerOff, restart | -| **Disks / Snapshots / Images / SSH Public Keys** | `Microsoft.Compute/{disks,snapshots,images,sshPublicKeys}`: full CRUD | +| **Disks / Snapshots / Images / SSH Public Keys** | `Microsoft.Compute/{disks,snapshots,images,sshPublicKeys}`: full CRUD. Disks also serve Update (PATCH, 202 + Azure-AsyncOperation, so `armcompute` `DisksClient.BeginUpdate` pollers complete): tags, `sku`, `tier`, `diskSizeGB` (grow only; a shrink is a 400) and `diskIOPSReadWrite`/`diskMBpsReadWrite` (UltraSSD_LRS / PremiumV2_LRS only), plus beginGetAccess/endGetAccess. | | **Blob Storage** *(data plane)* | Containers + Blobs: Create/Delete/List, PutBlob, GetBlob, DeleteBlob, CopyBlob | | **Cosmos DB** *(data plane)* | Databases, Containers, Documents: full CRUD with `x-ms-documentdb-*` headers | | **Cosmos DB (SQL ARM control plane)** | `Microsoft.DocumentDB/databaseAccounts/{acct}/sqlDatabases[/containers]`: SQL databases (CreateUpdate/Get/List/Delete, cascading container delete), containers (CreateUpdate/Get/List/Delete with partitionKey, defaultTtl, uniqueKeyPolicy, indexingPolicy), and `throughputSettings/default` at database and container level (Get/Update manual RU/s or autoscale maxThroughput + migrateToAutoscale/migrateToManualThroughput). Real `armcosmos` `SQLResources` clients round-trip end-to-end, including the LRO pollers, so Terraform/Bicep/`az cosmosdb sql` can manage the data model. Shares state with the Cosmos data plane above; a control-plane database/container/throughput is visible to data-plane clients and vice versa. | | **Virtual Network** | `Microsoft.Network/{virtualNetworks,networkSecurityGroups,publicIPAddresses,networkInterfaces}`: CRUD + nested subnets; NICs bind a subnet and get a private IP | | **Azure Monitor** | `microsoft.insights/metricAlerts` and metric data ingest/read | -| **Functions** | `Microsoft.Web/sites` (Function Apps): CreateOrUpdate, Get, List, Delete + non-ARM `/api/{name}` invoke | +| **Functions** | `Microsoft.Web/sites` (Function Apps): CreateOrUpdate, Get, List, Update (PATCH), Delete + non-ARM `/api/{name}` invoke. `Microsoft.Web/serverfarms` (App Service plans): CreateOrUpdate, Get, List, ListWebApps, Update (PATCH: `kind`, `sku`, tags, `reserved`, `perSiteScaling`, `zoneRedundant`, `maximumElasticWorkerCount`), Delete. | | **Service Bus** | `Microsoft.ServiceBus/namespaces[/queues]` ARM CRUD + raw-HTTP REST data plane (`POST /{ns}/{queue}/messages`, `DELETE /messages/head`) | | **SQL Database** | `Microsoft.Sql/servers[/databases]`: servers and databases, full CRUD lifecycle | | **Managed Cassandra** | `Microsoft.DocumentDB/cassandraClusters[/dataCenters]`: clusters (CreateOrUpdate, Get, ListByResourceGroup, ListBySubscription, Update, Delete, deallocate, start, invokeCommand, status) and datacenters (CreateOrUpdate, Get, List, Update, Delete). Real `armcosmos` `CassandraClusters`/`CassandraDataCenters` clients round-trip end-to-end, including the LRO pollers. | diff --git a/internal/coveragegen/wireops.go b/internal/coveragegen/wireops.go index 49abed02c..554d3cf8e 100644 --- a/internal/coveragegen/wireops.go +++ b/internal/coveragegen/wireops.go @@ -64,6 +64,7 @@ var nativeWireOperations = map[string][]string{ //nolint:gochecknoglobals // gen }, "azure/disks": { "CreateOrUpdate", "Delete", "Get", "GrantAccess", "List", "ListByResourceGroup", "RevokeAccess", + "Update", //nolint:goconst // SDK operation names are table data; the file already repeats them (pre-existing goconst hits) }, "azure/snapshots": { "CreateOrUpdate", "Delete", "Get", "List", "ListByResourceGroup", diff --git a/providers/azure/functions/app_service_plan.go b/providers/azure/functions/app_service_plan.go index 6abc1b7e1..acd090529 100644 --- a/providers/azure/functions/app_service_plan.go +++ b/providers/azure/functions/app_service_plan.go @@ -28,6 +28,13 @@ type AppServicePlan struct { Kind string // app / functionapp / linux Capacity int Tags map[string]string + + // Plan properties a caller can change after create (armappservice + // PlanProperties / PlanPatchResourceProperties). + Reserved bool // true for a Linux plan + PerSiteScaling bool + ZoneRedundant bool + MaximumElasticWorkerCount int // Elastic Premium burst ceiling } // App Service plan pricing tiers, the tier real Azure derives from a SKU name @@ -138,13 +145,48 @@ func (m *Mock) CreateAppServicePlan(_ context.Context, p AppServicePlan) (*AppSe p.Capacity = 1 } - stored := p + if p.Capacity < 0 { + return nil, cerrors.Newf(cerrors.InvalidArgument, "Invalid sku.capacity %d: it must be at least 1.", p.Capacity) + } + + return m.putAppServicePlan(&p) +} - m.plans.Set(planKey(p.Subscription, p.ResourceGroup, p.Name), &stored) +// putAppServicePlan stores p, creating the plan or replacing an existing one. +// A replacement (an ARM re-PUT) is subject to the same checks as a PATCH, run +// under the store lock: in particular it cannot flip the plan's OS. +func (m *Mock) putAppServicePlan(p *AppServicePlan) (*AppServicePlan, error) { + key := planKey(p.Subscription, p.ResourceGroup, p.Name) - out := stored + for { + var putErr error - return &out, nil + replaced := m.plans.Update(key, func(cur *AppServicePlan) *AppServicePlan { + if err := checkPlanUpdate(cur, p); err != nil { + putErr = err + return cur + } + + stored := *p + + return &stored + }) + + if putErr != nil { + return nil, putErr + } + + if !replaced { + stored := *p + if !m.plans.SetIfAbsent(key, &stored) { + continue // created concurrently: replace it instead + } + } + + out := *p + + return &out, nil + } } // GetAppServicePlan returns one App Service plan scoped to the given @@ -224,3 +266,88 @@ func (m *Mock) ListAppServicePlans(_ context.Context, subscription, resourceGrou return out, nil } + +// AppServicePlanPatch is a partial App Service plan update (ARM PATCH +// .../serverfarms/{name}). A nil field leaves the stored value untouched; a +// non-nil Tags map replaces the plan's tags wholesale. +type AppServicePlanPatch struct { + Kind *string + SKUName *string + SKUTier *string + Capacity *int + Tags map[string]string + Reserved *bool + PerSiteScaling *bool + ZoneRedundant *bool + MaximumElasticWorkerCount *int +} + +// PatchAppServicePlan applies a partial update to one App Service plan scoped +// to the given subscription and resource group, returning the stored result or +// NotFound. The read-modify-write and the checks in checkPlanUpdate (no OS +// change, a known SKU in the same plan family, a matching tier, a capacity in +// range) run under the store lock. A SKU name change re-derives the tier. +func (m *Mock) PatchAppServicePlan( + _ context.Context, subscription, resourceGroup, name string, patch AppServicePlanPatch, +) (*AppServicePlan, error) { + var out AppServicePlan + + var patchErr error + + found := m.plans.Update(planKey(subscription, resourceGroup, name), func(p *AppServicePlan) *AppServicePlan { + next := *p + applyPlanPatch(&next, &patch) + + if err := checkPlanUpdate(p, &next); err != nil { + patchErr = err + return p + } + + out = next + + return &next + }) + if !found { + return nil, cerrors.Newf(cerrors.NotFound, "app service plan %s not found", name) + } + + if patchErr != nil { + return nil, patchErr + } + + return &out, nil +} + +func applyPlanPatch(p *AppServicePlan, patch *AppServicePlanPatch) { + setIf(&p.Kind, patch.Kind) + setIf(&p.Capacity, patch.Capacity) + setIf(&p.Reserved, patch.Reserved) + setIf(&p.PerSiteScaling, patch.PerSiteScaling) + setIf(&p.ZoneRedundant, patch.ZoneRedundant) + setIf(&p.MaximumElasticWorkerCount, patch.MaximumElasticWorkerCount) + + if patch.SKUName != nil && *patch.SKUName != "" && !strings.EqualFold(*patch.SKUName, p.SKUName) { + p.SKUName = *patch.SKUName + p.SKUTier = deriveSKUTier(p.SKUName) + } + + if patch.SKUTier != nil && *patch.SKUTier != "" { + p.SKUTier = *patch.SKUTier + } + + if patch.Tags != nil { + tags := make(map[string]string, len(patch.Tags)) + for k, v := range patch.Tags { + tags[k] = v + } + + p.Tags = tags + } +} + +// setIf assigns *src to *dst when src is non-nil. +func setIf[T any](dst, src *T) { + if src != nil { + *dst = *src + } +} diff --git a/providers/azure/functions/app_service_plan_rules.go b/providers/azure/functions/app_service_plan_rules.go new file mode 100644 index 000000000..173629a63 --- /dev/null +++ b/providers/azure/functions/app_service_plan_rules.go @@ -0,0 +1,141 @@ +package functions + +import ( + "regexp" + "strings" + + cerrors "github.com/stackshy/cloudemu/v2/errors" +) + +// codeBadRequest is the ARM error code real Azure answers an OS change on an +// existing plan with (Azure/bicep#5724: 400 BadRequest, "You cannot change the +// OS hosting your app at this time. Please recreate your app with the desired +// OS."). +const codeBadRequest = "BadRequest" + +// PlanError is an App Service plan refusal that real Azure reports with a +// specific ARM error code. It unwraps to the canonical cloudemu error, so a +// caller that only checks the code still sees it; the wire layer echoes Code. +type PlanError struct { + Code string + Err error +} + +// Error implements the error interface. +func (e *PlanError) Error() string { return e.Err.Error() } + +// Unwrap returns the canonical cloudemu error this one stands for. +func (e *PlanError) Unwrap() error { return e.Err } + +// knownPlanSKU matches the App Service plan SKU names a plan can be moved to +// and whose tier deriveSKUTier maps correctly: Free, Shared, Basic, Standard, +// Premium (v1/v2/v3), Isolated (v1/v2), Consumption, Elastic Premium and +// Workflow Standard. +var knownPlanSKU = regexp.MustCompile( + `^(F1|D1|B[1-3]|S[1-3]|P[1-3]|P[1-3]V2|P[0-3]V3|I[1-3]|I[1-6]V2|Y1|EP[1-3]|WS[1-3])$`) + +// maxPlanCapacity is the largest sku.capacity (instance count) of a dedicated +// tier, from the App Service limits (Azure subscription and service limits): +// Free and Shared 1, Basic 3, Standard 10, Premium 30, Isolated 100. Other +// tiers are not capped here. +var maxPlanCapacity = map[string]int{ //nolint:gochecknoglobals // lookup table + tierFree: 1, tierShared: 1, tierBasic: 3, tierStandard: 10, + tierPremium: 30, tierPremiumV2: 30, tierPremiumV3: 30, + tierIsolated: 100, tierIsolatedV2: 100, +} + +// checkPlanUpdate validates replacing plan cur with next (a PATCH or a +// re-PUT). Only what changes is checked, so a plan created with an unusual +// SKU can still be re-PUT unchanged. +func checkPlanUpdate(cur, next *AppServicePlan) error { + if err := checkPlanOS(cur, next); err != nil { + return err + } + + skuChanged := !strings.EqualFold(cur.SKUName, next.SKUName) + + if skuChanged { + if err := checkPlanSKUMove(cur, next); err != nil { + return err + } + } + + if skuChanged || !strings.EqualFold(cur.SKUTier, next.SKUTier) { + if want := deriveSKUTier(next.SKUName); !strings.EqualFold(next.SKUTier, want) { + return cerrors.Newf(cerrors.InvalidArgument, + "The sku tier '%s' does not match sku name '%s' (tier '%s').", next.SKUTier, next.SKUName, want) + } + } + + if skuChanged || cur.Capacity != next.Capacity { + return checkPlanCapacity(next) + } + + return nil +} + +// checkPlanOS refuses a change of the plan's OS: reserved (true for Linux), +// or a kind that moves between Linux and Windows. An omitted kind (a re-PUT +// without one) is not a change of OS. +func checkPlanOS(cur, next *AppServicePlan) error { + if cur.Reserved == next.Reserved && (next.Kind == "" || linuxKind(cur.Kind) == linuxKind(next.Kind)) { + return nil + } + + return &PlanError{Code: codeBadRequest, Err: cerrors.New(cerrors.InvalidArgument, + "You cannot change the OS hosting your app at this time. Please recreate your app with the desired OS.")} +} + +// checkPlanSKUMove rejects an unknown SKU, and a move between the +// Consumption, Elastic Premium, Workflow Standard and dedicated families: a +// plan's hosting model is fixed at create. +func checkPlanSKUMove(cur, next *AppServicePlan) error { + if !knownPlanSKU.MatchString(strings.ToUpper(next.SKUName)) { + return cerrors.Newf(cerrors.InvalidArgument, "The pricing tier '%s' is not allowed for this App Service plan.", + next.SKUName) + } + + from, to := planFamily(cur.SKUTier), planFamily(deriveSKUTier(next.SKUName)) + if from != to { + return cerrors.Newf(cerrors.InvalidArgument, + "Cannot change the App Service plan from %s (%s) to %s (%s). Create a new plan instead.", + cur.SKUName, cur.SKUTier, next.SKUName, deriveSKUTier(next.SKUName)) + } + + return nil +} + +// checkPlanCapacity rejects an instance count below 1 or above the tier's +// maximum. +func checkPlanCapacity(p *AppServicePlan) error { + if p.Capacity < 1 { + return cerrors.Newf(cerrors.InvalidArgument, "Invalid sku.capacity %d: it must be at least 1.", p.Capacity) + } + + if limit, ok := maxPlanCapacity[p.SKUTier]; ok && p.Capacity > limit { + return cerrors.Newf(cerrors.InvalidArgument, + "Invalid sku.capacity %d: the %s tier allows at most %d instances.", p.Capacity, p.SKUTier, limit) + } + + return nil +} + +// planFamily groups plan tiers by hosting model. +func planFamily(tier string) string { + switch { + case strings.EqualFold(tier, tierDynamic): + return tierDynamic + case strings.EqualFold(tier, tierElasticPremium): + return tierElasticPremium + case strings.EqualFold(tier, tierWorkflowStandard): + return tierWorkflowStandard + default: + return "Dedicated" + } +} + +// linuxKind reports whether a plan kind names a Linux plan ("linux", +// "functionapp,linux", ...). +func linuxKind(kind string) bool { + return strings.Contains(strings.ToLower(kind), "linux") +} diff --git a/providers/azure/functions/app_service_plan_update_test.go b/providers/azure/functions/app_service_plan_update_test.go new file mode 100644 index 000000000..0dd5cee56 --- /dev/null +++ b/providers/azure/functions/app_service_plan_update_test.go @@ -0,0 +1,198 @@ +package functions + +import ( + "context" + "errors" + "maps" + "testing" + + cerrors "github.com/stackshy/cloudemu/v2/errors" +) + +func planMustNotErr(t *testing.T, err error) { + t.Helper() + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func planWantEqual[T comparable](t *testing.T, what string, got, want T) { + t.Helper() + + if got != want { + t.Errorf("%s = %v, want %v", what, got, want) + } +} + +// planWantInvalid asserts err is an InvalidArgument (400) refusal. +func planWantInvalid(t *testing.T, what string, err error) { + t.Helper() + + if !cerrors.IsInvalidArgument(err) { + t.Errorf("%s: err = %v, want InvalidArgument", what, err) + } +} + +// planWantOSChangeRefused asserts err is the 400 BadRequest Azure answers an +// OS change with. +func planWantOSChangeRefused(t *testing.T, what string, err error) { + t.Helper() + + var pe *PlanError + if !errors.As(err, &pe) || pe.Code != codeBadRequest || !cerrors.IsInvalidArgument(err) { + t.Errorf("%s: err = %v, want a BadRequest PlanError", what, err) + } + + if err != nil && err.Error() == "" { + t.Errorf("%s: empty error text", what) + } +} + +func createPlanForUpdate(t *testing.T, m *Mock, p AppServicePlan) { + t.Helper() + + p.Subscription, p.ResourceGroup = "sub", "rg" + if p.Name == "" { + p.Name = "p" + } + + _, err := m.CreateAppServicePlan(context.Background(), p) + planMustNotErr(t, err) +} + +func patchPlan(m *Mock, patch AppServicePlanPatch) (*AppServicePlan, error) { + return m.PatchAppServicePlan(context.Background(), "sub", "rg", "p", patch) +} + +func TestPatchAppServicePlan(t *testing.T) { + m := newTestMock() + createPlanForUpdate(t, m, AppServicePlan{SKUName: "S1", Tags: map[string]string{"env": "dev"}}) + + workers, sku, perSite := 10, "P1v3", true + + got, err := patchPlan(m, AppServicePlanPatch{ + SKUName: &sku, PerSiteScaling: &perSite, MaximumElasticWorkerCount: &workers, + }) + planMustNotErr(t, err) + + planWantEqual(t, "sku name", got.SKUName, "P1v3") + planWantEqual(t, "sku tier (re-derived)", got.SKUTier, tierPremiumV3) + planWantEqual(t, "perSiteScaling", got.PerSiteScaling, true) + planWantEqual(t, "maximumElasticWorkerCount", got.MaximumElasticWorkerCount, 10) + planWantEqual(t, "capacity (omitted, kept)", got.Capacity, 1) + + if !maps.Equal(got.Tags, map[string]string{"env": "dev"}) { + t.Errorf("tags = %v, want the omitted tags kept", got.Tags) + } + + got, err = patchPlan(m, AppServicePlanPatch{Tags: map[string]string{}}) + planMustNotErr(t, err) + + if len(got.Tags) != 0 { + t.Errorf("tags = %v, want an empty tags map to clear them", got.Tags) + } + + planWantEqual(t, "perSiteScaling (omitted, kept)", got.PerSiteScaling, true) + + stored, err := m.GetAppServicePlan(context.Background(), "sub", "rg", "p") + planMustNotErr(t, err) + planWantEqual(t, "stored sku", stored.SKUName, got.SKUName) + + _, err = m.PatchAppServicePlan(context.Background(), "sub", "other-rg", "p", AppServicePlanPatch{PerSiteScaling: &perSite}) + if !cerrors.IsNotFound(err) { + t.Errorf("PATCH in another resource group: err = %v, want NotFound", err) + } +} + +// TestPatchAppServicePlanRefusesOSChange: real Azure refuses to change the OS +// of an existing plan (Azure/bicep#5724), by reserved or by kind. +func TestPatchAppServicePlanRefusesOSChange(t *testing.T) { + m := newTestMock() + createPlanForUpdate(t, m, AppServicePlan{SKUName: "P1v3", Kind: "linux", Reserved: true}) + + windows, appKind, linuxFunc := false, "app", "functionapp,linux" + + _, err := patchPlan(m, AppServicePlanPatch{Reserved: &windows}) + planWantOSChangeRefused(t, "reserved:false on a Linux plan", err) + + _, err = patchPlan(m, AppServicePlanPatch{Kind: &appKind}) + planWantOSChangeRefused(t, "kind:app on a Linux plan", err) + + got, err := patchPlan(m, AppServicePlanPatch{Kind: &linuxFunc}) + planMustNotErr(t, err) + planWantEqual(t, "kind (same OS)", got.Kind, "functionapp,linux") + planWantEqual(t, "reserved", got.Reserved, true) +} + +// TestPutAppServicePlanRefusesOSChange: a re-PUT that omits reserved on a +// Linux plan used to turn it into a Windows plan silently. +func TestPutAppServicePlanRefusesOSChange(t *testing.T) { + m := newTestMock() + createPlanForUpdate(t, m, AppServicePlan{SKUName: "P1v3", Kind: "linux", Reserved: true}) + + _, err := m.CreateAppServicePlan(context.Background(), AppServicePlan{ + Name: "p", Subscription: "sub", ResourceGroup: "rg", SKUName: "P1v3", Kind: "linux", + }) + planWantOSChangeRefused(t, "re-PUT without reserved", err) + + stored, err := m.GetAppServicePlan(context.Background(), "sub", "rg", "p") + planMustNotErr(t, err) + planWantEqual(t, "reserved after refused re-PUT", stored.Reserved, true) + + got, err := m.CreateAppServicePlan(context.Background(), AppServicePlan{ + Name: "p", Subscription: "sub", ResourceGroup: "rg", SKUName: "P2v3", Kind: "linux", Reserved: true, Capacity: 3, + }) + planMustNotErr(t, err) + planWantEqual(t, "re-PUT sku", got.SKUName, "P2v3") + planWantEqual(t, "re-PUT capacity", got.Capacity, 3) +} + +// TestPatchAppServicePlanSKUAndCapacityRules covers the SKU and capacity +// checks: capacity at least 1 and at most the tier's instance limit, a known +// SKU, no move between hosting families, and a tier that matches the name. +func TestPatchAppServicePlanSKUAndCapacityRules(t *testing.T) { + m := newTestMock() + createPlanForUpdate(t, m, AppServicePlan{SKUName: "B1"}) + createPlanForUpdate(t, m, AppServicePlan{Name: "consumption", SKUName: "Y1"}) + + ptr := func(s string) *string { return &s } + n := func(i int) *int { return &i } + + for _, tc := range []struct { + what string + patch AppServicePlanPatch + }{ + {"capacity 0", AppServicePlanPatch{Capacity: n(0)}}, + {"capacity -5", AppServicePlanPatch{Capacity: n(-5)}}, + {"capacity above the Basic limit of 3", AppServicePlanPatch{Capacity: n(4)}}, + {"unknown sku", AppServicePlanPatch{SKUName: ptr("ZZ9")}}, + {"dedicated to Elastic Premium", AppServicePlanPatch{SKUName: ptr("EP1")}}, + {"dedicated to Consumption", AppServicePlanPatch{SKUName: ptr("Y1")}}, + {"mismatched explicit tier", AppServicePlanPatch{SKUName: ptr("S1"), SKUTier: ptr("Premium")}}, + {"tier alone", AppServicePlanPatch{SKUTier: ptr("Standard")}}, + {"sku move above the new tier limit", AppServicePlanPatch{SKUName: ptr("S1"), Capacity: n(11)}}, + } { + _, err := patchPlan(m, tc.patch) + planWantInvalid(t, tc.what, err) + } + + stored, err := m.GetAppServicePlan(context.Background(), "sub", "rg", "p") + planMustNotErr(t, err) + planWantEqual(t, "sku after refusals", stored.SKUName, "B1") + planWantEqual(t, "capacity after refusals", stored.Capacity, 1) + + got, err := patchPlan(m, AppServicePlanPatch{SKUName: ptr("S1"), SKUTier: ptr("Standard"), Capacity: n(10)}) + planMustNotErr(t, err) + planWantEqual(t, "tier", got.SKUTier, tierStandard) + planWantEqual(t, "capacity", got.Capacity, 10) + + _, err = m.PatchAppServicePlan(context.Background(), "sub", "rg", "consumption", + AppServicePlanPatch{SKUName: ptr("S1")}) + planWantInvalid(t, "Consumption to dedicated", err) + + _, err = m.CreateAppServicePlan(context.Background(), AppServicePlan{ + Name: "neg", Subscription: "sub", ResourceGroup: "rg", SKUName: "S1", Capacity: -1, + }) + planWantInvalid(t, "create with negative capacity", err) +} diff --git a/providers/azure/virtualmachines/disk_update.go b/providers/azure/virtualmachines/disk_update.go new file mode 100644 index 000000000..5f3ff7d01 --- /dev/null +++ b/providers/azure/virtualmachines/disk_update.go @@ -0,0 +1,500 @@ +package virtualmachines + +import ( + "net/url" + "strings" + "time" + + cerrors "github.com/stackshy/cloudemu/v2/errors" + "github.com/stackshy/cloudemu/v2/services/compute/driver" +) + +// Managed disk SKUs (armcompute DiskStorageAccountTypes). +const ( + skuStandardLRS = "Standard_LRS" + skuStandardSSDLRS = "StandardSSD_LRS" + skuStandardSSDZRS = "StandardSSD_ZRS" + skuPremiumLRS = "Premium_LRS" + skuPremiumZRS = "Premium_ZRS" + skuPremiumV2LRS = "PremiumV2_LRS" + skuUltraSSDLRS = "UltraSSD_LRS" +) + +const ( + // osDiskDevice mirrors the Device marker server/azure/virtualmachines + // attaches a VM's OS disk at, so the disk rules can tell an OS disk from a + // data disk. + osDiskDevice = "osdisk" + + // largeDiskBoundaryGiB is where Standard HDD/SSD and Premium SSD move to a + // different storage back end. Crossing it needs the disk detached. + largeDiskBoundaryGiB = 4096 + + // maxDiskSizeGiB / maxProvisionedDiskSizeGiB are the largest managed disk + // sizes (Standard/Premium SSD, and Ultra Disk / Premium SSD v2). + maxDiskSizeGiB = 32767 + maxProvisionedDiskSizeGiB = 65536 + + // sasTimeLayout is the se= (signed expiry) format GrantDiskAccess writes. + sasTimeLayout = "2006-01-02T15:04:05Z" + + // ARM error codes for a disk update Azure refuses because of the disk's + // state rather than the request. + codeOperationNotAllowed = "OperationNotAllowed" + codeResizeWhileActiveSAS = "ChangeDiskSizeWhileActiveSasNotAllowed" + codeInvalidResizeLargeDisks = "InvalidResizeForLargeDisks" +) + +// knownDiskSKUs are the storage account types a disk can be converted to, by +// lower-cased name, mapped to their canonical spelling. +var knownDiskSKUs = map[string]string{ //nolint:gochecknoglobals // lookup table + strings.ToLower(skuStandardLRS): skuStandardLRS, + strings.ToLower(skuStandardSSDLRS): skuStandardSSDLRS, + strings.ToLower(skuStandardSSDZRS): skuStandardSSDZRS, + strings.ToLower(skuPremiumLRS): skuPremiumLRS, + strings.ToLower(skuPremiumZRS): skuPremiumZRS, + strings.ToLower(skuPremiumV2LRS): skuPremiumV2LRS, + strings.ToLower(skuUltraSSDLRS): skuUltraSSDLRS, +} + +// premiumTiers are the Premium SSD performance tiers in ascending order, each +// with the largest disk size (GiB) it is the baseline for. From the +// "Performance tiers for Azure Premium SSD managed disks" table on Microsoft +// Learn (virtual-machines/disks-change-performance). +var premiumTiers = []struct { //nolint:gochecknoglobals // lookup table + name string + maxSize int +}{ + {"P1", 4}, {"P2", 8}, {"P3", 16}, {"P4", 32}, {"P6", 64}, {"P10", 128}, + {"P15", 256}, {"P20", 512}, {"P30", 1024}, {"P40", 2048}, {"P50", 4096}, + {"P60", 8192}, {"P70", 16384}, {"P80", maxDiskSizeGiB}, +} + +// p50Index is the position of P50, the highest tier a disk of 4 TiB or less +// may use: P60, P70 and P80 need a disk larger than 4,096 GiB. +const p50Index = 10 + +// patchVolume merges patch over the stored volume id and validates the result +// against Azure's managed-disk update rules, all inside one store-lock span +// with copy-on-write (like AttachVolume), so a concurrent attach, detach or +// update is never overwritten with a stale copy. location, when non-empty, +// replaces the stored region (the PUT path). +func (m *Mock) patchVolume(id string, patch *driver.AzureDiskPatch, location string) (*driver.VolumeInfo, error) { + var ( + out driver.VolumeInfo + patchErr error + ) + + ok := m.volumes.Update(id, func(cur *driver.VolumeInfo) *driver.VolumeInfo { + next, err := m.mergeVolumePatch(cur, patch) + if err != nil { + patchErr = err + return cur + } + + if location != "" { + next.Location = location + } + + out = *next + out.Tags = copyTags(next.Tags) + + return next + }) + if !ok { + return nil, cerrors.Newf(cerrors.NotFound, "disk %q not found", id) + } + + if patchErr != nil { + return nil, patchErr + } + + return &out, nil +} + +// volumeChange records which disk properties a patch actually changes. +type volumeChange struct { + size, sku, tier, perf bool +} + +func (c volumeChange) any() bool { return c.size || c.sku || c.tier || c.perf } + +// mergeVolumePatch returns a new VolumeInfo with patch applied over cur, or +// the error real Azure answers the update with. It never mutates cur. Request +// errors (400) are checked before state conflicts (409). +func (m *Mock) mergeVolumePatch(cur *driver.VolumeInfo, patch *driver.AzureDiskPatch) (*driver.VolumeInfo, error) { + next := *cur + next.Tags = patchVolumeTags(cur.Tags, patch.Tags, patch.KeepTags) + + var chg volumeChange + + if err := applySizePatch(&next, cur, patch.Size, &chg); err != nil { + return nil, err + } + + if err := applySKUPatch(&next, cur, patch.VolumeType, &chg); err != nil { + return nil, err + } + + if err := applyPerfPatch(&next, cur, patch, &chg); err != nil { + return nil, err + } + + if err := applyTierPatch(&next, cur, patch.Tier, &chg); err != nil { + return nil, err + } + + if !chg.any() { + return &next, nil + } + + if chg.size { + if err := checkMaxSize(&next); err != nil { + return nil, err + } + } + + if err := m.checkVolumeState(cur, &next, chg); err != nil { + return nil, err + } + + return &next, nil +} + +// applySizePatch grows the disk. Azure never shrinks a managed disk +// (InvalidParameter / InvalidResizeWithName) and rejects a size above the +// SKU's maximum. +func applySizePatch(next, cur *driver.VolumeInfo, size *int, chg *volumeChange) error { + if size == nil || *size == cur.Size { + return nil + } + + if *size < cur.Size { + return cerrors.Newf(cerrors.InvalidArgument, + "Disk size can only be increased. Current size is %d GB, requested size is %d GB.", cur.Size, *size) + } + + next.Size = *size + chg.size = true + + return nil +} + +// applySKUPatch converts the disk to another storage account type, subject to +// checkSKUConversion. A conversion drops the provisioned IOPS/throughput of a +// SKU that does not carry them. +func applySKUPatch(next, cur *driver.VolumeInfo, sku *string, chg *volumeChange) error { + if sku == nil || *sku == "" || strings.EqualFold(*sku, cur.VolumeType) { + return nil + } + + target, ok := knownDiskSKUs[strings.ToLower(*sku)] + if !ok { + return cerrors.Newf(cerrors.InvalidArgument, "The value '%s' of parameter 'sku.name' is not valid.", *sku) + } + + if err := checkSKUConversion(cur, target); err != nil { + return err + } + + next.VolumeType = target + chg.sku = true + + if !provisionedPerfSKU(target) { + next.IOPS = 0 + next.Throughput = 0 + } + + return nil +} + +// checkSKUConversion applies Microsoft Learn (virtual-machines/ +// disks-convert-types): an Ultra Disk can be neither the source nor the target +// of a conversion, a Premium SSD v2 cannot be converted to anything else, and +// an OS disk cannot become a Premium SSD v2. +func checkSKUConversion(cur *driver.VolumeInfo, target string) error { + switch { + case strings.EqualFold(cur.VolumeType, skuUltraSSDLRS) || target == skuUltraSSDLRS: + return cerrors.Newf(cerrors.InvalidArgument, + "Changing the SKU of disk %q from %s to %s is not supported. Create a new disk from a snapshot instead.", + cur.ID, cur.VolumeType, target) + case strings.EqualFold(cur.VolumeType, skuPremiumV2LRS): + return cerrors.Newf(cerrors.InvalidArgument, + "A %s disk cannot be converted to another disk type. Migrate it using a snapshot instead.", skuPremiumV2LRS) + case target == skuPremiumV2LRS && cur.Device == osDiskDevice: + return cerrors.Newf(cerrors.InvalidArgument, "An OS disk cannot be converted to %s.", skuPremiumV2LRS) + } + + return nil +} + +// applyPerfPatch sets diskIOPSReadWrite / diskMBpsReadWrite, which Azure only +// accepts on UltraSSD_LRS and PremiumV2_LRS. Clearing them is always allowed +// (a re-PUT that omits them). +func applyPerfPatch(next, cur *driver.VolumeInfo, patch *driver.AzureDiskPatch, chg *volumeChange) error { + iops := changedInt(patch.IOPS, cur.IOPS) + mbps := changedInt(patch.Throughput, cur.Throughput) + + if iops == nil && mbps == nil { + return nil + } + + provisioned := provisionedPerfSKU(next.VolumeType) + + if !provisioned && (nonZero(iops) || nonZero(mbps)) { + return cerrors.Newf(cerrors.InvalidArgument, + "Property 'diskIOPSReadWrite'/'diskMBpsReadWrite' can only be set on %s and %s disks; disk SKU is %q.", + skuUltraSSDLRS, skuPremiumV2LRS, next.VolumeType) + } + + setInt(&next.IOPS, iops) + setInt(&next.Throughput, mbps) + + chg.perf = provisioned + + return nil +} + +// changedInt returns v when it is set and differs from cur, else nil. +func changedInt(v *int, cur int) *int { + if v == nil || *v == cur { + return nil + } + + return v +} + +func nonZero(v *int) bool { return v != nil && *v != 0 } + +func setInt(dst, v *int) { + if v != nil { + *dst = *v + } +} + +// applyTierPatch sets the performance tier and keeps it consistent with the +// SKU and size. Per Microsoft Learn (virtual-machines/disks-change-performance) +// a tier is only supported on Premium SSD, cannot be below the size's +// baseline tier, and P60-P80 need a disk larger than 4,096 GiB. Moving to +// Premium SSD sets the baseline tier, moving off it clears the tier, and +// growing a Premium SSD past its tier raises the tier to the new baseline. +func applyTierPatch(next, cur *driver.VolumeInfo, tier *string, chg *volumeChange) error { + if tier != nil && *tier != "" && !strings.EqualFold(*tier, cur.Tier) { + return applyExplicitTier(next, *tier, chg) + } + + premium := premiumSSD(next.VolumeType) + baseline := premiumTiers[baselineTierIndex(next.Size)].name + + switch { + case chg.sku && premium: + next.Tier = baseline + case chg.sku: + next.Tier = "" + case chg.size && premium && tierIndex(next.Tier) < baselineTierIndex(next.Size): + next.Tier = baseline + } + + return nil +} + +// applyExplicitTier sets a requested performance tier. +func applyExplicitTier(next *driver.VolumeInfo, tier string, chg *volumeChange) error { + if !premiumSSD(next.VolumeType) { + return cerrors.Newf(cerrors.InvalidArgument, + "Performance tier '%s' can only be set on %s and %s disks; disk SKU is %q.", + tier, skuPremiumLRS, skuPremiumZRS, next.VolumeType) + } + + idx := tierIndex(tier) + if err := validTierFor(tier, idx, next.Size); err != nil { + return err + } + + next.Tier = premiumTiers[idx].name + chg.tier = true + + return nil +} + +// validTierFor checks that the tier at idx is a valid performance tier for a +// Premium SSD of size GiB. +func validTierFor(tier string, idx, size int) error { + if idx < 0 { + return cerrors.Newf(cerrors.InvalidArgument, "The value '%s' of parameter 'tier' is not valid.", tier) + } + + base := baselineTierIndex(size) + if idx < base { + return cerrors.Newf(cerrors.InvalidArgument, + "Performance tier '%s' is below the baseline tier '%s' of a %d GiB disk.", tier, premiumTiers[base].name, size) + } + + if idx > p50Index && size <= largeDiskBoundaryGiB { + return cerrors.Newf(cerrors.InvalidArgument, + "Performance tier '%s' is only available on disks larger than %d GiB.", tier, largeDiskBoundaryGiB) + } + + return nil +} + +// checkVolumeState refuses a property change the disk's current state does not +// allow (Microsoft Learn: troubleshoot/azure/virtual-machines/windows/ +// troubleshoot-disk-resize and virtual-machines/windows/expand-disks). An +// active SAS (beginGetAccess) holds a lease and blocks every change; the +// attachment rules are in checkAttachedChange. +func (m *Mock) checkVolumeState(cur, next *driver.VolumeInfo, chg volumeChange) error { + if m.activeDiskSAS(cur.ID) { + code := codeOperationNotAllowed + if chg.size { + code = codeResizeWhileActiveSAS + } + + return diskConflict(code, "Cannot change disk %q while it has an active SAS URI. Revoke access to the disk "+ + "(endGetAccess) and retry the operation.", cur.ID) + } + + if cur.State != stateInUse || cur.AttachedTo == "" { + return nil + } + + return checkAttachedChange(cur, next, chg, m.vmAllocated(cur.AttachedTo)) +} + +// checkAttachedChange applies the rules for an attached disk: an OS disk is +// only resized, and any disk only converted to another SKU, while its VM is +// deallocated (data disks can be expanded online), and a Standard HDD/SSD or +// Premium SSD of 4 TiB or less cannot grow past 4 TiB while attached. +func checkAttachedChange(cur, next *driver.VolumeInfo, chg volumeChange, allocated bool) error { + if chg.sku && allocated { + return diskConflict(codeOperationNotAllowed, "Cannot change the SKU of disk %q while it is attached to running "+ + "VM %q. Changing the disk type requires the virtual machine to be deallocated.", cur.ID, cur.AttachedTo) + } + + if !chg.size { + return nil + } + + if cur.Device == osDiskDevice && allocated { + return diskConflict(codeOperationNotAllowed, "Cannot resize disk %q while it is attached to running VM %q. "+ + "Resizing a disk of an Azure Virtual Machine requires the virtual machine to be deallocated. "+ + "Please stop your VM and retry the operation.", cur.ID, cur.AttachedTo) + } + + if !provisionedPerfSKU(cur.VolumeType) && cur.Size <= largeDiskBoundaryGiB && next.Size > largeDiskBoundaryGiB { + return diskConflict(codeInvalidResizeLargeDisks, "Disk %q of %d GiB cannot be expanded beyond %d GiB while "+ + "it is attached. Detach the disk and retry the operation.", cur.ID, cur.Size, largeDiskBoundaryGiB) + } + + return nil +} + +// checkMaxSize rejects a size above the largest disk the SKU offers. +func checkMaxSize(vol *driver.VolumeInfo) error { + limit := maxDiskSizeGiB + if provisionedPerfSKU(vol.VolumeType) { + limit = maxProvisionedDiskSizeGiB + } + + if vol.Size > limit { + return cerrors.Newf(cerrors.InvalidArgument, + "Disk size %d GB is larger than the maximum %d GB for a %s disk.", vol.Size, limit, vol.VolumeType) + } + + return nil +} + +// diskConflict builds the 409 an ARM disk update is refused with. +func diskConflict(code, format string, args ...any) error { + return &driver.AzureDiskError{Code: code, Err: cerrors.Newf(cerrors.FailedPrecondition, format, args...)} +} + +// vmAllocated reports whether the VM holds compute (any power state but +// deallocated). A VM stopped from inside the guest is still allocated, and +// Azure still refuses the change. An unknown VM counts as not allocated. +func (m *Mock) vmAllocated(instanceID string) bool { + inst, ok := m.instances.Get(instanceID) + if !ok { + return false + } + + return inst.PowerState != powerStateDeallocated +} + +// activeDiskSAS reports whether a SAS granted by GrantDiskAccess is still +// live: not revoked and not past its se= expiry on the mock's clock. +func (m *Mock) activeDiskSAS(volumeID string) bool { + sas, ok := m.diskAccess.Get(volumeID) + if !ok { + return false + } + + u, err := url.Parse(sas) + if err != nil { + return true + } + + expiry, err := time.Parse(sasTimeLayout, u.Query().Get("se")) + if err != nil { + return true + } + + return m.opts.Clock.Now().Before(expiry) +} + +// patchVolumeTags returns the tag set to store: a copy of existing when the +// patch omits tags, otherwise the patch tags plus the existing keys in keep. +func patchVolumeTags(existing, patch map[string]string, keep []string) map[string]string { + if patch == nil { + return copyTags(existing) + } + + out := make(map[string]string, len(patch)+len(keep)) + for k, v := range patch { + out[k] = v + } + + for _, k := range keep { + if v, ok := existing[k]; ok { + out[k] = v + } + } + + return out +} + +// provisionedPerfSKU reports whether a disk SKU lets the caller set IOPS and +// throughput independently of size. +func provisionedPerfSKU(sku string) bool { + return strings.EqualFold(sku, skuUltraSSDLRS) || strings.EqualFold(sku, skuPremiumV2LRS) +} + +// premiumSSD reports whether a disk SKU is Premium SSD, the only type with +// performance tiers. +func premiumSSD(sku string) bool { + return strings.EqualFold(sku, skuPremiumLRS) || strings.EqualFold(sku, skuPremiumZRS) +} + +// tierIndex returns the position of a performance tier name in premiumTiers, +// or -1 for an unknown name (including ""). +func tierIndex(tier string) int { + for i := range premiumTiers { + if strings.EqualFold(premiumTiers[i].name, tier) { + return i + } + } + + return -1 +} + +// baselineTierIndex returns the position of the baseline tier Azure selects +// for a Premium SSD of size GiB. +func baselineTierIndex(size int) int { + for i := range premiumTiers { + if size <= premiumTiers[i].maxSize { + return i + } + } + + return len(premiumTiers) - 1 +} diff --git a/providers/azure/virtualmachines/disk_update_test.go b/providers/azure/virtualmachines/disk_update_test.go new file mode 100644 index 000000000..8a8c12945 --- /dev/null +++ b/providers/azure/virtualmachines/disk_update_test.go @@ -0,0 +1,268 @@ +package virtualmachines + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stackshy/cloudemu/v2/config" + cerrors "github.com/stackshy/cloudemu/v2/errors" + "github.com/stackshy/cloudemu/v2/services/compute/driver" +) + +// TestConcurrentAttachAndUpdateVolumeKeepsAttachment runs AttachVolume, +// UpdateVolume (a disk re-PUT) and DescribeVolumes on the same disks in +// parallel. The update must merge under the store lock with copy-on-write: +// mutating the stored *VolumeInfo in place is a data race with the reader, and +// writing back a pointer read before a concurrent attach silently drops the +// attachment (the disk reads "available" while the VM still owns it). +func TestConcurrentAttachAndUpdateVolumeKeepsAttachment(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + insts, err := m.RunInstances(ctx, driver.InstanceConfig{ImageID: "img-1", InstanceType: "Standard_D2s_v3"}, 1) + require.NoError(t, err) + + vmID := insts[0].ID + + const rounds = 300 + + ids := make([]string, rounds) + + for i := range ids { + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{Size: 32, VolumeType: "Premium_LRS"}) + require.NoError(t, err) + + ids[i] = vol.ID + } + + var wg sync.WaitGroup + + for _, id := range ids { + wg.Add(3) + + go func() { + defer wg.Done() + + assert.NoError(t, m.AttachVolume(ctx, id, vmID, "0")) + }() + + go func() { + defer wg.Done() + + _, err := m.UpdateVolume(ctx, id, driver.VolumeConfig{ + Size: 64, VolumeType: "Premium_LRS", Tags: map[string]string{"env": "prod"}, + }) + assert.NoError(t, err) + }() + + go func() { + defer wg.Done() + + _, _ = m.DescribeVolumes(ctx, nil) + }() + } + + wg.Wait() + + vols, err := m.DescribeVolumes(ctx, ids) + require.NoError(t, err) + require.Len(t, vols, rounds) + + for i := range vols { + assert.Equal(t, stateInUse, vols[i].State, "disk %s lost its attachment", vols[i].ID) + assert.Equal(t, vmID, vols[i].AttachedTo, "disk %s lost its attachment", vols[i].ID) + assert.Equal(t, 64, vols[i].Size, "disk %s lost its resize", vols[i].ID) + } +} + +// TestConcurrentAttachAndPatchVolumeKeepsAttachment is the PATCH variant of +// the race above: PatchVolume must not drop a concurrent attachment either. +func TestConcurrentAttachAndPatchVolumeKeepsAttachment(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + insts, err := m.RunInstances(ctx, driver.InstanceConfig{ImageID: "img-1", InstanceType: "Standard_D2s_v3"}, 1) + require.NoError(t, err) + + const rounds = 200 + + ids := make([]string, rounds) + + for i := range ids { + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{Size: 32, VolumeType: "Premium_LRS"}) + require.NoError(t, err) + + ids[i] = vol.ID + } + + size := 64 + + var wg sync.WaitGroup + + for _, id := range ids { + wg.Add(3) + + go func() { + defer wg.Done() + + assert.NoError(t, m.AttachVolume(ctx, id, insts[0].ID, "0")) + }() + + go func() { + defer wg.Done() + + _, err := m.PatchVolume(ctx, id, driver.AzureDiskPatch{Size: &size}) + assert.NoError(t, err) + }() + + go func() { + defer wg.Done() + + _, _ = m.DescribeVolumes(ctx, nil) + }() + } + + wg.Wait() + + vols, err := m.DescribeVolumes(ctx, ids) + require.NoError(t, err) + + for i := range vols { + assert.Equal(t, insts[0].ID, vols[i].AttachedTo, "disk %s lost its attachment", vols[i].ID) + assert.Equal(t, 64, vols[i].Size, "disk %s lost its resize", vols[i].ID) + } +} + +func ptrTo[T any](v T) *T { return &v } + +// TestPatchVolumeRules exercises the disk update rules through the Go library, +// the same code the ARM wire handler and a re-PUT go through. +func TestPatchVolumeRules(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + newVol := func(size int, sku string) string { + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{ + Size: size, VolumeType: sku, Tags: map[string]string{"cloudemu:azureDiskName": "d", "env": "dev"}, + }) + require.NoError(t, err) + + return vol.ID + } + + premium := newVol(64, skuPremiumLRS) + standard := newVol(64, skuStandardLRS) + + for _, tc := range []struct { + name string + id string + patch driver.AzureDiskPatch + }{ + {"shrink", premium, driver.AzureDiskPatch{Size: ptrTo(32)}}, + {"unknown sku", premium, driver.AzureDiskPatch{VolumeType: ptrTo("Gold_LRS")}}, + {"above the 32767 GiB maximum", standard, driver.AzureDiskPatch{Size: ptrTo(40000)}}, + {"iops on Premium_LRS", premium, driver.AzureDiskPatch{IOPS: ptrTo(5000)}}, + {"tier on Standard_LRS", standard, driver.AzureDiskPatch{Tier: ptrTo("P10")}}, + {"unknown tier", premium, driver.AzureDiskPatch{Tier: ptrTo("P99")}}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := m.PatchVolume(ctx, tc.id, tc.patch) + assert.True(t, cerrors.IsInvalidArgument(err), "err = %v, want InvalidArgument", err) + }) + } + + // A re-PUT (UpdateVolume) is held to the grow-only rule too. + _, err := m.UpdateVolume(ctx, premium, driver.VolumeConfig{Size: 16, VolumeType: skuPremiumLRS}) + assert.True(t, cerrors.IsInvalidArgument(err), "re-PUT shrink: err = %v, want InvalidArgument", err) + + // Growing a Premium SSD past its tier raises the tier to the new baseline, + // and a tags replacement keeps the named bookkeeping keys. + got, err := m.PatchVolume(ctx, premium, driver.AzureDiskPatch{ + Size: ptrTo(600), Tags: map[string]string{"team": "x"}, KeepTags: []string{"cloudemu:azureDiskName"}, + }) + require.NoError(t, err) + assert.Equal(t, "P30", got.Tier) + assert.Equal(t, map[string]string{"team": "x", "cloudemu:azureDiskName": "d"}, got.Tags) + + // A Standard disk may grow past 4 TiB while detached, and a PremiumV2 disk + // up to 64 TiB. + _, err = m.PatchVolume(ctx, standard, driver.AzureDiskPatch{Size: ptrTo(8192)}) + require.NoError(t, err) + + v2 := newVol(64, skuPremiumV2LRS) + _, err = m.PatchVolume(ctx, v2, driver.AzureDiskPatch{Size: ptrTo(40000), IOPS: ptrTo(8000)}) + require.NoError(t, err) + + _, err = m.PatchVolume(ctx, "no-such-disk", driver.AzureDiskPatch{Size: ptrTo(1)}) + assert.True(t, cerrors.IsNotFound(err), "err = %v, want NotFound", err) +} + +// TestPatchVolumeOSDiskRules: an OS disk cannot become a Premium SSD v2, and +// is only resized while its VM is deallocated. +func TestPatchVolumeOSDiskRules(t *testing.T) { + ctx := context.Background() + m := newTestMock() + + insts, err := m.RunInstances(ctx, driver.InstanceConfig{ImageID: "img-1", InstanceType: "Standard_D2s_v3"}, 1) + require.NoError(t, err) + + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{Size: 64, VolumeType: skuPremiumLRS}) + require.NoError(t, err) + require.NoError(t, m.AttachVolume(ctx, vol.ID, insts[0].ID, osDiskDevice)) + + _, err = m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{VolumeType: ptrTo(skuPremiumV2LRS)}) + assert.True(t, cerrors.IsInvalidArgument(err), "OS disk to PremiumV2: err = %v, want InvalidArgument", err) + + _, err = m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{Size: ptrTo(128)}) + assertDiskErr(t, err, codeOperationNotAllowed) + + // Powered off from the guest is still allocated. + require.NoError(t, m.PowerOff(ctx, insts[0].ID)) + + _, err = m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{Size: ptrTo(128)}) + assertDiskErr(t, err, codeOperationNotAllowed) + + require.NoError(t, m.Deallocate(ctx, insts[0].ID)) + + got, err := m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{Size: ptrTo(128)}) + require.NoError(t, err) + assert.Equal(t, 128, got.Size) +} + +// TestPatchVolumeSASExpires: a granted SAS blocks a resize only until it is +// revoked or reaches its expiry. +func TestPatchVolumeSASExpires(t *testing.T) { + ctx := context.Background() + clk := config.NewFakeClock(time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)) + m := New(config.NewOptions(config.WithClock(clk))) + + vol, err := m.CreateVolume(ctx, driver.VolumeConfig{Size: 64, VolumeType: skuStandardLRS}) + require.NoError(t, err) + + _, err = m.GrantDiskAccess(ctx, vol.ID, "Read", 60) + require.NoError(t, err) + + _, err = m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{Size: ptrTo(128)}) + assertDiskErr(t, err, codeResizeWhileActiveSAS) + assert.True(t, cerrors.IsFailedPrecondition(err), "an active-SAS refusal is FailedPrecondition (409)") + + clk.Advance(61 * time.Second) + + _, err = m.PatchVolume(ctx, vol.ID, driver.AzureDiskPatch{Size: ptrTo(128)}) + require.NoError(t, err) +} + +func assertDiskErr(t *testing.T, err error, code string) { + t.Helper() + + var de *driver.AzureDiskError + if assert.True(t, errors.As(err, &de), "err = %v, want *driver.AzureDiskError", err) { + assert.Equal(t, code, de.Code) + } +} diff --git a/providers/azure/virtualmachines/vm.go b/providers/azure/virtualmachines/vm.go index 03a549a58..d5dccf220 100644 --- a/providers/azure/virtualmachines/vm.go +++ b/providers/azure/virtualmachines/vm.go @@ -35,6 +35,7 @@ var ( _ driver.ConsoleReader = (*Mock)(nil) _ driver.AzureVMController = (*Mock)(nil) _ driver.AzureDiskUpdater = (*Mock)(nil) + _ driver.AzureDiskPatcher = (*Mock)(nil) _ driver.KeyPairGenerator = (*Mock)(nil) _ driver.AzureDiskAccessor = (*Mock)(nil) _ driver.AzureSSHKeyUpdater = (*Mock)(nil) @@ -1152,40 +1153,38 @@ func (m *Mock) CreateVolume(_ context.Context, cfg driver.VolumeConfig) (*driver // uniqueId and timeCreated stay stable and an attached disk is not duplicated, // while updating the mutable cost fields from cfg. A non-zero Size, non-empty // VolumeType/Tier are applied; IOPS/Throughput and Tags are replaced from cfg -// (PUT is a full resource replacement). +// (PUT is a full resource replacement). The same Azure rules as PatchVolume +// apply (a re-PUT cannot shrink a disk), and the merge runs under the store +// lock with copy-on-write. // //nolint:gocritic // hugeParam: cfg mirrors the driver-interface signature. func (m *Mock) UpdateVolume(_ context.Context, id string, cfg driver.VolumeConfig) (*driver.VolumeInfo, error) { - vol, ok := m.volumes.Get(id) - if !ok { - return nil, cerrors.Newf(cerrors.NotFound, "disk %q not found", id) + tags := copyTags(cfg.Tags) + if tags == nil { + tags = map[string]string{} } + patch := driver.AzureDiskPatch{IOPS: &cfg.IOPS, Throughput: &cfg.Throughput, Tags: tags} + if cfg.Size != 0 { - vol.Size = cfg.Size + patch.Size = &cfg.Size } if cfg.VolumeType != "" { - vol.VolumeType = cfg.VolumeType + patch.VolumeType = &cfg.VolumeType } if cfg.Tier != "" { - vol.Tier = cfg.Tier + patch.Tier = &cfg.Tier } - if cfg.Location != "" { - vol.Location = cfg.Location - } - - vol.IOPS = cfg.IOPS - vol.Throughput = cfg.Throughput - vol.Tags = copyTags(cfg.Tags) - - m.volumes.Set(id, vol) - - result := *vol + return m.patchVolume(id, &patch, cfg.Location) +} - return &result, nil +// PatchVolume applies a partial managed-disk update (ARM Disks Update). See +// driver.AzureDiskPatcher. +func (m *Mock) PatchVolume(_ context.Context, id string, patch driver.AzureDiskPatch) (*driver.VolumeInfo, error) { + return m.patchVolume(id, &patch, "") } func (m *Mock) DeleteVolume(_ context.Context, id string) error { diff --git a/server/azure/disks/handler.go b/server/azure/disks/handler.go index 3c6ec816f..a40e17320 100644 --- a/server/azure/disks/handler.go +++ b/server/azure/disks/handler.go @@ -4,6 +4,7 @@ // Supported operations: // // PUT .../disks/{name} : CreateOrUpdate (returns 202 + Azure-AsyncOperation) +// PATCH .../disks/{name} : Update (returns 202 + Azure-AsyncOperation) // GET .../disks/{name} : Get // GET .../disks : List in resource group // DELETE .../disks/{name} : Delete (returns 202 + Azure-AsyncOperation) @@ -97,6 +98,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodPut: h.createOrUpdate(w, r, rp) + case http.MethodPatch: + h.update(w, r, rp) case http.MethodGet: h.get(w, r, rp) case http.MethodDelete: @@ -269,10 +272,14 @@ func (h *Handler) createOrUpdate(w http.ResponseWriter, r *http.Request, rp azur // attachment, rather than delete+recreate, which would leave a duplicate // phantom volume when the disk is attached (DeleteVolume rejects an attached // disk) and churn the uniqueId/timeCreated on every re-PUT. + // sku.tier is read-only in the ARM contract, so an update takes the + // performance tier from properties.tier alone. if existing, err := findDiskByName(r.Context(), h.compute, rp.ResourceGroup, rp.ResourceName); err == nil { + cfg.Tier = req.Properties.Tier + vol, err := h.updateExistingDisk(r.Context(), existing, cfg) if err != nil { - azurearm.WriteCErr(w, err) + writeDiskErr(w, err) return } diff --git a/server/azure/disks/types.go b/server/azure/disks/types.go index 501f18b90..bf51ad713 100644 --- a/server/azure/disks/types.go +++ b/server/azure/disks/types.go @@ -24,6 +24,22 @@ type diskRequestProps struct { Tier string `json:"tier"` } +// diskUpdateRequest is the PATCH body (armcompute.DiskUpdate). Every field is +// optional; pointers and a nil map distinguish "omitted" from a zero value, so +// an omitted field leaves the disk unchanged while "tags":{} clears the tags. +type diskUpdateRequest struct { + SKU *diskSKU `json:"sku"` + Tags map[string]string `json:"tags"` + Properties *diskUpdateRequestProps `json:"properties"` +} + +type diskUpdateRequestProps struct { + DiskSizeGB *int `json:"diskSizeGB"` + DiskIOPSReadWrite *int `json:"diskIOPSReadWrite"` + DiskMBpsReadWrite *int `json:"diskMBpsReadWrite"` + Tier *string `json:"tier"` +} + type creationData struct { CreateOption string `json:"createOption,omitempty"` SourceURI string `json:"sourceUri,omitempty"` diff --git a/server/azure/disks/update.go b/server/azure/disks/update.go new file mode 100644 index 000000000..5300ed187 --- /dev/null +++ b/server/azure/disks/update.go @@ -0,0 +1,95 @@ +package disks + +import ( + "errors" + "net/http" + + cerrors "github.com/stackshy/cloudemu/v2/errors" + "github.com/stackshy/cloudemu/v2/server/wire/azurearm" + computedriver "github.com/stackshy/cloudemu/v2/services/compute/driver" +) + +// internalDiskTags are the cloudemu bookkeeping tags a disk carries. A PATCH +// that replaces the user tags keeps them. +var internalDiskTags = []string{armNameTag, rgTag, createOptionTag, sourceIDTag} //nolint:gochecknoglobals // fixed list + +// update handles PATCH .../disks/{name} (armcompute DisksClient.BeginUpdate). +// The body is a DiskUpdate: every field is optional and only the supplied ones +// change. Like CreateOrUpdate it answers 202 + Azure-AsyncOperation, and the +// SDK poller's final GET reads the updated disk. The merge and Azure's rules +// (grow-only, SKU conversion, performance tier, attached disk, active SAS) run +// in the provider, so the Go library and serve behave the same. +// +//nolint:gocritic // rp is a request-scoped value +func (h *Handler) update(w http.ResponseWriter, r *http.Request, rp azurearm.ResourcePath) { + if rp.ResourceGroup == "" { + azurearm.WriteError(w, http.StatusBadRequest, "InvalidPath", "missing resourceGroups segment") + return + } + + patcher, ok := h.compute.(computedriver.AzureDiskPatcher) + if !ok { + azurearm.WriteError(w, http.StatusNotImplemented, "NotImplemented", "disk update not supported") + return + } + + existing, err := findDiskByName(r.Context(), h.compute, rp.ResourceGroup, rp.ResourceName) + if err != nil { + azurearm.WriteCErr(w, err) + return + } + + var req diskUpdateRequest + + if !azurearm.DecodeJSON(w, r, &req) { + return + } + + vol, err := patcher.PatchVolume(r.Context(), existing.ID, toDiskPatch(&req)) + if err != nil { + writeDiskErr(w, err) + return + } + + writeDiskAsync(w, r, rp.Subscription, "disk-update-"+rp.ResourceName, + h.toDiskResponse(r.Context(), vol, rp, "")) +} + +// toDiskPatch maps a DiskUpdate body onto the provider's partial update. +// sku.tier is read-only in the ARM contract (it follows sku.name) and is +// ignored; the performance tier is properties.tier. +func toDiskPatch(req *diskUpdateRequest) computedriver.AzureDiskPatch { + patch := computedriver.AzureDiskPatch{Tags: req.Tags, KeepTags: internalDiskTags} + + if req.SKU != nil && req.SKU.Name != "" { + name := req.SKU.Name + patch.VolumeType = &name + } + + if p := req.Properties; p != nil { + patch.Size = p.DiskSizeGB + patch.Tier = p.Tier + patch.IOPS = p.DiskIOPSReadWrite + patch.Throughput = p.DiskMBpsReadWrite + } + + return patch +} + +// writeDiskErr writes a disk update error. A refusal the provider tagged with +// an ARM error code (OperationNotAllowed, ChangeDiskSizeWhileActiveSasNotAllowed, +// ...) is echoed with that code; anything else maps as usual. +func writeDiskErr(w http.ResponseWriter, err error) { + var de *computedriver.AzureDiskError + if !errors.As(err, &de) { + azurearm.WriteCErr(w, err) + return + } + + status := http.StatusConflict + if cerrors.IsInvalidArgument(err) { + status = http.StatusBadRequest + } + + azurearm.WriteError(w, status, de.Code, cerrors.Message(err)) +} diff --git a/server/azure/disks/update_rules_test.go b/server/azure/disks/update_rules_test.go new file mode 100644 index 000000000..880650c0b --- /dev/null +++ b/server/azure/disks/update_rules_test.go @@ -0,0 +1,364 @@ +package disks_test + +import ( + "context" + "fmt" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" + + "github.com/stackshy/cloudemu/v2/providers/azure/virtualmachines" + computedriver "github.com/stackshy/cloudemu/v2/services/compute/driver" +) + +// diskVolumeID resolves an ARM disk name to the driver volume behind it. +func diskVolumeID(t *testing.T, vm *virtualmachines.Mock, name string) string { + t.Helper() + + vols, err := vm.DescribeVolumes(context.Background(), nil) + if err != nil { + t.Fatal(err) + } + + for i := range vols { + if vols[i].Tags["cloudemu:azureDiskName"] == name { + return vols[i].ID + } + } + + t.Fatalf("no volume for disk %s", name) + + return "" +} + +// attachToNewVM starts a VM and attaches the named disk to it at device. +func attachToNewVM(t *testing.T, vm *virtualmachines.Mock, disk, device string) string { + t.Helper() + + ctx := context.Background() + + insts, err := vm.RunInstances(ctx, computedriver.InstanceConfig{ImageID: "img", InstanceType: "Standard_D2s_v3"}, 1) + if err != nil { + t.Fatal(err) + } + + if err := vm.AttachVolume(ctx, diskVolumeID(t, vm, disk), insts[0].ID, device); err != nil { + t.Fatal(err) + } + + return insts[0].ID +} + +func sizeUpdate(gb int32) armcompute.DiskUpdate { + return armcompute.DiskUpdate{Properties: &armcompute.DiskUpdateProperties{DiskSizeGB: to.Ptr(gb)}} +} + +func skuUpdate(sku armcompute.DiskStorageAccountTypes) armcompute.DiskUpdate { + return armcompute.DiskUpdate{SKU: &armcompute.DiskSKU{Name: to.Ptr(sku)}} +} + +func tierUpdate(tier string) armcompute.DiskUpdate { + return armcompute.DiskUpdate{Properties: &armcompute.DiskUpdateProperties{Tier: to.Ptr(tier)}} +} + +func diskSize(t *testing.T, client *armcompute.DisksClient, name string) int32 { + t.Helper() + + got, err := client.Get(context.Background(), "rg-1", name, nil) + if err != nil { + t.Fatalf("Get %s: %v", name, err) + } + + return *got.Properties.DiskSizeGB +} + +// TestSDKDiskRePutCannotShrink checks that the grow-only rule lives in the +// provider: a CreateOrUpdate re-PUT on an existing disk cannot shrink it +// either (it used to answer 202 and store the smaller size). +func TestSDKDiskRePutCannotShrink(t *testing.T) { + _, client := newUpdateTestServer(t) + ctx := context.Background() + + createDiskForUpdate(t, client, "shrink-disk", armcompute.DiskStorageAccountTypesPremiumLRS) + + if _, err := beginUpdate(t, client, "shrink-disk", sizeUpdate(128)); err != nil { + t.Fatalf("grow: %v", err) + } + + _, err := client.BeginCreateOrUpdate(ctx, "rg-1", "shrink-disk", armcompute.Disk{ + Location: to.Ptr("eastus"), + SKU: &armcompute.DiskSKU{Name: to.Ptr(armcompute.DiskStorageAccountTypesPremiumLRS)}, + Properties: &armcompute.DiskProperties{ + CreationData: &armcompute.CreationData{CreateOption: to.Ptr(armcompute.DiskCreateOptionEmpty)}, + DiskSizeGB: to.Ptr[int32](16), + }, + }, nil) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + + if got := diskSize(t, client, "shrink-disk"); got != 128 { + t.Errorf("diskSizeGB=%d after a rejected shrink, want 128", got) + } +} + +// TestSDKDiskUpdateAttachedToRunningVM covers the attached-disk rules from +// Microsoft Learn (troubleshoot-disk-resize, expand-disks): an OS disk is only +// resized and any disk only converted while the VM is deallocated, a data disk +// can grow online, and a Standard/Premium disk of 4 TiB or less cannot grow +// past 4 TiB while attached. +func TestSDKDiskUpdateAttachedToRunningVM(t *testing.T) { + _, client, vm := newUpdateTestEnv(t) + ctx := context.Background() + + createDiskForUpdate(t, client, "os-disk", armcompute.DiskStorageAccountTypesPremiumLRS) + createDiskForUpdate(t, client, "data-disk", armcompute.DiskStorageAccountTypesStandardLRS) + + osVM := attachToNewVM(t, vm, "os-disk", "osdisk") + attachToNewVM(t, vm, "data-disk", "0") + + _, err := beginUpdate(t, client, "os-disk", sizeUpdate(128)) + wantDiskErr(t, err, http.StatusConflict, "OperationNotAllowed") + + _, err = beginUpdate(t, client, "os-disk", skuUpdate(armcompute.DiskStorageAccountTypesStandardSSDLRS)) + wantDiskErr(t, err, http.StatusConflict, "OperationNotAllowed") + + _, err = beginUpdate(t, client, "data-disk", sizeUpdate(8192)) + wantDiskErr(t, err, http.StatusConflict, "InvalidResizeForLargeDisks") + + if _, err := beginUpdate(t, client, "data-disk", sizeUpdate(128)); err != nil { + t.Fatalf("online data-disk grow: %v", err) + } + + if _, err := beginUpdate(t, client, "os-disk", armcompute.DiskUpdate{ + Tags: map[string]*string{"k": to.Ptr("v")}, + }); err != nil { + t.Fatalf("tags on an attached disk: %v", err) + } + + if got := diskSize(t, client, "os-disk"); got != 64 { + t.Errorf("os-disk diskSizeGB=%d after rejected resize, want 64", got) + } + + if err := vm.Deallocate(ctx, osVM); err != nil { + t.Fatal(err) + } + + if _, err := beginUpdate(t, client, "os-disk", sizeUpdate(128)); err != nil { + t.Fatalf("resize after deallocate: %v", err) + } + + if _, err := beginUpdate(t, client, "os-disk", skuUpdate(armcompute.DiskStorageAccountTypesStandardSSDLRS)); err != nil { + t.Fatalf("sku change after deallocate: %v", err) + } +} + +// TestSDKDiskUpdateWithActiveSAS checks that an active beginGetAccess SAS +// blocks a resize (ChangeDiskSizeWhileActiveSasNotAllowed) and a conversion, +// while a tags-only update still goes through, and that endGetAccess lifts it. +func TestSDKDiskUpdateWithActiveSAS(t *testing.T) { + _, client := newUpdateTestServer(t) + ctx := context.Background() + + createDiskForUpdate(t, client, "sas-upd", armcompute.DiskStorageAccountTypesStandardLRS) + + grant, err := client.BeginGrantAccess(ctx, "rg-1", "sas-upd", armcompute.GrantAccessData{ + Access: to.Ptr(armcompute.AccessLevelRead), DurationInSeconds: to.Ptr[int32](3600), + }, nil) + if err != nil { + t.Fatalf("BeginGrantAccess: %v", err) + } + + if _, err := grant.PollUntilDone(ctx, fastPoll); err != nil { + t.Fatalf("grant poll: %v", err) + } + + _, err = beginUpdate(t, client, "sas-upd", sizeUpdate(256)) + wantDiskErr(t, err, http.StatusConflict, "ChangeDiskSizeWhileActiveSasNotAllowed") + + _, err = beginUpdate(t, client, "sas-upd", skuUpdate(armcompute.DiskStorageAccountTypesPremiumLRS)) + wantDiskErr(t, err, http.StatusConflict, "OperationNotAllowed") + + if _, err := beginUpdate(t, client, "sas-upd", armcompute.DiskUpdate{ + Tags: map[string]*string{"k": to.Ptr("v")}, + }); err != nil { + t.Fatalf("tags with an active SAS: %v", err) + } + + revoke, err := client.BeginRevokeAccess(ctx, "rg-1", "sas-upd", nil) + if err != nil { + t.Fatalf("BeginRevokeAccess: %v", err) + } + + if _, err := revoke.PollUntilDone(ctx, fastPoll); err != nil { + t.Fatalf("revoke poll: %v", err) + } + + if _, err := beginUpdate(t, client, "sas-upd", sizeUpdate(256)); err != nil { + t.Fatalf("resize after revoke: %v", err) + } +} + +// TestSDKDiskUpdatePerformanceTier covers the Premium SSD performance tier +// rules (Microsoft Learn, disks-change-performance) and the SKU conversion +// rules (disks-convert-types). +func TestSDKDiskUpdatePerformanceTier(t *testing.T) { + _, client := newUpdateTestServer(t) + + createDiskForUpdate(t, client, "tier-disk", armcompute.DiskStorageAccountTypesStandardSSDLRS) + createDiskForUpdate(t, client, "ultra-disk", armcompute.DiskStorageAccountTypesUltraSSDLRS) + + // A tier only applies to Premium SSD. + _, err := beginUpdate(t, client, "tier-disk", tierUpdate("P30")) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + + _, err = beginUpdate(t, client, "ultra-disk", tierUpdate("P50")) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + + // Moving to Premium SSD sets the baseline tier for 64 GiB (P6). + got, err := beginUpdate(t, client, "tier-disk", skuUpdate(armcompute.DiskStorageAccountTypesPremiumLRS)) + if err != nil { + t.Fatalf("to Premium_LRS: %v", err) + } + + wantTier(t, got, "P6") + + for _, tc := range []struct{ tier, why string }{ + {"P4", "below the P6 baseline"}, + {"P70", "P60-P80 need a disk above 4 TiB"}, + {"Premium", "sku.tier value, not a performance tier"}, + } { + t.Run(tc.why, func(t *testing.T) { + _, err := beginUpdate(t, client, "tier-disk", tierUpdate(tc.tier)) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + }) + } + + got, err = beginUpdate(t, client, "tier-disk", tierUpdate("P50")) + if err != nil { + t.Fatalf("tier P50: %v", err) + } + + wantTier(t, got, "P50") + + // sku.tier is read-only: sending it does not change the performance tier. + got, err = beginUpdate(t, client, "tier-disk", armcompute.DiskUpdate{ + SKU: &armcompute.DiskSKU{Name: to.Ptr(armcompute.DiskStorageAccountTypesPremiumLRS), Tier: to.Ptr("Premium")}, + }) + if err != nil { + t.Fatalf("sku.tier: %v", err) + } + + wantTier(t, got, "P50") + + // Moving off Premium SSD clears the tier. + got, err = beginUpdate(t, client, "tier-disk", skuUpdate(armcompute.DiskStorageAccountTypesStandardSSDLRS)) + if err != nil { + t.Fatalf("to StandardSSD_LRS: %v", err) + } + + if got.Properties.Tier != nil { + t.Errorf("tier=%s after moving off Premium SSD, want omitted", *got.Properties.Tier) + } + + // Ultra Disk is neither a conversion source nor a target. + _, err = beginUpdate(t, client, "tier-disk", skuUpdate(armcompute.DiskStorageAccountTypesUltraSSDLRS)) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + + _, err = beginUpdate(t, client, "ultra-disk", skuUpdate(armcompute.DiskStorageAccountTypesPremiumLRS)) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") +} + +func wantTier(t *testing.T, d armcompute.Disk, tier string) { + t.Helper() + + if d.Properties.Tier == nil || *d.Properties.Tier != tier { + t.Errorf("tier=%v want %s", d.Properties.Tier, tier) + } +} + +// TestSDKDiskConcurrentPatchesBothLand runs a size PATCH and a tags PATCH on +// the same disks at once. The merge happens in the provider under the store +// lock, so neither overwrites the other with a stale snapshot. +func TestSDKDiskConcurrentPatchesBothLand(t *testing.T) { + _, client := newUpdateTestServer(t) + + const disks = 20 + + for i := range disks { + createDiskForUpdate(t, client, fmt.Sprintf("race-%d", i), armcompute.DiskStorageAccountTypesPremiumLRS) + } + + var wg sync.WaitGroup + + for i := range disks { + name := fmt.Sprintf("race-%d", i) + + wg.Add(2) + + go func() { + defer wg.Done() + + if _, err := beginUpdate(t, client, name, sizeUpdate(128)); err != nil { + t.Errorf("size PATCH %s: %v", name, err) + } + }() + + go func() { + defer wg.Done() + + if _, err := beginUpdate(t, client, name, armcompute.DiskUpdate{ + Tags: map[string]*string{"owner": to.Ptr("bob")}, + }); err != nil { + t.Errorf("tags PATCH %s: %v", name, err) + } + }() + } + + wg.Wait() + + for i := range disks { + name := fmt.Sprintf("race-%d", i) + + got, err := client.Get(context.Background(), "rg-1", name, nil) + if err != nil { + t.Fatalf("Get %s: %v", name, err) + } + + if *got.Properties.DiskSizeGB != 128 || got.Tags["owner"] == nil { + t.Errorf("%s: size=%d tags=%v, want both PATCHes applied", name, *got.Properties.DiskSizeGB, got.Tags) + } + } +} + +// TestDiskPatchMalformedBody checks a PATCH body that is not JSON is the 400 +// InvalidRequestContent ARM returns, and leaves the disk alone. +func TestDiskPatchMalformedBody(t *testing.T) { + ts, client := newUpdateTestServer(t) + + createDiskForUpdate(t, client, "bad-body", armcompute.DiskStorageAccountTypesPremiumLRS) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPatch, + ts.URL+diskPath("rg-1", "bad-body")+wireAPIVersion, strings.NewReader(`{"properties":`)) + if err != nil { + t.Fatal(err) + } + + resp, err := ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != http.StatusBadRequest || !strings.Contains(string(body), "InvalidRequestContent") { + t.Errorf("status=%d body=%s, want 400 InvalidRequestContent", resp.StatusCode, body) + } + + if got := diskSize(t, client, "bad-body"); got != 64 { + t.Errorf("diskSizeGB=%d, want 64", got) + } +} diff --git a/server/azure/disks/update_test.go b/server/azure/disks/update_test.go new file mode 100644 index 000000000..e46004084 --- /dev/null +++ b/server/azure/disks/update_test.go @@ -0,0 +1,367 @@ +package disks_test + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/cloud" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resourcegraph/armresourcegraph" + + "github.com/stackshy/cloudemu/v2" + "github.com/stackshy/cloudemu/v2/providers/azure/virtualmachines" + azureserver "github.com/stackshy/cloudemu/v2/server/azure" +) + +var fastPoll = &runtime.PollUntilDoneOptions{Frequency: time.Millisecond} + +func newUpdateTestServer(t *testing.T) (*httptest.Server, *armcompute.DisksClient) { + t.Helper() + + ts, client, _ := newUpdateTestEnv(t) + + return ts, client +} + +// newUpdateTestEnv is newUpdateTestServer plus the Azure VM mock behind it, so +// a test can attach disks to VMs and change power state directly. +func newUpdateTestEnv(t *testing.T) (*httptest.Server, *armcompute.DisksClient, *virtualmachines.Mock) { + t.Helper() + + cloudP := cloudemu.NewAzure() + srv := azureserver.New(azureserver.Drivers{ + VirtualMachines: cloudP.VirtualMachines, + Disks: cloudP.VirtualMachines, + ResourceDiscovery: cloudP.ResourceDiscovery, + }) + + ts := httptest.NewTLSServer(srv) + t.Cleanup(ts.Close) + ensureRG(t, ts, "sub-1", "rg-1") + + return ts, newDisksClient(t, ts), cloudP.VirtualMachines +} + +func createDiskForUpdate(t *testing.T, client *armcompute.DisksClient, name string, sku armcompute.DiskStorageAccountTypes) { + t.Helper() + + ctx := context.Background() + + poller, err := client.BeginCreateOrUpdate(ctx, "rg-1", name, armcompute.Disk{ + Location: to.Ptr("eastus"), + SKU: &armcompute.DiskSKU{Name: to.Ptr(sku)}, + Tags: map[string]*string{"env": to.Ptr("dev")}, + Properties: &armcompute.DiskProperties{ + CreationData: &armcompute.CreationData{CreateOption: to.Ptr(armcompute.DiskCreateOptionEmpty)}, + DiskSizeGB: to.Ptr[int32](64), + }, + }, nil) + if err != nil { + t.Fatalf("BeginCreateOrUpdate: %v", err) + } + + if _, err := poller.PollUntilDone(ctx, fastPoll); err != nil { + t.Fatalf("create poll: %v", err) + } +} + +func beginUpdate(t *testing.T, client *armcompute.DisksClient, name string, upd armcompute.DiskUpdate) (armcompute.Disk, error) { + t.Helper() + + ctx := context.Background() + + poller, err := client.BeginUpdate(ctx, "rg-1", name, upd, nil) + if err != nil { + return armcompute.Disk{}, err + } + + resp, err := poller.PollUntilDone(ctx, fastPoll) + if err != nil { + t.Fatalf("update poll: %v", err) + } + + return resp.Disk, nil +} + +// TestSDKDiskUpdate drives armcompute DisksClient.BeginUpdate + PollUntilDone +// end to end: size grows, SKU switches to PremiumV2_LRS with provisioned +// performance, tags are replaced, and identity (id/uniqueId/timeCreated) is kept. +func TestSDKDiskUpdate(t *testing.T) { + _, client := newUpdateTestServer(t) + ctx := context.Background() + + createDiskForUpdate(t, client, "upd-disk", armcompute.DiskStorageAccountTypesPremiumLRS) + + before, err := client.Get(ctx, "rg-1", "upd-disk", nil) + if err != nil { + t.Fatalf("Get before: %v", err) + } + + updated, err := beginUpdate(t, client, "upd-disk", armcompute.DiskUpdate{ + SKU: &armcompute.DiskSKU{Name: to.Ptr(armcompute.DiskStorageAccountTypesPremiumV2LRS)}, + Tags: map[string]*string{"env": to.Ptr("prod"), "team": to.Ptr("storage")}, + Properties: &armcompute.DiskUpdateProperties{ + DiskSizeGB: to.Ptr[int32](128), + DiskIOPSReadWrite: to.Ptr[int64](4000), + DiskMBpsReadWrite: to.Ptr[int64](250), + }, + }) + if err != nil { + t.Fatalf("BeginUpdate: %v", err) + } + + assertUpdatedDisk(t, "poller result", updated) + + got, err := client.Get(ctx, "rg-1", "upd-disk", nil) + if err != nil { + t.Fatalf("Get after: %v", err) + } + + assertUpdatedDisk(t, "get", got.Disk) + + if *got.ID != *before.ID || *got.Properties.UniqueID != *before.Properties.UniqueID { + t.Errorf("identity changed: id %s -> %s, uniqueId %s -> %s", + *before.ID, *got.ID, *before.Properties.UniqueID, *got.Properties.UniqueID) + } + + if !got.Properties.TimeCreated.Equal(*before.Properties.TimeCreated) { + t.Errorf("timeCreated changed: %v -> %v", before.Properties.TimeCreated, got.Properties.TimeCreated) + } +} + +func assertUpdatedDisk(t *testing.T, stage string, d armcompute.Disk) { + t.Helper() + + p := d.Properties + if p == nil { + t.Fatalf("%s: properties nil", stage) + } + + if p.DiskSizeGB == nil || *p.DiskSizeGB != 128 { + t.Errorf("%s: diskSizeGB=%v want 128", stage, p.DiskSizeGB) + } + + if p.DiskIOPSReadWrite == nil || *p.DiskIOPSReadWrite != 4000 { + t.Errorf("%s: diskIOPSReadWrite=%v want 4000", stage, p.DiskIOPSReadWrite) + } + + if p.DiskMBpsReadWrite == nil || *p.DiskMBpsReadWrite != 250 { + t.Errorf("%s: diskMBpsReadWrite=%v want 250", stage, p.DiskMBpsReadWrite) + } + + if d.SKU == nil || d.SKU.Name == nil || *d.SKU.Name != armcompute.DiskStorageAccountTypesPremiumV2LRS { + t.Errorf("%s: sku=%v want PremiumV2_LRS", stage, d.SKU) + } + + if len(d.Tags) != 2 || d.Tags["env"] == nil || *d.Tags["env"] != "prod" || d.Tags["team"] == nil { + t.Errorf("%s: tags=%v want {env:prod, team:storage}", stage, d.Tags) + } +} + +// TestSDKDiskUpdateOmittedFieldsKept checks that a tags-only PATCH leaves size +// and SKU alone, and that an empty-body PATCH is a no-op. +func TestSDKDiskUpdateOmittedFieldsKept(t *testing.T) { + _, client := newUpdateTestServer(t) + + createDiskForUpdate(t, client, "keep-disk", armcompute.DiskStorageAccountTypesStandardSSDLRS) + + got, err := beginUpdate(t, client, "keep-disk", armcompute.DiskUpdate{ + Tags: map[string]*string{"owner": to.Ptr("alice")}, + }) + if err != nil { + t.Fatalf("BeginUpdate: %v", err) + } + + if *got.Properties.DiskSizeGB != 64 { + t.Errorf("diskSizeGB=%d want 64 (unchanged)", *got.Properties.DiskSizeGB) + } + + if *got.SKU.Name != armcompute.DiskStorageAccountTypesStandardSSDLRS { + t.Errorf("sku=%s want StandardSSD_LRS (unchanged)", *got.SKU.Name) + } + + if len(got.Tags) != 1 || *got.Tags["owner"] != "alice" { + t.Errorf("tags=%v want exactly {owner:alice}", got.Tags) + } + + got, err = beginUpdate(t, client, "keep-disk", armcompute.DiskUpdate{}) + if err != nil { + t.Fatalf("empty BeginUpdate: %v", err) + } + + if *got.Properties.DiskSizeGB != 64 || len(got.Tags) != 1 { + t.Errorf("empty PATCH changed the disk: size=%d tags=%v", *got.Properties.DiskSizeGB, got.Tags) + } +} + +// TestSDKDiskUpdateRejections covers the 400s real Azure returns for a shrink +// and for provisioned performance on a SKU that does not support it, plus the +// 404 for a missing disk. A rejected PATCH must leave the disk untouched. +func TestSDKDiskUpdateRejections(t *testing.T) { + _, client := newUpdateTestServer(t) + ctx := context.Background() + + createDiskForUpdate(t, client, "rej-disk", armcompute.DiskStorageAccountTypesStandardLRS) + + cases := []struct { + name string + disk string + upd armcompute.DiskUpdate + status int + wantCode string + }{ + {"shrink", "rej-disk", armcompute.DiskUpdate{ + Properties: &armcompute.DiskUpdateProperties{DiskSizeGB: to.Ptr[int32](32)}, + }, http.StatusBadRequest, "InvalidParameter"}, + {"iops on Standard_LRS", "rej-disk", armcompute.DiskUpdate{ + Properties: &armcompute.DiskUpdateProperties{DiskIOPSReadWrite: to.Ptr[int64](3000)}, + }, http.StatusBadRequest, "InvalidParameter"}, + {"missing disk", "no-such-disk", armcompute.DiskUpdate{ + Tags: map[string]*string{"a": to.Ptr("b")}, + }, http.StatusNotFound, ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := beginUpdate(t, client, tc.disk, tc.upd) + + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) { + t.Fatalf("err=%v, want *azcore.ResponseError", err) + } + + if respErr.StatusCode != tc.status { + t.Errorf("status=%d want %d", respErr.StatusCode, tc.status) + } + + if tc.wantCode != "" && respErr.ErrorCode != tc.wantCode { + t.Errorf("code=%q want %q", respErr.ErrorCode, tc.wantCode) + } + }) + } + + got, err := client.Get(ctx, "rg-1", "rej-disk", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if *got.Properties.DiskSizeGB != 64 || got.Properties.DiskIOPSReadWrite != nil { + t.Errorf("rejected PATCH mutated disk: size=%d iops=%v", *got.Properties.DiskSizeGB, got.Properties.DiskIOPSReadWrite) + } +} + +// TestSDKDiskUpdatePremiumV2CannotConvertBack checks that a PremiumV2_LRS disk +// keeps its provisioned performance and refuses a conversion to another type: +// Microsoft Learn (disks-convert-types) says a Premium SSD v2 cannot be +// switched directly to another disk type, only migrated via a snapshot. +func TestSDKDiskUpdatePremiumV2CannotConvertBack(t *testing.T) { + _, client := newUpdateTestServer(t) + + createDiskForUpdate(t, client, "perf-disk", armcompute.DiskStorageAccountTypesPremiumV2LRS) + + if _, err := beginUpdate(t, client, "perf-disk", armcompute.DiskUpdate{ + Properties: &armcompute.DiskUpdateProperties{DiskIOPSReadWrite: to.Ptr[int64](5000)}, + }); err != nil { + t.Fatalf("set iops: %v", err) + } + + _, err := beginUpdate(t, client, "perf-disk", armcompute.DiskUpdate{ + SKU: &armcompute.DiskSKU{Name: to.Ptr(armcompute.DiskStorageAccountTypesPremiumLRS)}, + }) + wantDiskErr(t, err, http.StatusBadRequest, "InvalidParameter") + + got, err := client.Get(context.Background(), "rg-1", "perf-disk", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if *got.SKU.Name != armcompute.DiskStorageAccountTypesPremiumV2LRS || *got.Properties.DiskIOPSReadWrite != 5000 { + t.Errorf("rejected conversion changed the disk: sku=%s iops=%d", *got.SKU.Name, *got.Properties.DiskIOPSReadWrite) + } +} + +// wantDiskErr asserts err is an ARM error with the given status and code. +func wantDiskErr(t *testing.T, err error, status int, code string) { + t.Helper() + + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) { + t.Fatalf("err=%v, want *azcore.ResponseError %d %s", err, status, code) + } + + if respErr.StatusCode != status || respErr.ErrorCode != code { + t.Errorf("got %d %q, want %d %q", respErr.StatusCode, respErr.ErrorCode, status, code) + } +} + +// TestSDKDiskUpdateVisibleInResourceGraph checks that a PATCHed size and tag +// set is what Resource Graph reports for the disk afterwards. +func TestSDKDiskUpdateVisibleInResourceGraph(t *testing.T) { + ts, client := newUpdateTestServer(t) + + createDiskForUpdate(t, client, "arg-disk", armcompute.DiskStorageAccountTypesPremiumLRS) + + if _, err := beginUpdate(t, client, "arg-disk", armcompute.DiskUpdate{ + Tags: map[string]*string{"env": to.Ptr("prod")}, + Properties: &armcompute.DiskUpdateProperties{DiskSizeGB: to.Ptr[int32](256)}, + }); err != nil { + t.Fatalf("BeginUpdate: %v", err) + } + + row := queryDiskRow(t, ts) + + props, _ := row["properties"].(map[string]any) + if size, _ := props["diskSizeGB"].(float64); size != 256 { + t.Errorf("resource graph diskSizeGB=%v want 256", props["diskSizeGB"]) + } + + tags, _ := row["tags"].(map[string]any) + if tags["env"] != "prod" { + t.Errorf("resource graph tags=%v want env=prod", tags) + } +} + +func queryDiskRow(t *testing.T, ts *httptest.Server) map[string]any { + t.Helper() + + opts := &arm.ClientOptions{ClientOptions: azcore.ClientOptions{ + Cloud: cloud.Configuration{ + ActiveDirectoryAuthorityHost: "https://login.microsoftonline.com/", + Services: map[cloud.ServiceName]cloud.ServiceConfiguration{ + cloud.ResourceManager: {Endpoint: ts.URL, Audience: "https://management.azure.com"}, + }, + }, + Transport: ts.Client(), + Retry: policy.RetryOptions{MaxRetries: -1}, + }} + + cf, err := armresourcegraph.NewClientFactory(fakeCred{}, opts) + if err != nil { + t.Fatal(err) + } + + out, err := cf.NewClient().Resources(context.Background(), armresourcegraph.QueryRequest{ + Query: to.Ptr("Resources | where type =~ 'microsoft.compute/disks'"), + }, nil) + if err != nil { + t.Fatalf("Resources: %v", err) + } + + data, _ := out.Data.([]any) + if len(data) != 1 { + t.Fatalf("resource graph rows=%d want 1", len(data)) + } + + row, _ := data[0].(map[string]any) + + return row +} diff --git a/server/azure/functions/handler.go b/server/azure/functions/handler.go index c315ce2a7..98a20d28a 100644 --- a/server/azure/functions/handler.go +++ b/server/azure/functions/handler.go @@ -10,6 +10,7 @@ // GET .../sites : List in resource group / subscription // DELETE .../sites/{name} : Delete // PUT .../serverfarms/{name} : CreateOrUpdate App Service plan +// PATCH .../serverfarms/{name} : Update App Service plan (partial) // GET .../serverfarms/{name} : Get App Service plan // POST /api/{name} : Synchronous invoke (non-ARM, mirrors how // real Function Apps are hit at @@ -37,6 +38,9 @@ const ( resourceType = "sites" serverFarmsType = "serverfarms" + // provisioningSucceeded is the ARM provisioningState of a settled resource. + provisioningSucceeded = "Succeeded" + functionAppKind = "functionapp" defaultLocation = "eastus" invokePathPrefix = "/api/" @@ -65,6 +69,9 @@ type appServicePlanStore interface { GetAppServicePlan(ctx context.Context, subscription, resourceGroup, name string) (*azfunctions.AppServicePlan, error) DeleteAppServicePlan(ctx context.Context, subscription, resourceGroup, name string) error ListAppServicePlans(ctx context.Context, subscription, resourceGroup string) ([]azfunctions.AppServicePlan, error) + PatchAppServicePlan( + ctx context.Context, subscription, resourceGroup, name string, patch azfunctions.AppServicePlanPatch, + ) (*azfunctions.AppServicePlan, error) } // azureFunctionApps is the Azure-only site surface the handler layers on top of @@ -669,6 +676,8 @@ func (h *Handler) servePlan(w http.ResponseWriter, r *http.Request, rp azurearm. switch r.Method { case http.MethodPut: createPlan(w, r, rp, store) + case http.MethodPatch: + patchPlan(w, r, rp, store) case http.MethodGet: getPlan(w, r, rp, store) case http.MethodDelete: @@ -718,19 +727,28 @@ func createPlan(w http.ResponseWriter, r *http.Request, rp azurearm.ResourcePath return } + props := req.Properties + if props == nil { + props = &serverFarmPatchProperties{} + } + plan, err := store.CreateAppServicePlan(r.Context(), azfunctions.AppServicePlan{ - Name: rp.ResourceName, - Subscription: rp.Subscription, - ResourceGroup: rp.ResourceGroup, - Location: req.Location, - SKUName: req.SKU.Name, - SKUTier: req.SKU.Tier, - Kind: req.Kind, - Capacity: req.SKU.Capacity, - Tags: req.Tags, + Name: rp.ResourceName, + Subscription: rp.Subscription, + ResourceGroup: rp.ResourceGroup, + Location: req.Location, + SKUName: req.SKU.Name, + SKUTier: req.SKU.Tier, + Kind: req.Kind, + Capacity: req.SKU.Capacity, + Tags: req.Tags, + Reserved: boolOr(props.Reserved), + PerSiteScaling: boolOr(props.PerSiteScaling), + ZoneRedundant: boolOr(props.ZoneRedundant), + MaximumElasticWorkerCount: intOr(props.MaximumElasticWorkerCount), }) if err != nil { - azurearm.WriteCErr(w, err) + writePlanErr(w, err) return } @@ -835,8 +853,12 @@ func toServerFarmResource(rp azurearm.ResourcePath, plan *azfunctions.AppService Capacity: plan.Capacity, }, Properties: serverFarmProperties{ - ProvisioningState: "Succeeded", - Status: "Ready", + ProvisioningState: provisioningSucceeded, + Status: "Ready", + Reserved: plan.Reserved, + PerSiteScaling: plan.PerSiteScaling, + ZoneRedundant: plan.ZoneRedundant, + MaximumElasticWorkerCount: plan.MaximumElasticWorkerCount, }, } } @@ -912,7 +934,7 @@ func upsertFunction(r *http.Request, fn sdrv.Serverless, cfg sdrv.FunctionConfig //nolint:gocritic // rp is request-scoped. func toSiteResource(rp azurearm.ResourcePath, info *sdrv.FunctionInfo, meta *azfunctions.SiteMeta) siteResource { location := defaultLocation - provisioningState := "Succeeded" + provisioningState := provisioningSucceeded kind := functionAppKind state := siteStateRunning diff --git a/server/azure/functions/plan_patch.go b/server/azure/functions/plan_patch.go new file mode 100644 index 000000000..6ca3e6701 --- /dev/null +++ b/server/azure/functions/plan_patch.go @@ -0,0 +1,88 @@ +package functions + +import ( + "encoding/json" + "errors" + "io" + "net/http" + + cerrors "github.com/stackshy/cloudemu/v2/errors" + azfunctions "github.com/stackshy/cloudemu/v2/providers/azure/functions" + "github.com/stackshy/cloudemu/v2/server/wire/azurearm" +) + +// patchPlan serves PATCH .../serverfarms/{name} (armappservice +// PlansClient.Update). Only the fields present in the body change; the rest of +// the plan is kept. Like the sites PATCH it answers 200 synchronously with the +// updated plan, one of the two statuses (200/202) the SDK accepts. A PATCH +// against a missing plan, or the wrong resource group, is a 404. +// +//nolint:gocritic // rp travels the dispatch chain once per request. +func patchPlan(w http.ResponseWriter, r *http.Request, rp azurearm.ResourcePath, store appServicePlanStore) { + if rp.ResourceGroup == "" { + azurearm.WriteError(w, http.StatusBadRequest, "InvalidPath", "missing resourceGroups segment") + return + } + + r.Body = http.MaxBytesReader(w, r.Body, maxControlBytes) + + var req patchServerFarmRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil && err != io.EOF { + azurearm.WriteError(w, http.StatusBadRequest, "InvalidRequestContent", err.Error()) + return + } + + plan, err := store.PatchAppServicePlan(r.Context(), rp.Subscription, rp.ResourceGroup, rp.ResourceName, + toPlanPatch(&req)) + if err != nil { + writePlanErr(w, err) + return + } + + azurearm.WriteJSON(w, http.StatusOK, toServerFarmResource(rp, plan)) +} + +// toPlanPatch maps the wire PATCH body onto the provider's partial update. +func toPlanPatch(req *patchServerFarmRequest) azfunctions.AppServicePlanPatch { + patch := azfunctions.AppServicePlanPatch{Kind: req.Kind, Tags: req.Tags} + + if req.SKU != nil { + patch.SKUName = req.SKU.Name + patch.SKUTier = req.SKU.Tier + patch.Capacity = req.SKU.Capacity + } + + if req.Properties != nil { + patch.Reserved = req.Properties.Reserved + patch.PerSiteScaling = req.Properties.PerSiteScaling + patch.ZoneRedundant = req.Properties.ZoneRedundant + patch.MaximumElasticWorkerCount = req.Properties.MaximumElasticWorkerCount + } + + return patch +} + +func boolOr(p *bool) bool { + return p != nil && *p +} + +func intOr(p *int) int { + if p == nil { + return 0 + } + + return *p +} + +// writePlanErr writes a plan PUT/PATCH error. A refusal the provider tagged +// with an ARM error code (the 400 BadRequest for an OS change) is echoed with +// that code; anything else maps as usual. +func writePlanErr(w http.ResponseWriter, err error) { + var pe *azfunctions.PlanError + if !errors.As(err, &pe) { + azurearm.WriteCErr(w, err) + return + } + + azurearm.WriteError(w, http.StatusBadRequest, pe.Code, cerrors.Message(err)) +} diff --git a/server/azure/functions/plan_patch_sdk_test.go b/server/azure/functions/plan_patch_sdk_test.go new file mode 100644 index 000000000..53f171da3 --- /dev/null +++ b/server/azure/functions/plan_patch_sdk_test.go @@ -0,0 +1,287 @@ +package functions_test + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/appservice/armappservice/v3" + + "github.com/stackshy/cloudemu/v2" + azureserver "github.com/stackshy/cloudemu/v2/server/azure" +) + +func newPlanPatchServer(t *testing.T) (*httptest.Server, *armappservice.PlansClient) { + t.Helper() + + cloudP := cloudemu.NewAzure() + ts := httptest.NewTLSServer(azureserver.New(azureserver.Drivers{Functions: cloudP.Functions})) + t.Cleanup(ts.Close) + + ensureRG(t, ts.Client(), ts.URL, subID, rgName) + + client := newPlansClient(t, ts) + + poller, err := client.BeginCreateOrUpdate(context.Background(), rgName, "patch-plan", armappservice.Plan{ + Kind: to.Ptr("elastic"), + Location: to.Ptr("eastus"), + Tags: map[string]*string{"env": to.Ptr("dev")}, + SKU: &armappservice.SKUDescription{Name: to.Ptr("EP1"), Capacity: to.Ptr[int32](1)}, + Properties: &armappservice.PlanProperties{ + MaximumElasticWorkerCount: to.Ptr[int32](5), + }, + }, nil) + if err != nil { + t.Fatalf("BeginCreateOrUpdate: %v", err) + } + + if _, err := poller.PollUntilDone(context.Background(), &runtimePollerOptions); err != nil { + t.Fatalf("PollUntilDone: %v", err) + } + + return ts, client +} + +// TestSDKAzureAppServicePlanUpdate drives armappservice PlansClient.Update +// (PATCH): the changed properties land, and everything the body omitted +// (SKU, tags, kind, reserved) is kept. +func TestSDKAzureAppServicePlanUpdate(t *testing.T) { + _, client := newPlanPatchServer(t) + ctx := context.Background() + + resp, err := client.Update(ctx, rgName, "patch-plan", armappservice.PlanPatchResource{ + Properties: &armappservice.PlanPatchResourceProperties{ + MaximumElasticWorkerCount: to.Ptr[int32](20), + PerSiteScaling: to.Ptr(true), + ZoneRedundant: to.Ptr(true), + }, + }, nil) + if err != nil { + t.Fatalf("Update: %v", err) + } + + assertPatchedPlan(t, "update response", resp.Plan) + + got, err := client.Get(ctx, rgName, "patch-plan", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + assertPatchedPlan(t, "get", got.Plan) +} + +func assertPatchedPlan(t *testing.T, stage string, p armappservice.Plan) { + t.Helper() + + if p.Properties == nil { + t.Fatalf("%s: properties nil", stage) + } + + pp := p.Properties + + if pp.MaximumElasticWorkerCount == nil || *pp.MaximumElasticWorkerCount != 20 { + t.Errorf("%s: maximumElasticWorkerCount=%v want 20", stage, pp.MaximumElasticWorkerCount) + } + + if pp.PerSiteScaling == nil || !*pp.PerSiteScaling { + t.Errorf("%s: perSiteScaling=%v want true", stage, pp.PerSiteScaling) + } + + if pp.Reserved == nil || *pp.Reserved { + t.Errorf("%s: reserved=%v want false (unchanged)", stage, pp.Reserved) + } + + if pp.ZoneRedundant == nil || !*pp.ZoneRedundant { + t.Errorf("%s: zoneRedundant=%v want true", stage, pp.ZoneRedundant) + } + + if p.SKU == nil || *p.SKU.Name != "EP1" || *p.SKU.Tier != "ElasticPremium" { + t.Errorf("%s: sku=%+v want EP1/ElasticPremium (unchanged)", stage, p.SKU) + } + + if p.Kind == nil || *p.Kind != "elastic" { + t.Errorf("%s: kind=%v want elastic (unchanged)", stage, p.Kind) + } + + if len(p.Tags) != 1 || *p.Tags["env"] != "dev" { + t.Errorf("%s: tags=%v want {env:dev} (unchanged)", stage, p.Tags) + } +} + +// TestAzureAppServicePlanPatchSKUAndTags sends the sku and tags a raw ARM +// PATCH may carry (armappservice.PlanPatchResource has no fields for them), and +// reads the result back through the SDK. A new SKU name re-derives the tier; +// tags are replaced wholesale. +func TestAzureAppServicePlanPatchSKUAndTags(t *testing.T) { + ts, client := newPlanPatchServer(t) + + body := `{"sku":{"name":"EP2","capacity":3},"tags":{"team":"payments"}}` + + status, out := rawPlanPatch(t, ts, "patch-plan", body) + if status != http.StatusOK { + t.Fatalf("PATCH status=%d body=%s", status, out) + } + + got, err := client.Get(context.Background(), rgName, "patch-plan", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if *got.SKU.Name != "EP2" || *got.SKU.Tier != "ElasticPremium" || *got.SKU.Capacity != 3 { + t.Errorf("sku=%s/%s/%d want EP2/ElasticPremium/3", *got.SKU.Name, *got.SKU.Tier, *got.SKU.Capacity) + } + + if len(got.Tags) != 1 || got.Tags["team"] == nil || *got.Tags["team"] != "payments" { + t.Errorf("tags=%v want exactly {team:payments}", got.Tags) + } + + if got.Properties.MaximumElasticWorkerCount == nil || *got.Properties.MaximumElasticWorkerCount != 5 { + t.Errorf("maximumElasticWorkerCount=%v want 5 (unchanged)", got.Properties.MaximumElasticWorkerCount) + } + + var resp map[string]any + if err := json.Unmarshal(out, &resp); err != nil { + t.Fatalf("decode PATCH body: %v", err) + } + + if resp["name"] != "patch-plan" { + t.Errorf("PATCH response name=%v want patch-plan", resp["name"]) + } +} + +// TestSDKAzureAppServicePlanUpdateMissing checks a PATCH against a plan that +// does not exist (or lives in another resource group) is a 404. +func TestSDKAzureAppServicePlanUpdateMissing(t *testing.T) { + _, client := newPlanPatchServer(t) + + _, err := client.Update(context.Background(), rgName, "no-such-plan", armappservice.PlanPatchResource{ + Properties: &armappservice.PlanPatchResourceProperties{Reserved: to.Ptr(true)}, + }, nil) + + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) || respErr.StatusCode != http.StatusNotFound { + t.Fatalf("err=%v, want 404 ResponseError", err) + } +} + +func rawPlanPatch(t *testing.T, ts *httptest.Server, name, body string) (int, []byte) { + t.Helper() + + url := ts.URL + "/subscriptions/" + subID + "/resourceGroups/" + rgName + + "/providers/Microsoft.Web/serverfarms/" + name + "?api-version=2023-01-01" + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPatch, url, strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + + req.Header.Set("Content-Type", "application/json") + + resp, err := ts.Client().Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + var buf strings.Builder + + if _, err := io.Copy(&buf, resp.Body); err != nil { + t.Fatal(err) + } + + return resp.StatusCode, []byte(buf.String()) +} + +// TestSDKAzureAppServicePlanOSChangeRefused checks both write paths refuse to +// change an existing plan's OS with the 400 BadRequest real Azure returns +// ("You cannot change the OS hosting your app at this time", Azure/bicep#5724): +// a PATCH of reserved or kind, and a re-PUT that omits reserved. +func TestSDKAzureAppServicePlanOSChangeRefused(t *testing.T) { + ts, client := newPlanPatchServer(t) + ctx := context.Background() + + poller, err := client.BeginCreateOrUpdate(ctx, rgName, "linux-plan", armappservice.Plan{ + Kind: to.Ptr("linux"), + Location: to.Ptr("eastus"), + SKU: &armappservice.SKUDescription{Name: to.Ptr("P1v3")}, + Properties: &armappservice.PlanProperties{Reserved: to.Ptr(true)}, + }, nil) + if err != nil { + t.Fatalf("create linux plan: %v", err) + } + + if _, err := poller.PollUntilDone(ctx, &runtimePollerOptions); err != nil { + t.Fatalf("PollUntilDone: %v", err) + } + + _, err = client.Update(ctx, rgName, "linux-plan", armappservice.PlanPatchResource{ + Properties: &armappservice.PlanPatchResourceProperties{Reserved: to.Ptr(false)}, + }, nil) + wantPlanErr(t, "PATCH reserved:false", err, "BadRequest") + + status, out := rawPlanPatch(t, ts, "linux-plan", `{"kind":"app"}`) + if status != http.StatusBadRequest || !strings.Contains(string(out), `"BadRequest"`) { + t.Errorf("PATCH kind:app: status=%d body=%s, want 400 BadRequest", status, out) + } + + _, err = client.BeginCreateOrUpdate(ctx, rgName, "linux-plan", armappservice.Plan{ + Kind: to.Ptr("linux"), + Location: to.Ptr("eastus"), + SKU: &armappservice.SKUDescription{Name: to.Ptr("P1v3")}, + }, nil) + wantPlanErr(t, "re-PUT without reserved", err, "BadRequest") + + got, err := client.Get(ctx, rgName, "linux-plan", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if got.Properties.Reserved == nil || !*got.Properties.Reserved || *got.Kind != "linux" { + t.Errorf("refused OS change mutated the plan: kind=%s reserved=%v", *got.Kind, got.Properties.Reserved) + } +} + +// TestAzureAppServicePlanPatchSKUValidation checks a PATCH with a capacity +// below 1, above the tier maximum, an unknown SKU or a move out of the Elastic +// Premium family is a 400, and leaves the plan unchanged. +func TestAzureAppServicePlanPatchSKUValidation(t *testing.T) { + ts, client := newPlanPatchServer(t) + + for _, body := range []string{ + `{"sku":{"capacity":-5}}`, + `{"sku":{"capacity":0}}`, + `{"sku":{"name":"ZZ9"}}`, + `{"sku":{"name":"S1"}}`, + `{"sku":{"name":"EP2","tier":"Standard"}}`, + } { + status, out := rawPlanPatch(t, ts, "patch-plan", body) + if status != http.StatusBadRequest || !strings.Contains(string(out), `"InvalidParameter"`) { + t.Errorf("PATCH %s: status=%d body=%s, want 400 InvalidParameter", body, status, out) + } + } + + got, err := client.Get(context.Background(), rgName, "patch-plan", nil) + if err != nil { + t.Fatalf("Get: %v", err) + } + + if *got.SKU.Name != "EP1" || *got.SKU.Tier != "ElasticPremium" || *got.SKU.Capacity != 1 { + t.Errorf("sku=%s/%s/%d want EP1/ElasticPremium/1 (unchanged)", *got.SKU.Name, *got.SKU.Tier, *got.SKU.Capacity) + } +} + +func wantPlanErr(t *testing.T, what string, err error, code string) { + t.Helper() + + var respErr *azcore.ResponseError + if !errors.As(err, &respErr) || respErr.StatusCode != http.StatusBadRequest || respErr.ErrorCode != code { + t.Errorf("%s: err=%v, want 400 %s", what, err, code) + } +} diff --git a/server/azure/functions/types.go b/server/azure/functions/types.go index fbd6cf0a3..f8aa1e04d 100644 --- a/server/azure/functions/types.go +++ b/server/azure/functions/types.go @@ -167,8 +167,12 @@ type serverFarmSKU struct { } type serverFarmProperties struct { - ProvisioningState string `json:"provisioningState,omitempty"` - Status string `json:"status,omitempty"` + ProvisioningState string `json:"provisioningState,omitempty"` + Status string `json:"status,omitempty"` + Reserved bool `json:"reserved"` + PerSiteScaling bool `json:"perSiteScaling"` + ZoneRedundant bool `json:"zoneRedundant"` + MaximumElasticWorkerCount int `json:"maximumElasticWorkerCount,omitempty"` } // serverFarmListResponse is the {value:[...]} envelope for the serverfarms @@ -179,10 +183,37 @@ type serverFarmListResponse struct { // createServerFarmRequest captures the fields read from a serverfarms PUT body. type createServerFarmRequest struct { - Kind string `json:"kind"` - Location string `json:"location"` - Tags map[string]string `json:"tags"` - SKU serverFarmSKU `json:"sku"` + Kind string `json:"kind"` + Location string `json:"location"` + Tags map[string]string `json:"tags"` + SKU serverFarmSKU `json:"sku"` + Properties *serverFarmPatchProperties `json:"properties"` +} + +// patchServerFarmRequest captures a serverfarms PATCH body +// (armappservice.PlanPatchResource plus the sku/tags the ARM REST API also +// accepts). Every field is optional: a nil pointer or map leaves the stored +// value unchanged, and a present tags map replaces the tags wholesale. +type patchServerFarmRequest struct { + Kind *string `json:"kind"` + Tags map[string]string `json:"tags"` + SKU *serverFarmPatchSKU `json:"sku"` + Properties *serverFarmPatchProperties `json:"properties"` +} + +type serverFarmPatchSKU struct { + Name *string `json:"name"` + Tier *string `json:"tier"` + Capacity *int `json:"capacity"` +} + +// serverFarmPatchProperties are the mutable plan properties read from a PUT +// or PATCH body. +type serverFarmPatchProperties struct { + Reserved *bool `json:"reserved"` + PerSiteScaling *bool `json:"perSiteScaling"` + ZoneRedundant *bool `json:"zoneRedundant"` + MaximumElasticWorkerCount *int `json:"maximumElasticWorkerCount"` } // stringDictionary is the ARM StringDictionary shape returned by diff --git a/services/compute/driver/driver.go b/services/compute/driver/driver.go index 131fe6e3d..f8965949d 100644 --- a/services/compute/driver/driver.go +++ b/services/compute/driver/driver.go @@ -821,6 +821,55 @@ type AzureDiskUpdater interface { UpdateVolume(ctx context.Context, id string, cfg VolumeConfig) (*VolumeInfo, error) } +// AzureDiskPatch is a partial managed-disk update (ARM Disks Update, the +// PATCH of a DiskUpdate body). A nil field leaves the stored value unchanged. +type AzureDiskPatch struct { + // Size is the new diskSizeGB. Azure only grows a disk. + Size *int + // VolumeType is the new sku.name (storage account type). + VolumeType *string + // Tier is the new Premium SSD performance tier (properties.tier, e.g. P30). + Tier *string + // IOPS / Throughput are the provisioned diskIOPSReadWrite / + // diskMBpsReadWrite, settable only on UltraSSD_LRS and PremiumV2_LRS. + IOPS *int + Throughput *int + // Tags, when non-nil, replaces the tag set wholesale (ARM resource-level + // PATCH semantics). KeepTags names existing keys that survive the + // replacement: the wire layer's own bookkeeping tags. + Tags map[string]string + KeepTags []string +} + +// AzureDiskPatcher is an optional Azure-only capability for a partial managed +// disk update. The merge and every Azure rule (grow-only, SKU conversion, +// performance tier, attached-disk and active-SAS restrictions) run inside the +// provider's store lock, so a concurrent attach/detach or another update is +// never overwritten with a stale copy. Only the Azure VM mock implements it. +type AzureDiskPatcher interface { + // PatchVolume applies patch to the volume id and returns the stored + // result. NotFound when id is unknown, InvalidArgument for a request Azure + // rejects with 400, and an *AzureDiskError for one it rejects with a + // specific ARM error code. + PatchVolume(ctx context.Context, id string, patch AzureDiskPatch) (*VolumeInfo, error) +} + +// AzureDiskError is a managed-disk update refusal that real Azure reports with +// a specific ARM error code (e.g. OperationNotAllowed, +// ChangeDiskSizeWhileActiveSasNotAllowed). It unwraps to the canonical cloudemu +// error, so a caller that only checks the code (FailedPrecondition → 409, +// InvalidArgument → 400) still sees it; the Azure wire layer echoes Code. +type AzureDiskError struct { + Code string + Err error +} + +// Error implements the error interface. +func (e *AzureDiskError) Error() string { return e.Err.Error() } + +// Unwrap returns the canonical cloudemu error this one stands for. +func (e *AzureDiskError) Unwrap() error { return e.Err } + // AzureDiskDeleteOptioner is an optional Azure-only capability that records a // disk attachment's ARM deleteOption on the attached volume, mapped onto the // shared VolumeInfo.DeleteOnTermination (deleteOption "Delete" ⟷ true). The