Validate resources only during build time

[VenelinMartinov]

Should address pulumi/pulumi-aws#3330 after aws picks this up.

The resource validation runs the SchemaFuncs which is quite slow for some resources. This PR makes this run only during tfgen, to save time and memory during runtime.

Tested in pulumi/pulumi-aws#3368 but I CI doesn't seem to run for some reason.

[t0yv0]

// InternalValidate should be called to validate the structure
// of the provider.
//
// This should be called in a unit test for any provider to verify
// before release that a provider is properly configured for use with
// this library.

This is interesting.. Can you help me understand why this is called at runtime by us at all? Stack trace perhaps from the panic?

[t0yv0]

This is a really gnarly approach and I'm a little sad it's getting copied :) This for example does not detect unit test context properly. We don't have context.Context to propagate an option with but perhaps we could use a public global var that can be set by the unit test and TFGEN to enable the check.

[t0yv0]

Apart from the implementation nits I'm very eager to land this.

[VenelinMartinov]

This is interesting.. Can you help me understand why this is called at runtime by us at all? Stack trace perhaps from the panic?

Yeah, here is the full stack trace.

panic: WAFV2 resource schema func called!

goroutine 14 [running]:
github.com/hashicorp/terraform-provider-aws/internal/service/wafv2.ResourceIPSet.func2()
	/Users/vvm/code/pulumi-aws/upstream/internal/service/wafv2/ip_set.go:59 +0x2c
github.com/hashicorp/terraform-provider-aws/shim.markTagsAllNotComputedForResource.func1()
	/Users/vvm/code/pulumi-aws/upstream/shim/shim.go:60 +0x38
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).SchemaMap(...)
	/Users/vvm/code/terraform-plugin-sdk/helper/schema/resource.go:655
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).InternalValidate(0x140022f5500, 0x0, 0x1)
	/Users/vvm/code/terraform-plugin-sdk/helper/schema/resource.go:1148 +0x9c
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Provider).InternalValidate(0x14000c0a6c0)
	/Users/vvm/code/terraform-plugin-sdk/helper/schema/provider.go:148 +0x208
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Provider).Validate(0x14000c0a6c0, 0x9cb?)
	/Users/vvm/code/terraform-plugin-sdk/helper/schema/provider.go:224 +0x24
github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.v2Provider.Validate({0x14000c0a6c0?, {0x1400108a470?, 0x1130e05b8?, 0x14000f11680?}}, {0x0?, 0x0?}, {0x112f13760?, 0x14002b0a330?})
	/Users/vvm/code/pulumi-terraform-bridge/pkg/tfshim/sdk-v2/provider.go:82 +0x54
github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.validateProviderConfig({0x11308a770?, 0x14002b0a180?}, {0x14000d72ac0, 0x40}, 0x14000c39980, {0x112f13760?, 0x14002b0a330})
	/Users/vvm/code/pulumi-terraform-bridge/pkg/tfbridge/provider.go:559 +0x210
github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.(*Provider).CheckConfig(0x14000c39980, {0x11308a1c0, 0x11aa9ff80}, 0x14002b00000)
	/Users/vvm/code/pulumi-terraform-bridge/pkg/tfbridge/provider.go:457 +0x6e8
github.com/pulumi/pulumi-terraform-bridge/x/muxer.(*muxer).CheckConfig.func1()
	/Users/vvm/code/pulumi-terraform-bridge/x/muxer/muxer.go:127 +0x74
github.com/pulumi/pulumi-terraform-bridge/x/muxer.asyncJoin[...].func1()
	/Users/vvm/code/pulumi-terraform-bridge/x/muxer/muxer.go:583 +0x48
created by github.com/pulumi/pulumi-terraform-bridge/x/muxer.asyncJoin[...] in goroutine 13
	/Users/vvm/code/pulumi-terraform-bridge/x/muxer/muxer.go:582 +0x6c

We call the tf provider's Validate method:

terraform-plugin-sdk/helper/schema/provider.go

Lines 223 to 238 in ecba526

	func (p *Provider) Validate(c *terraform.ResourceConfig) diag.Diagnostics {
	if err := p.InternalValidate(); err != nil {
	return []diag.Diagnostic{
	{
	Severity: diag.Error,
	Summary: "InternalValidate",
	Detail: fmt.Sprintf("Internal validation of the provider failed! This is always a bug\n"+
	"with the provider itself, and not a user issue. Please report\n"+
	"this bug:\n\n%s", err),
	},
	}
	}
	return schemaMap(p.Schema).Validate(c)
	}

from

https://github.com/pulumi/pulumi-terraform-bridge/blob/e91ed7ee525a89502467902ae4f8e52d1e2fa694/pkg/tfbridge/provider.go#L559

It looks like the tf Validate method then calls the InternalValidate, which doesn't look like is necessary during runtime since it only deals with the schema. It then validates the config.

I'll change the PR to only call the InternalValidate during tfgen.

[VenelinMartinov]

I'll add a test for this behaviour in the bridge

[iwahbe]

This change looks good.

Is there a reason why we are also not skipping schemaMap(p.Schema).Validate(c) on line (234)?

[VenelinMartinov]

I believe that validates the config passed to the provider, which depends on runtime behaviour, so sounds useful

[VenelinMartinov]

Test added in pulumi/pulumi-terraform-bridge#1669, I'm going to merge this so I can get the right dependencies.

[t0yv0]

This is VERY unfortunate that their code calls InternalValidate which was intended for the static world of unit tests as part of validating the actual configuration data.

[t0yv0]

I like how this change turned out. That is perfect. This internal validation should not be the default behavior. Thank you!