Skip to content

fix: list every service's resources in the ARM resource index, at both scopes - #270

Open
MichaelDanCurtis wants to merge 2 commits into
floci-io:mainfrom
MichaelDanCurtis:fix/rg-resource-index-all-services
Open

fix: list every service's resources in the ARM resource index, at both scopes#270
MichaelDanCurtis wants to merge 2 commits into
floci-io:mainfrom
MichaelDanCurtis:fix/rg-resource-index-all-services

Conversation

@MichaelDanCurtis

@MichaelDanCurtis MichaelDanCurtis commented Sep 2, 2026

Copy link
Copy Markdown

Summary

Both of ARM's generic resource listings were incomplete, for the same reason: nothing assembled them from a single source of truth about what the estate holds.

GET .../resourceGroups/{rg}/resources built its answer from a hand-maintained set — ArmHandler's own Storage / Key Vault / Web state, plus Network, API Management and Managed Identity — plus the CDI ResourceIndexContributor lane, where AciHandler was the only implementation. Every other service with a management plane was missing from its own resource group.

GET /subscriptions/{sub}/resources aggregated only Key Vaults and API Management services, so a subscription holding neither answered {"value":[]} while its groups held VMs, virtual networks, storage accounts and database servers.

This PR registers the eight absent providers through the existing extension point:

Provider Handler
Microsoft.Compute/virtualMachines VmHandler
Microsoft.ContainerService/managedClusters AksHandler
Microsoft.ContainerRegistry/registries AcrHandler
Microsoft.Cache/Redis RedisHandler
Microsoft.DBforPostgreSQL/flexibleServers PostgresHandler
Microsoft.DBforMySQL/flexibleServers MySqlHandler
Microsoft.DBforMariaDB/servers MariaDbHandler
Microsoft.Sql/servers SqlHandler

and then makes both listings answer from one assembly, ArmHandler.indexedResources(sub, rg), where a null resource group means subscription scope — so they cannot disagree about the estate. ResourceIndexContributor gains listSubscriptionResources(sub) with no default implementation, so a service cannot appear in one listing while falling silently out of the other; Network and Managed Identity gain the subscription-scoped overload they lacked beside their resource-group one.

Each contributor answers from the state its own list endpoint already reads (scanAll() for the container-backed services, state.listServersBy... for the DB family), so the index cannot drift from the type-scoped listing. ArmResources.indexEntry(...) centralises the entry shape — id, name, type, location, tags, and no properties, which real Azure returns only under $expand. AciModels builds its entry through it too, so nine contributors share one definition rather than nine copies.

One deliberate deviation is preserved and now documented where the assembly happens: ArmHandler's own Storage / Key Vault / Web state contributes its full stored body, not the trimmed entry, because the azurerm provider reads this listing to populate its Key Vault cache and looks vaults up by properties.vaultUri. A test pins it.

Cosmos, Service Bus, Event Hubs and App Configuration are deliberately untouched: they have no ARM management plane in floci-az, so they own no resource to index.

Closes #269.

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

Azure Compatibility

Incorrect behavior: real Azure returns every resource in scope from both listings. floci-az returned a subset from one and almost nothing from the other. A VM created in a group answered a direct GET with 200 and appeared under /providers/Microsoft.Compute/virtualMachines, yet the group listing came back without it — and the subscription listing came back empty on an estate with a storage account and a virtual network in it.

That resource-group listing is what the azurerm provider reads before deleting a resource group to verify it is empty; ResourceIndexContributor's own javadoc records that a service which skips registration lets terraform destroy remove a group whose resources still exist. Both listings are also what anything enumerating an estate generically depends on — az resource list, the Resource Management SDKs, and drift checkers that diff deployed state against expected, which report real resources as missing.

Verified live against the release image and a build of this branch, same requests both times:

BEFORE — resource group holding a storage account and a VM
  rg index   -> [('reprosa', 'Microsoft.Storage/storageAccounts')]

BEFORE — subscription holding a storage account and a virtual network
  sub index  -> {"value":[]}

AFTER — two groups, each with a VM and a vnet; one also with a storage account
  rg-one     -> Microsoft.Storage/storageAccounts   subidxsa
                Microsoft.Network/virtualNetworks   vnet-rg-one
                Microsoft.Compute/virtualMachines   vm-rg-one

  subscription -> Microsoft.Storage/storageAccounts subidxsa
                  Microsoft.Network/virtualNetworks vnet-rg-one
                  Microsoft.Network/virtualNetworks vnet-rg-two
                  Microsoft.Compute/virtualMachines vm-rg-one
                  Microsoft.Compute/virtualMachines vm-rg-two

Tags round-trip into the index entry, properties stays out of it (matching GenericResourceExpanded), and the group listing stays scoped to its own group while the subscription listing spans every group.

Checklist

  • Tests pass locally — a bounded gate of 415 tests, 0 failures on JDK 25: the whole io.floci.az.core package plus every test class in the packages this touches (aci, acr, aks, vm, redis, postgres, mysql, mariadb, sql, arm, apim, managedidentity, network), excluding the *DockerTest classes. Full disclosure, same as in fix: honor X-Forwarded-Proto in /metadata/endpoints so URLs are https behind a TLS proxy #253: the complete ./mvnw test spins up docker-based service emulators I can't run in my sandbox, so I gated on those packages instead. CI ran the rest — the first commit came back green on all 13 checks including every SDK compat suite and compat-terraform/opentofu/azcli.
  • New or updated integration test added — one RestAssured case per contributing provider, in each service's existing mocked-mode test class, asserting both listings; plus a new ArmResourceIndexTest for what a per-service test cannot show: that the group listing stays scoped while the subscription listing spans groups, that a contributor's entry carries identity without properties, and that a key vault's vaultUri survives.
  • Every assertion was proven to fire. Reverting src/main to the pre-fix state leaves 8 failures, one per newly-registered provider; reverting it to the resource-group-only state leaves 11, exactly the subscription-scope assertions. With both commits applied, all pass.
  • Commit messages follow Conventional Commits

GET subscriptions/{sub}/resourceGroups/{rg}/resources returned only the
subsystems ArmHandler knows inline (Storage, Key Vault, Web) plus Network,
API Management, Managed Identity, and whatever implements the CDI
ResourceIndexContributor interface — where AciHandler was the sole
implementation. Every other service with a management plane was absent from
its own resource group: a VM answered a direct GET with 200 and appeared
under /providers/Microsoft.Compute/virtualMachines, yet the group listing
came back without it.

That listing is what the azurerm provider reads before deleting a resource
group to verify it is empty (the reason ResourceIndexContributor exists), and
what any caller enumerating a group generically — az resource list -g, the
Resource Management SDKs, drift-checking tools — depends on.

Registers the eight missing providers through the existing extension point:
Compute/virtualMachines, ContainerService/managedClusters,
ContainerRegistry/registries, Cache/Redis, DBforPostgreSQL/flexibleServers,
DBforMySQL/flexibleServers, DBforMariaDB/servers and Sql/servers. Each
contributes from the state its own list endpoint already reads, so the index
cannot drift from the type-scoped listing.

ArmResources.indexEntry centralises the entry shape Azure returns for a
generic resource — id, name, type, location, tags, and no properties, which
arrive only under $expand. AciModels now builds its entry through it, so the
one pre-existing contributor and the eight new ones share a single
definition.
@greptile-apps

greptile-apps Bot commented Sep 2, 2026

Copy link
Copy Markdown

Greptile Summary

The PR unifies subscription- and resource-group-level generic ARM resource aggregation and registers previously absent service handlers as resource-index contributors.

  • Adds minimal shared ARM index-entry construction.
  • Contributes VM, AKS, ACR, Redis, PostgreSQL, MySQL, MariaDB, and SQL resources.
  • Extends Network, Managed Identity, ACI, and existing contributors to subscription scope.
  • Adds scoped resource-index coverage across the affected services.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/main/java/io/floci/az/services/arm/ArmHandler.java Centralizes subscription and resource-group resource aggregation through one scope-aware assembly path.
src/main/java/io/floci/az/core/arm/ResourceIndexContributor.java Extends the contributor contract with an explicit subscription-scoped listing method implemented by every in-repository contributor.
src/main/java/io/floci/az/core/arm/ArmResources.java Adds a shared constructor for minimal generic ARM index entries.
src/main/java/io/floci/az/services/vm/VmHandler.java Registers VMs in both generic resource indexes using the handler's persisted VM state.
src/main/java/io/floci/az/services/aks/AksHandler.java Registers managed clusters in subscription and resource-group indexes with correctly scoped storage-key filtering.
src/main/java/io/floci/az/services/acr/AcrHandler.java Registers container registries in generic ARM indexes using minimal resource entries.
src/main/java/io/floci/az/services/redis/RedisHandler.java Registers Redis caches in both generic indexes while honoring service enablement.
src/main/java/io/floci/az/services/sql/SqlHandler.java Projects SQL server state into subscription- and resource-group-scoped generic listings.
src/test/java/io/floci/az/services/arm/ArmResourceIndexTest.java Covers scope separation, subscription aggregation, minimal contributor shape, and retained Key Vault compatibility fields.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Client[Generic ARM list request] --> Scope{Requested scope}
  Scope -->|Subscription| Subscription[Subscription resource index]
  Scope -->|Resource group| ResourceGroup[Resource-group resource index]
  Subscription --> Aggregate[indexedResources]
  ResourceGroup --> Aggregate
  Aggregate --> Own[Storage / Key Vault / Web]
  Aggregate --> Providers[Network / API Management / Managed Identity]
  Aggregate --> Contributors[CDI ResourceIndexContributor implementations]
  Contributors --> Services[ACI / ACR / AKS / VM / Redis / SQL-family]
  Own --> Response[ARM value array]
  Providers --> Response
  Services --> Response
Loading

Reviews (2): Last reviewed commit: "fix: aggregate the subscription resource..." | Re-trigger Greptile

GET subscriptions/{sub}/resources returned only Key Vaults and API
Management services, so a subscription holding neither answered with an empty
list while its resource groups held VMs, virtual networks, storage accounts
and database servers. Azure's subscription-scoped listing returns everything
the subscription holds.

Widens ResourceIndexContributor with listSubscriptionResources(sub) — no
default implementation, so a service cannot appear in one listing while
falling silently out of the other, which is the failure this interface exists
to prevent — and gives Network and Managed Identity the subscription-scoped
overload they lacked beside their resource-group one.

Both ARM listings now answer from one assembly, indexedResources(sub, rg),
where a null resource group means subscription scope. They cannot disagree
about what the estate holds.

ArmHandler's own Storage / Key Vault / Web state keeps contributing its full
stored body rather than the trimmed index entry: the azurerm provider reads
this listing to populate its Key Vault cache and looks vaults up by
properties.vaultUri, which a properties-free entry would not carry. That
deviation from Azure — which returns properties only under $expand — is now
recorded where the assembly happens, and pinned by a test.
@MichaelDanCurtis MichaelDanCurtis changed the title fix: list every service's resources in the resource-group index fix: list every service's resources in the ARM resource index, at both scopes Sep 2, 2026
@hectorvent

Copy link
Copy Markdown
Contributor

Thank you for extending this to the subscription scope; the estate-level test is the right shape. The widened listing breaks one real client, which I reproduced with the repo's own Terraform config.

(blocking) azurerm fills its Key Vault cache from GET /subscriptions/{sub}/resources?$filter=resourceType eq 'Microsoft.KeyVault/vaults' and parses every returned id as a vault id, failing hard on any other type (populate_cache.go#L72). The handler ignores $filter, so the listing now carries the storage account and VM as well, and terraform destroy fails with parsing ".../Microsoft.Storage/storageAccounts/flocitestsa" as a Key Vault ID. Main destroys cleanly. CI missed it because the bats script runs destroy with || true. Honouring resourceType eq '...' on both listings fixes it, and a test asserting the filtered result would pin it. The parameter is part of the listing's contract (resources.json#L481).

(follow-up, separate PR) The $expand wording from my earlier note now also lives in the indexedResources javadoc.

@hectorvent hectorvent added waiting-contributor bug Something isn't working arm Azure Resource Manager (ARM) labels Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arm Azure Resource Manager (ARM) bug Something isn't working waiting-contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] resourceGroups/{rg}/resources omits Compute, ContainerService, ContainerRegistry, Cache and every DB provider

2 participants