hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.71k stars 9.55k forks source link

When using 'removed' block, terraform plan should return removed: n in the summary. #34612

Open deitChi opened 9 months ago

deitChi commented 9 months ago

Terraform Version

1.7.2

Use Cases

Current plan message outputs that TF has no actions to perform.

{"@level":"info","@message":"Plan: 0 to add, 0 to change, 0 to destroy.","@module":"terraform.ui","@timestamp":"2024-02-02T11:16:28.954581Z","changes":{"add":0,"change":0,"import":0,"remove":0,"operation":"plan"},"type":"change_summary"}

Even though once terraform apply is run - there is a change to the state.

{"@level":"info","@message":"module.prodcctn-tgw-attachment.aws_ec2_transit_gateway_vpc_attachment.tgw_attachment: Plan to remove","@module":"terraform.ui","@timestamp":"2024-02-02T11:16:28.954490Z","change":{"resource":{"addr":"module.xxyyzz.aws_ec2_transit_gateway_vpc_attachment.tgw_attachment","module":"module.xxyyzz","resource":"aws_ec2_transit_gateway_vpc_attachment.tgw_attachment","implied_provider":"aws","resource_type":"aws_ec2_transit_gateway_vpc_attachment","resource_name":"tgw_attachment","resource_key":null},"action":"remove","reason":"delete_because_no_resource_config"},"type":"planned_change"}

If using Plan: 0 to add, 0 to change, 0 to destroy. as validation of whether to run TF apply or no - this will always fail.

Attempted Solutions

Alternative solution (more a workaround really), is to grab each line of json, and scan for "type":"planned_change" and use that as a metric for 'Actions taken if we were to apply', but it seems ugly, especially as an import gets added to summary if something is imported.

The fact that import is also no in the summary consistently irkes me, but in fairness that is another issue.

Proposal

Change the summary output to be consistent.

Plan: 0 to add, 0 to change, 0 to destroy, 0 to remove, 0 to import

References

No response

apparentlymart commented 9 months ago

Hi @deitChi,

Thanks for reporting this.

To start, I do agree that it would be useful to include the summary counts for other kinds of action in the JSON output, and ideally also in the human-readable output, but the latter is harder because various parties have (despite our warnings to the contrary) built integrations that regex-match that line and so it's become a de-facto compatibility constraint that we cannot practically change.

However, I think your report here betrays another more fundamental need: there should be a single, unambiguous signal somewhere in the Terraform JSON output for whether a plan contains at least one action that would make it useful to apply, and then automation built around Terraform should rely on that signal exclusively, rather than trying to make its own guesses based on other details.

One possible design for that would be an extra boolean property inside the existing change_summary object:

{"changes":{"add":0,"change":0,"import":0,"remove":0,"operation":"plan","applyable":true},"type":"change_summary"}

It might also/instead make sense to include this flag in the terraform show -json PLANFILE output.

Over time there will be even more different kinds of actions that can appear in a plan, and we can arrange for the "plan should be applied" flag to be true in those cases, so that software which relies on it will just work without needing to be modified for each new action type.

We already started thinking about such a thing as part of the ongoing Terraform Stacks project, because the new stacks language also needs a way to know if a particular component's plan includes actionable changes or not, but Terraform Stacks does not integrate through the Terraform CLI layer, and so this issue could potentially represent the work to also expose the new flag as part of the traditional terraform plan command and its associated JSON formats.

That doesn't mean that we couldn't/shouldn't also expose the number of items that are being "forgotten" (deleted from the state without actually destroying them), but the reason to do that should not be to allow surrounding automation to decide whether a plan needs to be applied, because that approach would not be robust to future Terraform features.

deitChi commented 9 months ago

Thank you for your detailed reply and consideration.

It's worth noting here that actually the -detailed-exitcode cli parameter did function as expected, and luckily was only a minor adjustment for our integration.

In order to emulate this functionality previously, I used the terraform state rm -dry-run to cycle a .staterm file, which appended a staterm count to the summary output ( to help our engineers distinguish between destroyed and state removed resources by count)

This can still be technically done using some breakout json processing, but I'd rather hoped that the removed block would already handle this.

An action of intent ( to apply or not) would help here, and if the intent dict could be expanded to include staterm alongside removed count, just to account for teams wishing to add validation actions ( I build github comments with dropdowns for actions, for approval review), dynamically based on the summary json, which matches actions from planned resources.