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

Provider panics when resource creation fails without a valid state #2706

Closed
VenelinMartinov opened this issue Dec 10, 2024 · 1 comment · Fixed by #2707
Closed

Provider panics when resource creation fails without a valid state #2706

VenelinMartinov opened this issue Dec 10, 2024 · 1 comment · Fixed by #2707
Assignees
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed

Comments

@VenelinMartinov
Copy link
Contributor

What happened?

Likely a consequence of #2695

The provider panics when creating a resource fails. Hit this on GCP 8.10.1 with bridge version 3.97.0

Example

    panic: interface conversion: interface {} is nil, not map[string]interface {}
    goroutine 89 [running]:
    github.com/hashicorp/go-cty/cty.Value.GetAttr({{{0x1081fab10?, 0x1400275fa08?}}, {0x0?, 0x0?}}, {0x1068f1b0e, 0x2})
    	/home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/cty/value_ops.go:711 +0x25c
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v2.(*v2InstanceState2).ID(0x1400184be60?)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfshim/sdk-v2/provider2.go:92 +0x3c
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.(*Provider).Create(0x14000755988, {0x1081fa0b8?, 0x14002341620?}, 0x140072c4e00)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/provider.go:1349 +0x728
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/x/muxer.(*muxer).Create.func1({0x10821bda0?, 0x14000755988?})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/x/muxer/muxer.go:376 +0x3c
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/x/muxer.resourceMethod[...](0x14006032240?, 0x107f4e5c0, 0x14007333698?)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/x/muxer/muxer.go:335 +0xc4
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/x/muxer.(*muxer).Create(0x10cf64b60?, {0x1081fa0b8?, 0x14002341620?}, 0x1076c9600?)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/x/muxer/muxer.go:375 +0x50
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Create_Handler.func1({0x1081fa0b8?, 0x14002341620?}, {0x107f213e0?, 0x140072c4e00?})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/[email protected]/proto/go/provider_grpc.pb.go:815 +0xd0
    github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0x1081fa0b8, 0x140023411d0}, {0x107f213e0, 0x140072c4e00}, 0x14007384d60, 0x14007115878)
    	/home/runner/go/pkg/mod/github.com/grpc-ecosystem/[email protected]/go/otgrpc/server.go:57 +0x2d4
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Create_Handler({0x108119300, 0x14006032240}, {0x1081fa0b8, 0x140023411d0}, 0x14007386e00, 0x140007a65a0)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/[email protected]/proto/go/provider_grpc.pb.go:817 +0x148
    google.golang.org/grpc.(*Server).processUnaryRPC(0x14000d7e200, {0x1081fa0b8, 0x14002341140}, {0x108211660, 0x14005ec9380}, 0x140074bcd80, 0x14001de4510, 0x10cfcd8b8, 0x0)
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1394 +0xb64
    google.golang.org/grpc.(*Server).handleStream(0x14000d7e200, {0x108211660, 0x14005ec9380}, 0x140074bcd80)
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1805 +0xb20
    google.golang.org/grpc.(*Server).serveStreams.func2.1()
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1029 +0x84
    created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 16
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1040 +0x13c

Output of pulumi about

.

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).

@VenelinMartinov VenelinMartinov added impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec labels Dec 10, 2024
@VenelinMartinov VenelinMartinov self-assigned this Dec 10, 2024
@VenelinMartinov VenelinMartinov added the p1 A bug severe enough to be the next item assigned to an engineer label Dec 10, 2024
@iwahbe iwahbe closed this as completed in b1f2d4f Dec 10, 2024
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Dec 10, 2024
flostadler added a commit to pulumi/pulumi-aws that referenced this issue Dec 10, 2024
VenelinMartinov added a commit that referenced this issue Dec 10, 2024
Additional tests for failure modes in `provider.go`'s `Create` method.
This would have caught
#2706
@pulumi-bot
Copy link
Contributor

This issue has been addressed in PR #2707 and shipped in release v3.97.1.

VenelinMartinov added a commit that referenced this issue Dec 11, 2024
In the SDKv2 bridge under PlanResourceChange we are not passing any
state we receive during TF Apply back to the engine if we also received
an error. This causes us to incorrectly miss any resources which were
created but encountered errors during the creation process. The engine
should see these as ResourceInitError, which allows the engine to
attempt to update the partially created resource on the next up.

This PR fixes the issue by passing the state down to the engine in the
case when we receive an error and a non-nil state from TF during Apply.

This is the second attempt at this. The first was
#2695 but was
reverted because it caused a different panic:
#2706. We added
a regression test for that in
#2710

The reason for that panic was that we were now creating a non-nil
`InstanceState` with a nil `stateValue` which causes the `ID` function
to panic. This PR fixes both issues by not allowing non-nil states with
nil `stateValue`s and by preventing the panic in `ID`.

There was also a bit of fun with go nil interfaces along the way, which
is the reason why `ApplyResourceChange` now returns a
`shim.InstanceState` interface instead of a `*v2InstanceState2`.
Otherwise we end up creating a non-nil interface with a nil value.

related to pulumi/pulumi-gcp#2700
related to pulumi/pulumi-aws#4759

fixes #2696
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/regression Something that used to work, but is now broken kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants