PSS: Swap to gating the experimental feature using configuration, instead of CLI flags

[SarahFrench]

Description

Currently the PSS feature is gated by a combination of:

1. Requiring experiments to be enabled (by building from source or using an alpha release).
2. Requiring the flag -enable-pluggable-state-storage-experiment=true to be passed to init commands, to choose an experimental version of the command
3. Requiring the configuration to include the feature's new state_store block.

There is also error feedback telling users when either of 1 or 2 are missing.

This PR changes this, so that the experiment is controlled by configuration instead of a CLI flag.

This makes testing the feature easier, as adding the experiment to the configuration occurs once and can be forgotten. Using the CLI flag requires users to remember it is needed each time they run init, which adds friction and in the past it has been confusing to diagnose errors when the flag is missing.

Now users will need configuration like this (plus an experimental build of TF) to use PSS:

terraform {
  experiments = [pluggable_state_stores]
  # more config here
}

Things to note during review

Needing to parse the root module early to access experiments risks losing diagnostics

The flag -enable-pluggable-state-storage-experiment=true was used in the past because Terraform needed to know if the experimental feature should be used early in the init command before configuration was parsed. Flags are present immediately and allow early logic to decide what to do. With the changes in this PR we now need to parse configuration to learn which experiments are named in the configuration. This happens here, before the fork in the logic between old-init behaviour and new-/experimental-init behaviour:

terraform/internal/command/init.go

Lines 59 to 69 in 987e8ab

	path, err := ModulePath(initArgs.Args)
	if err != nil {
	diags = diags.Append(err)
	view.Diagnostics(diags)
	return 1
	}
	rootMod, rootModDiags := c.loadSingleModuleWithTests(path, initArgs.TestsDirectory)
	// We purposefully don't exit early if there are error diagnostics returned here; there are errors related to the Terraform version
	// that have precedence and are detected downstream.
	// We pass the configuration and diagnostic values from here into downstream code, replacing where the files are parsed there.
	// This prevents the diagnostics being lost, as re-parsing the same config results in lost diagnostics.

Note the code comment about why we need to retain the diagnostics returned here, instead of trusting that the same diagnostics will be encountered when files are parsed in downstream logic. We cannot return any errors here because that'll change the return order of different types of errors that are possible in init: "init will error out with an invalid version constraint, even if there are other invalid configuration constructs" (see TestInit_checkRequiredVersionFirst).

When I previously did this:

terraform/internal/command/init.go

Lines 59 to 68 in 812a33d

	path, err := ModulePath(initArgs.Args)
	if err != nil {
	diags = diags.Append(err)
	view.Diagnostics(diags)
	return 1
	}
	rootMod, _ := c.loadSingleModuleWithTests(path, initArgs.TestsDirectory)
	// We purposefully ignore any diagnostics returned here. They will be encountered downstream,
	// when the 'run' logic below is executed. If we return early due to error diagnostics here we
	// will break the order that errors are expected to be raised in.

it resulted in test failures due to the ignored diagnostics being lost completely.

The -from-module flag requires special handling

When using the -from-module flag the working directory only gets populated with config later in the init process, long after the point that we need to use experiments to decide which version of init logic to use. Due to this, we aren't supporting the PSS experiment in directories created by downloading and copying a module during init. To allow the copied module to be parsed we need parsing of the root module to happen later in the init process. That has to be the first time that the root module is parsed to be able to get diagnostics that are relevant to the copied module's config, so in e9b7549 I made it so earlier parsing happens when -from-module is unused and later parsing happens when -from-module is in use.

Target Release

1.15.x

Rollback Plan

• If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

• This change is user-facing and I added a changelog entry.
• This change is not user-facing.

[SarahFrench]

There's a problem with this approach- loading config isn't idempotent.

When you parse HCL files using (p *Parser) ParseHCL from hashicorp/hcl it stores the parsed file in an internal map. Any diagnostics raised from parsing the file the first time are only returned when the file is first parsed. If you re-parse the same file the old parsed data is returned with no associated diagnostics.

[SarahFrench]

This problem was apparent through TestInit_invalidSyntaxBackendAttribute failing - the diagnostics we expect in the test get lost between the 1st and 2nd time the config is parsed.

[SarahFrench]

Ah the hclparse package is clear:

// Any diagnostics for parsing a file are only returned once on the first
// call to parse that file. Callers are expected to collect up diagnostics
// and present them together, so returning diagnostics for the same file
// multiple times would create a confusing result.

[SarahFrench]

I've addressed this in 672b0e7

[SarahFrench]

There are some unrelated test failures in the E2E test TestInit_fromModule. I've tried exploring the problem and found that I can't even run the test locally, so I've addressed that in #38015

Edit: they were related, once I got through issues with the test module containing outdated providers 🤦🏻 . I've addressed this in 263844b and e9b7549. The second commit is due to how we can only parse the config once when -from-module is in use; if you parse the config at the earlier time then the working directory is empty before the module is copied, and if you re-parse after the module is downloaded then you don't get diagnostics returned due to caching (despite the file content changing).