Skip to content

Conversation

ajeffowens
Copy link
Contributor

This PR introduces three new variables which should enable CMK encryption on the ANF account.

variable "netapp_enable_cmk_encryption" {
  description = "Setting this variable to true enables CMK encryption on the netapp account.  Only relevant when storage_type=ha."
  type        = bool
  default     = false
}

variable "netapp_cmk_encryption_key_id" {
  description = "The ID of the key in keyvault to Encrypt ANF with (i.e. https://<keyvault-name>.vault.azure.net/keys/<key-name>).  Must exist before running terraform.  Only relevant when storage_type=ha.  Required if enable_anf_cmk_encryption is true."
  type        = string
  default     = null
}

variable "netapp_cmk_encryption_key_uai" {
  description = "The user assigned identity that will be used to access the key (i.e. /subscriptions/<sub>/resourceGroups/<rg>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<uai name>).  Must exist and have Key Vault Crypto Service Encryption User permission on the keyvault before running terraform.  Only relevant when storage_type=ha.  Required if enable_anf_cmk_encryption is true."
  type        = string
  default     = null
}

The vars are not doc-ed to follow the pattern of other encryption-related vars.

This should be a non-breaking change. If netapp_enable_cmk_encryption is false, and/or if the variables are omitted entirely, the ANF module should continue to work exactly as before.

@Carus11
Copy link
Contributor

Carus11 commented Jan 14, 2025

Hey Jeff, you will need to amend your commit and use --signoff if using the git cli to adhere to the DCO requirements https://github.com/sassoftware/viya4-iac-azure/pull/424/checks?check_run_id=35447478424

@dhoucgitter
Copy link
Member

dhoucgitter commented Mar 27, 2025

Hi @ajeffowens, I'm looking into this PR. Did you follow a set of published steps to create the key and set up permissions for the uai to access the keyvault when you tested this?

Came across these Azure key vault requirements in Azure doc, does that match your experience?

Requirements
Before creating your first customer-managed key volume, you must set up:
An Azure Key Vault, containing at least one key.
The key vault must have soft delete and purge protection enabled.
The key must be of type RSA.

@dhoucgitter
Copy link
Member

Hi @ajeffowens, are you able to share the terraform-input.tfvars file that you used for your testing? AS part of the steps to create the key vault, it looks like this will need to be a BYON scenario where you create a pre-existing VNet to both create your AKS cluster in and that your key vault private endpoint can live in, does that sound similar to the configuration that you used?

@ajeffowens
Copy link
Contributor Author

You are correct, David, that this implies not only a BYOKeyvault/key, but also BYON/pep/mi. This code would not support assigning the required permissions to the MI, let alone creating it (let alone creating the KV). I do not know whether this would work with a public keyvault, but that is probably an unlikely scenario to support.

In our case, we always pre-create the MI, then the keyvault and keys, then the Vnet and PE. So we use standard BYON inputs to iac + the three additional inputs defined by this PR.

Similar pattern to the aks_cluster_enable_host_encryption, enable_vm_host_encryption, vm_disk_encryption_set_id, and aks_node_disk_encryption_set_id vars. (where keyvault/key/des/mi/rbac must be sorted out ahead of time).

Copy link

This PR is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the stale Open for 30 days with no activity label Apr 28, 2025
@saschjmil saschjmil deleted the branch sassoftware:main May 8, 2025 18:30
@saschjmil saschjmil closed this May 8, 2025
@saschjmil saschjmil reopened this May 8, 2025
@saschjmil saschjmil changed the base branch from staging to main May 8, 2025 18:47
@saschjmil
Copy link
Contributor

Hey @ajeffowens, sorry for closing and re-opening your PR. I deleted the staging branch, which closed all PR's targeting it. I've updated your PR to point to the main branch.

@ajeffowens
Copy link
Contributor Author

Cool, and no worries. I will get my branch rebased to main as well

@github-actions github-actions bot removed the stale Open for 30 days with no activity label May 9, 2025
Copy link

github-actions bot commented Jun 8, 2025

This PR is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the stale Open for 30 days with no activity label Jun 8, 2025
@github-actions github-actions bot removed the stale Open for 30 days with no activity label Jun 21, 2025
Copy link

This PR is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the stale Open for 30 days with no activity label Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Open for 30 days with no activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants