terraform-plugin-sdk icon indicating copy to clipboard operation
terraform-plugin-sdk copied to clipboard

ImportStateCheck func receives states other than the imported resource

Open SarahFrench opened this issue 2 years ago • 2 comments

SDK version

v2.18.0

Relevant provider source code

func TestAccPrivatecaCertificateTemplateIamPolicyGenerated_withCondition(t *testing.T) {
    t.Parallel()

    context := map[string]interface{}{
        "random_suffix":           randString(t, 10),
        "role":                    "roles/privateca.templateUser",
        "condition_title":         "expires_after_2019_12_31",
        "condition_expr":          `request.time < timestamp(\"2020-01-01T00:00:00Z\")`,
        "condition_desc":          "Expiring at midnight of 2019-12-31",
        "condition_title_no_desc": "expires_after_2019_12_31-no-description",
        "condition_expr_no_desc":  `request.time < timestamp(\"2020-01-01T00:00:00Z\")`,
    }

    // Test should have 2 bindings: one with a description and one without. Any < chars are converted to a unicode character by the API.
    expectedPolicyData := Nprintf(`{"bindings":[{"condition":{"description":"%{condition_desc}","expression":"%{condition_expr}","title":"%{condition_title}"},"members":["user:[email protected]"],"role":"%{role}"},{"condition":{"expression":"%{condition_expr}","title":"%{condition_title}-no-description"},"members":["user:[email protected]"],"role":"%{role}"}]}`, context)
    expectedPolicyData = strings.Replace(expectedPolicyData, "<", "\\u003c", -1)

    vcrTest(t, resource.TestCase{
        PreCheck:  func() { testAccPreCheck(t) },
        Providers: testAccProviders,
        Steps: []resource.TestStep{
            {
                Config: testAccPrivatecaCertificateTemplateIamPolicy_withConditionGenerated(context),
            },
            {
                ResourceName:  "google_privateca_certificate_template_iam_policy.foo",
                ImportStateId: fmt.Sprintf("projects/%s/locations/%s/certificateTemplates/%s", getTestProjectFromEnv(), getTestRegionFromEnv(), fmt.Sprintf("tf-test-my-template%s", context["random_suffix"])),
                ImportState:   true,
                // We cannot verify state with ImportStateVerify because the data.google_iam_policy.foo data source causes an issue,
                // which results in an error referencing the google_iam_policy data source's ID
                // e.g. Failed state verification, resource with ID 2561318194 not found
                // ImportStateVerify: true,
                ImportStateCheck: func(s []*terraform.InstanceState) error {
                    var policy *terraform.InstanceState
                    if len(s) != 1 {
                        return fmt.Errorf("expected 1 state: %#v", s)
                    }
                    policy = s[0]
                    if policy.Attributes["policy_data"] != expectedPolicyData {
                        return fmt.Errorf("expected google_privateca_certificate_template_iam_policy `policy_data` attribute to be %s, got: %s", expectedPolicyData, policy.Attributes["policy_data"])
                    }
                    return nil
                },
            },
        },
    })
}

func testAccPrivatecaCertificateTemplateIamPolicy_withConditionGenerated(context map[string]interface{}) string {
	return Nprintf(`
resource "google_privateca_certificate_template" "default" {
  name = "tf-test-my-template%{random_suffix}"
  location = "us-central1"

  identity_constraints {
    allow_subject_alt_names_passthrough = true
    allow_subject_passthrough           = true

    cel_expression {
      description = "Always true"
      expression  = "true"
      location    = "any.file.anywhere"
      title       = "Sample expression"
    }
  }
}

data "google_iam_policy" "foo" {
  binding {
    role = "%{role}"
    members = ["user:[email protected]"]
    condition {
      // Check that lack of description doesn't have non-empty plan after
      title       = "%{condition_title_no_desc}"
      expression  = "%{condition_expr_no_desc}"
    }
  }
  binding {
    role = "%{role}"
    members = ["user:[email protected]"]
    condition {
      title       = "%{condition_title}"
      description = "%{condition_desc}"
      expression  = "%{condition_expr}"
    }
  }
}

resource "google_privateca_certificate_template_iam_policy" "foo" {
  certificate_template = google_privateca_certificate_template.default.id
  policy_data = data.google_iam_policy.foo.policy_data
}
`, context)
}

// This is a Printf sibling (Nprintf; Named Printf), which handles strings like
// Nprintf("Hello %{target}!", map[string]interface{}{"target":"world"}) == "Hello world!".
// This is particularly useful for generated tests, where we don't want to use Printf,
// since that would require us to generate a very particular ordering of arguments.
func Nprintf(format string, params map[string]interface{}) string {
	for key, val := range params {
		format = strings.Replace(format, "%{"+key+"}", fmt.Sprintf("%v", val), -1)
	}
	return format
}

Terraform Configuration Files

See above

Debug Output

Expected Behavior

As 1 resource is being imported I'd expect the argument passed to the ImportStateCheck func to contain state for the google_privateca_certificate_template_iam_policy.foo resource only. Instead, there is state for both the google_privateca_certificate_template_iam_policy.foo resource and data.google_iam_policy.foo data source.

Actual Behavior

The test fails and in the output is:

    provider_test.go:307: expected 1 state: []*terraform.InstanceState{(*terraform.InstanceState)(0x14000fa4270), (*terraform.InstanceState)(0x14000fa4410)}

The file line being referred to is here in the google provider.

👉 Click to expand and see the 2 items in the state array above

# google_privateca_ca_pool_iam_policy resource state

&terraform.InstanceState{ID:"projects/[PROJECT_ID]/locations/us-central1/caPools/tf-test-my-poolom8j1hjruo", Attributes:map[string]string{"%":"6", "ca_pool":"projects/[PROJECT_ID]/locations/us-central1/caPools/tf-test-my-poolom8j1hjruo", "etag":"BwXouFsomjI=", "id":"projects/[PROJECT_ID]/locations/us-central1/caPools/tf-test-my-poolom8j1hjruo", "location":"us-central1", "policy_data":"{\"bindings\":[{\"condition\":{\"expression\":\"request.time \\u003c timestamp(\\\"2020-01-01T00:00:00Z\\\")\",\"title\":\"expires_after_2019_12_31\"},\"members\":[\"user:[email protected]\"],\"role\":\"roles/privateca.certificateManager\"},{\"condition\":{\"description\":\"Expiring at midnight of 2019-12-31\",\"expression\":\"request.time \\u003c timestamp(\\\"2020-01-01T00:00:00Z\\\")\",\"title\":\"expires_after_2019_12_31\"},\"members\":[\"user:[email protected]\"],\"role\":\"roles/privateca.certificateManager\"}]}", "project":"[PROJECT_ID]"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string(nil), Type:"google_privateca_ca_pool_iam_policy"}, Meta:map[string]interface {}{"schema_version":0}, ProviderMeta:cty.NilVal, RawConfig:cty.NilVal, RawState:cty.NilVal, RawPlan:cty.NilVal, Tainted:false, mu:sync.Mutex{state:0, sema:0x0}}

# google_iam_policy data source state

&terraform.InstanceState{ID:"2561318194", Attributes:map[string]string{"%":"4", "binding.#":"2", "binding.0.%":"3", "binding.0.condition.#":"1", "binding.0.condition.0.%":"3", "binding.0.condition.0.description":"", "binding.0.condition.0.expression":"request.time < timestamp(\"2020-01-01T00:00:00Z\")", "binding.0.condition.0.title":"expires_after_2019_12_31", "binding.0.members.#":"1", "binding.0.members.0":"user:[email protected]", "binding.0.role":"roles/privateca.certificateManager", "binding.1.%":"3", "binding.1.condition.#":"1", "binding.1.condition.0.%":"3", "binding.1.condition.0.description":"Expiring at midnight of 2019-12-31", "binding.1.condition.0.expression":"request.time < timestamp(\"2020-01-01T00:00:00Z\")", "binding.1.condition.0.title":"expires_after_2019_12_31", "binding.1.members.#":"1", "binding.1.members.0":"user:[email protected]", "binding.1.role":"roles/privateca.certificateManager", "id":"2561318194", "policy_data":"{\"bindings\":[{\"condition\":{\"description\":\"Expiring at midnight of 2019-12-31\",\"expression\":\"request.time \\u003c timestamp(\\\"2020-01-01T00:00:00Z\\\")\",\"title\":\"expires_after_2019_12_31\"},\"members\":[\"user:[email protected]\"],\"role\":\"roles/privateca.certificateManager\"},{\"condition\":{\"expression\":\"request.time \\u003c timestamp(\\\"2020-01-01T00:00:00Z\\\")\",\"title\":\"expires_after_2019_12_31\"},\"members\":[\"user:[email protected]\"],\"role\":\"roles/privateca.certificateManager\"}]}"}, Ephemeral:terraform.EphemeralState{ConnInfo:map[string]string(nil), Type:"google_iam_policy"}, Meta:map[string]interface {}{"schema_version":0}, ProviderMeta:cty.NilVal, RawConfig:cty.NilVal, RawState:cty.NilVal, RawPlan:cty.NilVal, Tainted:false, mu:sync.Mutex{state:0, sema:0x0}}

Steps to Reproduce

  • You can get a version of the google provider containing these code changes by checking out this commit in a fork of the provider: https://github.com/modular-magician/terraform-provider-google/commit/d38cd23bade3e8c3a27b94c98f95695e2695db1a
  • In the root of the repo, run make testacc TEST=./google TESTARGS="-run=^TestAccComputeSubnetworkIamPolicyGenerated_withCondition$"
    • You would need these ENVs set for acceptance tests to work:
      • GOOGLE_PROJECT=<project ID>
      • GOOGLE_REGION=us-central1
      • GOOGLE_ZONE=us-central1-a
      • GOOGLE_CREDENTIALS=<path to a .json key file for a service account> - this is the identity use by Terraform
      • GOOGLE_SERVICE_ACCOUNT=<email of the service account above>

References

  • ⭐ This issue is very similar to https://github.com/hashicorp/terraform-plugin-sdk/issues/531 which has been fixed in the version of the SDK used by the google provider currently

SarahFrench avatar Sep 15 '22 17:09 SarahFrench

To help it stand out from the description above: I found this very similar issue (https://github.com/hashicorp/terraform-plugin-sdk/issues/531) that was closed in 2020.

SarahFrench avatar Sep 15 '22 17:09 SarahFrench

The tests I've experienced failures with have passed in the the automated tests on PRs in the terraform-provider-google repo (link to tests on the relevant PR here), which is interesting

SarahFrench avatar Sep 16 '22 07:09 SarahFrench

hey 👋

we are facing the same problem in our Terraform provider, please see: https://github.com/aiven/terraform-provider-aiven/actions/runs/3386997636/jobs/5627140403#step:6:60

there are, indeed, multiple states passed to the ImportStateCheck function, and this behavior has only been introduced recently, so we believe this is a bug

Serpentiel avatar Nov 03 '22 15:11 Serpentiel

Hi @Serpentiel 👋 Can you confirm if this is only an issue when using Terraform 1.3 and later?

bflad avatar Nov 03 '22 17:11 bflad

I have submitted https://github.com/hashicorp/terraform-plugin-sdk/pull/1089 which will strip data source states (seemingly present after import with Terraform 1.3 and later) from ImportStateCheck. Please note as mentioned in the PR description and the updated Go documentation that ImportStateVerify is generally preferable over ImportStateCheck, as it can ensure that the entire resource state for all attributes match between created and imported resources.

bflad avatar Nov 03 '22 20:11 bflad

Hi @Serpentiel 👋 Can you confirm if this is only an issue when using Terraform 1.3 and later?

hey, thank you for your reply! 👋

I can confirm that this issue appeared in recent releases of the Terraform

Serpentiel avatar Nov 04 '22 11:11 Serpentiel

Thanks for addressing this! In my case I must have been using a newer version of TF locally versus the version used in terraform-provider-google's automated tests.

I'll revisit the affected tests and the possibility of using ImportStateVerify

SarahFrench avatar Nov 04 '22 15:11 SarahFrench

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[bot] avatar Dec 05 '22 02:12 github-actions[bot]