generate clean terraform plan with only changes

[ghost]

TL;DR

This PR addresses various issues occurring when:

• terraform plan is generated,
• posted to Git
• and comapred before applying

Causes of 'Plan has changed' issue in various scenarios:

1. Plan has Releasing state lock. This may take a few moments... in either git comment or in the plan generated before apply for comparing with git comment
2. Warning in the plan: There can be multiple terraform resources of same type having same warning, terraform reports warning for the first resource it tried to refresh and this first resource could be any any one of the same resources (not same one due to parallel processing) hence causes plan has changed issue.

Changes

• Generating the plan.txt file from the terraform's plan.out file instead of the console output of terraform plan command. This ensures that only the plan (add, change and destroy) is included in the git comment.
• Added a boolean flag trim_plan to disable or enable this trimmed version of plan.

Tests

I have manually verified the plan.txt files in above scenarios. Also tested the dev version of the orb as part of this PR Also tested when there is no change, it generates simple message

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

[sweatherly-ovo]

Hi @ovotech/production-eng could we please get a review of this PR as this issue is causing major problems with our CI pipelines. Many thanks.

[enicholson125]

Removing warnings concerns me as they're normally there for a good reason, e.g. to warn of upcoming deprecations. It feels to me like it would be better to fix the causes of the warnings than to stop them from being output.

[ghost]

Removing warnings concerns me as they're normally there for a good reason, e.g. to warn of upcoming deprecations. It feels to me like it would be better to fix the causes of the warnings than to stop them from being output.

Hi @enicholson125 , As I mentioned in the PR, there are 2 reasons why we see the 'Plan has changed' issue. And via this PR I am trying to address both. I agree with you that not all teams may want to exclude the warnings from the PR comment, in our case the warnings are coming from the upstream TF provider which are incorrect, and it's an open issue at provider's end. Hence we can't get rid of the warnings.

The reason I had to make this PR is because this issue has become a major blocker for us to deploy our infrastructure and tools for a lot of teams who are using it actively in production.

Could you please provide any other alternative on how to fix this issue for both the scenarios and may still manage to keep the warnings in the git comment?

[enicholson125]

https://github.com/ovotech/terrapin/commit/acb27dc98e0a75c45c64b2ac05f81c01d6f51363 - this was a similar change made in another Terraform plan parsing CI tool to do this. I would expect a similar tail based approach to work here.

[ghost]

ovotech/terrapin@acb27dc - this was a similar change made in another Terraform plan parsing CI tool to do this. I would expect a similar tail based approach to work here.

Thank you for sharing that. please correct me if I misunderstood anything below. Based on as far as I understood the tail based approach It can address the first scenario of omitting the Releasing state lock... line but won't address the 2nd scenario due to the following:

• If we apply grep -v filter on the warnings, which contain the random Terraform resource coordinates causing plan has changed, then it'll simply exclude that line from the plan and the git comment, and this will happen for all warnings if we have to use grep with a generic pattern. Unless we hardcode specific warnings in this approach (not ideal).

And this defeats the purpose of having the warnings in the plan and git comment as you mentioned earlier. Adding a sample warning for the reference ( we shouldn't exclude with and on lines from the warning but they contain the resource which can be different in each plan):

Warning: "default_secret_name" is no longer applicable for Kubernetes v1.24.0 and above

  with module.carbon-zero-datasets.module.autocapture_manage_compaction.module.bq-views-and-compaction[0].kubernetes_service_account.bq_compaction_k8s_service_account,
  on .terraform/modules/carbon-zero-datasets.autocapture_manage_compaction.bq-views-and-compaction/service_account.tf line 1, in resource "kubernetes_service_account" "bq_compaction_k8s_service_account":
   1: resource "kubernetes_service_account" "bq_compaction_k8s_service_account" {

Starting from version 1.24.0 Kubernetes does not automatically generate a token for service accounts, in this case, "default_secret_name" will be empty

(and 11 more similar warnings elsewhere)

[enicholson125]

I'm not aware of a way to selectively remove warnings without hardcoding some element of the warning you wish to remove - and again, I don't believe that removing warnings at all is desirable in an orb that's as widely used as this one is. If you really need to remove warnings for your use-case, I'd suggest forking the orb or adding a flag so that the default behaviour remains unchanged.

[ghost]

Thanks for the suggestion @enicholson125 . I have added a boolean parameter for plan and apply trim_plan making this optional and set to false by default