Add task_env to provider blocks

[findkim]

Adds support for terraform_provider blocks to configure provider environment variables using the meta block task_env. Dynamic config source is supported within the task_env block.

terraform_provider "foo" {
  address = "foo.example.com"
  task_env {
     "FOO_TOKEN" = "{{ env \"CTS_FOO_TOKEN\" }}"
  }
}

This is useful to keep sensitive provider arguments out of the generated terraform.tfvars for each task. The environment variables are passed to Terraform workspaces that use that provider. The envvars are not accessible to other tasks or set to the global env scope.

The task_env is also useful to decouple the expected environment name for Terraform providers. A few use cases here:

• CTS configuring multiple instances of a provider for different tasks. Without the task_env block, the provider instances would consume from the same environment value which may not be desired in preference for different tokens per instance.
• An existing process is consuming from an env name that a provider in CTS also consumes from but requires different configuration.

[lornasong]

Looks good! Excited for this feature! I'm not sure what remaining work if any you have in mind for this feature. The logic is pretty straightforward, but I was thinking it would be nice (not necessarily this PR) to have some unit tests for terraform_provider.go. Slightly motivated by our driver package coverage being a little low (~35%).

[lornasong]

Suggestion (minor optional, not blocker) to name block to something else - like arguments mentioned in the comments. For me, at first read, block sounded broader/inclusive of env i.e. environment variables are declared in the block. For me the block splits into environment and provider config information. I might be misunderstanding 'block' terminology though.

[findkim]

Heh not "block"-er. To provide a bit more background on why I chose to keep block here. Block is an HCL terminology for an object

object {
  arg = "value"
}

And in context of Terraform configuration, it would be the provider block which includes an HCL label to the block. I felt that renaming to arguments would imply only the body of the block and not the HCL label/provider name, which isn't the case.

provider "foo" {
  arg = "value"
}

Block in both of these context refers to the syntax for a configuration file and I would believe is exclusive of the environment. However, since this adds CTS support for task_env as a meta-argument, which is not directly supported by the provider, the task_env is initially in the block when loading the user-CTS configs, which then gets removed before it gets written as TF config files.

Does block sound a bit more ok in context of HCL and Terraform syntax?

[lornasong]

Thanks for taking the time to share your thinking behind it! To your point about task_env initially being in the block, when I came across NewTerraformProviderBlock, I understood the relationship between the two, so it was more of an initial thing.

[findkim]

Thanks Lorna for the review! No remaining work. The task_env block is evaluated for dynamic configs and fetched from external sources along with the other configs (#143), then the changes in this PR splits the meta-arguments from the provider config that ultimately gets written to Terraform configuration files.

On the point about tests for terraform_provider.go, I don't think it's valuable to write tests for functions that are wrappers around creating maps and lists. The bulk of the uncovered code in the driver package would require integration tests (TF download and generating TF files, which are already tested individually in the templates/tftmpl package)

[lornasong]

Sorry for the confusion - the question about the remaining work was more that I didn't want to assume you didn't have other plans for tests. Yea, it sounds like this feature is complete! I look at test coverage when deciding to use a repo so had thought that if others were like me they might have reservations. I think this is personal preference so ok with you to go ahead and merge when ready

[findkim]

Appreciate the follow-up -- I'll go ahead and merge

Increasing the coverage is a good goal but I don't like to strive for numbers for the sake of numbers. I can see value in writing an integration test for generation of all TF files. However for the TF downloads, if we do have an integration test in the future, it's not valuable to be included as a part of the CI tests which would still keep the coverage on the lower side.