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

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.

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 987e8ab

[SarahFrench]

Note: I don't expect a nil rootModEarly to be passed in, unless there's a future refactor that alters the calling code. Mainly I changed the code here in this way to allow the new method arguments to be used but still leave the appropriate position in the method body marked with that TODO comment.