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

gRPC bridge: fix unknowns in Update previews #6006

Merged
merged 6 commits into from
Dec 23, 2020
Merged

Conversation

pgavlin
Copy link
Member

@pgavlin pgavlin commented Dec 23, 2020

These changes are a combination of three commits, each of which
contributes to the testing and/or fixing of a problem with marshaling
unknowns in plugin.provider.Update when preview is true.

deploytest: add support for gRPC adapters.

These changes add support for communicating with providers using the
gRPC adapters to the deploytest pacakage. This makes it easier to test
the gRPC adapters across typical lifecycle patterns.

Supporting these changes are two additions to the resource/plugin
package:

  1. A type that bridges between the plugin.Provider interface and the
    pulumirpc.ResourceProviderServer
  2. A function to create a plugin.Provider given a
    pulumirpc.ResourceProviderClient

The deploytest package uses these to wrap an in-process
plugin.Provider in a gRPC interface and connect to it without using
the default plugin host, respectively.

pulumi_test: test provider preview over gRPC.

Add a test that runs the provider preview lifecycle, but using a
provider that communicates over gRPC.

gRPC bridge: fix unknowns in Update previews

Set the KeepUnknowns and RejectUnknowns bits in the MarshalOptions
used when unmarshaling update results to preserve unknowns during a
preview and reject them otherwise.

These changes also set the RejectUnknowns bit in the MarshalOptions
used by Create if preview is false, and fix a bug in the array
unmarshaler that could cause out-of-bounds accesses.

Fixes #6004.

These changes add support for communicating with providers using the
gRPC adapters to the deploytest pacakage. This makes it easier to test
the gRPC adapters across typical lifecycle patterns.

Supporting these changes are two additions to the `resource/plugin`
package:

1. A type that bridges between the `plugin.Provider` interface and the
  `pulumirpc.ResourceProviderServer`
2. A function to create a `plugin.Provider` given a
  `pulumirpc.ResourceProviderClient`

The deploytest package uses these to wrap an in-process
`plugin.Provider` in a gRPC interface and connect to it without using
the default plugin host, respectively.
Add a test that runs the provider preview lifecycle, but using a
provider that communicates over gRPC.
Set the `KeepUnknowns` and `RejectUnknowns` bits in the `MarshalOptions`
used when unmarshaling update results to preserve unknowns during a
preview and reject them otherwise.

These changes also set the `RejectUnknowns` bit in the `MarshalOptions`
used by `Create` if `preview` is false, and fix a bug in the array
unmarshaler that could cause out-of-bounds accesses.
@pgavlin
Copy link
Member Author

pgavlin commented Dec 23, 2020

regarding "deploytest: add support for gRPC adapters": my plan is to update the testing targets s.t. we run all of the tests in the lifecycle harness with and without the gRPC adapter except for those that explicitly opt out. I opted to skip that for now in order to deliver a more targeted test for the issue at hand.

@pgavlin
Copy link
Member Author

pgavlin commented Dec 23, 2020

I opted to skip that for now in order to deliver a more targeted test for the issue at hand.

I did add support for a command-line flag that enables the gRPC adapter for all of the tests, however. This should let us work on fixing them one-by-one.

@pgavlin
Copy link
Member Author

pgavlin commented Dec 23, 2020

( I think that this fixes #6004)

@pgavlin pgavlin merged commit c6d22a2 into master Dec 23, 2020
@pulumi-bot pulumi-bot deleted the pgavlin/updatePreview branch December 23, 2020 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview failure: "unexpected unknown property value"
2 participants