Skip to content

Split Terraform customer managed key interface into two variants - #2863

Open
Jared Holgate (jaredfholgate) wants to merge 6 commits into
mainfrom
jaredfholgate-cmk-interface-client-id
Open

Split Terraform customer managed key interface into two variants#2863
Jared Holgate (jaredfholgate) wants to merge 6 commits into
mainfrom
jaredfholgate-cmk-interface-client-id

Conversation

@jaredfholgate

@jaredfholgate Jared Holgate (jaredfholgate) commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

Splits the Terraform Customer Managed Key interface into two variants, mirroring the Variant 1 / Variant 2 pattern already used by the Bicep spec.

Variant 1 is the existing shape, unchanged. Existing modules and existing consumers stay valid.

Variant 2 is new and minimal:

key_vault_key_uri                = string
user_assigned_identity_client_id = optional(string, null)

Why

Terraform reads a data source during plan whenever its arguments are already known.

  • azurerm_key_vault_key takes a vault resource ID and a key name.
  • azurerm_user_assigned_identity takes a name and a resource group name.

Those arguments are known literals even when the key or the identity is created by the same terraform apply, so the read runs before the resource exists and the plan fails. Modules that need an identity's client IDMicrosoft.ContainerRegistry/registries is the current example — have no way to obtain it from a resource ID without hitting exactly this problem. That is what drove the non-standard customer_managed_key_direct_values workaround in terraform-azurerm-avm-res-containerregistry-registry#201.

Reading the vault by key_vault_resource_id is safe in both directions, because that argument is unknown at plan time whenever the vault is created by the same apply, which defers the read. Variant 1 therefore keeps working as it always has.

Why two variants rather than extending the existing one

Adding client_id to the single existing object is a breaking change for every module that consumes it, and it permanently clutters the interface for resources such as Microsoft.Storage/storageAccounts that will never use it. Splitting means a module only ever sees the attributes its resource provider actually consumes.

Linting accepts either shape.

Which variant applies

This is determined by the resource provider's API, not by module owner preference.

Variant 1 Variant 2
Identity identified by resource ID client ID
Key supplied as vault URI + key name + version single key URI
Example provider Microsoft.Storage/storageAccounts Microsoft.ContainerRegistry/registries

Side effects of Variant 2

  • Carrying the full key URI means the consumer owns the host, so sovereign clouds and Managed HSM work unchanged with no DNS suffix derivation in the module.
  • Omitting the trailing version segment leaves the resource provider following key rotations automatically.
  • Variant 2 carries no identity resource ID, so a module cannot cross-check the encryption identity against managed_identities.user_assigned_resource_ids. This is intentional — the spec instead requires the module to document that the consumer assigns the identity.

Validation

Both schemas were exercised with terraform test: 6 cases for Variant 1 and 12 for Variant 2, covering versionless and versioned URIs, Managed HSM and sovereign hosts, and rejection of secret URIs, key resource IDs, http scheme and non-GUID client IDs. 18/18 pass. terraform fmt -check is clean on all six include files.

Companion changes

  • Azure/terraform-azure-avm-utl-interfaces — add the Variant 2 shape.
  • Azure/tflint-ruleset-avm — accept either shape.
  • The corresponding update to Azure-Verified-Modules-Docs.

Note on CI

The buildpr failure on this branch is pre-existing and unrelated. Commit 11edbf8 (#2861) flipped two avm-ptn-alz-application-landing-zone-cicd-bootstrap-* rows in TerraformPatternModules.csv to Available, and their registry URLs return 404, so Hugo fails resolving them. Unrelated PRs fail identically. It needs a separate fix in the CSV.

@myaschmitz

Copy link
Copy Markdown

I like the approach here, since it would remove the data sources as well as fix the root issue of Azure/terraform-azurerm-avm-res-containerregistry-registry#201.

@jaredfholgate Jared Holgate (jaredfholgate) changed the title feat(spec): allow client_id in the Terraform CMK interface and remove data source reliance Split Terraform customer managed key interface into two variants Aug 6, 2026
… data source reliance

Adds `user_assigned_identity.client_id` alongside `resource_id`, making both
optional with validation that at least one is supplied. Resource providers
differ in how they identify the encryption identity, so modules need to be able
to accept either.

Also removes the need for `azurerm_key_vault_key` and
`azurerm_user_assigned_identity` data sources. Terraform reads a data source
during plan whenever its arguments are known, so resolving the key URI that way
fails when the key or identity is created by the same apply. The key URI is
derived from the existing inputs instead, with a new optional `key_vault_uri`
to override the derived host in sovereign clouds.

Adds `int.cmk.usage.tf` showing the derivation and the ACR-shaped consumption
of it, including versionless URI support for auto-rotation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Make `key_vault_resource_id` optional, requiring at least one of it or
`key_vault_uri`, because no resource provider consumes the vault's ARM
resource ID for encryption.

Replace the azurerm implementation example with two azapi examples that
show the two shapes resource providers use: a combined key identifier
plus client ID (Microsoft.ContainerRegistry/registries), and a split
vault URI, key name and key version plus identity resource ID
(Microsoft.Storage/storageAccounts).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a52aad12-17f1-4da4-8c8f-316469ee6958
`Microsoft.Compute/diskEncryptionSets` does consume the vault's ARM
resource ID via `activeKey.sourceVault.id`, and requires a fully
versioned key URL, so state both as module-level validations rather
than claiming no provider uses the vault resource ID.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a52aad12-17f1-4da4-8c8f-316469ee6958
Replace the `cmk` abbreviation with the full `customer_managed_key`
prefix and drop the redundant `local.cmk` alias in favour of reading
`var.customer_managed_key` directly. Also split the DNS suffix into its
own local so the derived vault URI stays readable.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a52aad12-17f1-4da4-8c8f-316469ee6958
Retain the original customer managed key shape unchanged as Variant 1, and
add a minimal Variant 2 that carries only the full key URI and the encryption
identity client ID.

Terraform reads a data source during plan whenever its arguments are already
known, so resolving a key URI or an identity client ID from names or resource
IDs fails when either is created by the same apply. Variant 2 removes that
class of failure by having the consumer supply both values directly, which
also makes the input cloud agnostic and Managed HSM capable.

Modules implement whichever variant their resource provider requires, so
existing modules stay valid and no interface is cluttered with attributes it
will never use.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a52aad12-17f1-4da4-8c8f-316469ee6958
Variant 2 carried a flat user_assigned_identity_client_id, which followed
the Bicep UDT convention rather than the Terraform interface it sits beside.
Variant 1 nests a single-attribute identity object, so nesting keeps the two
Terraform variants consistent and makes adding a companion resource_id an
additive change rather than a rename.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a52aad12-17f1-4da4-8c8f-316469ee6958
@jaredfholgate
Jared Holgate (jaredfholgate) force-pushed the jaredfholgate-cmk-interface-client-id branch from 8bd657a to 89c2d28 Compare August 6, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants