This change changes how unexpected positional arguments in the `init` command are detected and used to raise errors.
Previously unexpected positional arguments were returned to the command implementation and validated in the `ModulePath` function. That function used to use a positional argument, DIR, which was removed and replaced with the -chdir flag. Now the ModulePath function raises errors if there are any unexpected arguments and obtains the working directory's path is a way that doesn't need info about arguments.
After this change, the arguments package is used to raise errors when additional positional arguments are present. Use of the `ModulePath` function has been removed from init; the command can use the Meta's internal data about the working directory ((Meta).WorkingDir) and doesn't need to use `os` from the stdlib to get the working directory.
In #38648 the provider download process was changed:
Previously the two steps were:
1. Download providers based on configuration
2. Download providers based on state
The process remains two steps but the new steps are:
1. Download 0-1 provider if one is needed for PSS (1 provider downloaded if PSS is in use)
2. Download all other providers (configuration + state providers)
Because the first download step is specific to the provider used for state storage we can specifically block that provider being upgraded during init. We block upgrade unless the -reconfigure flag is supplied (i.e. the user isn't wanting to migrate state). Users see a warning when they perform an upgrade action and PSS is in use.
Prior to this, users would get an error if the upgrade process impacted the state store provider, now it's never impacts and warnings remind users to upgrade that provider via another command.
Updates a bunch of tests in the command package to use t.Parallel, without needing to refactor them to enable it.
Also, replaces use of `io/ioutil` in `TestPluginSHA256LockFile_Read`
When Terraform CLI starts running the working directory is changed, using `os.Chdir`, if the -chdir global option is set. This means when downstream code uses os.Getwd it will get the directory intended by the user. There currently isn't a bug in the `state show` command, so this change is just a refactor. Also, if the local backend is in use then the `Filesystem` state manager implementation that's used relies on the CLI running `os.Chdir` previously if no explicit -state flag is supplied by the user.
When Terraform CLI starts running the working directory is changed, using `os.Chdir`, if the -chdir global option is set. This means when downstream code uses os.Getwd it will get the directory intended by the user. There currently isn't a bug in the `providers schema` command, so this change is just a refactor.
This walks back some of the decisions we made earlier around two-step provider downloading process. However, it should not impact the safety features and it should not have a tangible (functional) impact on the end-user aside from the log output outlined below.
The motivation is to accommodate some needs of the upcoming policy features.
Previously the two steps were:
1. Download providers based on configuration
2. Download providers based on state
The process remains two steps but the new steps are:
1. Download 0-1 provider if one is needed for PSS (PSS is in use)
2. Download all other providers (configuration + state)
End-user facing changes are mainly related to terminal output - what messages are included and their order:
- initializing_provider_plugin_message (re-introduction after removal in 1.14)
- initializing_state_store_provider_plugin_message (net new but PSS specific)
After #38678 a number of tests no longer use t.Setenv, which means they can now be run in parallel. This change makes those tests use t.Parallel and the time taken to run the test suite has gone down:
Before
PASS
ok github.com/hashicorp/terraform/internal/command/e2etest 172.190s
After
PASS
ok github.com/hashicorp/terraform/internal/command/e2etest 62.911s
In the e2etest package tests reuse a single build of Terraform accessible via this global scope var: `terraformBin` in internal/command/e2etest/main_test.go. A setup function (`func TestMain(m *testing.M)` in the same file) runs once before any tests run, makes a Terraform binary, and enables tests to reuse that binary like this:
```go
fixturePath := filepath.Join("testdata", "full-workflow-null")
tf := e2e.NewBinary(t, terraformBin, fixturePath)
```
This change updates that process to also build a binary with experiments enabled, which can be reused in tests that cover experimental features. By preventing experimental tests from all building their separate experimental builds of Terraform we can reduce the package's test suite duration by about half.
---------
Co-authored-by: Daniel Banck <dbanck@users.noreply.github.com>
When rendering a plan file in JSON format we filter out both deleted and 'forgotten' resources when creating the `planned_values` section of the JSON; there is no value to be planned in those scenarios. Prior to this commit forgotten resources (i.e. removed via a `removed` block with `lifecycle { destroy = false }`) were erroneously still included in the planned values.
When developer overrides warnings were first added in #26605 we warned users about the potential impacts of dev_overrides on resource state. If users performed apply using a dev override in a project that wasn't for testing purposes their state could become incompatible with published providers and require manual edits to state to reverse the effects. Not good.
Warnings were added to init, apply, and eventually to plan.
Later, in #27514, the warning in init was made more specific than warnings in plan/apply and just instructed users to not run init when overrides are used. This was based on the assumption that the provider would not be used during init in any way. This also was intended as a way for users to avoid confusing errors when their config includes an unpublished provider in required_providers, supply that provider via overrides, and then experience installation errors during init.
Recently, in #37884, dev_overrides were made more compatible with init, avoiding the unpublished provider installation errors from happening. Therefore the old guidance of 'don't run init when using dev_overrides' became outdated. At that time the dev_override warning was updated to stop instructing users to not perform init while a dev_override is present, and instead the warning tells users about the overrides' potential effects on provider download.
Given that latest change, this PR adds a warning to init highlighting the same effects of unmanaged providers on the provider download process. Unmanaged providers have the same impact as overridden providers.
In the `init` command we skip downloading any providers that are overridden or aren't managed by Terraform, but during `providers lock` the installer is not configured with knowledge about those affected providers so they aren't similarly skipped. Compare how the init installer is created (cf0aae24f4/internal/command/meta_providers.go (L63-L92)) to how the providers lock installer is created (cf0aae24f4/internal/command/providers_lock.go (L258-L265)). Therefore we don't need to warn about the effect of dev_overrides as they have no effect.
Prior to this commit we wanted to validate the provider upgrade process using data that was only populated during backend initialisation (as that was the only place using that data previously). Now we enable the provider supply mode data to be available before backends are initialised.
After making that available, we can update the check after getProvidersFromConfig that protects against the provider version being changed outside of a deliberate state migration process. If the state store provider is overridden then the upgrade process doesn't impact that provider anyway, so the check can be skipped. Also, this protects against errors if there was ever a built in provider supplying a state store implementation.
Terraform cannot prompt users to establish trust in a newly downloaded provider used for state storage if input is disabled. This commit's changes enable people using Terraform in automation (defined as `-input=false` being set) to establish trust for a provider in the same situation.
We expect users to follow this workflow:
1. Practitioner initialises a Terraform project with minimal config describing how they'd like to use PSS in other production configurations. This is done manually, so they get an interactive prompt for approval and create a lock file describing the downloaded provider.
2. Practitioner copies that .terraform.lock.hcl artefact to a location that can be used in their automation environment.
3. Projects being initialised with PSS for the first time (no prior state) in automation are initialised using the command terraform init -state-provider-lock=<path-to-reused-lock-file>.
The `-state-provider-lock-file` flag is expected to be a path to a .terraform.lock.hcl file. If the flag is missing it defaults to a path for the project's own lock file. The lock file is expected to describe the provider used for state storage (can be among other locks present in the file) and the version must match the requirements of the config used.
Use of the lock file supplied via the flag is at the same point that a user would otherwise interactively approve a provider. If the lock file is insufficient to establish trust it'll be similar to a user declining a prompt to trust a provider.
This PR makes a few related changes:
1. Construct provider cache Installers so they have knowledge about dev_override providers
Prior to this change, providercache.Installer variables were made in the command package with knowledge about reattached/unmanaged providers, but not with knowledge about dev_override providers. Now both sets of overridden providers are present in the installer and can affect installation logic.
2. Make the provider installation logic skip any dev_override providers from being installed from the Registry (or other configured sources)
This means that if a provider is first added to a config while someone uses a dev_override an init command will not add that provider to the lock file. If the overridden provider is already in the lock file then the lock will be unchanged. An edge case, that already exists for unmanaged providers, is that if a dev_override is in play while an init -upgrade command is used, only the providers that aren't overridden or unmanaged will be upgraded
This change is coupled to another change in the PR described below.
3. Fix the provider download process (in context of init) so that dev_overrides are not removed from the provider requirements.
This reverts a change in https://github.com/hashicorp/terraform/pull/37884. The original motivation of that PR was to address a situation where the provider supplied by dev_overrides isn't published to the Registry yet. The config may need to include an entry in required_providers for that provider, which means that init would always fail due to the provider being unavailable in the Registry for download. The prior commit (bfc08b5d96) changed installer logic to skip dev_override providers, which cancels out this commit's changes; dev_override providers will remain in the provider requirements passed to installation logic, but that logic will now ignore them. As a consequence of no longer removing these provider requirements we will retain any pre-existing locks for the provider through the init process.
4. The `providers locks` command will now warn users about any dev_overrides in effect, as these will stop provider locks from being downloaded.
We're blocking commands from using an invalid workspace name, as this mitigates some risks if the invalid name attempts to cause path traversal. A workspace is selected if its name is in the .terraform/environment file, and something other than Terraform could change the contents of that file. To fix this, we expect users to use Terraform to update that file via Terraform using the `workspace select` command. This will allow the user to select or create a valid workspace instead, while also removing the bad data from the file without expecting the user to know about that file (it's an implementation detail, not a public API!).
* test: Update reattachedProviderForTest to return the provider server used, so tests can choose to make assertions against it. Refactor relevant E2E tests to use new reattachedProviderForTest helper function.
* test: Add test coverage to provider installer logic defining how providers being unmanaged/reattached or dev_overrides impacts the provider installation process
These tests show that the two methods of overriding a provider behave differently. This is due to how reattached/unmanaged providers are skipped during the provider installation process whereas development overrides are removed from the provider requirements passed into the installation logic. This results in Terraform treating dev_override providers similarly to a provider that's been completely removed from a configuration. I believe this is a bug and will fix in a future commit, but these tests help to highlight the current problem.
The -reconfigure flag makes Terraform ignore the backend state file when deciding what scenario is happening, which leads to Terraform acting like it's initialising a working dir for the first time. However downstream code then needs to be prepared for both of these scenarios:
- The work dir has never been initialised before and the backend state file doesn't exist yet
- The work dir has been used before but Terraform is being forced to treat it as new.
This change helps with the second point - we need to remove any old descriptions of a Backend to avoid a situation where the backend state file describes both a backend and state_store.
Follow up to https://github.com/hashicorp/terraform/pull/38391
That PR added tests showing that upgrading the provider used for PSS is blocked in init. There are no other upgrade-related user stories related to PSS in init, as upgrading the state storage provider will only be possible through a new `state migrate` command in future. Therefore these TODOs etc should be removed.
* Fix resource leaks in provisioners and commands
Close file handles, HTTP response bodies, and pipe ends that were
previously leaked on certain code paths:
- workspace_new.go: Close state file opened via os.Open
- file/resource_provisioner.go: Close temp file handle before returning
the file path to caller
- login.go: Move defer resp.Body.Close() before ioutil.ReadAll so the
body is closed even when ReadAll fails
- local-exec/resource_provisioner.go: Close the read end of os.Pipe
after the copy goroutine completes
---------
Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
The `-upgrade` and `-lockfile=readonly` flags are mutually exclusive, and this is now enforced by the arguments package. This refactor has also fixed a historical bug where that validation error was output twice.
The getProvidersFromConfig code now depends on the flag validation happening in the calling code. However there is another check in downstream code, in `saveDependencyLockFile` when we overwrite the dependency lock file's contents. There, an error occurs if a diff in the locks is detected during `readonly` mode. This protects against incorrect use of `getProvidersFromConfig` with unvalidated inputs, if that happens in future.
Added details about the default behaviour for the lock-file CLI flags, and made it clearer that the upgrade flag only enables upgrading the provider used for state storage.
* refactor: Replace use of prepareInstallerEvents method. This will allow finer control of callbacks when implementing security related features
* feat: Users are prompted to approve a provider used for PSS on first use, and only if downloaded via HTTP.
Prompts include signer details and key ID data.
* test: Users see "Authentication: unauthenticated" in prompt if network mirror doesn't include hashes
They'll see authentication data in all other prompt scenarios. There's no auth when using an fs mirror, but when those are in use we trust the providers already and no prompts are raised.
* refactor: Simplify how we prepare installation event callbacks by defining reused callbacks
* refactor: Remove unused parameters from `getProvidersFromState`
* refactor: Move method `ValidWorkspaceName` and related const into arguments package, update references
This function is primarily used to validate arguments, but it's used once (in (m *Meta) Workspace) in a different context, so I've left the original function signature in the command package.
* feat: Add `workspace new`-related code to arguments package
* refactor: Update `workspace new` to use the arguments package when parsing arguments
Note that this changes the format of errors returned when arg parsing fails - this is now wrapped in a diagnostic so now starts with `\nError: ` and has an extra trailing newline.
* feat: Add `workspace select`-related code to arguments package
* refactor: Update `workspace select` to use the arguments package when parsing arguments
Note that this changes the format of errors returned when arg parsing fails - this is now wrapped in a diagnostic so now starts with `\nError: ` and has an extra trailing newline.
* test: Add pre-release to available providers. Existing tests don't use it because the configuration doesn't specify the prerelease as the version constraint.
This was the gap in my understanding - prerelease providers are handled differently in the installer logic. I believe it boils down to this: 29863fd2ba/versions/set_released.go (L5-L7)
* test: Add failing tests that show expected behaviour when a pre-release of a provider is used
* fix: Ensure that any relevant version constraints from the config are available to use when installing providers present in the state
* test: Add test defining pre-release download behaviour that was present before v1.15
* test: Update test to reflect that 2 step init is no longer experimental or specific to the PSS project
We can't detect why a provider function is missing is all cases, so
get rid of the extra logic trying to do that.
We also admit here that we need to allow a provider function within a
variable validation condition, because that is not the only way these
could fail during init, and it's otherwise valid during the next phase.
We can't fully detect why a module source or version might be unknown,
so instead use a simpler error that covers all possible reasons. If we
do find a way to get more specific in the future, we can extent it then,
but the current errors were trying to be specific when they couldn't.
