Skip to content

Commit

Permalink
Tolerate Configure panics if one provider errors out (#2931)
Browse files Browse the repository at this point in the history
Minimally scoped panic recovery to address the scenario found in
pulumi/pulumi-gcp#3007

GCP expects failure in Configure for SDKv2 to be final. When our code
attempts to call Configure for PF after that failure, it results in a
panic.

I do not know if fail-on-first-error is the desired behavior as it seems
preferable to collect all the errors to show to the user if possible.
Therefore this PR is scoped to tolerate panics if there have been
preceding errors.
  • Loading branch information
t0yv0 authored Mar 3, 2025
1 parent bc3688f commit eac88d7
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 1 deletion.
29 changes: 28 additions & 1 deletion pkg/x/muxer/muxer.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,20 @@ func (m *muxer) Configure(ctx context.Context, req *pulumirpc.ConfigureRequest)

for _, s := range m.servers {
req := proto.Clone(req).(*pulumirpc.ConfigureRequest)
r, err := s.Configure(ctx, req)

var r *pulumirpc.ConfigureResponse
var err error

if errs.Len() > 0 {
var panicked bool
r, panicked, err = panicRecoveringConfigure(ctx, s, req)
if panicked {
continue
}
} else {
r, err = s.Configure(ctx, req)
}

if err != nil {
errs.Errors = append(errs.Errors, err)
continue
Expand All @@ -317,6 +330,20 @@ func (m *muxer) Configure(ctx context.Context, req *pulumirpc.ConfigureRequest)
return response, m.muxedErrors(errs)
}

func panicRecoveringConfigure(
ctx context.Context,
s server,
req *pulumirpc.ConfigureRequest,
) (response *pulumirpc.ConfigureResponse, panicked bool, finalError error) {
defer func() {
if p := recover(); p != nil {
panicked = true
}
}()
r, err := s.Configure(ctx, req)
return r, false, err
}

type resourceRequest interface {
GetUrn() string
}
Expand Down
40 changes: 40 additions & 0 deletions pkg/x/muxer/muxer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,46 @@ func TestConfigureInSequence(t *testing.T) {
}
}

// Some providers such as pulumi-gcp will panic in PluginFramework Configure if SDKv2 Configure has
// produced errors. That is they do not expect both being called in the error case. This test checks
// that such panics are ignored and processed as expected.
func TestConfigureIgnorePanics(t *testing.T) {
t.Parallel()
ctx := context.Background()

m := &muxer{
host: &host{},
servers: []server{
configureReturnsErrors{},
configurePanics{},
},
}
_, err := m.Configure(ctx, &pulumirpc.ConfigureRequest{})
require.Error(t, err)
}

type configureReturnsErrors struct {
pulumirpc.UnimplementedResourceProviderServer
}

func (x configureReturnsErrors) Configure(
ctx context.Context,
req *pulumirpc.ConfigureRequest,
) (*pulumirpc.ConfigureResponse, error) {
return nil, fmt.Errorf("Required configuration values have not been set")
}

type configurePanics struct {
pulumirpc.UnimplementedResourceProviderServer
}

func (x configurePanics) Configure(
ctx context.Context,
req *pulumirpc.ConfigureRequest,
) (*pulumirpc.ConfigureResponse, error) {
panic("Configure panics unexpectedly")
}

type configure struct {
pulumirpc.UnimplementedResourceProviderServer
t *testing.T
Expand Down

0 comments on commit eac88d7

Please sign in to comment.