Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing soft delete option in azurerm_key_vault #1329

Closed
Beczka opened this issue Jun 1, 2018 · 26 comments · Fixed by #5344
Closed

Missing soft delete option in azurerm_key_vault #1329

Beczka opened this issue Jun 1, 2018 · 26 comments · Fixed by #5344
Assignees
Milestone

Comments

@Beczka
Copy link

Beczka commented Jun 1, 2018

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Please add option for soft delete

New or Affected Resource(s)

  • azurerm_key_vault

Potential Terraform Configuration

resource "azurerm_key_vault" "test" {
  name                = "testvault"
  location            = "West US"
  resource_group_name = "${azurerm_resource_group.test.name}"

  sku {
    name = "standard"
  }

  tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610"

  access_policy {
    tenant_id = "d6e396d0-5584-41dc-9fc0-268df99bc610"
    object_id = "d746815a-0433-4a21-b95d-fc437d2d475b"

    key_permissions = [
      "get",
    ]

    secret_permissions = [
      "get",
    ]
  }
  
  soft_delete = true
  enabled_for_disk_encryption = true

  tags {
    environment = "Production"
  }
}
@tombuildsstuff
Copy link
Contributor

hey @Beczka

Thanks for opening this feature request :)

Would it be possible to give some more information on the usage of the Soft Deletion property here (e.g. is this for items in the Key Vault, or for the Key Vault itself)? Terraform's destroy command is intended to completely remove objects, rather than soft-deleting them (since we prompt users for confirmation.

Thanks!

@Beczka
Copy link
Author

Beczka commented Jun 4, 2018

@tombuildsstuff

This feature is specific for the key vault itself. It very useful (as I painfully found out recently) when you by accident delete some values (secret/key or cert) from key vault basically it enables you to restore them using PowerShell or Azure Cli.

More info can be found here :
https://blogs.technet.microsoft.com/kv/2017/05/10/azure-key-vault-recovery-options/

It is already enabled in ARM template. But I think it would be cool to have it also in native terraform resources.

I was looking at source code and found out that this is already in terraform repo in models.go/VaultPatchProperties struct so I assume the reason for it not to be available at resource level is that it is in preview right?

@katbyte
Copy link
Collaborator

katbyte commented Jun 6, 2018

Seems like a sensible use-case to me, however for it to be effective the corresponding properties would also need to be added to resource_arm_key_vault_key and resource_arm_key_vault_certificate resources.

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Jun 19, 2018

Seems like a sensible use-case to me, however for it to be effective the corresponding properties would also need to be added to resource_arm_key_vault_key and resource_arm_key_vault_certificate resources.

@katbyte I don't think we need code changes in the Certificate/Key resources, since setting Soft Delete on the top level would cause any child items to be soft-deleted rather than hard-deleted - as such I think this only needs to be added to the Key Vault resource?

@liemnotliam
Copy link
Contributor

enablePurgeProtection and enableSoftDelete are key vault level properties and only changes to the key vault resource are needed, https://blogs.technet.microsoft.com/kv/2017/05/10/azure-key-vault-recovery-options/

The two properties are required to enable custom keys for storage account encryption, https://docs.microsoft.com/en-us/azure/storage/common/storage-service-encryption-customer-managed-keys#step-2-enable-sse-for-blob-and-file-storage

@LaurentLesle
Copy link
Contributor

PR #1805 has implemented it. to be reviewed

@marty2bell
Copy link

This issue seems to be sitting blocked to be reviewed for the 2.0 release at some point. Is there a timeline for that as the soft delete enablement is very useful in protecting the key vaults?

@ranokarno
Copy link

@tombuildsstuff ,
I notice that PR #1805 already closed while waiting for 2.0
nowadays lots of services required Azure keyVault with soft delete enabled.

Will it cause add complexity to run Azurerm_keyvault as current API and enable soft-delete from azure powershell e.g. keyvault delete, certificates update, etc.?

@PirateBread
Copy link

PirateBread commented Jul 4, 2019

+1 on a very needed feature. Right now we have to run some manual CLI commands following every new keyvault create. It seems like one extra API call: properties.enableSoftDelete=true

@nexxai
Copy link
Contributor

nexxai commented Jul 24, 2019

@tombuildsstuff Can we please get an update on where this one sits? The PR was closed in March (over 4 months ago) - there's obviously a bunch of us who would like to know if there's something in the pipe or what, especially if it's just relating to the decision of what to do when a terraform destroy is called on a Key Vault that has enable_purge_protection on it.

@nexxai
Copy link
Contributor

nexxai commented Jul 25, 2019

In case anyone is interested in a workaround until the option is added to the provider, here's what I did (using the command suggested by @slebodat here: https://docs.microsoft.com/en-us/azure/application-gateway/key-vault-certs in the feedback section):

resource "azurerm_key_vault" "keyvault" {
  name                        = "swoop-keyvault-tf-${var.environment_name}"
  location                    = "${azurerm_resource_group.keyvault.location}"
  resource_group_name         = "${azurerm_resource_group.keyvault.name}"
  enabled_for_disk_encryption = true
  tenant_id                   = "[OUR TENANT ID]"

  sku { 
    name = "standard"
  }

  access_policy {
    tenant_id = "[OUR TENANT ID]"
    object_id = "${azurerm_user_assigned_identity.keyvault-managed-identity.principal_id}"

    key_permissions = [
      "get",
    ]

    secret_permissions = [
      "get",
    ]

    storage_permissions = [
      "get",
    ]

    certificate_permissions = [
      "get"
    ]
  }

  network_acls {
    default_action = "Allow"
    bypass         = "AzureServices"
  }

  tags = "${azurerm_resource_group.keyvault.tags}"
}

resource "null_resource" "key-vault-enable-soft-delete" {
  provisioner "local-exec" {
    command = "az resource update --id $(az keyvault show --name ${azurerm_key_vault.keyvault.name} -o tsv | awk '{print $1}') --set properties.enableSoftDelete=true"
  }
}

@ranokarno
Copy link

@nexxai ,
I have similar solution in-place, however need to take note that any content of key/certiificate using terraform destroy /apply will fail as the record still in deleted-mode.

resource "azurerm_key_vault_secret" "test" {
  name         = "secret-sauce"   #<--- any redeployment (destroy/apply) with same name will fail
  value        = "szechuan"
  key_vault_id = "${azurerm_key_vault.test.id}"

  tags = {
    environment = "Production"
  }
}

@steffencircle
Copy link

Hi,

I have tried to enable Storage Account Encryption with Customer Managed Keys and ended up here.

Is there any progress on this matter and maybe a timeline the implementation ?

@mjiderhamn
Copy link

The workaround by @nexxai can be simplified (and awk dependency removed) to

resource "null_resource" "azure_storage_queue-enable-soft-delete" {
  provisioner "local-exec" {
    command = "az resource update --id ${azurerm_key_vault.mykeyvault.id} --set properties.enableSoftDelete=true"
  }
}

@J0F3
Copy link

J0F3 commented Oct 4, 2019

@tombuildsstuff can you give use please an update what is going on here now?
To be honest I don't quite understand all the discussion around the soft delete feature here. The soft delete feature on a Key Vault is an essential feature in Azure and many other services have dependencies to it.

For example, for the customer managed key for a storage account (mentioned above) or for the Key Vault integration for the Application Gateway needs this feature to be enable. Not implementing this in the Terraform provider leads to bigger and bigger feature gaps compared to what is possible with ARM. Which would be very unfortunate.

For the App Gateway part there is actually already a pull request which would implement also the necessary part on for Key Vault which is open since 16 days. But with no comment or update why it is not get merged. (#4366)

The workaround with Azure CLI described here a fine as a temporary solution but the initial request is now over an year old...

So in essence please, please make it possible that we can enable the soft delete feature natively with Terraform. 😁
Or give us at least a rough timeline when we can expect the implementation of this feature.

Thanks and sorry for my insistence but I need this feature really urgent and I don't like "hacks" with local-exec as a long term solution. 😉

@tombuildsstuff
Copy link
Contributor

@J0F3 since this would be a breaking behavioural change to the Delete (since we'd need to Purge the Key Vault during deletion) this is something we're planning to ship as a part of 2.0, with the other breaking changes; apologies, I thought we'd tagged this as a part of that milestone.

From our side we've started working on 2.0 and we've been adding support for the new features over the past couple of months - at this point in time they're intentionally feature-toggled off so that we can continue shipping 1.x features until the new features are available. Once we've finished the last of the new resources for 2.0 (which we're planning to do over the next few weeks); then we'll circle around and add support for Soft Deletion - however at this time since this'd be a breaking change to the behaviour of the Delete method this is currently waiting to ship as a part of 2.0

Thanks!

@sean-nixon
Copy link
Contributor

@tombuildsstuff You mentioned purging the key vault during the Delete. Could you clarify more on what the planned behavior for that will look like? I ask because the service integration with Azure Key Vault and SQL Server for TDE requires both soft delete AND purge protection to be enabled. Will there be a way to override this automatic purge and/or soft failing if the purge fails due to purge protection being enabled while still removing the resource from state? What about for keys/secrets? Obviously, trying to recreate the resources at the point would fail, but it stays in line with the purposes of those settings on Key Vault. It's supposed to act as a check against accidental deletion and that could happen with Terraform just as easily as without.

@J0F3
Copy link

J0F3 commented Nov 1, 2019

@tombuildsstuff thank you for the update!

However @sean-nixon brought up an very good point. I thought the same. Actually Terraform should not purge the Key Vault during deletion because this would make the actual meaning of the soft delete feature useless. The idea behind this feature is the ability to restore a deleted Key Vault after an accidental deletion and this may happend, as @sean-nixon mentioned also, done very easily with Terraform.

Imagine the Szenario where you accidental remove a Key Vault resource from your Terraform configuration which leads in the deletion of a Key Vault full with secrets which other resources and services depends on. When soft delete was enable you have the ability to restore the Key Vault with all its secrets very quickly. But when Terraform would completely purge the Key Vault you would be really in trouble because a simple re-creation of an empty Key Vault with the same name would not be enough.

Furthermore I think the behavior of soft delete is just how the platform (Azure in this case) works behind Terraform and Terraform should not change that in any way. Which means it should just send the delete "order" to Azure and remove the Key Vault form the state. Because that is what should be expected when a Key Vault (with enabled soft delete) is removed in Terraform. And if I want actually delete a Key Vault by intention because I do not need it anymore the fact that I can not recreate it with the same name should no be a problem at all. (And after 90 days it get purged automatically anyway)

But may be you could give us some insights about the planned changes so we can understand it better.

@tombuildsstuff
Copy link
Contributor

@sean-nixon @J0F3

You mentioned purging the key vault during the Delete. Could you clarify more on what the planned behavior for that will look like?

Honestly, we've not defined that yet - it's something we need to investigate; our initial thinking has been an option in the Provider block to control this behaviour; like so:

provider "azurerm" {
  purge_on_deletion {
    resource_groups = true # defaulted to true; this one deletes a Resource Group even if it contains things, matching the behaviour today
    key_vaults = true # probably defaulted to true, but maybe not ¯\_(ツ)_/¯
  }
}

(note to future readers - this doesn't exist today)

But there's downsides to that; namely that this is defined once in the Provider block for all resources. Unfortunately we're unable to place this within the specific resource since this resource may not be present in the Terraform Configuration during deletion - meaning that looking up the value of (say) a purge_on_deletion field within the Resource will fail if a user has deleted the Key Vault from their Terraform Configuration and run terraform apply; rather than running terraform destroy (where it still exists).

The issue here is there's valid use-cases for purging Key Vaults within Terraform (since, if the Resource Group contains a deleted but unpurged Key Vault, then the Resource Group deletion will fail, when it shouldn't). Whilst we could add some logic within the Resource Group deletion to handle this; this affects a number of resources now, so it needs some broader thought / investigation (one other option for example would be to store this "purge" information in a Tag, but some organizations have policies only allowing certain tags, as an example which would break that).

As mentioned above this is something we're hoping to deep-dive into in the near future / ship in 2.0 - but at this time we're focused on some of the other components - we'll post an update closer to the time. Definitely interested to hear thoughts though - we're trying to balance the fact that people use many different kinds of configuration here 😄

@J0F3
Copy link

J0F3 commented Nov 5, 2019

@tombuildsstuff

Ah, thank you for the clarification. The option on the provider block seems legit to me. 👍
Actually I did not know the part about the Resource Groups which could not be deleted when there is a delete but not purged Key Vault in it. In this case it makes probably really sense when Terrform purges the Key Vault then.

Thx

@sean-nixon
Copy link
Contributor

@tombuildsstuff Thank you very much for your response. I also like the provider approach. I don't think I have a use case right now that would require finer grain control than that, and it's always possible to use multiple aliased providers if absolutely necessary. And I'm glad to here there will be at the very least be a way to disable purging. Since you asked for thoughts, though, here are mine😄

My two cents on the defaults is I am strongly against purging by default for the following reasons:

  1. It is a breaking change with potentially significant consequences (see item 3.)
  2. It is entirely against the spirit of the soft-delete feature and so should require explicit opt-in
  3. The result of forgetting to disable automatic purging of a key vault could potentially be catastrophic data loss while forgetting to enable purge has a worst case of some Terraform errors deleting/creating resources and maybe some manual rework
  4. In many environments, the user / service principal credentials being used probably don't have permissions to execute purge operations. Purge permissions are typically not given out lightly and by default for Azure Key Vault only the subscription administrator can purge. For keys, secrets, and certificates, this is similar with all default permissions templates in the Portal NOT giving out purge permissions.
  5. Some Azure service integrations (namely SQL Server TDE) also require that purge protection be enabled on the Key Vault, which would then break the default delete behavior

I'm curious about your example use case you gave for requiring purging of Key Vaults. Based on the documentation and my own testing just now, it's completely possible to delete a resource group after a Key Vault has been soft deleted. It just needs to be re-created if you try to restore a key vault.

When a key vault is recovered, a new resource is created with the key vault's original resource ID. If the original resource group is removed, one must be created with same name before attempting recovery.

From Terraform’s perspective, it’s deleted and there would only be issues if we tried to create a new vault with the same name as the deleted one (I’m not sure which other resources you were referring too that were also affected related to deletion/purging so I can't speak to those)

In light of that, the main use case I see for purging Key Vaults is to re-create a Key Vault with the same name in a different region due to a misconfiguration, migration, etc. If purging is off my default and the vault was deleted without the override, that would result in a state that Terraform alone could not recover from as the vault would need to be manually purged or manually restored before Terraform could manage it again. That is not ideal but it is identical behavior to if someone enables soft-delete using one of the workaround above (which is obviously happening given the comments here and the number of service integrations that require it).

In that sense, with the assumption of NOT making purging the default, this really doesn't have to be a breaking change IMHO. Accompanied by reasonable warnings in the documentation for what could happen when deleting a vault with soft-delete enabled, it seems to me like waiting for 2.0 isn't strictly necessary (although I understand there may be other priorities as I don’t have the whole picture)


On another note, as a workaround to the case of a soft-deleted but not purged vault, my suggestion would be to add an additional parameter recover_deleted_if_exists to the azurerm_key_vault resource. The idea is that it would customize the create behavior to first check if there is an existing deleted vault and if so attempt to restore the vault instead of creating a new one (it's one simple property in the create request). This would allow previously deleted vaults to be brought back under Terraform management and then used or purged as necessary. The parameter would only take effect during the creation of the resource, so it would need to be effectively ignored on all update operations which I know is not typical. I don't know if there's precedent for this on other Terraform resources.

@grusak
Copy link

grusak commented Nov 6, 2019

I like @sean-nixon's idea of using a recover_deleted_if_exists parameter as it can be extended to any resources with soft delete functionality. My specific case has to do with azurerm_recovery_services_protected_vm. In a soft delete enabled Recovery Service Vault, data in a soft-deleted state cannot be deleted until it is automatically deleted 14 days after the soft delete date. There is no force purge capability. Currently an apply/delete tries until the timeout time and then fails. Since soft delete is driven by Microsoft, Terraform should be able to cleanly deal with it in resources where its available.

@tombuildsstuff
Copy link
Contributor

@grusak

There is no force purge capability. Currently an apply/delete tries until the timeout time and then fails. Since soft delete is driven by Microsoft, Terraform should be able to cleanly deal with it in resources where its available.

FWIW we've been chatting with the Recovery Services team and they've agreed there should be a means of disabling/toggling Soft Deletion off - so whilst that's not an option today, it sounds like it may be in the future :)

@grusak
Copy link

grusak commented Nov 7, 2019

@tombuildsstuff I received the same information from MS as well, so its coming.

@ghost
Copy link

ghost commented Feb 24, 2020

This has been released in version 2.0.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.0.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Mar 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.