upbound / provider-terraform

A @crossplane provider for Terraform
Apache License 2.0
126 stars 54 forks source link

Can terraform plan shows diff in the provider for humanbeing review? #86

Open janeyu205 opened 1 year ago

janeyu205 commented 1 year ago

What problem are you facing?

How could Official Terraform Provider help solve your problem?

bobh66 commented 1 year ago

This is not possible today - all reconciliation is done automatically. The assumption is that you want what is documented in the spec and the provider will work to make that happen.

What do you think the user interface for this feature would look like? I could envision something like a spec.planOnly: true option that causes the provider to NOT run terraform apply, in which case the resource would remain in the Ready: False state until planOnly is set to false. The plan output would have to be made available in the status object or in an event, though events can be short lived and hard to find.

There is some related discussion at the crossplane level in https://github.com/crossplane/crossplane/issues/1805

nagyv commented 1 year ago

Taking the Flux Terraform controller for inspiration

If I remember well, the controller writes the plan to status field together with a key. You need to pass the key to the approvePlan property to have it applied.

wojtekszpunar commented 1 year ago

Feature like this would be really nice. Sometimes you want to have breaking changes in TF code but since you are deploying it through provider you do not really see output of what meant to be changed which can lead to unintended deletion of resources. When you work manually with terraform you have feedback loop by executing plan which can let you know about possible changes, and you can decide if those are good or not.

You can test module locally, but then you are not testing inputs to that module which in some cases can be destructive in nature.

bobh66 commented 1 year ago

Summarizing the discussion from #162 :

yardbirdsax commented 1 year ago

From my (now closed) issue just to keep things in one place:

including the exposure of sensitive values in cleartext

I could be mistaken, but I believe so long as the (Terraform native, not Crossplane) providers and variables have the fields marked as sensitive, that shouldn’t happen, at least not in the human readable output. The binary plan file certainly can contain sensitive values though, so if that’s what you’re referring to I completely agree the provider shouldn’t do that. And it’s true that the human plan output can get pretty verbose, though that’s kind of an indicator that you’re likely managing too many resources in one place. Maybe using the same zipped output style with the summary text being the “X to create, X to modify, X to destroy” text from the plan might be a good approach?

Regarding the “observe only mode”, is my understanding that Crossplane handles this above the level of the provider correct? E.g., the provider itself doesn’t need to implement this interface?