tfexec: Initial support for Terraform 0.14 -chdir global option

[bflad]

Closes #99

[paultyng]

Does chdir replace the work dir args that are in use in some commands already? We may need to look at this a bit more holistically, so we can utilize those options or something.

[bflad]

@paultyng I believe so, please see hashicorp/terraform#27664

[bflad]

Rebased 👍 We will probably want to go through and mark all the removed "args" with tf.compatible(ctx, nil, tf0_15_0) as well -- happy to do that here or leave as a followup item.

[radeksimko]

Sorry for the long delay in reviewing this @bflad - I finally found a spare moment to look at this PR of yours!

Do you have a use case (in SDK or elsewhere) for changing workdir during the lifetime of *Terraform instance?

I always worked with the assumption that the *Terraform is short-lived in a similar way to context.Context, which coincides with the fact that many commands didn't have an explicit argument (until recently) to run in a directory different to the one provided via NewTerraform(workdir, ...).

I think it is fair to support both workflows, but I think it will require a little bit more effort involved to make this safe.

We currently persist version output in the struct

terraform-exec/tfexec/terraform.go

Lines 56 to 58 in 43ec6e1

	versionLock sync.Mutex
	execVersion *version.Version
	provVersions map[string]*version.Version

to be able to run compatibility checks without having to re-execute version repeatedly before each command.

The core version is (naturally) scoped to the Terraform binary (execPath), but the version command also outputs versions of providers which are scoped to the workdir. I worry slightly that users won't realize this side effect while reading the cached output

terraform-exec/tfexec/version.go

Lines 27 to 36 in 43ec6e1

	func (tf *Terraform) Version(ctx context.Context, skipCache bool) (tfVersion *version.Version, providerVersions map[string]*version.Version, err error) {
	tf.versionLock.Lock()
	defer tf.versionLock.Unlock()
	if tf.execVersion == nil || skipCache {
	tf.execVersion, tf.provVersions, err = tf.version(ctx)
	if err != nil {
	return nil, nil, err
	}
	}

Obviously this problem already exists given that we support the dir args in various commands already, but this is much more radical change which brings this problem more into spotlight IMO.

One way we could address this is by scoping the cache by workdir - as a map, or more likely as sync.Map to make it safe for multiple commands to be executed at once. We should probably also consider some mechanism for purging the cache, like Cleanup(workdir string) - something that can be (optionally) called by downstream long-running process. This is to prevent any unnecessary memory consumption in case the consumer e.g. creates a high number of temporary directories in which it runs Terraform commands.

On a related note there is still a few commands which are missing the opts argument for passing chdir:

• ProvidersSchema
• Version
• Validate

I think the skipCache argument should probably stay there as 1st class argument (as opposed to being hidden under opts ...VersionOption) to communicate the side effect of the builtin cache.

We will probably want to go through and mark all the removed "args" with tf.compatible(ctx, nil, tf0_15_0) as well -- happy to do that here

Yes please - if you can do that as part of this PR that would be great! 👍🏻

[radeksimko]

I think in order to actually expose the chdir flag here we'd need to introduce opts ...ProvidersSchemaOption here as well, no?

[radeksimko]

I remember there were some format changes at some point - is it perhaps safer to test both 0.12 and >=0.14 which introduced the format change?

[radeksimko]

I think this is no longer relevant?

Suggested change

// TODO: [DIR] param option

[radeksimko]

Suggested change

// TODO: [DIR] param option

That doesn't seem relevant anymore

[hashicorp-cla]

All committers have signed the CLA.