Ensure InternalValidate is called at build time #1852
Assignees
Labels
impact/regression
Something that used to work, but is now broken
kind/bug
Some behavior is incorrect or out of spec
resolution/fixed
This issue was fixed
Comments
t0yv0
added
needs-triage
|
Note also that the referenced links validate SchemaMap for the provider configuration only! However, the schemas we are interested in as well (or mostly) are SchemaMap declarations for resources and datasources. We need to make sure that if there are any errors in those they are detected at build time. |
|
Note that the provider |
|
Unfortunately I think we've regressed here in #1669 I injected a panic in the InternalValidate method in the plugin-sdk but running I'll address this next. |
VenelinMartinov
added
the
impact/regression
|
Yeah perhaps some e2e test to hold this in place can help. Thank you! |
This was referenced Apr 11, 2024
VenelinMartinov
added a commit
to pulumi/pulumi-aws
that referenced
this issue
Apr 12, 2024
In #3638 - we introduced a diff customizer to change the schema as an alternative to patching. The change does not pass InternalValidate but because of pulumi/pulumi-terraform-bridge#1852 it was not flagged up. After turning InternalValidate back on (pulumi/pulumi-terraform-bridge#1852) we discovered an issue with this parameter: ``` make tfgen_no_deps (cd provider && go build -p 2 -o /Users/vvm/code/pulumi-aws/bin/pulumi-tfgen-aws -ldflags "-X github.com/pulumi/pulumi-aws/provider/v6/pkg/version.Version=6.29.1+dirty" github.com/pulumi/pulumi-aws/provider/v6/cmd/pulumi-tfgen-aws) /Users/vvm/code/pulumi-aws/bin/pulumi-tfgen-aws schema --out provider/cmd/pulumi-resource-aws Errors occurred: [InternalValidate: Internal validation of the provider failed! This is always a bug with the provider itself, and not a user issue. Please report this bug: resource aws_db_parameter_group: apply_method: Default must be nil if computed] make: *** [tfgen_no_deps] Error 255 ``` This should fix it by also setting the Default to nil in the schema. To keep the current behaviour, we set the Default in the pulumi overlay instead.
t0yv0
added a commit
that referenced
this issue
Apr 12, 2024
Fixes #1790 by building a rapid generator for schemas and associated values. Large-ish problem 1: I do not have it figured out how to test unknown values. TF literals as unknown values are forbidden and do not make sense. We might need a helper resource so that testing unknown values generates references to an output of the helper resource. This is logged for future work. Large-ish problem 2: iteration is pretty slow (x-proc). Normal n=100 rapid tests can take up to 10min. Could try batching so several resources are tried in one shot say 100 resources. Large-ish problem 3: I'm not sure if no-op Update and Create implementations are acceptable. There is something to testing Computed attributes where provider has to set values. Possibly Update also needs to set values? Possibly not. Small problems: - [x] Using TF JSON syntax didn't handle null/empty correctly; that is now discarded, using actual HCL syntax - [x] TF representations are difficult to visualize in failing tests and difficult to assert against - [x] Lots of lost-in-translation papercuts possible between representations (cty.Value, resource.PropertyValue, tftypes.Value) - [x] this requires a change to providertest to abstract from testing.T so we can pass rapid.T - [x] it's very hard to disable annoying TF logging, using env vars for now We are starting to find bugs and discrepancies from this work: - #1856 panic corner-case - #1852 need to InternalValidate - #1828 Future work: - #1856 - #1857 - #1858 - #1859 - #1860 - #1861 - #1862 - #1863 - #1864 - #1865 - #1866 - #1867
VenelinMartinov
added a commit
that referenced
this issue
Apr 15, 2024
fixes #1852 #1669 removed the runtime calls to `InternalValidate` to speed up provider startup. This assumed it was called during tfgen but it turns out it was not. This introduces an explicit call to `InternalValidate` during tfgen. Thanks to @t0yv0 for prompting the investigation and @iwahbe for helping with the muxer. To facilitate that, I've added `InternalValidate` to the shim Provider interface. Technically breaking but seems unlikely to be depended on externally. Calling `Validate` does not do the job as that requires passing in a valid config which is not available during tfgen. I've added an e2e test for an sdk provider, need to add one for a pf provider. --------- Co-authored-by: Ian Wahbe <ian@wahbe.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What happened?
Schemas for sdk-v2 based providers are subject to InternalValidate that rejects invalid schemas. We would like to assume that a schema that the bridge needs to handle passes the validation so we do not need to support degenerate combinations. To make this safe, ensure that bridge providers run this validation at build-time and reject non-conforming schemas.
Example
https://github.com/pulumi/terraform-plugin-sdk/blob/4f60ee4e2975c25b88b392e69c87551bb0e26dfc/helper/schema/provider.go#L225
pulumi-terraform-bridge/pkg/tfbridge/provider.go
Line 567 in cffb553
pulumi-terraform-bridge/pkg/tfbridge/provider.go
Line 465 in cffb553
pulumi-terraform-bridge/pkg/tfgen/main.go
Line 38 in cffb553
Looks like tfgen does call schema.RunProviderInternalValidation = true which is good but need to also make sure that tfgen executes provider.Validate which in turn executes InternalValidate, as we were not sure.
Output of
pulumi aboutN/A
Additional context
No response
Contributing
Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
The text was updated successfully, but these errors were encountered: