Test HCL configs isolated in files

[radeksimko]

Problem Statement

The test framework today expects test configuration in the form of a string. This is a convention nearly all official providers comply with.

This approach makes it difficult, or rather impossible to parse test configs outside the context of a test framework, which in turn prevents us from:

• collecting any data about test infrastructure, e.g. to estimate how much it costs to run all acceptance tests
• knowing how many and which tests cover specific resource or data source
• formatting configs via terraform fmt
• upgrading configs via terraform 0.12upgrade
• and otherwise take advantage of any other tooling that users have available to them when dealing with raw HCL

It is technically possible to move these inlined strings into files, but we have no conventions in that area and no API surface helping with this.

Extra Context

One of the motivating factors behind this design was (probably?) to allow developers to easily generate many different versions of the same config by leveraging common Go tooling such as fmt.Sprintf or text/template.

It is expected that this requirement isn't going away and we need to find a solution that allows for such templating, e.g. via new 0.12 templatefile function or through other means.

Proposal

Come up with a convention for directory structure on how to structure configs and allow test framework to load valid files from there without providers having to deal with any low-level IO functions.

[bflad]

My suggestion in this area would be to allow provider developers to choose their directory/file structure by passing in the relative path, similar to what is possible today with file() usage in test configurations. Maybe we can just recommend certain conventions and follow them in official providers. Passing in the path will allow additional use cases, such as examples/ testing. These provider directories conventionally contain end-to-end multiple resource configurations for practitioners, typically split up by services/functionality and may contain multiple files. e.g.

• https://github.com/terraform-providers/terraform-provider-aws/tree/master/examples
• https://github.com/terraform-providers/terraform-provider-azurerm/tree/master/examples

Proposals

These are not necessarily mutually exclusive proposals, but presented in a personal preferential order. They are shown with a very similar developer experience to the Config handling today.

Proposal 1: (helper/resource.TestStep).ConfigDirectory

Example developer experience:

// provider/resource_example_thing_test.go

func TestAccExampleThing_basic(t *testing.T) {
	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				ConfigDirectory: "testdata/example_thing/basic", // conventional pathing, for large providers may be split into service directories, etc.
				Check: /* ... */,
			},
		},
	})
}

// provider/testdata/example_thing/basic/main.tf

resource "example_thing" "example" {
  # ...
}

// examples/examples_test.go

func TestAccExamples(t *testing.T) {
        // loop through all child directories and t.Run() the below:
	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				ConfigDirectory: /* filled in via loop above */,
				ExpectNonEmptyPlan: true,
			},
		},
	})
}

// examples/example1/main.tf

# This would be handled!

// examples/example1/more.tf

# Same here!

// examples/example2/main.tf

# Yay tested!

Underlying details:

• New ConfigDirectory field added to TestStep
• While handling existing TestStep behaviors, if ConfigDirectory declared:
  • If Config also declared, return error
  • Verify path exists, otherwise return error
  • Copy directory to temporary directory and set temporary directory as working directory for existing test handling

Proposal 2: (helper/resource.TestStep).ConfigFile

Example developer experience:

// provider/resource_example_thing_test.go

func TestAccExampleThing_basic(t *testing.T) {
	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				ConfigFile: "testdata/example_thing/basic/main.tf", // conventional pathing, for large providers may be split into service directories, etc.
				Check: /* ... */,
			},
		},
	})
}

// provider/testdata/example_thing/basic/main.tf

resource "example_thing" "example" {
  # ...
}

// examples/examples_test.go

func TestAccExamples(t *testing.T) {
        // loop through all child directories with an expected file name (e.g. main.tf) and t.Run() the below:
	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				ConfigFile: /* filled in via loop above */,
				ExpectNonEmptyPlan: true,
			},
		},
	})
}

// examples/example1/main.tf

# ...

// examples/example1/more.tf

# This would not work

// examples/example2/main.tf

# ...

Underlying details:

• New ConfigFile field added to TestStep
• While handling existing TestStep behaviors, if ConfigFile declared:
  • If Config also declared, return error
  • Verify file path exists, otherwise return error
  • Create temporary directory
  • Copy file to temporary directory and set temporary directory as working directory for existing test handling

Additional Considerations

I believe these ideas are outside the base use case (any provider could fulfill these needs manually, by convention, or with some additional wrapper coding), but these could present opportunities for nice enhancements to the developer experience with the functionality above. For what its worth, I feel like "Injected Variables" is much more desirable that "Automatic Variables" below.

Automatic Variables

In the Terraform AWS Provider, we provide the majority of tests with a random name (created via acctest.RandomWithPrefix("tf-acc-test") or friends). e.g.

func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
	/* ... */
	rName := acctest.RandomWithPrefix("tf-acc-test")

	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				Config: testAccBatchJobDefinitionConfigName(rName),
				Check: /* ... */,
			},
		},
	})
}

func testAccBatchJobDefinitionConfigName(rName string) string {
	return fmt.Sprintf(`
resource "aws_batch_job_definition" "test" {
  container_properties = jsonencode({
    command = ["echo", "test"]
    image   = "busybox"
    memory  = 128
    vcpus   = 1
  })
  name = %[1]q
  type = "container"
}
`, rName)
}

This could be an interesting opportunity! To assist with a common use case like this, we could automatically inject certain variable values in the testing framework command calls (-var=random_name=tf-acc-test-12345678) or create a tfvars files if the configuration has the variable already declared so it is valid:

variable "random_name" {
  type = string
}

Which the other configuration could use like normal:

resource "aws_batch_job_definition" "test" {
  container_properties = jsonencode({
    command = ["echo", "test"]
    image   = "busybox"
    memory  = 128
    vcpus   = 1
  })
  name = var.random_name
  type = "container"
}

Injected Variables and Values

Similar to above but less "magic", the TestStep could allow configuration variable handling:

func TestAccAWSBatchJobDefinition_basic(t *testing.T) {
	/* ... */
	rName := acctest.RandomWithPrefix("tf-acc-test")

	resource.ParallelTest(t, resource.TestCase{
		/* ... */
		Steps: []resource.TestStep{
			{
				ConfigDirectory: "testdata/aws_batch_job_definition/basic",
				ConfigVariables: map[string]interface{}{
					"random_name": rName,
				},
				Check: /* ... */,
			},
		},
	})
}

Where the ConfigVariables are converted to -var parameters or a tfvars file (with automatic quoting/escaping of strings, etc.). The test configurations would still have the same restriction as the automatic variable handling mentioned above to declare the variables to keep the configuration valid:

variable "random_name" {
  type = string
}

Helper Configurations

Larger providers can potentially want to utilize "shared" configurations for common functionality to reduce maintenance burdens. For example, in the Terraform AWS Provider, we have a configuration function for Amazon Linux AMI lookup:

func testAccLatestAmazonLinuxHvmEbsAmiConfig() string {
	return `
data "aws_ami" "amzn-ami-minimal-hvm-ebs" {
  most_recent = true
  owners      = ["amazon"]

  filter {
    name   = "name"
    values = ["amzn-ami-minimal-hvm-*"]
  }

  filter {
    name   = "root-device-type"
    values = ["ebs"]
  }
}
`
}

That is appended to many test configurations and used as ami = data.aws_ami.amzn-ami-minimal-hvm-ebs.id. It would be neat if we could include that configuration (or inject its values) with this new configuration file/directory handling. Some potential options, although each has its own drawbacks:

1. Creating a Terraform module relative to other configurations and using module in test configuration (e.g. source = "../shared/myhelper")
   • Pros: Follows standard practitioner practices and configuration, test configuration file/directory is always valid
   • Cons: Unsure if relative parent pathing can work like this, unsure how well the test framework handles modules, additional module configuration in every test configuration
2. Creating a Terraform module, publishing in the Terraform Registry, using module in test configuration
   • Pros: Follows standard practitioner practices and configuration, test configuration file/directory is always valid
   • Cons: Additional testing dependency on networked service, complicated update process (module(s) may need to live separate from provider repository), unsure how well the test framework handles modules, additional module configuration in every test configuration
3. Adding the ability to pass values between TestStep as variables
   • Pros: Separation of duties
   • Cons: Additional TestStep code for every test needing the shared configuration, test configuration file/directory is not valid until runtime (e.g. due to missing data "aws_ami" ... in above example), sounds complex to implement and understand
4. Adding a new TestStep configuration for "helper configuration paths"
   • Pros: Relatively simple to include these additional configurations
   • Cons: Test configuration file/directory is not valid until runtime (e.g. due to missing data "aws_ami" ... in above example)
5. Using PreCheck or other new functionality to run shared configuration outside of configuration and inject variable values
   • Pros: Potentially only run once per test, may not need additional functionality added
   • Cons: High level of configuration indirection

Having rubber ducked these, I feel like option 1 above if verified or implemented to work, may actually be the best solution here. 😄

[paultyng]

Following the tighter integration with terraform-exec will probably make this much easier as it more easily exposes variable passing, etc.

[paddycarver]

This has been a pipedream of mine for a while and something I've wanted since the 0.12 upgrade. I think for me the big question is whether a test that uses variables will ever be unrepresentative of a config that doesn't use variables. Basically, almost every test I can think of uses a test-run-unique value of some sort, which means almost all tests will be using Terraform variables of some sort, and I worry that leaves us with a blind spot in our testing. What if something works when specified by a variable, but doesn't when hardcoded? Or vice-versa?

The only proposal I can come up with to fix this is an extension to HCL/JSON that allows us to have pre-processor placeholders that all tooling treats as a placeholder and works around, but Terraform itself errors on if it finds it in a config during a regular run. Something like {% foo %} or whatever. That would be more work, but would avoid a potential difference between our test environment and how providers are used in production (yes, there would still be the difference, but variables are used in a lot more codepaths than what I'm envisioning). I don't know if that's feasible or even better, but I'm interested in hearing people's thoughts about the perceived danger in dramatically increasing our usage of variables in testing like this.

[paultyng]

Are variables and literals materially different in core? I guess technically in a module a variable could be coming from a computed source, but I'd think in a root module there was not difference? I see what you are saying though, probably worth posing the question to the core team if this would cause missing coverage or not.

[paultyng]

The placeholder though is probably a non-starter as we want these files to be actual configs that can be fmt'd and run manually, etc. So i think we'd need to have some basic expression parsing or something instead.