Support for manual approval of Terraform plan

[yardbirdsax]

What problem are you facing?

As a user of the Terraform provider, there are situations where I need to view the Terraform plan that results from making a change in a Workspace resource before the change is applied. At present, it doesn't appear that there is any way to do this since the apply command is always run with the -auto-approve flag set.

How could Official Terraform Provider help solve your problem?

I would propose something similar to what the Flux Terraform controller implements; basically, if a field is set to a certain value, a plan would be generated and presented much in the same way other Terraform related messages are surfaced (e.g., through Status fields or events), along with a unique value that, if the same field is set to, the controller will apply the change.

I believe this is somewhat related to the capability to function in Observe only mode, though somewhat different. I didn't see an issue for that either; if there isn't one I'm happy to open one as well.

[bobh66]

We will likely get the core of this feature "for free" with the new managementPolicy field as described in https://github.com/crossplane/crossplane/pull/3822

We would still need to make the plan output available to the user, but setting managementPolicy: [Observe] would allow the provider to only run terraform plan and not run the apply. Then the user can update the Workspace to have managementPolicy: [Create, Observe] to allow the apply to run. The user can also add Update and Delete as they desire.

[yardbirdsax]

I think this could potentially work, but it would be a bit labor intensive. For example, here's what I imagine the flow would look like for making a change to an existing resource. (Keep in mind, this is in a GitOps style environment, such that the definitions of the resources live in files in a repository.)

• Developer makes a change to the resource in a feature branch and opens a pull request.
• A teammate reviews the changes and approves them.
• Developer merges the pull request, causing the resource to be updated in the cluster. Crossplane reconciles the Kubernetes resource, and because the Management Policy doesn't include Update, just stores a plan for someone to review.
• Developer reviews plan, and, seeing no problems, opens another pull request that changes the Management Policy to Create | Observe | Update.
• Teammate reviews the change and approves it.
• Developer merges the pull request, causing the resource to be updated in the cluster. Crossplane reconciles the Kubernetes resource, and because the Management Policy includes Update, it applies the changes.
• Developer opens another pull request to set the Management Policy back to Create | Observe, so that future changes don't occur.
• Teammate reviews the change and approves it.
• Developer merges the pull request, causing the resource to be updated in the cluster. Crossplane reconciles the Kubernetes resource.

This is fairly high touch, in that it involves nine (9) different human interactions just for making a change to an existing resource. It also doesn't guarantee that the plan viewed will be the plan that is applied, since there's no comparison of the hash/signature of the plan before the ultimate apply runs.

I'm thinking about this from two angles that would help cut down the work here quite a bit.

1. Allow a Terraform plan to be generated when the resource change is still in a pull request, such that it can be reviewed prior to the change actually occurring.
2. Allow the setting of a specific hash/signature value (similar to how Flux's controller does it), and only apply the changes if the generated plan's signature matches that.

I'm not sure if item 1 is really a Crossplane/Terraform provider problem to solve. Crossplane, and the Terraform provider by relation, are Kubernetes controllers, and therefore depend on things being in, well, Kubernetes in order to reconcile them. I can definitely see a way to accomplish this, by, say, watching for pull requests and then creating a new Workspace resource pointing to the feature branch with ManagementPolicy = Observe, causing a plan to be generated. However, that (watching for PRs) is wildly outside the core responsibility of Crossplane and its ecosystem, at least by my understanding. So likely it's something that's more of a recipe that composes Crossplane with other things.

For item 2, we'd have to think about how to ensure that the plan viewed is the plan applied. Flux's controller uses the Git revision as a marker, as well as saving the actual plan to a Secret, which it then loads and uses at run time. I suppose something similar could be implemented here. Combine this with what I mentioned on the linked Crossplane issue, and I think that would be a fairly nice workflow.

[bobh66]

Allow the setting of a specific hash/signature value (similar to how Flux's controller does it), and only apply the changes if the generated plan's signature matches that.

I definitely see value in controlling which steps in the reconciliation loop the provider will execute, but what value does a signature add? I don't see how populating a specific string value in a dedicated field is any different than updating managementPolicy to a specific set of values. The process described above can be automated at a higher level via a pipeline or other workflow.

I don't think Crossplane/the providers should need to worry about whether you really want it to do what you've asked it to do.

how to ensure that the plan viewed is the plan applied

That's definitely an issue - since the provider is constantly reconciling, and Update is not set in managementPolicy, it is entirely possible that the plan you reviewed is now out of date compared to what actually exists and the subsequent plan/apply will be different. By leaving Update out of the managementPolicy the provider is unable to restore the previously-expected state before applying the "new" state changes.

[yardbirdsax]

That's definitely an issue - since the provider is constantly reconciling, and Update is not set in managementPolicy, it is entirely possible that the plan you reviewed is now out of date compared to what actually exists and the subsequent plan/apply will be different.

Yep, exactly the scenario I was thinking of. 😄 That's why I suggested a way to say "I approve this plan", rather than simply "I approve you reconciling to the desired state, regardless of the plan to get there". If I understand correctly, just updating the managementPolicy field would be more of the latter, since there's no way to relate that change to a specific plan that was generated?

I may be wildly off here, since I'm coming from a very Terraform heavy perspective. I.e., very centric on the idea of "make a proposed change, see what operations are going to happen in order to get there, approve that plan, execute (apply) plan." This admittedly seems a little antithetical to the model the Crossplane ecosystem adopts in general, in that it's a continuous reconciliation and drift correction model (kinda more like an autopilot system). So if I'm trying too hard to fit a square peg in a round hole, feel free to say so.

[bobh66]

If I understand correctly, just updating the managementPolicy field would be more of the latter, since there's no way to relate that change to a specific plan that was generated?

Yes - reconciliation will always apply the "latest" plan, which is generated by the terraform apply itself using the provided terraform resources and the current state. There isn't currently a way to "stop the world" and only apply a specific plan. Which I think is a good thing, because while you think the world is stopped in reality someone may be mucking around in your resources and may have changed things again while you were reviewing the plan from 5 minutes ago.

I think of it as trusting Crossplane to keep things the way I want them and not worrying too much about the exact path it takes to get there (which can also be different at different times). In the end what matters is that the state matches what I specified, not necessarily how it got there.

So if I'm trying too hard to fit a square peg in a round hole, feel free to say so.

I don't think it's square but it may be octagonal 😄

Crossplane's model is definitely both more and less rigid than traditional Terraform. Crossplane (by default) will keep things the way you said you wanted them to be, all of the time, with no questions asked. It's a feature not a bug, but it definitely goes against the manual-approval workflow that Terraform traditionally wants to require. If you're using Crossplane, you're expecting it to keep things the way they should be, and anyone making changes to those things outside of Crossplane is going to see their changes reverted. Which means no more fixing things by hand in the cloud provider - fix it upstream in git where it belongs.

Typically if you want to see what changes will result from an update you would run the update in a duplicate dev/test cluster so you know what will happen before you apply the changes in prod. Of course that's not always practical or possible, so I think there is definitely room for making things more controllable/visible.

I'm a little concerned about putting the terraform plan output in status.atProvider - depending on what you're doing the plan can be huge, and it can also expose sensitive variables in cleartext, which is obviously a security issue. We have had other requests for it (see #86 ) so I know it's desirable, I just don't know if it's worth the risks. Maybe it needs a feature flag or a flag in the spec to control whether it's stored or not. Then it could be used for debugging but disabled in production.

[bobh66]

@yardbirdsax I'm thinking this is a duplicate of #86 - is it okay to close this issue and continue the discussion there?

[yardbirdsax]

I think this definitely covers the “show me the plan” part.

depending on what you're doing the plan can be huge, and it can also expose sensitive variables in cleartext

I could be mistaken, but I believe so long as the (Terraform) 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. Regardless these are probably points for that other issue. 🙂

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? If so then yeah, we can close this out.

[yardbirdsax]

Ok, having read that in more detail it really does cover this. So, close it I shall.