diff --git a/pkg/tfbridge/provider.go b/pkg/tfbridge/provider.go old mode 100644 new mode 100755 index a2f071938..787f0f217 --- a/pkg/tfbridge/provider.go +++ b/pkg/tfbridge/provider.go @@ -74,6 +74,7 @@ type Provider struct { pulumiSchema []byte // the JSON-encoded Pulumi schema. pulumiSchemaSpec *pschema.PackageSpec memStats memStatCollector + hasTypeErrors map[resource.URN]struct{} } // MuxProvider defines an interface which must be implemented by providers @@ -182,6 +183,46 @@ func (e CheckFailureError) Error() string { return fmt.Sprintf("CheckFailureErrors %s", e.Failures) } +// callWithRecover is a generic function that takes a resource URN, a recovery +// function, and a callable function as parameters. It attempts to call the +// provided callable function and returns its results. If the callable function +// panics, callWithRecover will recover from the panic and call the recovery +// function with the resource URN and the panic value. The recovery function is +// expected to convert the panic value into an error, which is then returned by +// callWithRecover. This function is useful for handling panics in a controlled +// manner and converting them into errors. +func callWithRecover[T any]( + urn resource.URN, + rec func(resource.URN, any) error, + call func() (T, error), +) (_ T, err error) { + defer func() { + if r := recover(); r != nil { + err = rec(urn, r) + contract.Assertf(err != nil, "Panic handlers must return an error") + } + }() + + return call() +} + +// if we have type errors that were generated during check +// we don't want to log the panic. In the future these type errors +// will hard fail during check and we will never make it to create +// +// We do our best to restrict catching panics to where the panic will +// occur. The type checking will catch panics that occur as part +// of the `Diff` function +func (p *Provider) recoverOnTypeError(urn resource.URN, payload any) error { + if _, ok := p.hasTypeErrors[urn]; !ok { + panic(payload) + } + if err, ok := payload.(error); ok { + return fmt.Errorf("panicked: %w", err) + } + return fmt.Errorf("panicked: %#v", payload) +} + // NewProvider creates a new Pulumi RPC server wired up to the given host and wrapping the given Terraform provider. func NewProvider(ctx context.Context, host *provider.HostClient, module, version string, tf shim.Provider, info ProviderInfo, pulumiSchema []byte, @@ -200,13 +241,14 @@ func newProvider(ctx context.Context, host *provider.HostClient, module, version string, tf shim.Provider, info ProviderInfo, pulumiSchema []byte, ) *Provider { p := &Provider{ - host: host, - module: module, - version: version, - tf: tf, - info: info, - config: tf.Schema(), - pulumiSchema: pulumiSchema, + host: host, + module: module, + version: version, + tf: tf, + info: info, + config: tf.Schema(), + pulumiSchema: pulumiSchema, + hasTypeErrors: make(map[resource.URN]struct{}), } ctx = p.loggingContext(ctx, "") p.initResourceMaps() @@ -802,9 +844,10 @@ func (p *Provider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (*pul if schema != nil { iv := NewInputValidator(urn, *schema) typeFailures := iv.ValidateInputs(t, news) - if typeFailures != nil { + if len(typeFailures) > 0 { + p.hasTypeErrors[urn] = struct{}{} logger.Warn("Type checking failed: ") - for _, e := range *typeFailures { + for _, e := range typeFailures { if validateShouldError { pp := NewCheckFailurePath(schemaMap, schemaInfos, e.ResourcePath) cf := NewCheckFailure(MiscFailure, e.Reason, &pp, urn, false, p.module, schemaMap, schemaInfos) @@ -954,8 +997,10 @@ func (p *Provider) Diff(ctx context.Context, req *pulumirpc.DiffRequest) (*pulum ic := newIgnoreChanges(ctx, schema, fields, olds, news, req.GetIgnoreChanges()) - diff, err := p.tf.Diff(ctx, res.TFName, state, config, shim.DiffOptions{ - IgnoreChanges: ic, + diff, err := callWithRecover(urn, p.recoverOnTypeError, func() (shim.InstanceDiff, error) { + return p.tf.Diff(ctx, res.TFName, state, config, shim.DiffOptions{ + IgnoreChanges: ic, + }) }) if err != nil { return nil, errors.Wrapf(err, "diffing %s", urn) @@ -1098,11 +1143,13 @@ func (p *Provider) Create(ctx context.Context, req *pulumirpc.CreateRequest) (*p return nil, errors.Errorf("error decoding timeout: %s", err) } - diff, err := p.tf.Diff(ctx, res.TFName, nil, config, shim.DiffOptions{ - TimeoutOptions: shim.TimeoutOptions{ - ResourceTimeout: timeouts, - TimeoutOverrides: newTimeoutOverrides(shim.TimeoutCreate, req.Timeout), - }, + diff, err := callWithRecover(urn, p.recoverOnTypeError, func() (shim.InstanceDiff, error) { + return p.tf.Diff(ctx, res.TFName, nil, config, shim.DiffOptions{ + TimeoutOptions: shim.TimeoutOptions{ + ResourceTimeout: timeouts, + TimeoutOverrides: newTimeoutOverrides(shim.TimeoutCreate, req.Timeout), + }, + }) }) if err != nil { return nil, errors.Wrapf(err, "diffing %s", urn) @@ -1324,12 +1371,14 @@ func (p *Provider) Update(ctx context.Context, req *pulumirpc.UpdateRequest) (*p return nil, errors.Errorf("error decoding timeout: %s", err) } - diff, err := p.tf.Diff(ctx, res.TFName, state, config, shim.DiffOptions{ - IgnoreChanges: ic, - TimeoutOptions: shim.TimeoutOptions{ - TimeoutOverrides: newTimeoutOverrides(shim.TimeoutUpdate, req.Timeout), - ResourceTimeout: timeouts, - }, + diff, err := callWithRecover(urn, p.recoverOnTypeError, func() (shim.InstanceDiff, error) { + return p.tf.Diff(ctx, res.TFName, state, config, shim.DiffOptions{ + IgnoreChanges: ic, + TimeoutOptions: shim.TimeoutOptions{ + TimeoutOverrides: newTimeoutOverrides(shim.TimeoutUpdate, req.Timeout), + ResourceTimeout: timeouts, + }, + }) }) if err != nil { return nil, errors.Wrapf(err, "diffing %s", urn) diff --git a/pkg/tfbridge/provider_test.go b/pkg/tfbridge/provider_test.go index 6dd6061da..77ec0ee14 100644 --- a/pkg/tfbridge/provider_test.go +++ b/pkg/tfbridge/provider_test.go @@ -1260,6 +1260,7 @@ func TestCheckWarnings(t *testing.T) { config: shimv2.NewSchemaMap(p.Schema), pulumiSchema: []byte("hello"), // we only check whether this is nil in type checking pulumiSchemaSpec: pulumiSchemaSpec, + hasTypeErrors: make(map[resource.URN]struct{}), resources: map[tokens.Type]Resource{ "ExampleResource": { TF: shimv2.NewResource(p.ResourcesMap["example_resource"]), diff --git a/pkg/tfbridge/tests/provider_test.go b/pkg/tfbridge/tests/provider_test.go index b84b3bd96..b9e1d8d00 100644 --- a/pkg/tfbridge/tests/provider_test.go +++ b/pkg/tfbridge/tests/provider_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/pulumi/pulumi/sdk/v3/go/common/util/contract" pulumirpc "github.com/pulumi/pulumi/sdk/v3/proto/go" + "github.com/stretchr/testify/assert" "github.com/pulumi/providertest/replay" "github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge" @@ -283,6 +284,385 @@ func TestReproMinimalDiffCycle(t *testing.T) { }`) } +func TestValidateInputsPanic(t *testing.T) { + ctx := context.Background() + p := newTestProvider(ctx, tfbridge.ProviderInfo{ + P: shimv2.NewProvider(&schema.Provider{ + Schema: map[string]*schema.Schema{}, + ResourcesMap: map[string]*schema.Resource{ + "example_resource": { + Schema: map[string]*schema.Schema{ + "network_configuration": { + Type: schema.TypeList, + Optional: true, + MaxItems: 1, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "assign_public_ip": { + Type: schema.TypeBool, + Optional: true, + Default: false, + }, + "security_groups": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + "subnets": { + Type: schema.TypeSet, + Required: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + }, + }, + }, + }, + }, + }, + }, shimv2.WithDiffStrategy(shimv2.PlanState)), + Name: "testprov", + ResourcePrefix: "example", + Resources: map[string]*tfbridge.ResourceInfo{ + "example_resource": {Tok: "testprov:index:ExampleResource"}, + }, + }, newTestProviderOptions{}) + + t.Run("diff_panic", func(t *testing.T) { + assert.Panics(t, func() { + replay.ReplaySequence(t, p, ` + [ + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres", + "olds": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "news": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + }, + "response": { + } + } + ] + `) + }) + }) + + t.Run("diff_no_panic", func(t *testing.T) { + replay.ReplaySequence(t, p, fmt.Sprintf(` + [ + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres", + "randomSeed": "ZCiVOcvG/CT5jx4XriguWgj2iMpQEb8P3ZLqU/AS2yg=", + "olds": { + "__defaults": [] + }, + "news": { + "networkConfiguration": { + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + }, + "response": { + "inputs": { + "__defaults": [], + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + } + }, + { + "method": "/pulumirpc.ResourceProvider/Diff", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres", + "olds": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "news": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + }, + "response": { + }, + "errors": [ + "%s" + ] + } + ] + `, "diffing urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres: "+ + `panicked: \"value has no attribute of that name\"`)) + }) + + t.Run("update_panic", func(t *testing.T) { + assert.Panics(t, func() { + replay.ReplaySequence(t, p, ` + [ + { + "method": "/pulumirpc.ResourceProvider/Update", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres1", + "olds": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "news": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "preview": true + }, + "response": { + } + } + ] + `) + + }) + }) + + t.Run("update_no_panic", func(t *testing.T) { + replay.ReplaySequence(t, p, fmt.Sprintf(` + [ + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres2", + "randomSeed": "ZCiVOcvG/CT5jx4XriguWgj2iMpQEb8P3ZLqU/AS2yg=", + "olds": { + "__defaults": [] + }, + "news": { + "networkConfiguration": { + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + }, + "response": { + "inputs": { + "__defaults": [], + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + } + }, + { + "method": "/pulumirpc.ResourceProvider/Update", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres2", + "olds": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "news": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "preview": true + }, + "response": { + }, + "errors": [ + "%s" + ] + } + ] + `, "diffing urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres2: "+ + `panicked: \"value has no attribute of that name\"`)) + + }) + + t.Run("create_no_panic", func(t *testing.T) { + replay.ReplaySequence(t, p, fmt.Sprintf(` + [ + { + "method": "/pulumirpc.ResourceProvider/Check", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres3", + "randomSeed": "ZCiVOcvG/CT5jx4XriguWgj2iMpQEb8P3ZLqU/AS2yg=", + "olds": { + "__defaults": [] + }, + "news": { + "networkConfiguration": { + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + }, + "response": { + "inputs": { + "__defaults": [], + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + } + } + }, + { + "method": "/pulumirpc.ResourceProvider/Create", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres3", + "properties": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "preview": true + }, + "response": { + }, + "errors": [ + "%s" + ] + } + ] + `, "diffing urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres3: "+ + `panicked: \"value has no attribute of that name\"`)) + }) + + t.Run("create_panic", func(t *testing.T) { + assert.Panics(t, func() { + + replay.ReplaySequence(t, p, ` + [ + { + "method": "/pulumirpc.ResourceProvider/Create", + "request": { + "urn": "urn:pulumi:dev::teststack::testprov:index:ExampleResource::exres4", + "properties": { + "networkConfiguration": { + "__defaults": [ + "assignPublicIp" + ], + "assignPublicIp": false, + "securityGroups": [ + "04da6b54-80e4-46f7-96ec-b56ff0331ba9" + ], + "subnets": "[\"first\",\"second\"]" + } + }, + "preview": true + }, + "response": { + } + } + ] + `) + }) + }) + +} + func nilSink() diag.Sink { nilSink := diag.DefaultSink(io.Discard, io.Discard, diag.FormatOptions{ Color: colors.Never, diff --git a/pkg/tfbridge/validate_input_types.go b/pkg/tfbridge/validate_input_types.go index f9ee393bc..161a33a23 100644 --- a/pkg/tfbridge/validate_input_types.go +++ b/pkg/tfbridge/validate_input_types.go @@ -258,7 +258,7 @@ func (v *PulumiInputValidator) getType(typeRef string) *pschema.ObjectTypeSpec { // ValidateInputs will validate a set of inputs against the pulumi schema. It will // return a list of type failures if any are found -func (v *PulumiInputValidator) ValidateInputs(resourceToken tokens.Type, inputs resource.PropertyMap) *[]TypeFailure { +func (v *PulumiInputValidator) ValidateInputs(resourceToken tokens.Type, inputs resource.PropertyMap) []TypeFailure { resourceSpec, knownResourceSpec := v.schema.Resources[string(resourceToken)] if !knownResourceSpec { return nil @@ -269,7 +269,7 @@ func (v *PulumiInputValidator) ValidateInputs(resourceToken tokens.Type, inputs resource.PropertyPath{}, ) if len(failures) > 0 { - return &failures + return failures } return nil diff --git a/pkg/tfbridge/validate_input_types_test.go b/pkg/tfbridge/validate_input_types_test.go index 838a67331..93bbdf9d7 100644 --- a/pkg/tfbridge/validate_input_types_test.go +++ b/pkg/tfbridge/validate_input_types_test.go @@ -805,14 +805,14 @@ func TestValidateInputType_objects(t *testing.T) { failures := v.ValidateInputs(tokens.Type("pkg:mod:ResA"), resource.PropertyMap{ resource.PropertyKey(tc.name): tc.input, }) - if failures != nil && len(*failures) != len(tc.failures) { - t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(*failures), *failures) + if failures != nil && len(failures) != len(tc.failures) { + t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(failures), failures) } if len(tc.failures) > 0 { if failures == nil { t.Fatalf("expected failures, got none") } else { - assert.Equal(t, tc.failures, *failures) + assert.Equal(t, tc.failures, failures) } } }) @@ -1305,14 +1305,14 @@ func TestValidateInputType_arrays(t *testing.T) { failures := v.ValidateInputs(tokens.Type("pkg:mod:ResA"), resource.PropertyMap{ resource.PropertyKey(tc.name): tc.input, }) - if failures != nil && len(*failures) != len(tc.failures) { - t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(*failures), *failures) + if failures != nil && len(failures) != len(tc.failures) { + t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(failures), failures) } if len(tc.failures) > 0 { if failures == nil { t.Fatalf("expected failures, got none") } else { - assert.Equal(t, tc.failures, *failures) + assert.Equal(t, tc.failures, failures) } } }) @@ -1626,14 +1626,14 @@ func TestValidateInputType_toplevel(t *testing.T) { failures := v.ValidateInputs(tokens.Type("pkg:mod:ResA"), resource.PropertyMap{ resource.PropertyKey(tc.name): tc.input, }) - if failures != nil && len(*failures) != len(tc.failures) { - t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(*failures), *failures) + if failures != nil && len(failures) != len(tc.failures) { + t.Fatalf("%d failures, got %d: %v", len(tc.failures), len(failures), failures) } if len(tc.failures) > 0 { if failures == nil { t.Fatalf("expected failures, got none") } else { - assert.Equal(t, tc.failures, *failures) + assert.Equal(t, tc.failures, failures) } } })