Skip to content
This repository has been archived by the owner on Apr 27, 2021. It is now read-only.

Propagate reattach info #38

Merged
merged 7 commits into from
Sep 9, 2020
Merged

Propagate reattach info #38

merged 7 commits into from
Sep 9, 2020

Conversation

kmoe
Copy link
Member

@kmoe kmoe commented Sep 7, 2020

Requires hashicorp/terraform-exec#78

This PR fixes issues introduced in #35, in which Terraform commands were reimplemented using terraform-exec. The tfexec paradigm does not permit transparent propagation of environment variables through to the Terraform CLI process, which meant that the crucial TF_PROVIDERS_REATTACH env var was not present in the binary testing environment. This behaviour, and the ability for the caller to set other env vars at arbitrary times, is restored by allowing WorkingDir to maintain a pseudo-environment.

Add wd.SetReattachInfo(reattachInfo string) and wd.UnsetReattachInfo()

The caller provides the reattach info (the contents of TF_PROVIDERS_REATTACH), and this is stored on WorkingDir and added to each tfexec command as necessary.

Restore previous behaviour of wd.Setenv(envVar, val string) (reverted, see comment below)

Since terraform-plugin-test is no longer in control of the environment variables for each Terraform command, we instead ensure the correct environment is set before each tfexec.Terraform command.

Note: wd.Setenv and wd.Unsetenv are not used in the SDK, and should be removed if there is no clear use case. This is a breaking change and so will be dealt with separately as https://github.com/hashicorp/terraform-plugin-test/issues/39.

@kmoe kmoe requested a review from paddycarver September 7, 2020 21:35
@kmoe kmoe marked this pull request as ready for review September 7, 2020 21:35
Copy link
Contributor

@paddycarver paddycarver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Called out some errors that need checked (sorry for the repetition).

We could also automate this with https://github.com/kisielk/errcheck or golangci-lint.

working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
working_dir.go Outdated Show resolved Hide resolved
@kmoe
Copy link
Member Author

kmoe commented Sep 9, 2020

After some discussion, I'm removing the propagation of the whole os.Environ() into the tfexec environment, because it causes unexpected results with reattach tests. Leaving the two (now a little counterintuitive, and in any case unused) wd.Setenv() and wd.Unsetenv(), which will be removed when tftest is very shortly moved into the SDK itself.

@kmoe kmoe merged commit 24abcfd into master Sep 9, 2020
@kmoe kmoe deleted the reattach-env branch September 9, 2020 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants