Skip to content

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

Description

@cristim

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions