[Feature Request] Add option to define behavior when running terraform destroy

[lhriley]

Summary

Currently when running terraform destroy or terraform apply after removing 1password provider resources, Terraform destroys any secrets that it has created or imported into state. This behavior can be dangerous or unwanted in the case of a need to preserve secrets outside the Terraform lifecycle.

For example, it could be problematic to import a set of database credentials for use in Terraform only to have them deleted automatically at some point in the future.

It would be preferable to allow users the option to configure what action the Terraform provider takes when an outcome could be the deletion of secrets from 1password vaults.

Use cases

Scenario A: A user needs to rename Terraform resources to follow a new naming convention (eg: fixing Terraform linting issues where resource names include -), the resulting vault item title and content of the secret will remain unchanged once the refactoring is complete.

Prior to the terraform apply, the vault item is imported into a new Terraform resource. While the terraform apply is occuring, the password must remain available to other consumers of the secret (eg: k8s pods are configured to restart automatically when the secret changes), and thus cannot be removed and recreated.

Scenario B: A specific vault contains secrets used in a CI/CD environment that validates infrastructure as code changes as part of the PR process. This vault is reused by all CI/CD runners which in turn import certain secrets into Terraform state, and thus must be available any time a PR is submitted or subsequent commits are pushed. Since these secrets are critical to the proper functioning of the CI/CD pipeline, they cannot be removed when terraform destroy is called.

Proposed solution

Options exist globally for the 1password Terraform provider and within individual provider resources that control what happens in the case of a terraform destroy or similarly destructive set of circumstances which would result in the deletion of critical 1password secrets.

Internally this might look like a check for the status of the bool at the time the API call is made and act or not act as appropriate. If the state is not to act, a warning should be logged to help users understand what the decision was and hint at why it was made.

Note that more appropriate / better names for these options are welcome.

Provider example:

provider "onepassword" {
  url = "http://localhost:8080"
  remove_secrets_from_vault_on_destroy = <bool>
}

Resource example:

resource "onepassword_item" "demo_login" {
  vault = var.vault_id

  remove_from_vault_on_destroy = <bool>

  title    = "Demo Terraform Login"
  category = "password"

  username = "demo-username"

  password_recipe {
    length  = 40
    symbols = false
  }

Is there a workaround to accomplish this today?

The only way to currently solve for this problem today would be to manually remove the resources from Terraform state prior to a destructive action being taken.

This might not always be feasible due to policies around individual contributor rights or when using CI/CD automation tools such as Atlantis.

Additionally, depending on how many secrets are being managed via Terraform or how the code is arranged (eg: when using nested modules), it might be prohibitively complex to attempt manual state manipulation.

References & Prior Work

These are examples of an open issues in Terraform itself looking for ways to "leave resources in place when destroying", some of which are ancient:

• https://github.com/hashicorp/terraform/issues/23547
• https://github.com/hashicorp/terraform/issues/2253
• https://github.com/hashicorp/terraform/issues/3874

These links are not examples of prior work, but they do relate to the problem at hand:

• https://www.terraform.io/docs/language/meta-arguments/lifecycle.html#create_before_destroy
• https://www.terraform.io/docs/language/meta-arguments/lifecycle.html#prevent_destroy
• https://www.terraform.io/docs/cli/commands/state/rm.html

[jpcoenen]

Hi @lhriley,

Thank you for your request and extensive. I understand your concern about Terraform destroying your items.

In the ticket you are mentioning importing items. Would it be possible to use a data source instead of a resource? Then it does not have to be imported and there is also no way for Terraform to accidentally destroy it.

[lhriley]

@jpcoenen Sorry for the delayed response, I was out on vacation for the last couple of weeks.

I included the use cases as examples of how this problem could manifest, not as an example of issues that I'm currently experiencing.

I'm not currently using this provider due to security concerns in Terraform itself, so the suggested workaround wouldn't apply to me at the moment. The way that Terraform stores sensitive data in the state file is inherently insecure and have gone unaddressed since at least 2014 (https://github.com/hashicorp/terraform/issues/516). There doesn't appear to be a good solution to this problem, but that's another matter entirely.

However, I intentionally gave the example as I have seen Terraform code which has been maintained in similar, less than ideal ways due to various historical reasons or specific use cases. It may not be possible for users to refactor their Terraform code to use data sources rather than providers.

Consider that you have some Terraform code that you need to test exhaustively in CI/CD before you release it to production. You need the same code to be able to successfully create temporary test environments (with or without existing secrets) as well as new long running environments. As such it also needs to gracefully generate secrets if they don't exist. It is easy to create pre-hooks that import existing known secrets to resources, but it is hard to generate secrets that would need to exist for the data source not to fail.

[jpcoenen]

Those clarifications help a lot, thanks.

If I understand you correctly, the situation where you'd want the requested feature, is when a 1Password item is imported into the Terraform state as an alternative to a data source? If that's the case, then I think we should also look at the underlying reason for this. Because forcing someone to import an item that should not be managed by Terraform, sounds like something that we should avoid.

I have not checked if this is possible, but would this problem be solved if the data source had something like a allow_not_found option, that would not error when the referenced item cannot be found?

[lhriley]

Because forcing someone to import an item that should not be managed by Terraform, sounds like something that we should avoid.

I would generally agree, but this might not always be possible for various reasons. I think we're probably getting too far into the weeds about hypothetical terraform use cases, though.

I don't think an allow_not_found would resolve this feature request (though it may resolve someone's use case) since if the secrets are required terraform should create them when missing or fail gracefully, and when terraform destroy is issued the secrets should still exist.