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

aws_secretsmanager_secret: interpolated tags are not included in depedency map #103

Closed
dekimsey opened this issue Apr 6, 2020 · 8 comments

Comments

@dekimsey
Copy link

dekimsey commented Apr 6, 2020

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 other comments that do not add relevant new information or questions, 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

Terraform Version

terraform -v
Terraform v0.12.20
+ provider.acme v1.5.0
+ provider.aws v2.53.0
+ provider.null v2.1.2
+ provider.random v2.2.1
+ provider.template v2.1.2
+ provider.tls v2.1.1

Affected Resource(s)

  • aws_secretsmanager_secret
  • acme_certificate

Terraform Configuration Files

resource "aws_route53_record" "demo" {
  name = "demo"
  ...
}
resource "acme_certificate" "demo" {
  common_name     = aws_route53_record.demo.fqdn
  ...
}

resource "aws_secretsmanager_secret" "demo_bundle_pem" {
  name        = "/demo.pem"
  description = "nep-proxy cert: ${acme_certificate.demo.certificate_domain}"

  tags = {
    namespace    = data.terraform_remote_state.main.outputs.namespace
    application     = "demo"
    terraform       = "true"
    acme_id         = acme_certificate.demo.id   # <--------- this value
  }
}

resource "aws_secretsmanager_secret_version" "demo_bundle_pem" {
  secret_id = aws_secretsmanager_secret.demo_bundle_pem.id
  secret_string = join(
    "\n",
    [
      acme_certificate.demo.certificate_pem,
      acme_certificate.demo.issuer_pem,
      acme_certificate.demo.private_key_pem,
    ],
  )
}

Debug Output

Panic Output

Expected Behavior

The ACME cert should have been refreshed and updated without issue.

Actual Behavior

Had to run terraform apply twice.

$ terraform apply

# acme_certificate.demo will be updated in-place
~ resource "acme_certificate" "demo" {
	  account_key_pem    = (sensitive value)
	~ certificate_domain = "demo.example.com -> (known after apply)
	~ certificate_p12    = (sensitive value)
	~ certificate_pem    = <<~EOT
		  -----BEGIN CERTIFICATE-----
		  ...
		  -----END CERTIFICATE-----
	  EOT -> (known after apply)
	~ certificate_url    = "https://acme-v02.api.letsencrypt.org/acme/cert/1337" -> (known after apply)
	  common_name        = "demo.example.com"
	  id                 = "https://acme-v02.api.letsencrypt.org/acme/cert/1337"
	~ issuer_pem         = <<~EOT
		  -----BEGIN CERTIFICATE-----
		  ...
		  -----END CERTIFICATE-----
	  EOT -> (known after apply)
	  key_type           = "P256"
	  min_days_remaining = 30
	  must_staple        = false
	~ private_key_pem    = (sensitive value)

	  dns_challenge {
		  config   = (sensitive value)
		  provider = "route53"
	  }
  }

# aws_secretsmanager_secret.demo_bundle_pem will be updated in-place
~ resource "aws_secretsmanager_secret" "demo_bundle_pem" {
	  arn                     = "arn:aws:secretsmanager:..:secret:/demo.pem-F4EXre"
	~ description             = "nep-proxy cert: demo.example.com" -> (known after apply)
	  id                      = "arn:aws:secretsmanager:...:secret:/demo.pem-F4EXre"
	  kms_key_id              = "deadbeef"
	  name                    = "/demo.pem"
	  recovery_window_in_days = 30
	  rotation_enabled        = false
	  tags                    = {
		  "acme_id"         = "https://acme-v02.api.letsencrypt.org/acme/cert/1337"
		  "application"     = "foo"
	  }
  }

# aws_secretsmanager_secret_version.demo_bundle_pem must be replaced
-/+ resource "aws_secretsmanager_secret_version" "demo_bundle_pem" {
	~ arn            = "arn:aws:secretsmanager:...:secret:/demo.pem-F4EXre" -> (known after apply)
	~ id             = "arn:aws:secretsmanager:...:secret:/demo.pem-F4EXre|E1B7E44A-E893-4808-BF3A-27CCBFE7BF4D" -> (known after apply)
	  secret_id      = "arn:aws:secretsmanager:...:secret:/demo.pem-F4EXre"
	~ secret_string  = (sensitive value)
	~ version_id     = "E1B7E44A-E893-4808-BF3A-27CCBFE7BF4D" -> (known after apply)
	~ version_stages = [
		- "AWSCURRENT",
	  ] -> (known after apply)
  }

  acme_certificate.demo: Modifying... [id=https://acme-v02.api.letsencrypt.org/acme/cert/1337]
  aws_secretsmanager_secret_version.demo_bundle_pem: Destroying... [id=arn:aws:secretsmanager:...:secret:/demo.pem-F4EXre|E1B7E44A-E893-4808-BF3A-27CCBFE7BF4D]
  aws_secretsmanager_secret_version.demo_bundle_pem: Destruction complete after 0s
  acme_certificate.demo: Still modifying... [id=https://acme-v02.api.letsencrypt.org/acme/cert/1337, 10s elapsed]
  ...
  acme_certificate.demo: Modifications complete after 1m15s [id=https://acme-v02.api.letsencrypt.org/acme/cert/abcd]

  Error: Provider produced inconsistent final plan

  When expanding the plan for
  aws_secretsmanager_secret.demo_bundle_pem to include new values
  learned so far during apply, provider "registry.terraform.io/-/aws" produced
  an invalid new value for .tags["acme_id"]: was
  cty.StringVal("https://acme-v02.api.letsencrypt.org/acme/cert/1337"),
  but now
  cty.StringVal("https://acme-v02.api.letsencrypt.org/acme/cert/abcd").

  This is a bug in the provider, which should be reported in the provider's own
  issue tracker.

Steps to Reproduce

  1. terraform apply # errors
  2. terraform apply # success

Important Factoids

It looks as if the tag changes, but it's dependency upon the output of acme_certificate isn't computed as part of the plan.

References

  • #0000
@bflad
Copy link

bflad commented Apr 6, 2020

Hi @dekimsey 👋 Thank you for reporting this and sorry you are running into trouble here.

This type of error:

When expanding the plan for
  aws_secretsmanager_secret.demo_bundle_pem to include new values
  learned so far during apply, provider "registry.terraform.io/-/aws" produced
  an invalid new value for .tags["acme_id"]: was
  cty.StringVal("https://acme-v02.api.letsencrypt.org/acme/cert/1337"),
  but now
  cty.StringVal("https://acme-v02.api.letsencrypt.org/acme/cert/abcd").

Indicates that the upstream resource attribute reference changed "unexpectedly" between plan and apply. Attribute values between plan and apply phases are expected to be known or otherwise marked as changed in Terraform resource code. Terraform resources have various ways to signal to Terraform (core/CLI) that their attributes might change during an update-in-place operation, however it appears the acme_certificate resource may not be doing this for the id attribute itself. The Terraform ACME Provider maintainers will need to update the resource (e.g. via CustomizeDiff and SetNewComputed for the attribute or adding ForceNew to attributes that should trigger a resource recreation plan instead of an in-place update plan).

Since there's nothing that we can do in this situation within the Terraform AWS Provider, I'm going to transfer this issue over to the Terraform ACME Provider due to the above, but in the meantime you may find success in switching to an attribute that does appear to be re-computed during updates, such as acme_certificate.demo.certificate_url.

@bflad bflad transferred this issue from hashicorp/terraform-provider-aws Apr 6, 2020
@dekimsey
Copy link
Author

dekimsey commented Apr 6, 2020

Thanks @bflad! I wasn't sure if this was an ACME or AWS provider issue, but I figured I'd start there.

@dekimsey
Copy link
Author

Just pinging this issue, it is still occurring every time we rotate the ACME certificates.

 terraform -v
Terraform v0.12.28
+ provider.acme v1.5.0
+ provider.aws v2.52.0
+ provider.local v1.4.0
+ provider.template v2.1.2

@vancluever
Copy link
Owner

vancluever commented Oct 8, 2020

@dekimsey indeed this is probably caused by the fact that the ID is changing on certificate update (as it's essentially getting replaced on renewal), so I will need to figure out what will be better - forcing a complete replacement of the resource on renewal (which may not be a good idea as we actually revoke the certificate when the resource is destroyed), or determining a different ID scheme, decoupling it from the actual certificate URL.

In the interim, you could see about changing the field you use from the ID of the certificate to certificate_url.

@dekimsey
Copy link
Author

dekimsey commented Oct 9, 2020

Thanks @vancluever, that's a good idea. I'll update our code to use that url and let you know when the next rotation happens.

For background, we actually squirrel away the certificate_id on the secretsmanager entry because it offers a nice way to identify the certificate bundle without having to grant access to read it's contents. We then have a lambda that periodically checks all the certificates by simply reading their tags and alerting for any pending expiration.

@dekimsey
Copy link
Author

Just circling back on this, it worked great. New secretsmanager entries were made and tagged without issue in a single run by using the .certificate_url instead of .id.

@vancluever
Copy link
Owner

Awesome, glad that's working for you @dekimsey. Will help once I decide the route to go with how we're going to manage the ID going forward!

@vancluever
Copy link
Owner

This is now closed; version 2.0.0 will use a UUID for the certificate field going forward.

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

No branches or pull requests

3 participants