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

Framework doesn't appear to support a computed list of objects. #713

Closed
liamawhite opened this issue Apr 3, 2023 · 6 comments
Closed

Framework doesn't appear to support a computed list of objects. #713

liamawhite opened this issue Apr 3, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@liamawhite
Copy link

liamawhite commented Apr 3, 2023

Module version

github.com/hashicorp/terraform-plugin-framework v1.2.0

Relevant provider source code

https://github.com/liamawhite/tf-issue-repro

Debug Output

make test                                                                                                                                                 1.20.2
TF_ACC=1 go test ./...
--- FAIL: TestAccServiceAccountResource (0.70s)
    resource_test.go:36: Step 1/3 error: Error running apply: exit status 1
        
        Error: Value Conversion Error
        
          with provider_service_account.some-name,
          on terraform_plugin_test.tf line 4, in resource "provider_service_account" "some-name":
           4: resource provider_service_account "some-name" {}
        
        An unexpected error was encountered trying to build a value. This is always
        an error in the provider. Please report the following to the provider
        developer:
        
        Received unknown value, however the target type cannot handle unknown values.
        Use the corresponding `types` package type or a custom type that handles
        unknown values.
        
        Path: keys
        Target Type: []*repro.KeyModel
        Suggested Type: basetypes.ListValue
FAIL
FAIL    github.com/liamawhite/tf-issue-repro    1.064s
FAIL
make: *** [test] Error 1

Expected Behavior

To populate the model without having to define a custom type just for computed lists of objects.

@liamawhite liamawhite added the bug Something isn't working label Apr 3, 2023
@bflad
Copy link
Member

bflad commented Apr 3, 2023

Hi @liamawhite 👋 Thank you for raising this and sorry you ran into trouble here.

No additional custom types are necessarily needed -- what the framework error diagnostic is trying to relay to you as the provider developer is that:

type ServiceAccountModel struct {
	Id   types.String `tfsdk:"id"`
	Keys []*KeyModel  `tfsdk:"keys"`
}

Should be:

type ServiceAccountModel struct {
	Id   types.String `tfsdk:"id"`
	Keys types.List   `tfsdk:"keys"`
}

Because a types.List value can contain unknown values per the conversion rules, while a standard Go slice ([]T) has no concept of "unknown". The reason this unknown value is showing up here is because during planning, the keys attribute will be automatically marked as unknown (known after apply), so Terraform core knows that the provider will fill in the attribute value after its applied. In your resource Create method, you are using:

	var model ServiceAccountModel
	resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...)
	if resp.Diagnostics.HasError() {
		return
	}

Which is generally correct when the model is implemented using all types types, however because the request plan is trying to convert the unknown value into an unsupported Go slice, the error is raised. Adjusting to types.List in the model should resolve it. The resource logic will need to handle the conversion of types.List from the API response. This can be handled by using any of the types.List value creation functions, such as types.ListValueFrom().

Does this make sense? If you have suggestions for how to improve documentation, the error messaging, etc., please let us know.

@liamawhite
Copy link
Author

Let me give this a try and I'll get back to you.

@masterfuzz
Copy link

Could you point to any examples on how to handle this with nested objects?

For now I have something like this:

	keys, err := types.ListValueFrom(ctx, req.Config.Schema.GetAttributes()["keys"].(schema.ListNestedAttribute).NestedObject.Type(), []*KeyModel{{Token: types.StringValue("some-token")}})
	if err != nil {
		resp.Diagnostics.Append(err...)
	}
	model.Keys = keys

But it seems a little tedious, especially if there are further levels of nesting.

Any guidance?

@chrismarget-j
Copy link

Could you point to any examples on how to handle this with nested objects?

In my world, every layer of nesting is a struct with Attributes() map[string]schema.Attribute (for use in nested Schema()) and AttrTypes() map[string]attr.Type (for use cases like this one) methods.

I'd probably wind up with something like:

type key struct {
	ThingOne types.String `tfsdk:"thing_one"`
	ThingTwo types.Int64  `tfsdk:"thing_two"`
}

func (o key) attrTypes() map[string]attr.Type {
	return map[string]attr.Type{
		"thing_one": types.StringType,
		"thing_two": types.Int64Type,
	}
}

func keySliceToList (ctx context.Context, keysSliceIn []key, diags *diag.Diagnostics) types.List {
	keys, d := types.ListValueFrom(ctx, types.ObjectType{AttrTypes: key{}.attrTypes()}, keysSliceIn)
	diags.Append(d...)
	return keys
}

This might not be the right way of doing things, but it's been a big improvement on what I had before.

@bflad
Copy link
Member

bflad commented Jun 14, 2023

Since the framework is working as expected for the original report, I'm going to close this issue. If you have documentation/error messaging suggestions or feature request proposals in this space, please open a new issue. We do have #695 pending to make the data handling documentation a lot more prescriptive for each schema type already. 👍

The approach above seems to be relatively popular though and some of the maintainer team's forward-looking code generation efforts to convert framework data to/from other Go types (typically an API SDK's types) are similar in nature. If you have general questions about implementation details when working with the framework, it might be good to check out or post in HashiCorp Discuss where the maintainers and other provider developers typically answer those.

@bflad bflad closed this as not planned Won't fix, can't repro, duplicate, stale Jun 14, 2023
@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 Jul 15, 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

4 participants