-
Notifications
You must be signed in to change notification settings - Fork 99
Add List Resource Config Validation #1178
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
Conversation
| "An unexpected error was encountered when converting the configuration from the protocol type. "+ | ||
| "This is always an issue in terraform-plugin-framework used to implement the provider and should be reported to the provider developers.\n\n"+ | ||
| "Please report this to the provider developer:\n\n"+ | ||
| "Missing schema.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 Here's a thought to consider – completely non-blocking for merging this PR.
While we're adding new kinds of schemas, I wonder if this diagnostic is precise enough to be actionable by a provider developer.
"Missing list resource schema for ``random_pet``" seems nice to have here. And also at odds with the universal message in fromproto6.Config().
So I'm curious how this diagnostic reads when it is rendered by the Terraform CLI and whether it includes precise context. If it's not precise, I suggest we adjust the Config diagnostic to be more flexible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, this error message is only for our team, and isn't actionable by the provider developer outside of reporting it to us:
terraform-plugin-framework/internal/fromproto5/config.go
Lines 25 to 26 in 3f40cbd
| // Panic prevention here to simplify the calling implementations. | |
| // This should not happen, but just in case. |
The only way we'd hit this error is if there was a bug in Terraform core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, no action here 🏁
| "An unexpected error was encountered when converting the configuration from the protocol type. "+ | ||
| "This is always an issue in terraform-plugin-framework used to implement the provider and should be reported to the provider developers.\n\n"+ | ||
| "Please report this to the provider developer:\n\n"+ | ||
| "Missing schema.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, this error message is only for our team, and isn't actionable by the provider developer outside of reporting it to us:
terraform-plugin-framework/internal/fromproto5/config.go
Lines 25 to 26 in 3f40cbd
| // Panic prevention here to simplify the calling implementations. | |
| // This should not happen, but just in case. |
The only way we'd hit this error is if there was a bug in Terraform core
| sort.Slice(got.ListResources, func(i int, j int) bool { | ||
| return got.ListResources[i].TypeName < got.ListResources[j].TypeName | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might have noticed random CI failures for actions because I forgot to add this 😆
It'll be added in #1186, but in the mean time you can just re-run the CI and it'll eventually pass 😆
stephybun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rainkwan LGTM 🌿
Related Issue
Description
Added list resource validation and some missing tests
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No