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

Error during provider startup for invalid provider configurations #206

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Mar 27, 2024

Fixes #205

This PR introduces checks for non-pointer (T instead of *T) values that cannot be safely round-tripped. So far, these include enum and struct values. Previously, users could specify a property like:

fieldName T `pulumi:"name,optional"`

This would cause confusing behavior at runtime if T transitively contained an enum without a pointer (or map, or array) indirection. To avoid a complicated rule set for where pointers are needed, we require a pointer where the ,optional was declared.

@iwahbe iwahbe requested review from blampe and a team March 27, 2024 20:01
@iwahbe iwahbe self-assigned this Mar 27, 2024
@iwahbe iwahbe force-pushed the iwahbe/205/prevent-invalid-struct-configurations branch from 283d9a3 to c6a378d Compare March 27, 2024 20:27
Fixes #205

This PR introduces checks for non-pointer (`T` instead of `*T`) values that cannot be
safely round-tripped. So far, these include enum and struct values. Previously, users
could specify a property like:

```
fieldName T `pulumi:"name,optional"`
```

This would cause confusing behavior at runtime if `T` transitively contained an enum
without a pointer (or map, or array) indirection. To avoid a complicated rule set for
where pointers are needed, we require a pointer where the `,optional` was declared.

Tests incoming shortly
@iwahbe iwahbe force-pushed the iwahbe/205/prevent-invalid-struct-configurations branch from c6a378d to d661a3a Compare March 28, 2024 00:04
@iwahbe iwahbe force-pushed the iwahbe/205/prevent-invalid-struct-configurations branch from d661a3a to 2926d3d Compare March 28, 2024 00:45
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at first glance, the feature is very compelling, probably slightly breaking? If users used optional X before now this will force upgrading to *X. But I think it's worth it. Make illegal states unrepresentable.

I admit I'm not very thoroughly reviewing the code itself, LMK if it would be helpful to take the time to dig in. New codebase for me.

@iwahbe iwahbe merged commit f1d1625 into main Apr 1, 2024
6 checks passed
@iwahbe iwahbe deleted the iwahbe/205/prevent-invalid-struct-configurations branch April 1, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[infer]: Prevent provider authors from describing optional fields as non-nullable types
2 participants