path not found error for null attributes in plancheck.ExpectUnknownValue and plancheck.ExpectSensitiveValue

[austinvalle]

Ref: hashicorp/terraform-plugin-framework#840 (comment)

The error message for an attribute that is null and being asserted as unknown or sensitive can be confusing for developers as it's the same error you'd receive if the attribute didn't exist in the schema.

--- FAIL: Test_ExpectUnknownValue_NullValue (233.24s)
        path not found: specified key string_attribute not found in map

terraform-plugin-testing version

1.5.1

Relevant provider source code

This recreation was done with the internal testing plugin-go provider in this Go module, but the same can be achieved with another TF SDK.

Schema: map[string]*schema.Schema{
	"string_attribute": {
		Optional: true,
		Type:     schema.TypeString,
	},

	"list_attribute": {
		Type: schema.TypeList,
		Elem: &schema.Schema{
			Type: schema.TypeString,
		},
		Optional: true,
	},
	"set_attribute": {
		Type: schema.TypeSet,
		Elem: &schema.Schema{
			Type: schema.TypeString,
		},
		Optional: true,
	},
	"map_attribute": {
		Type: schema.TypeMap,
		Elem: &schema.Schema{
			Type: schema.TypeString,
		},
		Optional: true,
	},
	"root_map_attribute": {
		Type: schema.TypeMap,
		Elem: &schema.Schema{
			Type: schema.TypeString,
		},
		Optional: true,
	},

	"list_nested_block": {
		Type:     schema.TypeList,
		Optional: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"list_nested_block_attribute": {
					Type:     schema.TypeString,
					Optional: true,
				},
			},
		},
	},
	"set_nested_block": {
		Type:     schema.TypeSet,
		Optional: true,
		Elem: &schema.Resource{
			Schema: map[string]*schema.Schema{
				"set_nested_block_attribute": {
					Type:     schema.TypeString,
					Optional: true,
				},
			},
		},
	},
},

Expected Behavior

Given the following test

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

	r.UnitTest(t, r.TestCase{
		ProviderFactories: map[string]func() (*schema.Provider, error){
			"test": func() (*schema.Provider, error) { //nolint:unparam // required signature
				return testProvider(), nil
			},
		},
		Steps: []r.TestStep{
			{
				Config: `
				resource "test_resource" "one" {}
				`,
				ConfigPlanChecks: r.ConfigPlanChecks{
					PreApply: []plancheck.PlanCheck{
						plancheck.ExpectUnknownValue("test_resource.one", tfjsonpath.New("string_attribute")),
					},
				},
			},
		},
	})
}

I would expect an error message similar to other assertion fails, like:

--- FAIL: Test_ExpectUnknownValue_NullValue (233.24s)
        specified key string_attribute was null, but expected it to be unknown

--- FAIL: Test_ExpectUnknownValue_NullValue (233.24s)
        specified key string_attribute was null, but expected it to be sensitive

Potential Solutions

This is occurring because Change.AfterUnknown and Change.AfterSensitive don't have the attribute value if it's nil. Perhaps we can cross-reference with the After property to provide a better error message?

[bflad]

For root attributes, this seems reasonable to me. The original implementations of these plan checks were intended as a 1:1 check of the data being inspected so we could determine what type of compatibility promises we might need before introducing any cross-checking of other data in the plan. Once we introduce those cross-checks, this Go module becomes beholden to handling those situations, even if Terraform core's implementation changes over time. It's probably safe in this case as "fallback" logic, at least for root attributes.

My wording with root attributes is intentional here -- Terraform will typically "stop" with null/unknown values at the first attribute in the schema data that is null/unknown, rather than showing that information with all nested attributes. I'm guessing that may have some less fortunate messaging in those situations already and this logic would also need to account for some potentially awkward cases where nested attributes cannot actually be determined whether they exist or not even from the After data. Whether that is enough awkwardness to not adjust these checks is debatable, since it may be difficult or not possible to "fix" this messaging in all situations.