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

Computed values within blocks that are nested within set blocks are unset by the framework

Open liamcervante opened this issue 3 years ago • 3 comments
trafficstars

Module version

v0.12.0

Relevant provider source code

PR reproducing the bug

Terraform Configuration Files

resource "tf6muxprovider_nested" "example" {
  set {
    id = "one"

    list {

    }

    list {

    }
  }

  set {

  }
}

Debug Output

N/A, the error is reproduced in a test case in the PR

Expected Behavior

The test case in the PR should pass.

Actual Behavior

=== RUN   TestAccResourceNested
    nested_resource_test.go:11: Step 1/1 error: Error running post-apply plan: exit status 1
        
        Error: Error modifying plan
        
          with tf6muxprovider_nested.example,
          on terraform_plugin_test.tf line 2, in resource "tf6muxprovider_nested" "example":
           2: resource "tf6muxprovider_nested" "example" {
        
        There was an unexpected error updating the plan. This is always a problem
        with the provider. Please report the following to the provider developer:
        
        AttributeName("set").ElementKeyValue(tftypes.Object["id":tftypes.String,
        "list":tftypes.List[tftypes.Object["id":tftypes.String]]]<"id":tftypes.String<"xhxKQFDa">,
        "list":tftypes.List[tftypes.Object["id":tftypes.String]]<>>).AttributeName("list"):
        couldn't find attribute in resource schema: path leads to block, not an
        attribute
--- FAIL: TestAccResourceNested (310.30s)

Steps to Reproduce

  1. Checkout the branch in terraform-provider-corner (or the forked repo)
  2. TF_ACC=1 go test internal/tf6muxprovider/provider1/*.go

References

  • https://github.com/hashicorp/terraform-plugin-framework/issues/479

liamcervante avatar Sep 14 '22 10:09 liamcervante

Some other observations.

If you replace the resource with any of the following the test works fine:

resource "tf6muxprovider_nested" "example" {
  set {
    id = "one"

    list {

    }

    list {

    }
  }

  set {
    id = "two"
  }
}
resource "tf6muxprovider_nested" "example" {
  set {

    list {

    }

    list {

    }
  }

  set {

  }
}
resource "tf6muxprovider_nested" "example" {
  set {
    id = "one"

    list {
      id = "one"
    }

    list {
      id = "two"
    }
  }

  set {

  }
}

The last case needs the inner id to be set to optional as well as some other tweaks. But the point I think is that there's some weird behaviour around nested computed values and when it does or doesn't have to compute values for them.

liamcervante avatar Sep 14 '22 10:09 liamcervante

The actual error message originates here: https://github.com/hashicorp/terraform-plugin-framework/blob/main/internal/fwserver/server_planresourcechange.go#L296

But if you jump back up the stack and inspect the execution during the test, it seems like the computed values for the inner nested block (list) are appearing as null (even though the provider logic sets them), while all the other computed values are being set as normal: https://github.com/hashicorp/terraform-plugin-framework/blob/main/internal/fwserver/server_planresourcechange.go#L160

liamcervante avatar Sep 14 '22 10:09 liamcervante

I'm curious how this hasn't been more of an issue until now, but the MarkComputedNilsAsUnknown logic should likely be returning the untransformed value if it encounters a tfsdk.ErrPathIsBlock error, similar to the other tfsdk.ErrPathInsideAtomicAttribute error handling. Blocks can't be computed so there isn't anything else to do with them in that case.

The testing for that function is a little heavy as it prefers to setup everything in a single, big schema with a single, big value. Ideally that testing would be converted to the parallel unit testing setup used elsewhere and where each test case can include its own smaller schema and values.

I'm not sure when we'll have time to potentially fix or review a fix for this, but please use 👍 on the issue if you are also running into this to help with prioritization.

bflad avatar Sep 20 '22 19:09 bflad

I think I'm running into this writing a new resource for the AWS provider (https://github.com/hashicorp/terraform-provider-aws/pull/27857). The schema includes an optional list within a required set (simplified below for brevity).

		Blocks: map[string]tfsdk.Block{
			"control_mapping_sources": {
				NestingMode: tfsdk.BlockNestingModeSet,
				MinItems:    1,
				Attributes: map[string]tfsdk.Attribute{
				        // omitted
				},
				Blocks: map[string]tfsdk.Block{
					"source_keyword": {
						NestingMode: tfsdk.BlockNestingModeList,
						MinItems:    0,
						MaxItems:    1,
						Attributes: map[string]tfsdk.Attribute{
							// omitted
						},
					},
				},

Tests pass if control_mapping_sources is a list, but in practice we need a set because the AWS API response does not guarantee order. Changing to a set type results in errors like:

=== CONT  TestAccAuditManagerControl_tags
    control_test.go:90: Step 3/4 error: Error running pre-apply refresh: exit status 1

        Error: Error modifying plan

          with aws_auditmanager_control.test,
          on terraform_plugin_test.tf line 2, in resource "aws_auditmanager_control" "test":
           2: resource "aws_auditmanager_control" "test" {

        There was an unexpected error updating the plan. This is always a problem
        with the provider. Please report the following to the provider developer:

        AttributeName("control_mapping_sources").ElementKeyValue(tftypes.Object["source_description":tftypes.String,
        "source_frequency":tftypes.String, "source_id":tftypes.String,
        "source_keyword":tftypes.Set[tftypes.Object["keyword_input_type":tftypes.String,
        "keyword_value":tftypes.String]], "source_name":tftypes.String,
        "source_set_up_option":tftypes.String, "source_type":tftypes.String,
        "troubleshooting_text":tftypes.String]<"source_description":tftypes.String<null>,
        "source_frequency":tftypes.String<null>,
        "source_id":tftypes.String<"0011761a-3c2f-46b3-bde8-c04232ad7b2f">,
        "source_keyword":tftypes.Set[tftypes.Object["keyword_input_type":tftypes.String,
        "keyword_value":tftypes.String]]<>,
        "source_name":tftypes.String<"tf-acc-test-3318555567884913938">,
        "source_set_up_option":tftypes.String<"Procedural_Controls_Mapping">,
        "source_type":tftypes.String<"MANUAL">,
        "troubleshooting_text":tftypes.String<null>>).AttributeName("source_keyword"):
        couldn't find attribute in resource schema: path leads to block, not an
        attribute

However, the suggestion above around passing through the value for ErrPathIsBlock errors resolves this (or at least my issue). I replaced the plugin framework dependency with a local clone including this extra conditional and all tests pass again.

			if errors.Is(err, tfsdk.ErrPathIsBlock) {
				// ignore blocks, they cannot be computed
				logging.FrameworkTrace(ctx, "attribute is a block, not marking unknown")
				return val, nil
			}

If this sounds like a reasonable solution, I'd be happy to get a PR started and take a shot at adding tests. Plugin SDKv2 presents separate challenges with nested computed attributes for this resource, so it'd be valuable to us to unblock use of framework here.

jar-b avatar Nov 23 '22 21:11 jar-b

I believe this has been resolved at least in v0.17.0, but please reach out if it has not. 👍

bflad avatar Dec 12 '22 17:12 bflad

Just closing the loop here - I can confirm this has been fixed for my use case. Thanks!

liamcervante avatar Dec 13 '22 08:12 liamcervante

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 Jan 13 '23 02:01 github-actions[bot]