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

Update certificate ready_for_renewal during refresh? #268

Closed
1 task done
apparentlymart opened this issue Sep 1, 2022 · 2 comments · Fixed by #278
Closed
1 task done

Update certificate ready_for_renewal during refresh? #268

apparentlymart opened this issue Sep 1, 2022 · 2 comments · Fixed by #278

Comments

@apparentlymart
Copy link
Contributor

Terraform CLI and Provider Versions

Terraform v1.3.0-beta1
on linux_amd64

  • provider registry.terraform.io/hashicorp/tls v4.0.2

Use Cases or Problem Statement

Currently the provider updates the ready_for_renewal flag during the plan phase, with the refresh phase just being a no-op.

While I would agree it seems kinda marginal to think about what is "refresh" vs. "plan" for an object that only exists within Terraform state anyway, conceptually it feels to me like the need for renewal is a property of the object that the certificate resources represent -- something "changing outside of Terraform" (the passage of time) -- and so would be appropriate to model during refresh instead of (or in addition to) planning.

A concrete use-case is that authors can then use postcondition to describe that a certificate should not be ready for renewal:

resource "tls_self_signed_cert" "example" {
  private_key_pem = tls_private_key.example.private_key_pem

  subject {
    common_name  = "example.com"
    organization = "ACME Examples, Inc"
  }

  validity_period_hours = 2
  early_renewal_hours   = 1

  allowed_uses = [
    "key_encipherment",
    "digital_signature",
    "server_auth",
  ]
  
  lifecycle {
    postcondition {
      condition     = !self.ready_for_renewal
      error_message = "Certificate will expire soon."
    }
  }
}

A system which uses periodic refresh-only plans to detect drift can then potentially raise an alert if the certificate becomes pending for renewal, while leaving it under the operator's control to actually renew it. (Postcondition failures during a refresh-only plan will raise a warning and update the check status in the state, which is an intended integration point for a monitoring system of this kind.)

Proposal

I'd propose to run the same logic that's currently in modifyPlanIfCertificateReadyForRenewal during the refresh step, thereby making a refresh-only plan set the flag that the certificate needs to be renewed without also planning to renew it.

This is consistent with how a refresh-only plan would behave if these resources were representing an object in a remote system, and can therefore integrate better with Terraform features that are designed with real remote objects in mind (as opposed to these special "state-only" objects).

How much impact is this issue causing?

Low

Additional Information

It is in principle possible to get the same effect by essentially re-implementing the "ready for renewal" logic using Terraform language functions:

    postcondition {
      condition     = timecmp(timestamp(), timeadd(self.validity_end_time, "-1h")) > 0
      error_message = "Certificate will expire soon."
    }

But it seems a shame to have to reimplement logic the provider already implements, and this expression is far more complex than just checking against ready_for_renewal directly.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@apparentlymart
Copy link
Contributor Author

apparentlymart commented Sep 21, 2022

In the "Additional Information" above I included a hypothetical way to address this without modifying the provider, but looking at that again in retrospect I see that it actually can't work: timestamp() returns the timestamp during the apply step, and so it will always be unknown during a refresh-only plan. That means any postcondition which depends on the result of timestamp() can only be checked during the apply phase.

The provider itself can make this work (in the way I proposed here) because it's reducing the information down to just a single boolean flag, and so it's safe to assume that if the flag is already true during planning then it will still be true during apply of that plan, because a certificate cannot become valid again after its expiration date.

I recently contributed a similar mechanism to the Vault provider in hashicorp/terraform-provider-vault#1597, which might serve as an example of what I mean by this proposal. (The Vault provider previously didn't have the flag at all, so the change there was broader in scope than what I proposed here but includes the same behavior of updating the flag during refresh.)

bendbennett added a commit that referenced this issue Sep 23, 2022
… in locally_signed_cert and self_signed_cert resources (#268)
bendbennett added a commit that referenced this issue Sep 29, 2022
bendbennett added a commit that referenced this issue Oct 5, 2022
…e state for ready_for_renewal and maintains the requires replace behaviour for terraform apply (#268)
bendbennett added a commit that referenced this issue Oct 5, 2022
…ty_period_hours is zero or early_renewal_hours is greater than or equal to validity_period_hours at time of locally or self-signed cert creation (#268)
bendbennett added a commit that referenced this issue Oct 11, 2022
…gned certificates are ready for renewal or have a validity period of zero at the time they are created (#268)

Reference: #282
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant