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

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

Closed
liamcervante opened this issue Sep 14, 2022 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@liamcervante
Copy link
Member

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

@liamcervante liamcervante added the bug Something isn't working label Sep 14, 2022
@liamcervante
Copy link
Member Author

liamcervante commented Sep 14, 2022

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
Copy link
Member Author

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

@bflad
Copy link
Member

bflad commented Sep 20, 2022

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.

@jar-b
Copy link
Member

jar-b commented Nov 23, 2022

I think I'm running into this writing a new resource for the AWS provider (hashicorp/terraform-provider-aws#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.

@bflad
Copy link
Member

bflad commented Dec 12, 2022

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

@bflad bflad closed this as completed Dec 12, 2022
@liamcervante
Copy link
Member Author

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

@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 Jan 13, 2023
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
Development

No branches or pull requests

3 participants