v0.12: TF_VAR_ environment not being used to override default values

[alexng-canuck]

Terraform Version

github.com/hashicorp/terraform v0.12.0-alpha4.0.20190429183344-30199600eb15

Terraform Configuration Files

variable "identity_provider_metadata_file" { default = "sampleFederationMetadata.xml" }

resource "oci_identity_identity_provider" "test_identity_provider" {
  #Required
  compartment_id = "${var.tenancy_ocid}"
  description    = "${var.identity_provider_description}"
  metadata       = "${file("${var.identity_provider_metadata_file}")}"
...
}

Along with this configuration, we set an environment variable called TF_VAR_identity_provider_metadata_file and set it to /usr/blahblahblah/sampleFederationMetadata.xml.

Debug Output

Expected Behavior

In v0.11, the TF_VAR_ prefixed environment variable would have overridden the default value of the configuration.

Actual Behavior

In v0.12, the default value is still being used, resulting in errors when passing an invalid file path to the file()

    testing.go:568: Step 0 error: errors during plan:
        
        Error: Error in function call
        
          on /var/folders/6r/8fk5dmbj4_z3sl0mc_y_fhjw0000gn/T/tf-test209101720/main.tf line 45:
          (source code not available)
            |----------------
            | var.identity_provider_metadata_file is "sampleFederationMetadata.xml"
        
        Call to function "file" failed: no file exists at
        sampleFederationMetadata.xml.

Steps to Reproduce

terraform apply

Additional Context

This was caught via a test case that had previously worked with v0.11 plugin SDKs. In v0.12, the same test fails.

[apparentlymart]

Hi @alexng-canuck!

My initial instinct was to flag this as a CLI bug, but on re-read I think it may actually be a testing-specific issue, but I'd like to learn a bit more about what you are doing so I can confirm: Are you setting this environment variable and then running the acceptance tests, expecting the acctests to pick it up?

This is not something we ever intentionally supported, and I guess it was working before only because that bit of logic happened to be inside the terraform package. We refactored it out into the frontend (in the command and backend packages) in 0.12 so that it could report errors better by being processed as part of the general CLI input processing, and so that mechanism now lives outside of the part of Terraform that the test harness runs.

We don't have any reasonable way to reinstate the previous unintended behavior of supporting these environment variables during testing. Moving this back to where it was in 0.11 would prevent the improved error messages that motivated this change, and I think the only other option would be to duplicate a bunch of that behavior into the test harness itself, which would risk it drifting in behavior from the main mechanism over time.

Rather than relying on TF_VAR_ here -- a mechanism that was intended only as a CLI thing, not a testing helper -- I'd suggest making the test code directly call os.Getenv and handle this itself. That way the dependency on an environment variable is represented much more explicitly in the test, so the reader can see more clearly that the behavior of this test depends on something other than the test's own source code when debugging it.

[alexng-canuck]

Thanks @apparentlymart

Are you setting this environment variable and then running the acceptance tests, expecting the acctests to pick it up?

That is precisely what we are doing.

While we can introduce a workaround in test code to use os.Getenv, I'm still a proponent of acceptance tests mimicking CLI behavior as much as possible. Short of rewriting new tests that invoke Terraform CLI directly, acceptance tests are all we have to reproduce scenarios that we would expect our customers to be following.

[apparentlymart]

The handling of environment variables is a Terraform Core feature and thus Terraform Core's responsibility to test, so provider developers shouldn't feel compelled to repeat those same tests.

With that said, one of the ideas we are exploring for the forthcoming SDK revamp is to run acceptance tests through a real build of the Terraform CLI rather than by using Terraform as a library. The main motivation for that is to allow the same provider code to be tested against multiple different Terraform versions without modification, but depending on how it is implemented it may also permit configuring the environment for a specific test to test how Terraform behaves.

I think the goal would still be for the test to explicitly choose the TF_VAR_... environment variable values rather than have them be inherited from the calling shell though: that way, the test is specifically testing how Terraform behaves with the environment set, rather than the outcome depending on the configuration of the surrounding test environment.

[tamccall]

I am possibly also seeing this error after updating to terraform 0.12.

Here is my failed build and the diff.

https://travis-ci.org/nukosuke/terraform-provider-zendesk/jobs/537885423

Diff is here. nukosuke/terraform-provider-zendesk@c417e18...efe8126.

[heldersepu]

I imagine this regression is breaking a bunch of integration tests all over the place...
@apparentlymart any chance this makes a comeback in the near future?

[apparentlymart]

As noted before, TF_VAR_ environment variables are not part of the acceptance test framework and provider acceptance tests cannot rely on them. There is no chance of that changing with the current acceptance test framework, for the reasons I stated before.

I'm not sure what you mean by "integration tests all over the place", though. The codepath we're discussing here is only for provider acceptance tests, so it should not be affecting anything except those.

[heldersepu]

package abc

import (
  "testing"
  "github.com/hashicorp/terraform/helper/resource"
)

func TestAbc(t *testing.T) {
  resource.Test(t, resource.TestCase{
    Steps: []resource.TestStep{
      { 
        Config: `variable "test" {} ...`,
      },
    }})
}

Something like that could be used outside provider acceptance test, right?
maybe it should not be used outside...

TF_VAR_ environment variables are not part of the acceptance test framework

But before all that was working fine, the fact that on latest is not working makes it a regression.
No biggie, I already made the correction on my side to read using os.Getenv and inject them

[apparentlymart]

Indeed, the Go packages inside the Terraform repository are for provider use only and are not intended as a general framework for integration tests. Third-party solutions like terratest are a better answer for testing Terraform modules, because they are built around the full Terraform lifecycle rather than supporting only a single config file given inline.

[tamccall]

So what exactly is our recommendation here if we have an acceptance test that is relying on environment variables at this point?

[kmoe]

This is fixed in #262, the new binary acceptance test driver, which can be enabled from v1.7.0 onwards. Environment variables in the acceptance test environment are now passed transparently to the terraform process used to run the tests.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.