Current comparison approach is non-deterministic resulting in false positives plan drifts

[toast-gear]

Problem description

Relates to #177

Terraform's human readable output, the output we attach to the PR and use as a point of comparison, is not deterministic and as a result it fails with false positive drift errors when comparing the plan at execution time with the plan in the PR.

As pointed out in hashicorp/terraform#30934

If you need more detail than just whether the plan includes changes at all, you can save the plan to a file with terraform plan -out=tfplan and then use terraform show -json tfplan to obtain a machine-readable description of the plan. A wrapper program can then use arbitrary logic against that data structure to decide how to proceed.

In order to have a determanistic comparison between the approved plan in the PR and the plan generated at execution time we should comparing the JSON. Perhaps the json version of the plan needs to be included in the PR comment (perhaps hidden maybe?) in addition to the human readable version so we can compare plans in a determanistic way preventing false positive plan drifts?

Terraform version

any

Backend

any

Workflow YAML

No response

Workflow log

No response

[dflook]

I'm working on this at the moment, but I can't say when it will be finished.

Github comments have size limits, so the json plan just won't fit. There are already cases where the text plan doesn't fit.
The idea is for the comment to have a hidden deterministic hash representing the plan which can be used to decide if the apply can proceed. This would resolve a number of open issues with matching the plan text in the comment.

The plan text is still useful for showing the differences when the plan has changed though, it would be great if was determinstic. We may have to do it ourselves.

[toast-gear]

The plan text is still useful for showing the differences when the plan has changed though

totally agree

it would be great if was determinstic

totally agree

The idea is for the comment to have a hidden deterministic hash representing the plan which can be used to decide if the apply can proceed. This would resolve a number of open issues with matching the plan text in the comment.

The plan text is still useful for showing the differences when the plan has changed though, it would be great if was determinstic. We may have to do it ourselves.

I think that is an excellent idea for how to work around to the PR comment size limitation whilst providing the missing determinism aspect as well as keeping the human readable format critical for accurate peer reviews.

[timdhoffmann]

Hi, I was wondering if there is any ETA on this. We are currently running into the problem that our terraform plans are always exceeding PR comment size limitations. Therefore, our apply is always failing when attempting to compare against the PR plan :/.
I also saw that there are alternative suggestions, that don't use this action suit: #195.

Could you please share an insight into the current state of this topic?

Thanks in advance, and thank you for the great work on this suite of actions!

[dflook]

Hi @timdhoffmann. The core issue of using the plan text output to compare plans is not resolved at this time, but progress has been made and i'm not aware of it causing any issues at present.

Since v1.27.0 plans that exceed the PR comment size will be truncated in the PR comment. As long as the plan doesn't change it should apply normally. If the plan does change, the apply job will fail. It will know the plan changed, but may not be able to show what changed between the plans.

It sounds like this is not working for you, can you created another issue to describe what you are seeing?

[timdhoffmann]

Thanks for getting back to me quickly!

Since v1.27.0 plans that exceed the PR comment size will be truncated in the PR comment. As long as the plan doesn't change it should apply normally. If the plan does change, the apply job will fail. It will know the plan changed, but may not be able to show what changed between the plans.

This is good to know. First of all, I will again test and observe the behavior. Maybe we have observed a case where both conditions were true, the plan comment having been truncated on the comment, and the plan having changed. We might have judged prematurely, that the action only reported a difference in plans because the one on the PR had been truncated.

[timdhoffmann]

I tested again and the apply succeeded with comparing against a PR plan with a truncated comment. So it looks like everything is working as expected in that regard. Sorry for the false negative.