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

Type-check incoming values against the Pulumi Package Schema #1328

Closed
2 tasks
t0yv0 opened this issue Aug 4, 2023 · 2 comments
Closed
2 tasks

Type-check incoming values against the Pulumi Package Schema #1328

t0yv0 opened this issue Aug 4, 2023 · 2 comments
Assignees
Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Aug 4, 2023

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Bridged providers receive resource.PropertyValue user data from the engine encoding resource or function arguments that might not have the expected shape, in the sense that it mismatches the type published in Pulumi Package Schema. In these cases the transformation to the Terraform-expected domain may result in data of unexpected shape passed down to the TF internals. This leads to panics.

This is especially likely to happen when using languages that do not guarantee type-correctness or where it can be bypassed, such as TypeScript and Python.

Link to related issues:

The suggested enhancement is to check the incoming data against the schema as soon as the provider receives it, and provide a nicely formatted error message to the user, closer to Pulumi terms, of how to fix the program.

Ideally this feature is built as a layer independent from the bridge as nothing here assumes that a provider is a bridged provider; it could be implemented as at a GRPC transforming layer or as middleware.

Affected area/feature

Tasks

  1. kind/task
  2. kind/enhancement
@t0yv0 t0yv0 added needs-triage Needs attention from the triage team kind/enhancement Improvements or new features area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers and removed needs-triage Needs attention from the triage team labels Aug 4, 2023
@t0yv0 t0yv0 changed the title Type-Check incoming values against the Pulumi Package Schema Type-check incoming values against the Pulumi Package Schema Aug 4, 2023
@t0yv0
Copy link
Member Author

t0yv0 commented Dec 6, 2023

Another GCP issue where the error message could have been better with a type-check pass: pulumi/pulumi-gcp#1377

@t0yv0
Copy link
Member Author

t0yv0 commented Dec 7, 2023

pulumi/pulumi-aws#3092 could have gotten a much more lucid error message here.

@t0yv0 t0yv0 added size/S Estimated effort to complete (1-2 days). impact/usability Something that impacts users' ability to use the product easily and intuitively labels Jan 22, 2024
@corymhall corymhall self-assigned this Mar 18, 2024
corymhall added a commit that referenced this issue Mar 27, 2024
This adds an initial type checking implementation to `pkg/tfbridge`. The
actually type checker is pretty generic and only has a dependency on
`pulumi/pulumi` so it would be possible to pull this out into a separate
library as a general type checker.

**Feature Support**
- [X] required properties
- [X] property names
- [X] `string` types
- [X] `number` types
- [X] `object` types
- [X] `array` types
- [X] `null` types
- [X] `bool` types
- [X] `output` types
- [X] `secret` types
- [ ] `asset` types
- [ ] `archive` types
- [ ] `resourceReference` types
- [ ] String Enums
- [ ] Number Enums
    - *It knows about enums, but it does not validate enum values since
      enums are not exhaustive*

Along with the type checker it will also build an error message that
contains the path to the property.

re #1328
corymhall added a commit that referenced this issue Mar 27, 2024
This adds an initial type checking implementation to `pkg/tfbridge`. The
actually type checker is pretty generic and only has a dependency on
`pulumi/pulumi` so it would be possible to pull this out into a separate
library as a general type checker.

**Feature Support**
- [X] required properties
- [X] property names
- [X] `string` types
- [X] `number` types
- [X] `object` types
- [X] `array` types
- [X] `null` types
- [X] `bool` types
- [X] `output` types
- [X] `secret` types
- [ ] `asset` types
- [ ] `archive` types
- [ ] `resourceReference` types
- [ ] String Enums
- [ ] Number Enums
    - *It knows about enums, but it does not validate enum values since
      enums are not exhaustive*

Along with the type checker it will also build an error message that
contains the path to the property.

re #1328
corymhall added a commit that referenced this issue Mar 27, 2024
This adds an initial type checking implementation to `pkg/tfbridge`. The
actually type checker is pretty generic and only has a dependency on
`pulumi/pulumi` so it would be possible to pull this out into a separate
library as a general type checker.

**Feature Support**
- [X] required properties
- [X] property names
- [X] `string` types
- [X] `number` types
- [X] `object` types
- [X] `array` types
- [X] `null` types
- [X] `bool` types
- [X] `output` types
- [X] `secret` types
- [ ] `asset` types
- [ ] `archive` types
- [ ] `resourceReference` types
- [ ] String Enums
- [ ] Number Enums
    - *It knows about enums, but it does not validate enum values since
      enums are not exhaustive*

Along with the type checker it will also build an error message that
contains the path to the property.

re #1328
corymhall added a commit that referenced this issue Apr 23, 2024
This adds an initial type checking implementation to `pkg/tfbridge`. The
actually type checker is pretty generic and only has a dependency on
`pulumi/pulumi` so it would be possible to pull this out into a separate
library as a general type checker.

**Feature Support**
- [ ] required properties
- [X] property names
- [X] `string` types
- [X] `number` types
- [X] `object` types
- [X] `array` types
- [X] `null` types
- [X] `bool` types
- [ ] `output` types
- [ ] `secret` types
- [ ] `asset` types
- [ ] `archive` types
- [ ] `resourceReference` types
- [ ] String Enums
- [ ] Number Enums - *It knows about enums, but it does not validate
enum values since enums are not exhaustive*

Along with the type checker it will also build an error message that
contains the path to the property.

**Todo**
- [ ] Add to `pf/tfbridge` (can be done in a later pr)
- [x] Add tests that hit the new type checker at the provider level
- [x] How do we roll this out?
- By default this will only warn users, but can override with an env var

re #1328
@iwahbe iwahbe added the resolution/fixed This issue was fixed label Jul 1, 2024
@iwahbe iwahbe closed this as completed Jul 1, 2024
@mjeffryes mjeffryes added this to the 0.107 milestone Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tfbridge Issues in pkg/tfbridge, which provides interop between Pulumi and TF providers impact/usability Something that impacts users' ability to use the product easily and intuitively kind/enhancement Improvements or new features resolution/fixed This issue was fixed size/S Estimated effort to complete (1-2 days).
Projects
None yet
Development

No branches or pull requests

4 participants