Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation of imported resources fails in integrated test #1066

Closed
dataclouder opened this issue Sep 22, 2022 · 1 comment · Fixed by #1077
Closed

Validation of imported resources fails in integrated test #1066

dataclouder opened this issue Sep 22, 2022 · 1 comment · Fixed by #1077
Labels
bug Something isn't working

Comments

@dataclouder
Copy link

I am one of the maintainers of vmware/terraform-provider-vcd. We are evaluating terraform v1.3.0 with SDK v2.23.0, and we have seen that with the updated tool many tests in our suite fail with an error as described in this issue.
With terraform up to 1.2.9, the tests run fine.

SDK version

github.com/hashicorp/terraform-plugin-sdk/v2 v2.23.0

Relevant provider source code

The problem arises when testing import , like in the test snippet here

			{
				ResourceName:      "vcd_catalog_item." + TestCatalogItemDS,
				ImportState:       true,           // << ---
				ImportStateVerify: true,           // <---
				ImportStateIdFunc: importStateIdOrgCatalogObject(TestCatalogItemDS),
				// These fields can't be retrieved from catalog item data
				ImportStateVerifyIgnore: []string{"ova_path", "upload_piece_size", "show_upload_progress"},
			},

Full test code available from datasource_vcd_catalog_test.go

Terraform Configuration Files

The kind of test that fails is the one that has a few data sources followed by one resource, usually created as a data source clone, like in the snippet below.

data "vcd_catalog" "cat-datacloud" {
  org  = "datacloud"
  name = "cat-datacloud"
}

data "vcd_catalog_item" "photon-hw11" {
  org     = "datacloud"
  catalog = data.vcd_catalog.cat-datacloud.name
  name    = "photon-hw11"
}

resource "vcd_catalog_item" "TestCatalogItemDS" {
  org     = "datacloud"
  catalog = data.vcd_catalog.cat-datacloud.name

  name                 = "TestCatalogItemDS"
  description          = data.vcd_catalog_item.photon-hw11.id
  ova_path             = "/some/path/to/terraform-provider-vcd/test-resources/test_vapp_template.ova"
  upload_piece_size    = 5
  show_upload_progress = "true"

  metadata = {
    catalogItem_metadata = "catalogItem Metadata"
    catalogItem_metadata2 = "catalogItem Metadata2"
  }
}

Debug Output

Skipped: see Root cause and fix below.

Expected Behavior

The imported fields should be validated.

Actual Behavior

All tests that use this paradigm fail with this kind of error

=== RUN   TestAccVcdCatalogAndItemDatasource
    datasource_vcd_catalog_item_test.go:44: Failed state verification, resource with ID urn:vcloud:catalog:65637586-c703-48ae-a7e2-82605d18db57 not found
--- FAIL: TestAccVcdCatalogAndItemDatasource (154.20s)
FAIL

Steps to Reproduce

The error only happens during the import test with this kind of configuration:

References

N/A

Root cause and fix

I followed the code and found where the problem arises. When testing imports, the validation happens in testing_new_import_source starting at line 164.

What happens here is that, when comparing the new set with the old set, the code skips the data sources from the old set (lines 172-174) but doesn't do so for the new set.
The result is that it tries to compare the first item in the new set (which is a data source) with the only remaining non-data-source item in the old set; the comparison fails, and the validation concludes that the imported set does not contain the wanted resource.

Here is a patch that solves this particular problem:

diff --git a/helper/resource/testing_new_import_state.go b/helper/resource/testing_new_import_state.go
index fc4ebc9c..e92e77c3 100644
--- a/helper/resource/testing_new_import_state.go
+++ b/helper/resource/testing_new_import_state.go
@@ -161,9 +161,12 @@ func testStepNewImportState(ctx context.Context, t testing.T, helper *plugintest
                newResources := importState.RootModule().Resources
                oldResources := state.RootModule().Resources

-               for _, r := range newResources {
+               for rKey, r := range newResources {
                        // Find the existing resource
                        var oldR *terraform.ResourceState
+                       if strings.HasPrefix(rKey, "data.") {
+                               continue
+                       }
                        for r2Key, r2 := range oldResources {
                                // Ensure that we do not match against data sources as they
                                // cannot be imported and are not what we want to verify.

By skipping the data sources in the new set, we only compare resources with resources, and the validation works.

@dataclouder dataclouder added the bug Something isn't working label Sep 22, 2022
bendbennett added a commit that referenced this issue Oct 13, 2022
bendbennett added a commit that referenced this issue Oct 13, 2022
…tate (#1077)

* Adding test coverage for changes in #1074 (#1066)

* Skips data sources from both original state and imported state

* Adding changelog entry (#1066)

Co-authored-by: Graham Davison <[email protected]>
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
1 participant