Skip to content

Live-verify Azure Advisor/Consumption resourceType filter literals (database, synapse, compute, cache, cosmosdb, managedredis) #1318

Closed
Closed
Live-verify Azure Advisor/Consumption resourceType filter literals (database, synapse, compute, cache, cosmosdb, managedredis)#1318
Labels
effort/mDaysimpact/fewLimited audiencepriority/p2Backlog-worthyseverity/mediumModerate harmtriagedItem has been triagedtype/bugDefecturgency/this-quarterWithin the quarter

Description

@cristim
cristim
opened on Jun 26, 2026

Context

Adversarial review of PR #1208 (ARCH-01) noted that the closing issue #1189 also flagged a related concern: the Advisor / armconsumption recommendation OData filter strings also use hand-typed resourceType literals. The PR explicitly scoped these out (they target a different API surface — Consumption Recommendations, not Capacity Reservations) and the PR description notes they need live verification before being touched.

Locations

providers/azure/services/database/client.go:174
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'SqlDatabase'"
providers/azure/services/cache/client.go:175
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'RedisCache'"
providers/azure/services/cosmosdb/client.go:169
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'CosmosDb'"
providers/azure/services/compute/client.go:199
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'VirtualMachines'"
providers/azure/services/synapse/client.go:130
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'SQLDatabaseDTU'"
providers/azure/services/managedredis/client.go:142
  filter := "properties/scope eq 'Shared' and properties/resourceType eq 'RedisCache'"

Concerns

  1. database/client.go:174 uses 'SqlDatabase' (singular) -- the same value PR fix(providers/azure): send canonical reservedResourceType enum values #1208 fixed on the purchase side from "SqlDatabase" to "SqlDatabases". If the Consumption Recommendations API expects the canonical (plural) form, this filter silently returns zero recommendations for Azure SQL Database. The PR description acknowledges this needs live verification.
  2. synapse/client.go:130 uses 'SQLDatabaseDTU' -- this doesn't match either the purchase enum (SqlDataWarehouse) or any obvious Consumption recommendation category. Suspicious; needs live verification.
  3. compute/client.go:199 uses 'VirtualMachines' while the inbound mapping at providers/azure/recommendations.go:404 lowercases to compare against "virtualmachines". Consistent in case, but the value source is still a literal not derived from the SDK.

Recommendation

  • Live-test each filter against a subscription with known recommendations to verify the literal returns rows (or document the live-verified canonical names).
  • For the armconsumption API there is no Go SDK enum for the filter resourceType values (it is an OData query parameter, not a typed field), so the fix is documentation / a var block at the top of each file naming the live-verified literal, not an SDK constant.
  • Add a smoke test in the e2e suite (or compose dev profile) that exercises each GetRecommendations against a live subscription and fails loud if the page count is zero AND zero recommendations were returned — distinguishes "no recommendations available" from "filter is wrong".

Out of scope for PR #1208

Per the closing issue #1189 ("Also check the singular 'SqlDatabase' Advisor filter at database/client.go:173") and the PR description ("intentionally left unchanged here; it needs live verification before touching the read path"), this is explicitly deferred.

Surfaced by

PR #1208 adversarial review sweep.

Activity

  1. cristim
    added
    triagedItem has been triaged
    priority/p2Backlog-worthy
    severity/mediumModerate harm
    urgency/this-quarterWithin the quarter
    impact/fewLimited audience
    effort/mDays
    type/bugDefect
    on Jun 26, 2026

  2. cristim commented on Jul 28, 2026

    @cristim
    cristim
    on Jul 28, 2026
    MemberAuthor

    Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

    The 2026-07-28 providers/azure review re-derived these literals from the SDK rather than by inspection, and the answer resolves most of this issue without a live subscription. Recording it here so it is not re-litigated.

    Where

    • SDK reference: armconsumption@v1.1.0/models.go:2463-2465
    • Current filter sites at be11bdcb5: compute/client.go:191, cache/client.go:157, managedredis/client.go:41, cosmosdb/client.go:159 (via the named constant reservationResourceTypeCosmosDB at :35), database/client.go:189 (via reservationResourceTypeSQLDB at :34), synapse/client.go:138 (via reservationResourceTypeSynapse at :34)
    • Purchase-side enum: armreservations@v1.1.0/constants.go:492-518

    What

    The armconsumption SDK does document the filter's allowed resourceType values, in the model doc comment rather than as a Go enum. models.go:2463-2465 lists them verbatim:

    ['VirtualMachines', 'SQLDatabases', 'PostgreSQL', 'ManagedDisk', 'MySQL', 'RedHat', 'MariaDB', 'RedisCache', 'CosmosDB', 'SqlDataWarehouse', 'SUSELinux', 'AppService', 'BlockBlob', ...]

    Checked against that list, every filter literal in the module today is correct:

    Site Literal In the documented list
    compute/client.go:191 VirtualMachines yes
    cache/client.go:157, managedredis/client.go:41 RedisCache yes
    cosmosdb/client.go:159 CosmosDB yes
    database/client.go:189 SQLDatabases yes
    synapse/client.go:138 SqlDataWarehouse yes

    This resolves the three concerns in the issue body, which were written when those sites still carried the older spellings:

    1. database no longer uses the singular 'SqlDatabase'; it uses the constant reservationResourceTypeSQLDB = "SQLDatabases", which is the documented value.
    2. synapse no longer uses 'SQLDatabaseDTU'; it uses reservationResourceTypeSynapse = "SqlDataWarehouse", which is the documented value.
    3. compute's 'VirtualMachines' is the documented value. It is still a hand-typed literal rather than an SDK constant (there is no Go enum for an OData query parameter), but its correctness is now established from the SDK docs.

    The casing divergence against the purchase side is correct, not drift. The Capacity Reservations API uses a different, typed enum that spells two of these differently: CosmosDb and SqlDatabases (armreservations@v1.1.0/constants.go:498,510). Two APIs, two spellings, both right. Any future sweep that "aligns" the Consumption filters with the Capacity enum casing would break the filters.

    What is still open

    Only the purchase-side literals that are genuinely unbacked by an SDK constant: "SearchService" (search/client.go:257) and the managedredis RedisCache product mismatch. Both are tracked on LeanerCloud/cloud-commitments-go#38 rather than here.

    Suggested disposition

    Narrow this issue to the remaining armconsumption values that are not on the documented list (none of the six current sites qualify), or close it and let LeanerCloud/cloud-commitments-go#38 carry the live-catalog verification of the purchase-side literals. If it stays open, the recommendation to pin the literals in a named const block per file is already implemented for cosmosdb, database and synapse and could be extended to compute, cache and managedredis, with the SDK doc citation above as the justification comment.

    Additional scope note from the CI/test-integrity reviewer, added here so the two halves are not litigated separately:

    Recording a verified negative result from the 2026-07-28 full review, so the next reviewer does not re-derive it. Read the scope note at the end carefully: this narrows what remains open here, it does not close this issue.

    What was verified: the Consumption reservation-details filter literals are correct

    armconsumption@v1.1.0/models.go:2463-2465 documents the filter's allowed values verbatim as:

    ['VirtualMachines', 'SQLDatabases', 'PostgreSQL', 'ManagedDisk', 'MySQL', 'RedHat', 'MariaDB', 'RedisCache', 'CosmosDB', 'SqlDataWarehouse', 'SUSELinux', 'AppService', 'BlockBlob', ...]

    Every filter at these call sites is drawn from that list:

    • 'VirtualMachines' - providers/azure/services/compute/client.go:191
    • 'RedisCache' - providers/azure/services/cache/client.go:157, providers/azure/services/managedredis/client.go:41
    • 'CosmosDB' - providers/azure/services/cosmosdb/client.go:35
    • 'SQLDatabases' - providers/azure/services/database/client.go:34
    • 'SqlDataWarehouse' - providers/azure/services/synapse/client.go:34

    The purchase side correctly uses the Capacity enum, which spells two of them differently: CosmosDb and SqlDatabases (armreservations@v1.1.0/constants.go:498,510).

    So the CosmosDB / CosmosDb and SQLDatabases / SqlDatabases divergences between the two sides are correct, not drift. They look like a copy-paste inconsistency and are not one: the two APIs genuinely use different spellings, and the repo has them right on both sides. Anyone "fixing" the apparent inconsistency by aligning the two spellings would break one of the two call paths.

    Scope: this does NOT clear the six literals this issue is about

    This issue tracks the Advisor / Consumption Recommendations OData filters, which are different call sites at different lines: database/client.go:174, cache/client.go:175, cosmosdb/client.go:169, compute/client.go:199, synapse/client.go:130, managedredis/client.go:142. Those still need the live verification described here. In particular:

    • database/client.go:174 uses 'SqlDatabase' (singular), which is not in the armconsumption allowed-value list quoted above.
    • synapse/client.go:130 uses 'SQLDatabaseDTU', which is not in that list either.

    Both remain unverified and both are the two this issue flagged as suspicious. Do not close this issue on the strength of the negative result above.

    Two literals confirmed as real defects, tracked separately

    The same review found the only two purchase-body literals in the module not backed by an SDK constant, filed separately as a MEDIUM in the Azure/GCP slice:

    • providers/azure/services/search/client.go:257 sends a bare "SearchService", which appears in neither armreservations v1.1.0 nor v2.0.0's ReservedResourceType enum (constants.go:492-518), and the in-code comment concedes it is unverified.
    • providers/azure/services/managedredis/client.go:274 purchases with ReservedResourceTypeRedisCache (Azure Cache for Redis) while being surfaced as Azure Managed Redis, a different product.

  3. cristim commented on Sep 2, 2026

    @cristim
    cristim
    on Sep 2, 2026
    MemberAuthor

    Verified resolved at 3c0f8ac: the two suspicious Consumption filter literals are gone. database now filters on reservationResourceTypeSQLDB = "SQLDatabases" and synapse on reservationResourceTypeSynapse = "SqlDataWarehouse", and every filter value in the six services (VirtualMachines, RedisCache, CosmosDB, SQLDatabases, SqlDataWarehouse) is on the allowed list the armconsumption v1.1.0 SDK documents at models.go:2464. Evidence: providers/azure/services/database/client.go:34,189; synapse/client.go:34,138; cosmosdb/client.go:35,159; compute/client.go:199; cache/client.go:157; managedredis/client.go:41 (#1455 commit 849a76e). Residual axes checked: no live e2e smoke test was added (the SDK-documented list makes it unnecessary for correctness); the managedredis client filtering on RedisCache for a different product is tracked on LeanerCloud/cloud-commitments-go#38. Closing as completed; reopen if the behaviour recurs.

  4. cristim
    closed this as completedon Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort/mDaysimpact/fewLimited audiencepriority/p2Backlog-worthyseverity/mediumModerate harmtriagedItem has been triagedtype/bugDefecturgency/this-quarterWithin the quarter

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions