Make config errors more important during init operations

[liamcervante]

This PR updates the backend initialisation logic so that if the value returned by parsing the configuration isn't wholly known it returns an error diagnostic instead of crashing. This happens because we try to delay returning diagnostics until we can validate the required_versions conditions.

This PR also tweaks the ordering of which diagnostics are returned first. Previously if the backend initialisation returned diagnostics those diagnostics would be reported instead of the config parsing diagnostics. I can see the logic in that, but there is no way to differentiate between there is an error in the backend initialisation and there was an error in the backend initialisation caused by an error in the configuration. Therefore, I think it's better to simply return the configuration errors as higher priority since in the end, both will have to be fixed anyway.

@radditude - you put together the logic I'm moving around here so would be glad to hear your thoughts on my reasoning!

Another solution could be to simply return all the early config diagnostics and the backend diagnostics together instead of either/or?

Fixes #33622

Target Release

1.5.5

Draft CHANGELOG entry

BUG FIXES

• init: Fix crash when using invalid configuration in backend blocks.

[radditude]

Hmm, earlyConfDiags doesn't include errors from backend initialization, so if I have a config that looks like this:

terraform {
  backend "local" {
    bad_attribute = "is bad"
  }
}

bad_block {

}

I won't see anything about the backend problems until after I fix any other config errors.

That doesn't seem like a terrible outcome, but I wonder if it might be just as well to return all these diags together and either get rid of or adjust errInitConfigError so that it makes sense for both config errors and backend errors - the exact wording of that error message used to be important because terraform-exec relied on it, but that matching was since removed.

[liamcervante]

Yep, sounds good! I've made it print both sets if either one fails and tweaked the wording a bit!

[radditude]

Ah, actually, an important bit of context came back to me and I fear I gave you a bad suggestion 😅 backDiags includes config validation errors as well, so appending/returning both together means we'll show errors twice.

We could consider:

1. making loadBackendConfig not also validate the entire root module so backDiags actually contains what it says on the tin
2. going back to what you had originally in order to fix the crash and straighten out this initialization logic later

2 seems reasonable to me, unless you have other suggestions - sorry for the false flag!

[liamcervante]

I'm more than happy to turn it back, but I don't think they will actually contain duplicate diagnostics. The HCL library itself caches the files it loads: https://github.com/hashicorp/hcl/blob/main/hclparse/parser.go#L56

So, the first time round it loads the partial file with the diagnostics, but on following reads it doesn't reproduce the diagnostics. It just returns the partial file directly with no diagnostics. The tests also don't show duplicate errors.

Either way, I'm more than happy to go back to the previous approach but I think it should be fine.

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.