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

Provider produced unexpected value after apply for a Computed attribute #840

Closed
mschuchard opened this issue Sep 11, 2023 · 22 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@mschuchard
Copy link

mschuchard commented Sep 11, 2023

This has been a discussion on plugin development for about two weeks with evidence leading to believe this more likely an issue than not, no hypothetical root cause or workaround, and the affected users in this situation are of interest to Hashicorp from a business perspective. I am therefore opening an issue here based on the above.

Module version

1.3.5

Relevant provider source code

See link to Hashicorp discuss.

Terraform Configuration Files

See link to Hashicorp discuss.

Debug Output

I cannot provide this easily as this is proprietary/NDA, but something could possibly be worked out if need be.

Expected Behavior

The plugin framework sets a Computed attribute to unknown during plan, and then allows setting the state to the correct actual values within the Create and/or Read.

Actual Behavior

If we are to believe the results of terraform-plugin-testing 1.5.1, and based on discussion thus far, then the Computed attribute value is set to unknown, then to null for unknown reasons post-plan creation (issue is here), and then to the correct accurate value. The plugin framework throws this behavior as an error during the State.set(). Error string is:

Error: Provider produced inconsistent result after apply
… produced an unexpected new value: .bar
was null, but now cty.ObjectValue(map[string]cty.Value{…})

Note the post-apply value is the expected and accurate value for the attribute according to the logs.

Steps to Reproduce

apply any resource with a nested type struct in the top level model that is also a schema nested attribute and tftype types.Object

References

n/a

@mschuchard mschuchard added the bug Something isn't working label Sep 11, 2023
@austinvalle
Copy link
Member

austinvalle commented Sep 12, 2023

hey there @mschuchard, thanks for opening the issue 👋🏻

Just wanted to let you know this is still on my radar, I just haven't found the time to recreate the issue. If others run into this bug and have a reproducible example they can share, that'd be helpful.

For now, I plan on continuing to iterate on the example you shared in the discuss post here 👍🏻

@austinvalle austinvalle self-assigned this Sep 12, 2023
@austinvalle
Copy link
Member

I just started looking at the example and I have a clarifying question about the schema/data model that was provided.

In the example, you have this model:

// provider plugin
// tf go models
type fooModel struct {
  Bar types.Object `tfsdk:"bar"`
}

type barModel struct {
  Baz types.String `tfsdk:"baz"`
}

This schema:

Attributes: map[string]schema.Attribute{
  "foo": resource.SingleNestedAttribute{
		Computed:    true,
		Description: "The foo attributes.",
		Attributes: map[string]resource.Attribute{
			"bar": resource.StringAttribute{
				Computed:    true,
				Description: "The bar.",
			},
		},
	},
}

Which is read in this Create from fooModel:

func (resource *fooResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
	// read Terraform plan data into the model
	var data util.fooModel
	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
	if resp.Diagnostics.HasError() {
		return
	}
	
	// ... remaining logic
}

I'm wondering if this was a copy/paste error? I would expect, based on the schema you provided and how you're reading the plan data, that your resource models would look like:

// provider plugin
// tf go models
type resourceModel struct {
  Foo types.Object `tfsdk:"foo"`
}

type fooModel struct {
  Bar types.String `tfsdk:"bar"`
}

func (resource *fooResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
	// read Terraform plan data into the model
	var data util.resourceModel
	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
	if resp.Diagnostics.HasError() {
		return
	}
	
	// ... remaining logic
}

@mschuchard
Copy link
Author

mschuchard commented Sep 12, 2023

Ok I see where the typo arose from the heavy amount of redacting and sanitization; there is an issue in the schema:

Attributes: map[string]schema.Attribute{
  "bar": resource.SingleNestedAttribute{
		Computed:    true,
		Description: "The bar attributes.",
		Attributes: map[string]resource.Attribute{
			"baz": resource.StringAttribute{
				Computed:    true,
				Description: "The baz.",
			},
		},
	},
}

That would probably actually deserialize and serialize for you accurately. If the TF Go models were to be renamed then the Create and Read would need to correspondingly be modified as the core issue here is for the struct field member/object type/single nested attribute.

Since this issue occurs for all resources with this model/schema/type, then you could probably also devise your own MCVE if there are any other typos.

@austinvalle
Copy link
Member

Cool, thanks for clarifying. Apologies if I'm coming across as pedantic with the specifics of your example models/schema. As I'm still unable to reproduce the behavior, I'm trying to change as little as possible from what you've given me.

Here is the example resource I ran in it's entirety from your MCVE: https://github.com/austinvalle/terraform-provider-sandbox/blob/9b559ce4fb11aaabf2b2cf0f166493d31b9ce17c/internal/provider/thing_resource.go

  • I stubbed out the DoFoo and BarGoToTerraform, if you have more detail on those methods I can update them.

Running with this config: https://github.com/austinvalle/terraform-provider-sandbox/blob/9b559ce4fb11aaabf2b2cf0f166493d31b9ce17c/examples/resources/resource.tf

  1. First apply, successful create

Output

 $ terraform apply -auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # examplecloud_thing.this will be created
  + resource "examplecloud_thing" "this" {
      + bar = (known after apply)
      + id  = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.
examplecloud_thing.this: Creating...
examplecloud_thing.this: Creation complete after 0s [id=test]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.



 $ cat terraform.tfstate                             
{
  "version": 4,
  "terraform_version": "1.5.4",
  "serial": 1,
  "lineage": "6bd42aa5-17fb-ad89-4d52-6e018719f3a6",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "examplecloud_thing",
      "name": "this",
      "provider": "provider[\"registry.terraform.io/austinvalle/sandbox\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "bar": {
              "baz": "baz-value"
            },
            "id": "test"
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],
  "check_results": null
}
  1. Second apply, read successful, no changes

Output

 $ terraform apply -auto-approve
examplecloud_thing.this: Refreshing state... [id=test]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

 $ cat terraform.tfstate
{
  "version": 4,
  "terraform_version": "1.5.4",
  "serial": 1,
  "lineage": "6bd42aa5-17fb-ad89-4d52-6e018719f3a6",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "examplecloud_thing",
      "name": "this",
      "provider": "provider[\"registry.terraform.io/austinvalle/sandbox\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "bar": {
              "baz": "baz-value"
            },
            "id": "test"
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],
  "check_results": null
}

As I wasn't able to reproduce the error:

  • Does my MCVE I linked above look correct to what you have?
  • Were you expecting additional attributes that I haven't included to force an Update?
  • Anything else I can add to the MCVE to more closely align with your provider?

Appreciate your time working through this 👋🏻

@mschuchard
Copy link
Author

mschuchard commented Sep 12, 2023

Update does not occur here to my knowledge. I had to glance three times before I noticed there is one dissimilarity between this MCVE and my code. The analogous definition of BarGoToTerraform (exported because in another package) is:

func BarGoToTerraform(ctx context.Context, b Bar) (types.Object, diag.Diagnostics) {
	return types.ObjectValueFrom(ctx, barModelTF, barModel{
            Baz: types.StringValue(b.Baz),
        })
}

The primary difference here being my usage of the intrinsic converter ObjectValueFrom() to convert frrom the SDK Go model to the TF Go model prior to converting to the TF type model. Your definition directly defines the TF type model with types.ObjectValue() and converts from the SDK Go model without an intermediate conversion to the TF Go model. If this is significant then it may be root cause.

Also if this is root cause then this relates to #822 where I offered to contribute more complex examples and algorithms for review and then addition to documentation but received no reply. The documentation currently recommends the usage of ObjectValueFrom() in this situation, and if this is incorrect, or not applicable to all use cases such as this one, then I would emphasize the need to accept assistance such as what I offered.

I know there are generally issues with availability for reviewing PR (same occurs in other TF projects, and Packer and Vault projects), and so I do understand why there was no reply to my offer of assistance. I would request the offer is accepted if you verify this is root cause however. Thank you.

@austinvalle
Copy link
Member

Your definition directly defines the TF type model with types.ObjectValue() and converts from the SDK Go model without an intermediate conversion to the TF Go model. If this is significant then it may be root cause.

I don't think it necessarily is significant in-terms of "you should be using one vs. the other". Using types.ObjectValueFrom() is totally valid as well, probably more common. If you're handling the diagnostics returned by types.ObjectValue* in your CRUD code, then you should see any mapping-based errors.

You can replace the stub function with the following and achieve the same results (updated sandbox example):

func DoFoo(client any, method string, id string) (Foo, error) {
	return Foo{
		Bar: Bar{
			Baz: "using-object-from-now",
		},
	}, nil
}

func BarGoToTerraform(ctx context.Context, b Bar) (types.Object, diag.Diagnostics) {
	return types.ObjectValueFrom(ctx, barModelTF, barModel{
		Baz: types.StringValue(b.Baz),
	})
}

Output

 $ terraform apply -auto-approve

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # examplecloud_thing.this will be created
  + resource "examplecloud_thing" "this" {
      + bar = (known after apply)
      + id  = (known after apply)
    }

Plan: 1 to add, 0 to change, 0 to destroy.
examplecloud_thing.this: Creating...
examplecloud_thing.this: Creation complete after 2s [id=test]

Apply complete! Resources: 1 added, 0 changed, 0 destroyed.


 $ cat terraform.tfstate
{
  "version": 4,
  "terraform_version": "1.5.4",
  "serial": 1,
  "lineage": "3e6bd6de-387f-1dd9-bcb2-65e2f578860b",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "examplecloud_thing",
      "name": "this",
      "provider": "provider[\"registry.terraform.io/austinvalle/sandbox\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "bar": {
              "baz": "using-object-from-now"
            },
            "id": "test"
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],
  "check_results": null
}


 $ terraform apply -auto-approve
examplecloud_thing.this: Refreshing state... [id=test]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.


 $ cat terraform.tfstate        
{
  "version": 4,
  "terraform_version": "1.5.4",
  "serial": 1,
  "lineage": "3e6bd6de-387f-1dd9-bcb2-65e2f578860b",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "examplecloud_thing",
      "name": "this",
      "provider": "provider[\"registry.terraform.io/austinvalle/sandbox\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "bar": {
              "baz": "using-object-from-now"
            },
            "id": "test"
          },
          "sensitive_attributes": []
        }
      ]
    }
  ],
  "check_results": null
}

I'm not 100% sure this is the root cause of your problem, but I also don't have any other ideas based on what we've looked at so far. If there's more detail you can provide about the schema/CRUD functionality that hasn't been shared then I can look further. I'll also bring this up with some of my teammates later this week and see if there are any other ideas.

Also if this is root cause then this relates to #822 where I offered to contribute more complex examples and algorithms for review and then addition to documentation but received no reply. The documentation currently recommends the usage of ObjectValueFrom() in this situation, and if this is incorrect, or not applicable to all use cases such as this one, then I would emphasize the need to accept assistance such as what I offered.

I know there are generally issues with availability for reviewing PR (same occurs in other TF projects, and Packer and Vault projects), and so I do understand why there was no reply to my offer of assistance. I would request the offer is accepted if you verify this is root cause however. Thank you.

As for this, we try our best to respond to requests, although it's less likely for us to find comments on merged PRs after the fact. If you'd like to open up a new issue to discuss further (more complex) documentation updates I think that'd be great! We do try to keep the docs simple where possible, but always welcome to hear potential improvements.

@mschuchard
Copy link
Author

mschuchard commented Sep 12, 2023

So now the MCVE functionality does match the code, and that is concerning.

The first thought would probably be matching the environment, but this is occurring for myself and the users for all resources with this structure across different operating systems, Terraform versions >= 1.2.0 < 1.6.0, and versions of Go within the minor of 1.20.

Then it becomes possibly bizarre differences that I would think should have no effect such as that my schema for bar is being returned by a function in a different sub-package. Along the same lines it could be some weird behavior with responses from the API (but I have no idea why). I am spitballing ideas here because "scientific method" seems to be completely failing.

A couple of questions though:

  • I thought a Create is followed by a subsequent Read to confirm creation, and I thought some previous comment implied that as well. In this discussion it seems I am mistaken though?
  • Since this whole issue is a false positive error returned from resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) then is there a workaround to ignore that completely (such as not appending to the diagnostics although that could have negative side-effects in other situations I would imagine)? According to the error logs the values in the state are being set to the correct and accurate values, and the issue is that for some unknown reason it was reset to null at some point without any intention from the code, and this triggers an error. If I could circumvent the error return due to the reset to null then that would be a workaround good enough for me.

Thank you.

@austinvalle
Copy link
Member

austinvalle commented Sep 12, 2023

Context

Since I'm going to reference the Terraform Resource lifecycle and the related RPCs in this response, here are helpful references that describe them:

Answering Questions

I thought a Create is followed by a subsequent Read to confirm creation, and I thought some previous comment implied that as well. In this discussion it seems I am mistaken though?

  • The Read function is only called during the ReadResource RPC, which occurs after validation and before the plan.
  • It's entirely possible that you may have read comments referencing an old SDKv2 pattern, where after creating a resource, you would return the read function (i.e. your create function would call your read function before returning).
    • The plugin framework won't call your Read function outside of the ReadResource RPC, so if you need to confirm creation, you can handle that in the Create.

Since this whole issue is a false positive error returned from resp.Diagnostics.Append(resp.State.Set(ctx, &data)...) then is there a workaround to ignore that completely (such as not appending to the diagnostics although that could have negative side-effects in other situations I would imagine)? According to the error logs the values in the state are being set to the correct and accurate values, and the issue is that for some unknown reason it was reset to null at some point without any intention from the code, and this triggers an error. If I could circumvent the error return due to the reset to null then that would be a workaround good enough for me.

  • I'm not convinced that the error is a false positive; I still believe there is something incorrectly setting your object to null during the plan, when it should be automatically marked as unknown (if there are no defaults, schema plan modifiers, or resource plan modifiers changing the value)
    • In the face of no plan modification or defaults, then I'd expect it to be a bug that can be recreated
  • To answer your question directly, this error is being raised on the Terraform core side as a part of it's data consistency checks, and there is no way for a provider to influence that assertion.

@austinvalle
Copy link
Member

I'll talk through this problem with a teammate and see if I can get any other ideas for recreating the behavior you're describing

@mschuchard
Copy link
Author

mschuchard commented Sep 12, 2023

It's entirely possible that you may have read comments referencing an old SDKv2 pattern, where after creating a resource, you would return the read function (i.e. your create function would call your read function before returning).

Yes thank you very likely that. I have been playing around with provider development since the days of direct package imports, but only began doing anything beyond toys with this plugin framework, and so I probably conflated the four different iterations here.

I still believe there is something incorrectly setting your object to null during the plan

Would that not cause the following to fail though because it is passing:

ConfigPlanChecks: resource.ConfigPlanChecks{
    PreApply: []plancheck.PlanCheck{
        plancheck.ExpectUnknownValue("provider_foo.test", tfjsonpath.New("bar")),
    },
},

I did observe that changing bar to id in the above test throws the error:

Step 1/1 error: Pre-apply plan check(s) failed:
path not found: specified key id not found in map

which was super weird to me and makes me question the test outcome.

@austinvalle
Copy link
Member

austinvalle commented Sep 12, 2023

Would that not cause the following to fail though because it is passing:

ConfigPlanChecks: resource.ConfigPlanChecks{
   PreApply: []plancheck.PlanCheck{
       plancheck.ExpectUnknownValue("provider_foo.test", tfjsonpath.New("bar")),
   },
},

Based on how you described your error, I would expect this to pass successfully, but the test would fail due to your unexpected value after apply error.

I would expect that plancheck assertion to fail on PostApplyPreRefresh, as I believe you stated that your error was occurring after the initial apply, during the second apply? So maybe something like this:

ConfigPlanChecks: resource.ConfigPlanChecks{
    PostApplyPreRefresh: []plancheck.PlanCheck{
        plancheck.ExpectUnknownValue("provider_foo.test", tfjsonpath.New("bar")),
    },
},

I did observe that changing bar to id in the above test throws the error:

Step 1/1 error: Pre-apply plan check(s) failed:
path not found: specified key id not found in map

This looks like a bug in the error message of that plan check when the attribute is nil, I actually can recreate that with this new test where I'm forcing the Terraform core "inconsistent value" error for our MCVE:
https://github.com/austinvalle/terraform-provider-sandbox/blob/dc12c4ec36a2437b65fea9caf6165dd9a3925fc8/internal/provider/thing_resource_test.go

--- FAIL: Test (0.22s)
    /Users/austin.valle/code/terraform-provider-sandbox/internal/provider/thing_resource_test.go:17: Step 1/1 error: Pre-apply plan check(s) failed:
        path not found: specified key bar not found in map
FAIL
FAIL	github.com/austinvalle/terraform-provider-sandbox/internal/provider	0.506s

Further inspection shows that the attribute does exist, but it's nil
image

I will write up an issue for terraform-plugin-testing to have that error message adjusted. It should be something like:

--- FAIL: Test (0.22s)
    /Users/austin.valle/code/terraform-provider-sandbox/internal/provider/thing_resource_test.go:17: Step 1/1 error: Pre-apply plan check(s) failed:
        specified key bar is nil, expected unknown
FAIL
FAIL	github.com/austinvalle/terraform-provider-sandbox/internal/provider	0.506s

@mschuchard
Copy link
Author

mschuchard commented Sep 13, 2023

I would expect that plancheck assertion to fail on PostApplyPreRefresh, as I believe you stated that your error was occurring after the initial apply, during the second apply?

It fails with that error message during initial apply for users and during acceptance testing for myself (equivalent to initial apply?). Based on the discussion thus far I believe this strongly implies we can discard Read as being relevant here.

@austinvalle
Copy link
Member

austinvalle commented Sep 14, 2023

Alright, so I spoke with one of my teammates and I'm still thinking that there is something affecting your PlanResourceRPC, although I'm not able to reproduce any behavior that resembles a bug in terraform-plugin-framework.

Reminder, that the only provider-defined code that are used during PlanResourceRPC are plan modifications or default values.

Since the code you're working with is proprietary and you believe the MCVE I presented is pretty close to the code exhibiting this bug, I'd like to see if you can recreate the bug using the MCVE in my github repo:

Here is the commit that is currently not exhibiting the bug that you can pull to your local machine and run: https://github.com/austinvalle/terraform-provider-sandbox/tree/0dcb5af2c256d3931ea87ee99b67bf89c47cb356
I created an acceptance test to make it easier to run, but you can also debug/build the provider yourself and run it with the config in examples/resources (you'll need to update your .terraformrc or use a REATTACH config for debugging)

I also added a commented-out line that will shows how this error can be caused by an "errant" plan modification: https://github.com/austinvalle/terraform-provider-sandbox/blob/0dcb5af2c256d3931ea87ee99b67bf89c47cb356/internal/provider/thing_resource.go#L74 - If you want to see the error purposefully, you can uncomment that line and run the acceptance test, which'll show:

--- FAIL: TestThingResource (0.31s)
    /Users/austin.valle/code/terraform-provider-sandbox/internal/provider/thing_resource_test.go:15: Step 1/1 error: Error running apply: exit status 1
        
        Error: Provider produced inconsistent result after apply
        
        When applying changes to examplecloud_thing.this, provider
        "provider[\"registry.terraform.io/hashicorp/examplecloud\"]" produced an
        unexpected new value: .bar: was null, but now
        cty.ObjectVal(map[string]cty.Value{"baz":cty.StringVal("baz-for-the-foo")}).
        
        This is a bug in the provider, which should be reported in the provider's own
        issue tracker.

If you're able to re-create this bug with the MCVE provided without a plan modifier or default value, please let me know so I can investigate further.

@mschuchard
Copy link
Author

Real quick sidebar that I just noticed is that the error message thrown for me on the plan test was:

Step 1/1 error: Pre-apply plan check(s) failed:
path not found: specified key id not found in map

and not:

path not found: specified key bar not found in map

I believe your discussed issue of a perhaps misleading error message is different than what I uncovered as your debug output confirms my test should pass the unknown check instead of throwing an internal error during execution, and this then does throw into question the validity of that test to verify that bar is unknown during plan generation.

@mschuchard
Copy link
Author

mschuchard commented Sep 14, 2023

I did clone and execute an acceptance test, and then uncommented the line and executed the acceptance test again. Unfortunately that triggered a missing package import error:

internal/provider/thing_resource.go:74:18: undefined: objectdefault

Regardless of this, I am unsure of the value of this experiment as all it would do is lend credence to a hypothesis that this is an environment issue. However, we are seeing this for all resources in all environments. I believe what is more likely here by mathematical logic/scientific reasoning is that this statement:

Reminder, that the only provider-defined code that are used during PlanResourceRPC are plan modifications or default values.

is likely false. We are seeing this error thrown for a computed attribute where the schema contains no plan modifiers or default values of any kind. One of the resources does contain:

PlanModifiers: []planmodifier.String{
				stringplanmodifier.UseStateForUnknown(),
			},

for the id, but it is a completely different attribute than what is throwing the error. Something else is insidiously causing a rest of the attribute from unknown to null before the correct value is set in the state at the end of Create, and it seems to be something possibly unique to my implementation (perhaps because of some early adoption of some feature). I can attempt removing this plan modifier, but if that fixes the error than that would also be an issue because it would be in a completely different resource not present in the same acceptance test.

I actually repped Hashicorp while working with this company previously, and so I know they have previously shared code with Hashicorp. At this point I believe that may be the only real path forward here. What is unclear to me is how legally or for your time this would possibly fit under their TFE support contract (if at all).

Alternatively I will begin experimenting with ignoring/circumventing the thrown error as I am convinced it is a false positive since the logs show the attribute is correctly computed, and according to terraform-plugin-testing it is unknown during plan. If it is not a false positive then that would be evidence of another issue with terraform-plugin-testing.

@mschuchard
Copy link
Author

mschuchard commented Sep 15, 2023

Bit of a breakthrough executing acceptance tests this morning: the exact same inputs began producing different behavior in a different resource. While this normally would be sanity-reducing (or because the API was silently updated), it actually provoked a path forward on the resource with the plan modifier. I had a stupid moment and was marking the id as Optional in the schema instead of Computed because of my confusion with another similar resource, and because of outdated API documentation. I also specified the id in the acceptance test because of this. This then caused the "unexpected value error" except this time correctly because the id was input as one value (I use ENV vars with string formatting in the acceptance tests), but was then re-assigned post-apply because it is actually computed. Also this API has some unusual behavior and characteristics, and one of these is that a set is duplicated as inputs versus outputs, and the inputs are nil in the response, and instead used to populate the outputs. This unsurprisingly also correctly threw the "unexpected value after apply" error because the inputs are defined, but then set to null in the state because the response does not contain the inputs and I have tagged them omitempty for the JSON deserialization. I would speculate continuing down this path will fix the unexpected value after apply for this resource.

One weird behavior I did notice is that a set(string) optional attribute is being generated with a value of null during the plan, but then afterward the response causes it to be re-assigned to an empty set because the response value is an empty slice. I honestly am unsure off the top of my head if this would be expected behavior or not, but I can certainly devise a workaround regardless.

The unfortunate part of this is that this is NOT the resource represented in the MCVE above, and that resource represented by the MCVE is still exhibiting the behavior. The encouraging aspect of this is that the issue is probably centralized to the MCVE, and not all resources defined using the terraform-plugin-framework (which makes sense).

@mschuchard
Copy link
Author

Ok I can confirm the resource(s) in question (not the MCVE though) are now "good to go" except for an optional set(string) initialized to null instead of an empty set when it is unspecified. Need to poke around to see if that is my responsibility or the plugin framework, but please chime in if you already know.

@mschuchard
Copy link
Author

re: set initialization to null

I worked around it by just setting the attribute to Computed in addition to Optional. I know that for most languages a set would be initialized to empty instead of null, but I assume the AST or whatever here is dervied from *[]T, and also I did a PR for the AWS provider where I literally debated with someone from cloudposse that it should be initialized to null (this was soon after TF 0.12 was released so the concept was kind of new at the time), and Bardin upheld my viewpoint, and so I would look ridiculous reversing now (even though that was SDKv1 or something I believe).

re: this issue

It is only occurring for this resource now thanks to my realization that the API was behaving differently from documentation, and that I should scrutinize the error messages from the plugin framework for the actual root cause and not believe what I read. The customer is fine with not using this resource for the time being, and will instead be using the data source. Hopefully this issue will be fixed indirectly in an update, but otherwise this can be triaged probably.

@bflad
Copy link
Member

bflad commented Jan 30, 2024

Hi @mschuchard 👋 I'm going through this issue tracker to ensure that there are actionable items in the backlog or otherwise closing them. It is a little unclear to me about the current status of this issue. Are you still having trouble and if so, can you let us know exactly what you are looking for in your situation? Thanks.

@bflad bflad added the waiting-response Issues or pull requests waiting for an external response label Jan 30, 2024
@mschuchard
Copy link
Author

This issue is still outstanding as far as all of us involved know, but I implemented a workaround and the plugin was successfully delivered to and accepted by the client.
Therefore I have no current stake in this issue's resolution unless it occurs again with another plugin in the future, and so I entrust you with the decision about what action to take on this issue, and am good with whatever you decide.

@github-actions github-actions bot removed the waiting-response Issues or pull requests waiting for an external response label Jan 30, 2024
@bflad
Copy link
Member

bflad commented Jan 30, 2024

Okay, thanks for the quick reply, @mschuchard. I chatted with @austinvalle and since we do not have a working reproduction to base any potential documentation or code changes, I am not sure what we can do further in this case. I am going to close this issue for now, but if you or someone else has a working reproduction case for this type of error, please open a new issue and we will be happy to investigate it further. 👍

@bflad bflad closed this as not planned Won't fix, can't repro, duplicate, stale Jan 30, 2024
Copy link

github-actions bot commented Mar 1, 2024

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 Mar 1, 2024
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