ImportStateVerify fails for TypeSet fields in terraform-plugin-testing but not terraform-plugin-sdk

[trodge]

terraform-plugin-testing version

github.com/hashicorp/terraform-plugin-testing v1.7.0

Expected Behavior

Set fields should pass ImportStateVerify regardless of the order of elements as they do in terraform-plugin-sdk.

Actual Behavior

ImportStateVerify fails with a different ordering of the same elements in a set field.

Steps to Reproduce

Run any of the tests from this list in the google provider after migrating to terraform-plugin-sdk.

References

This issue was previously closed as being caused by an upstream library, but it was never explained what the upstream issue was or why the tests pass in terraform-plugin-sdk but not in terraform-plugin-testing

#269

[austinvalle]

Hey there @trodge 👋🏻, thanks for submitting the bug and sorry you're running into an issue here.

I don't have access to the CI logs posted to that linked PR, would you mind taking one/a couple of those error message logs and publishing them in this GitHub issue if possible? (I've been chatting with @SarahFrench internally, so you both may be already working on this)

As for #269, that bug report didn't come to a conclusion on the specific root cause, but was closed by the reporter as one of the likely causes of the behavior were related to the upstream terraform-plugin-sdk/v2 library. There are a couple documented bugs in terraform-plugin-sdk/v2 related to TypeSet that could be creating inconsistencies now displayed by terraform-plugin-testing. A lot of these bugs we can't fix for compatibility reasons, however if the problem is in terraform-plugin-testing, we might be able to.

That being said, it would still be useful to see the exact error messages and related SDKv2 schemas, so I can recreate the bug you're experiencing and then definitively describe what's happening and whether we can actually fix it.

[trodge]

Here is the error message from one of the tests:

2024-04-11T20:50:05.607Z [ERROR] sdk.helper_resource: Error running import: test_working_directory=/tmp/plugintest2444090125
  error=
  | ImportStateVerify attributes not equivalent. Difference is shown below. The - symbol indicates attributes missing after import.
  | 
  | \u00a0\u00a0map[string]string{
  | -\u00a0\t"access.0.iam_member":     "allUsers",
  | +\u00a0\t"access.0.iam_member":     "",
  | -\u00a0\t"access.0.role":           "READER",
  | +\u00a0\t"access.0.role":           "OWNER",
  | -\u00a0\t"access.0.user_by_email":  "",
  | +\u00a0\t"access.0.user_by_email":  "Joe@example.com",
  | -\u00a0\t"access.1.domain":         "hashicorp.com",
  | +\u00a0\t"access.1.domain":         "",
  | -\u00a0\t"access.1.iam_member":     "",
  | +\u00a0\t"access.1.iam_member":     "allUsers",
  | +\u00a0\t"access.2.domain":         "hashicorp.com",
  | +\u00a0\t"access.2.group_by_email": "",
  | +\u00a0\t"access.2.iam_member":     "",
  | -\u00a0\t"access.2.role":           "OWNER",
  | +\u00a0\t"access.2.role":           "READER",
  | +\u00a0\t"access.2.special_group":  "",
  | -\u00a0\t"access.2.user_by_email":  "Joe@example.com",
  | +\u00a0\t"access.2.user_by_email":  "",
  | \u00a0\u00a0}
   test_name=TestAccBigQueryDataset_access test_step_number=4 test_terraform_path=/bin/terraform

The test makes this POST request:

---[ REQUEST ]---------------------------------------
POST /bigquery/v2/projects/ci-test-project-188019/datasets?alt=json HTTP/1.1
Host: bigquery.googleapis.com
User-Agent: Terraform/1.2.5 (+https://www.terraform.io) Terraform-Plugin-SDK/2.33.0 terraform-provider-google-beta/acc
Content-Length: 202
Content-Type: application/json
Accept-Encoding: gzip

{
 "access": [
  {
   "role": "OWNER",
   "userByEmail": "Joe@example.com"
  }
 ],
 "datasetReference": {
  "datasetId": "tf_test_access_68oy1cbnmn"
 },
 "labels": {
  "default_table_expiration_ms": "3600000",
  "env": "foo"
 },
 "location": "US"
}

Then updates with this PUT request:

---[ REQUEST ]---------------------------------------
PUT /bigquery/v2/projects/ci-test-project-188019/datasets/tf_test_access_68oy1cbnmn?alt=json HTTP/1.1
Host: bigquery.googleapis.com
User-Agent: Terraform/1.2.5 (+https://www.terraform.io) Terraform-Plugin-SDK/2.33.0 terraform-provider-google-beta/acc
Content-Length: 304
Content-Type: application/json
Accept-Encoding: gzip

{
 "access": [
  {
   "role": "OWNER",
   "userByEmail": "Joe@example.com"
  },
  {
   "domain": "hashicorp.com",
   "role": "READER"
  },
  {
   "iamMember": "allUsers",
   "role": "READER"
  }
 ],
 "datasetReference": {
  "datasetId": "tf_test_access_68oy1cbnmn"
 },
 "friendlyName": "",
 "labels": {
  "default_table_expiration_ms": "3600000",
  "env": "foo"
 },
 "location": "US"
}

With this response:

{
  "kind": "bigquery#dataset",
  "etag": "KCk+wkYOvmKwPt4onEZIAQ==",
  "id": "ci-test-project-188019:tf_test_access_68oy1cbnmn",
  "selfLink": "https://bigquery.googleapis.com/bigquery/v2/projects/ci-test-project-188019/datasets/tf_test_access_68oy1cbnmn",
  "datasetReference": {
    "datasetId": "tf_test_access_68oy1cbnmn",
    "projectId": "ci-test-project-188019"
  },
  "friendlyName": "",
  "labels": {
    "default_table_expiration_ms": "3600000",
    "env": "foo"
  },
  "access": [
    {
      "role": "OWNER",
      "userByEmail": "Joe@example.com"
    },
    {
      "role": "READER",
      "iamMember": "allUsers"
    },
    {
      "role": "READER",
      "domain": "hashicorp.com"
    }
  ],
  "creationTime": "1712868593855",
  "lastModifiedTime": "1712868600723",
  "location": "US",
  "type": "DEFAULT"
}

[austinvalle]

Alright, I pulled down the auto-pr-10339 branch and ran through the TestAccBigQueryDataset_access test. I found a bug that exists in both terraform-plugin-sdk/v2 and terraform-plugin-testing, however it's being exposed via an unrelated change we made in #223.

Background

In both terraform-plugin-sdk/v2 and terraform-plugin-testing, the ImportStateVerify logic creates a flatmap of the existing state and the imported state, which are then compared with the Go reflect.DeepEqual. This ignores that in the case of a TypeSet, the order should not be considered a difference, which is a bug in our testing logic.

The reason that this bug is only being exposed in your testing with the new terraform-plugin-testing Go module is because we removed explicit refresh commands (#223) from being called after applying the Config. So in the case of TestAccBigQueryDataset_access, this refresh was re-ordering the access set so that the imported state (which the terraform import command always performs a refresh) and the existing state would have a matching order for the access set, which passes.

I haven't run the other tests that were failing in your CI, but I'd guess a lot of them are failing either because of the set ordering bug described above, or another related bug to the removal of "explicit refreshes" (#223).

Solutions

Rewriting the ImportStateVerify comparison logic is likely the most correct way for us to fix this bug, using the actual schema to determine when to compare TypeSet without worrying about the order.

We also could reintroduce the explicit refresh command just for the Import mode, to preserve the original behavior, although this could introduce other bugs for provider tests relying on that explicit refresh not being executed 🙁.

With either solution, before applying a fix, it would be useful to know how many resources/tests are being affected by this set order bug.

Workarounds

• If the explicit refresh removal is the cause of a lot of the CI testing errors, you could downgrade your terraform-plugin-testing Go module to v1.5.1, as we introduced the changes from helper/resource: Refresh during plan rather than separately and remove state shimming when possible #223 in v1.6.0.
• If you're still experiencing bugs with v1.5.1, you could try downgrading to the initial v1.0.0 release, which is an exact copy of the prior terraform-plugin-sdk/v2 testing framework.
• And the most explicit workaround is to just ignore the set attribute itself with ImportStateVerifyIgnore if you want to continue forward with v1.7.0

{
	Config: testAccBigQueryDatasetWithOneAccess(datasetID),
},
{
	ResourceName:            "google_bigquery_dataset.access_test",
	ImportState:             true,
	ImportStateVerify:       true,
	ImportStateVerifyIgnore: []string{"labels", "terraform_labels", "access"},
},