Don't force a new resource when updating early renewal time. #34
Conversation
ghost
added
the
|
@mildwonkey Hope you can take a look at this PR, otherwise we'll have to directly muck with our state since we messed up the value :-) |
|
@mildwonkey Ping - would be cool if you get a chance to look at this. Thanks! |
|
Hi @anuraaga! Thanks for working on this and sorry for the slow response. The Terraform team is currently 100% focused on finishing up the v0.12.0 release so we probably won't be able to look at this for a few more weeks at least. Unfortunately I think directly editing the state here is probably the most expedient path, so that our work on the v0.12.0 release isn't a blocker for you to be able to move forward with |
|
Actually we still have about half a year (instead of the intended 8 and a half years) before the certs get renewed so we may be able to wait until after 0.12.0. Thanks for the note :) |
|
@apparentlymart Congratulations on the release of 0.12 :) Just wanted to bring this back in case it's possible to take a look now. |
There was a problem hiding this comment.
Hi @anuraaga! Sorry for the delay and thanks for your patience here.
I left a comment inline about a possible alternative way to get the result you wanted here that should avoid some annoying safety check errors from Terraform Core in the 0.12 release. Terraform Core includes various additional checks to ensure that providers behave as they planned because in prior versions such unexpected changes would tend to cause confusing downstream errors: dependent resources have their plans built from what they depend on, so it's very important that the apply step produce exactly the planned result to avoid invalidating the plans for other resources.
The different approach I proposed would integrate this logic into Terraform's plan step, thus avoiding the apply-time inconsistency and also (as a bonus) improving the rendered plan output.
However, I know you worked on this a while ago and may not have the time or motivation to rework it to a different approach. If you don't, then no worries! Totally understandable. The Terraform team should be able to spend time here in the not-too-distant future (once we've finished wrapping up some loose ends on the 0.12 release) and we'd be happy to take this over the line if you cannot.
Thanks again for working on this, and sorry for the delay in reviewing it. 😖
…g diff computation. Only force new resources when updating early_renewal_hours if it causes this time to go negative.
|
@apparentlymart Thanks for the advice - I've updated to use Also, while I have your attention, if I can point you at this completely unrelated PR for possibly a push... It's been approved for a while and is quite critical for using terraform with k8s - I ended up forking the provider since I couldn't upgrade to 0.12 without downtime since parser whitespace changes were triggering a destroy instead of update >< |
…e to prevent extraneous output during terraformApply before renewal.
|
Thanks for working on these updates @anuraaga! This looks great to me. I'm going to merge it now. (On the subject of the Kubernetes provider PR: I'm not on the team for that provider and so I can't influence that team's prioritization. It looks like one of the team members is already participating and has been repeatedly tagged on it, so I'm sure it's on the team's radar and they'll get to it.) |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Early renewal time is not a feature of a certificate and just used by terraform to determine whether to recreate a cert. Updating the time itself doesn't need to update the cert. If users used the wrong value for early renewal (for example, specifying a time from issuance instead of the correct time from expiration), they will be forced to rotate certs instead of just tweaking this value.
Also updates
go.mod/vendor- couldn't get it to build when it depended on the alpha SDK.