Skip to content

azure: the provider-default prune entries the claimed-field set now covers #2186

Description

@lex00

#2172 (issue #2160) added the claimed-field set: packages/core/src/claimed-fields.ts derives, from a declaration's own props, every path it ever set. A live value on a path outside that set now reports correctly instead of being read as drift. (#2181 is renaming that #2172 bucket from "held elsewhere" to "unclaimed": DeepDiffResult.unclaimed, heading UNCLAIMED. Using "unclaimed" throughout this issue.)

#2172's PR body names the counterpart-gated provider-default entries in the azure deep-observe hook as the largest shrink candidate for this lexicon, and says pruning and unclaimed report differently: pruned means the field never reaches the tree, unclaimed means it is reported with its live value under the UNCLAIMED heading. This issue is the per-lexicon follow-up its acceptance line asked for; nothing here has been shrunk yet.

What is in the file

azureDeepNormalizationHooks.prune (lexicons/azure/src/deep-observe.ts:252-290) checks, in order:

  • AZURE_READ_ONLY_NAMES (lines 134-143): provisioningState, resourceGuid, principalId, tenantId, etag. Unconditional, both sides (checked line 255).
  • AZURE_UNCONDITIONAL_PRUNE_NAMES (line 151): defaultSecurityRules. Same unconditional check as above, not gated to counterpart at all.
  • AZURE_SERVER_COMPUTED_NAMES (lines 163-167): currentKubernetesVersion, fqdn, nodeResourceGroup. Counterpart-gated live+absent (checked line 273).
  • The empty-tags-map rule (line 261): unconditional once gated absent+live (the side/counterpart gate is line 257).
  • The location rule (line 269): same gate, fires on any top-level path literally equal to location.
  • AZURE_OWNERSHIP_TAG_PATTERNS / AZURE_MANAGED_BY_TAG_PATTERNS (lines 202-215): chant's own tag signature, checked lines 276-277.
  • The [].id suffix rule (line 285): same gate, fires on the self-id ARM stamps on each element of a nested array (a security rule, an inline subnet).
  • AZURE_SERVICE_DEFAULTS (lines 176-180): Microsoft.Storage/storageAccounts.minimumTlsVersion = TLS1_2. One entry, one kind, checked lines 287-289.

Two of these tables have a second consumer beyond the diff. lexicons/azure/src/export-resources.ts:28,62 imports AZURE_READ_ONLY_NAMES and AZURE_SERVER_COMPUTED_NAMES to scrub the same field names out of generated declarations for chant import --from. That import is unconditional there (export has no declared side to gate against), and it answers a different question from the diff hook: whether generated code should ever contain the field, not whether an undeclared live value is noise. Grepped every other exported name here across the lexicon and packages/core: none of the rest has a second consumer.

Candidates versus must-stay

Candidates: AZURE_SERVICE_DEFAULTS (1 entry), the empty-tags rule, the location rule, and the [].id suffix rule. All four only fire when the declaration never set the property, the shape claimedFieldsOfProps now answers directly.

The location rule needs the most care of the four. It is not a per-kind table, it fires on the literal path location for every azure resource type, and most azure resources (any child that inherits its parent's placement, and most top-level ones too, since chant usually declares location via [resourceGroup().location], which already has a declared counterpart and is unaffected) would start reporting UNCLAIMED: location = <region> on every clean diff --live if this moves. It is a legitimate candidate by the same test as the rest, but it is also the one most likely to turn into permanent per-resource noise rather than a rare, useful finding; verify the actual volume against a real estate before deciding to keep it or shrink it.

AZURE_SERVER_COMPUTED_NAMES is a candidate for the diff hook specifically, but removing it from the shared const (rather than only from the prune() check) would also stop export-resources.ts from scrubbing fqdn/currentKubernetesVersion/nodeResourceGroup out of generated code, which is a separate, likely wrong, outcome (fqdn and currentKubernetesVersion have no create-time input at all). Shrinking this one correctly means splitting the diff-noise question from the export-scrub question, for example by giving export-resources.ts its own copy of the names it needs to keep scrubbing, rather than deleting entries from the table both consumers share.

Must stay:

  • AZURE_READ_ONLY_NAMES: server-assigned identity and bookkeeping, pruned unconditionally on both sides. The read-only-name category feat(core): the claimed-field set, and a live value chant never set stops being drift #2172's body calls must-stay by name, doubly so since export-resources.ts also depends on it for a different purpose.
  • AZURE_UNCONDITIONAL_PRUNE_NAMES (defaultSecurityRules): not gated on absence at all; it is server-injected boilerplate dropped outright regardless of what the declaration says, so the claimed-field set has nothing to say about it.
  • AZURE_OWNERSHIP_TAG_PATTERNS / AZURE_MANAGED_BY_TAG_PATTERNS: chant's own signature, not a provider default. Removing it would make every managed resource's own tags report as unclaimed noise on every diff.

Verification before merging

Run chant lifecycle diff --live against a real Azure resource group covering a storage account (for minimumTlsVersion and the empty-tags rule), an AKS managed cluster (for fqdn/currentKubernetesVersion/nodeResourceGroup), a network security group (for defaultSecurityRules staying pruned and securityRules[].id moving), and a virtual network with subnets (for location), each declared with the relevant property left unset, before and after removing an entry. Also run chant import --from against the same estate after any change to AZURE_READ_ONLY_NAMES or AZURE_SERVER_COMPUTED_NAMES and confirm the generated declarations are unchanged.

Ref #2160, #2172, #2181.

Acceptance

  • Each removed entry (AZURE_SERVICE_DEFAULTS, the empty-tags rule, the location rule, or the [].id suffix rule) has a test in lexicons/azure/src/deep-observe.test.ts asserting the field reports unclaimed with its live value.
  • If AZURE_SERVER_COMPUTED_NAMES moves, export-resources.ts's scrub behavior is preserved (its own copy of the names, or an equivalent split) and export-resources.test.ts still passes unchanged in what it asserts about generated output.
  • No entry moves without a matching real-estate run named in the PR (resource group, resource kinds, fields checked), including an import --from run for anything touching AZURE_READ_ONLY_NAMES or AZURE_SERVER_COMPUTED_NAMES.
  • No KNOWN_FAILURES entry added for any lexicon.
  • lexicons/azure/src/deep-observe.test.ts and lexicons/azure/src/export-resources.test.ts pass in full.
  • AZURE_READ_ONLY_NAMES, AZURE_UNCONDITIONAL_PRUNE_NAMES and the ownership-tag patterns are untouched, or the PR explains why one of them turned out to be coverable after all.

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

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions