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

cloudflare_origin_ca_certificate requested_validity causes new certificates to be created on every apply #1276

Closed
2 tasks done
mattmichal opened this issue Oct 25, 2021 · 2 comments · Fixed by #1289
Closed
2 tasks done
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@mattmichal
Copy link

Confirmation

  • My issue isn't already found on the issue tracker.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

Terraform 1.0.4
cloudflare 3.3.0

Affected resource(s)

cloudflare_origin_ca_certificate

Terraform configuration files

resource "tls_private_key" "app_origin" {
  algorithm = "RSA"
}

resource "tls_cert_request" "app_origin" {
  key_algorithm   = tls_private_key.app_origin.algorithm
  private_key_pem = tls_private_key.app_origin.private_key_pem

  subject {
    common_name    = "CloudFlare Origin Certificate"
    street_address = []
  }
}

resource "cloudflare_origin_ca_certificate" "app_origin" {
  csr                = tls_cert_request.app_origin.cert_request_pem
  hostnames          = [cloudflare_record.cname.hostname, "www.${cloudflare_record.cname.hostname}"]
  request_type       = "origin-rsa"
  requested_validity = 15 * 365
}

Debug output

Panic output

Expected output

No change to resource.

Actual output

   # module.app-xyz.cloudflare_origin_ca_certificate.app_origin will be updated in-place
  ~ resource "cloudflare_origin_ca_certificate" "app_origin" {
        id                 = "xxx"
      ~ requested_validity = 5472 -> 5475
        # (5 unchanged attributes hidden)
    }

3 days after the last apply and the resource is showing a diff. A new certificate is generated causing any dependent resources to be updated as well (Azure App Service Certificate for example).

Steps to reproduce

  1. Use cloudflare provider 3.3.0 to create a cloudflare_origin_ca_certificate resource with requested_validity.
  2. Run terraform apply.
  3. Wait at least one day.
  4. Run terraform plan and observe that the cloudflare_origin_ca_certificate resource will be updated.

Additional factoids

This was previously discussed in #1031 for version 2.x series. However, when I look at the current code I don't see the same logic as was implemented with PR #1078. The ignore_changes fix is still possible but it would be good to have a native fix. Is it possible to update the docs in the interim?

The API is still not returning requested_validity; is there any issue or documentation that can be referenced or voted on?

References

#1031
#1078

@mattmichal mattmichal added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Oct 25, 2021
@jacobbednarz
Copy link
Member

This was previously discussed in #1031 for version 2.x series. However, when I look at the current code I don't see the same logic as was implemented with PR #1078.

#1078 was superseded by #1214 due to an underlying bug in terraform-plugin-sdk which didn't solve the issue in all cases.

The API is still not returning requested_validity; is there any issue or documentation that can be referenced or voted on?

requested_validity is present on the create payload and the value is calculated based on the expires_on value that is returned for the state updates. I suspect you're seeing this issue here because the value in your config is higher than the value the state file has (computed) however it shouldn't be as the field is marked as optional and computed 🤔 I'll have to dig into this (again!) to work out what is happening.

jacobbednarz added a commit that referenced this issue Nov 1, 2021
…unc` for `requested_validity` changes

Re-introduces the `DiffSuppressFunc` to supplement handling the
`requested_validity` countdown.

Closes #1276
jacobbednarz added a commit that referenced this issue Nov 1, 2021
…unc` for `requested_validity` changes

Re-introduces the `DiffSuppressFunc` to supplement handling the
`requested_validity` countdown.

Closes #1276
@jacobbednarz
Copy link
Member

Alright, after a bit of head scratching, I got to the bottom of this. The short version is that we need both the DiffSuppressFunc and the Computed/Optional to cover both versions of the SDK and the provider otherwise one combination doesn't work as intended. #1289 has the fix so this should be all good for the next release. thanks for the report!

boekkooi-lengoo added a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Mar 10, 2022
Instead of trying and failing to calculate the requested validity of the certificate we can use the returned validity.
It looks like `requested_validity` was being used in 2 ways.
The first one was to request the certificate for a time period and the second to indicate the amount of day the certificate is still valid.

The calculation is no longer done and could be seen as a BC break but since the current setup is causing issue like cloudflare#1148, cloudflare#1276 and cloudflare#1031. I expect that this is acceptable.

Related API docs: https://api.cloudflare.com/#origin-ca-get-certificate
boekkooi-lengoo added a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Mar 10, 2022
Instead of trying and failing to calculate the requested validity of the certificate we can use the returned validity.
It looks like `requested_validity` was being used in 2 ways.
The first one was to request the certificate for a time period and the second to indicate the amount of day the certificate is still valid.

The calculation is no longer done and could be seen as a BC break but since the current setup is causing issue like cloudflare#1148, cloudflare#1276 and cloudflare#1031. I expect that this is acceptable.

Related API docs: https://api.cloudflare.com/#origin-ca-get-certificate
boekkooi-lengoo added a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Mar 10, 2022
Instead of trying and failing to calculate the requested validity of the certificate we can use the returned validity.
It looks like `requested_validity` was being used in 2 ways.
The first one was to request the certificate for a time period and the second to indicate the amount of day the certificate is still valid.

The calculation is no longer done and could be seen as a BC break but since the current setup is causing issue like cloudflare#1148, cloudflare#1276 and cloudflare#1031. I expect that this is acceptable.

Related API docs: https://api.cloudflare.com/#origin-ca-get-certificate
boekkooi-lengoo added a commit to boekkooi-lengoo/terraform-provider-cloudflare that referenced this issue Mar 10, 2022
Instead of trying and failing to calculate the requested validity of the certificate we can use the returned validity.
It looks like `requested_validity` was being used in 2 ways.
The first one was to request the certificate for a time period and the second to indicate the amount of day the certificate is still valid.

The calculation is no longer done and could be seen as a BC break but since the current setup is causing issue like cloudflare#1448, cloudflare#1276 and cloudflare#1031. I expect that this is acceptable.

Related API docs: https://api.cloudflare.com/#origin-ca-get-certificate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
None yet
2 participants