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

Adding CustomType to a ListAttribute changes generated description to List of Dynamic #342

Closed
1 task done
ewbankkit opened this issue Mar 7, 2024 · 6 comments
Closed
1 task done
Labels
bug Something isn't working

Comments

@ewbankkit
Copy link
Contributor

ewbankkit commented Mar 7, 2024

Terraform CLI and terraform-plugin-docs Versions

% tfplugindocs -v
tfplugindocs Version dev

In go.mod: github.com/hashicorp/terraform-plugin-docs v0.18.0

Provider Code

"health_check_arns": schema.ListAttribute{ /*START ATTRIBUTE*/
                        ElementType: types.StringType,
                        CustomType:  cctypes.MultisetType,
                        Description: "The Amazon Resource Names (ARNs) of the health check to associate with the protection.",
                        Optional:    true,
                        Computed:    true,
                        Validators: []validator.List{ /*START VALIDATORS*/
                                listvalidator.SizeAtMost(1),
                                listvalidator.ValueStringsAre(
                                        stringvalidator.LengthBetween(1, 2048),
                                ),
                        }, /*END VALIDATORS*/
                        PlanModifiers: []planmodifier.List{ /*START PLAN MODIFIERS*/
                                listplanmodifier.UseStateForUnknown(),
                        }, /*END PLAN MODIFIERS*/
                }, /*END ATTRIBUTE*/

Expected Behavior

Generate:

health_check_arns (List of String) The Amazon Resource Names (ARNs) of the health check to associate with the protection.

Actual Behavior

Generated:

health_check_arns (List of Dynamic) The Amazon Resource Names (ARNs) of the health check to associate with the protection.

Steps to Reproduce

  1. tfplugindocs generate --flags

How much impact is this issue causing?

Medium

Additional Information

Relates hashicorp/terraform-provider-awscc#1463.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ewbankkit ewbankkit added the bug Something isn't working label Mar 7, 2024
@bflad
Copy link
Member

bflad commented Mar 7, 2024

Drive-by note: The framework website documentation does not go into too many of the details about creating a custom collection type, but in that situation, the custom type "owns" the entire type definition (e.g. the attribute ElementType is not effective). You can see some of that implementation at play here:

https://github.com/hashicorp/terraform-plugin-framework/blob/4ca111592bfec2bec39662261ad147002bc56c34/resource/schema/list_attribute.go#L199-L208

Maybe the framework could/should raise an attribute implementation error when it detects both CustomType and ElementType being set.

In your situation it might be a matter of doing something like (removing the exported MultisetType variable and instead exporting the type directly):

CustomType:  cctypes.MultisetType{basetypes.ListType{ElementType: basetypes.StringType}},

That should cause the GetProviderSchema RPC to return the "full" type information for the attribute, which is what terraform-plugin-docs eventually uses.

@bflad
Copy link
Member

bflad commented Mar 7, 2024

For what its worth about:

Maybe the framework could/should raise an attribute implementation error when it detects both CustomType and ElementType being set.

Could also be turned around as a framework feature request that attributes could automatically do what you tried to do there with both fields. There is the attr.TypeWithElementType interface already available that could potentially help in this situation if the CustomType implemented it and the framework attribute GetType() logic checked for it. While it would likely solve this exact situation, I'm not sure off the top of my head whether just adjusting that GetType() logic would fully enable correct type handling everywhere else in the framework though. We would need to verify. 😄

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Mar 7, 2024

@bflad Thanks for the explanation.
I'll close this Issue once I've verified the solution.

@ewbankkit
Copy link
Contributor Author

ewbankkit commented Mar 7, 2024

+1 to

Maybe the framework could/should raise an attribute implementation error when it detects both CustomType and ElementType being set.

I'll add an Issue in https://github.com/hashicorp/terraform-plugin-framework.

@ewbankkit
Copy link
Contributor Author

Yes, removing the ElementType (and reworking MultisetType) works. Thanks.

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 May 23, 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

2 participants