hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.71k stars 9.55k forks source link

Optional skipping of `import` blocks during Terraform test plan operations #34150

Open Taha-cmd opened 1 year ago

Taha-cmd commented 1 year ago

Terraform Version

Terraform v1.6.0 on windows_amd64

Use Cases

I have a terraform configuration that includes declarative import blocks. For this configuration, I have unit tests (tests with command = plan) to assert that plan time values are calculated correctly. I run these tests against a dummy environment since I don't care about creating actual resources. However, my tests always fail because the dummy environment does not contain the resources that should be imported:

Error: Cannot import non-existent remote object While attempting to import an existing object to "azurerm_subnet.gateway", the provider detected that no object exists with the given id. Only pre-existing objects can be imported; check that the id is correct and that it is associated with the provider's configured region or endpoint, or use "terraform apply" to create a new remote object for this resource.

Attempted Solutions

Include the imports in the list of expected failures in my tests

Proposal

# in a *.tftest.hcl file
expect_failures = [
   import.azurerm_subnet.gateway # ignore this specific resource
]

# even better
expect_failures = [
   import # or import.* =>  ignore all imports
]

References

https://github.com/hashicorp/terraform/issues/33633 suggests adding a fail_on_missing flag to import blocks. This feature would solve my problem, it serves however a different use case. I would have to modify my import blocks (the actual terraform configuration), instead of the tests.

liamcervante commented 1 year ago

Hi @Taha-cmd, thanks for filing this request.

I wonder if you could expand on your use case more. Are you aiming to test a module you've built, or some standalone configuration for a service? Why do you require import blocks to be present in your configuration? What exactly do you want to test about the import blocks?

I don't think simply allowing import blocks to be referenced from expect_failures will achieve what you want it to. The expect_failures block only affects the output of the test, it doesn't affect the behaviour of the Terraform operation. If a condition referenced in expect_failures fails during a Terraform operation, then the overall operation will still fail. It's just the test framework acknowledges the reason for the failure, and marks the test as passing despite that. Expecting a failure from an import block would mean the overall operation would still fail at the same time it does now, just that the testing framework would consider that a successful test.

It doesn't seem right to me that you'd want to use import blocks in tests at all. The import block is generally a one-time thing, that should be used to onboard already existing infrastructure into your Terraform state on a single use-basis, as if you were onboarding an existing service onto Terraform and you didn't want to recreate anything. Once a resource has been imported, you can remove the import block and Terraform will continue to manage the resource even without the import block being present.

Your use-case here, as I understand it, suggests that you have some configuration that requires someone to create infrastructure outside of Terraform, just so that it can then be imported and managed by Terraform later. In this situation, why can't you just create the resources in Terraform to begin with, and then nothing needs to be imported? Alternatively, If the configuration you want to test has already imported the resources in your main state, then you can just take the import blocks away, and the testing framework will create the resources that were imported for your main configuration when it executes anyway.

As a further alternative, if you really do require the import blocks in your configuration, you could create a setup step in your test file that creates the resources that are to be imported before your main configuration executes.

Taha-cmd commented 1 year ago

Hi @liamcervante, thanks for the detailed response. Let me elaborate on our use case a bit more.

Taha-cmd commented 1 year ago

As a further alternative, if you really do require the import blocks in your configuration, you could create a setup step in your test file that creates the resources that are to be imported before your main configuration executes.

This not feasible for us, because the resources in question are considered heavyweight and will require 45-60 mins to create. We can't afford running this setup just to assert that plan-time values are calculated correctly.

liamcervante commented 1 year ago

Hi @Taha-cmd, thanks for the additional context.

I think adding import blocks into the expect_failures block isn't the right thing for us to do at a language level. The behaviour of the expect_failures block doesn't currently modify the execution of Terraform in any way, and I think your use-case would require that. Skipping parts of the configuration is a different action to expecting parts of the configuration to fail, and treating that as a success. We'll have to think more about the right way to do this so I'll leave this open and get some eyes on it from our product team.

In the meantime, have you tried using the target command in the run blocks? This would allow you to only plan the resources you want to test and potentially skip any resources that require importing. It might be that your import targets are referenced by the resources you want to test, which does mean this might not work.

Another upcoming alternative, we also plan to add support for mocking resources and data sources in the testing framework soon (hopefully 1.7), this would allow you to define a fake resource for your import targets within the test.

AMEvers commented 10 months ago

@liamcervante

I think adding import blocks into the expect_failures block isn't the right thing for us to do at a language level. The behaviour of the expect_failures block doesn't currently modify the execution of Terraform in any way, and I think your use-case would require that. Skipping parts of the configuration is a different action to expecting parts of the configuration to fail, and treating that as a success. We'll have to think more about the right way to do this so I'll leave this open and get some eyes on it from our product team.

So, we also have an interest in allowing imports to ignore missing resources. Our company has implemented generic terraform modules for our cloud deployment that allows us to provide a yaml definition of our azure deployments at runtime. Terraform consumes this yaml and creates the infrastructure for us, injecting certain automated configuration. Because the declaration occurs in our yaml definition and not strictly the terraform, importing is not a one and done. The same terraform code could be used to create any number of deployments, some of which will have existing resources we would want to import into our state as part of migration. Then our DevOps CI pipelines can easily spit out json/yaml to provision or manage resources without having to strictly write terraform.

The desired behavior would be to "import if exists, create if it doesn't" which appears to be a commonly requested feature from what I've seen. Honestly, I think this could be abstracted to a more general error handling meta-argument. Something along the lines of...

data "azurerm_linux_virtual_machine" "frontend_host" {
   ...

   ignore_errors [
      "missing_resource", ...
   ]
}

import {
   to = azurerm_linux_virtual_machine.frontend_host
   id = ...

   ignore_errors [
      "missing_resource", ...
   ]
}

resource"azurerm_linux_virtual_machine" "frontend_host" {
   ...

   ignore_errors [
      "*", ...
   ]
}

Where a 'caught' error by ignore_error meta-argument would result in a null datasource/resource instead of failed terraform operations. Maybe couple with a warning that the operation failed so the possible problem is still visible. An implementation like this would create a LOT of functionality. A couple use cases that occur to me off the top of my head

I'm fully aware that it introduces some complexity into troubleshooting but I'd argue that is complexity/risk the developer should be given a choice to adopt in exchange for more flexibility.

jbardin commented 10 months ago

@AMEvers, the possibility of "import if exists, create if it doesn't" is separate from the terraform test functionality being discussed here. That is currently possible given the availability of cooperating data sources, and may become easier with future enhancements. See also #33465 and #26558

bladechild commented 2 months ago

For those who still experienced this issue, I do have a workaround to fix this issue when we have imports blocks.

According to imports documentation, it is recommended that we put the import blocks into a file called imports.tf.

If we can comment out the imports.tf file before running terraform test, this error will be gone.