Support defining remote components in Go #6403
Conversation
justinvp
force-pushed
the
justin/goprovider
branch
2 times, most recently
from
8e705df
to
afe1096
Compare
|
I've force pushed a new commit that addresses the feedback and adds integration tests. I am finishing support for prompt values for a separate PR, rather than including it here. Here's a summary of what's changed:
import (
"github.com/pulumi/pulumi/pkg/v3/resource/provider"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil"
)
// Serve launches the gRPC server for the resource provider.
func Serve(providerName, version string, schema []byte) {
// Start gRPC service.
if err := provider.ComponentMain(providerName, version, schema, construct); err != nil {
cmdutil.ExitError(err.Error())
}
}import (
"github.com/pkg/errors"
"github.com/pulumi/pulumi/sdk/v3/go/pulumi"
"github.com/pulumi/pulumi/sdk/v3/go/pulumi/provider"
)
func construct(ctx *pulumi.Context, typ, name string, inputs provider.ConstructInputs,
options pulumi.ResourceOption) (*provider.ConstructResult, error) {
// TODO: Add support for additional component resources here.
switch typ {
case "xyz:index:StaticPage":
return constructStaticPage(ctx, name, inputs, options)
default:
return nil, errors.Errorf("unknown resource type %s", typ)
}
}
// constructStaticPage is an implementation of Construct for the example StaticPage component.
// It demonstrates converting the raw ConstructInputs to the component's args struct, creating
// the component, and returning its URN and state (outputs).
func constructStaticPage(ctx *pulumi.Context, name string, inputs provider.ConstructInputs,
options pulumi.ResourceOption) (*provider.ConstructResult, error) {
// Copy the raw inputs to StaticPageArgs. `inputs.SetArgs` uses the types and `pulumi:` tags
// on the struct's fields to convert the raw values to the appropriate Input types.
args := &StaticPageArgs{}
if err := inputs.SetArgs(args); err != nil {
return nil, errors.Wrap(err, "setting args")
}
// Create the component resource.
staticPage, err := NewStaticPage(ctx, name, args, options)
if err != nil {
return nil, errors.Wrap(err, "creating component")
}
// Return the component resource's URN and state. `NewConstructResult` automatically sets the
// ConstructResult's state based on resource struct fields tagged with `pulumi:` tags with a value
// that is convertible to `pulumi.Input`.
return provider.NewConstructResult(staticPage)
} |
| continue | ||
| } | ||
|
|
||
| if !field.Type.Implements(reflect.TypeOf((*Input)(nil)).Elem()) { |
There was a problem hiding this comment.
[I just found these two comments in drafts that I had intended to send a couple weeks ago! I believe they are still relevant to the design here though - so sharing now ahead of a more thorough review of the updated PR shortly]
I didn't want to force component authors to have to define it twice manually like this. But curious if you think we should force component authors to define these the same way as the Go codegen does.
Two things that I've gotten tripped up on myself working with this:
- How do I write my own nested type to use here? Like if I want a
Routes: []Route, how do I write[]Routeso that it works? It appears I need to create atype RouteArrayInput, ensure that has aToRouteArrayOutputWithContext, and then also a customOutputstruct that wraps the rawRoutestruct? I haven't yet quite figured out what works fully here. - If we had just alllowed using the same thing that codegen produces, then I could just use the types that the Go SDK codegen already generates for me - by having the provider actually depend on the SDK. That may be a terrible idea, but it is nice in that the implementation gets to work with the exact same structs/interfaces as consumers will. It also means I don't have to recreate all of this manually inside the implementation - since there is already a codegen process to create these for the SDK. However, the types implemented in the SDK don't seem to work correctly with the logic as implemented here.
There was a problem hiding this comment.
It's also a little strange that these accept Input types - given that at least currently they will always be Outputs, and even once prompt values are possible, the user will always want to be clear here in the types which ones are prompt and which are Output. It will never be desirable for the implementor to see Input types vs. Output types. The implementation will always have to immediately turn the inputs back into outputs to work with them.
If this args struct used Outputs, the implementation here would also be much simpler and less "weird" in that it wouldn't have to discover To<>OutputWithContext to figure out what the output type was to create.
There was a problem hiding this comment.
How do I write my own nested type to use here?
The idea was to follow our documented pattern for inputs:
Lines 767 to 829 in afe1096
But it is an unfortunate amount of boilerplate to have to write manually (especially so if you'd need to also manually write associated boilerplate for array variants, map variants, etc.), so I fully empathize with the desire from (2) to use the types that the Go SDK codegen generates. However, ideally, we'd have a way to write a much simpler type in the implementation without all the extra ceremony/boilerplate, and drive the schema + SDK generation based on it, not the other way around.
However, the types implemented in the SDK don't seem to work correctly with the logic as implemented here.
Hmmm, do you happen to recall the specific issue? I believe there is a bug in the current implementation in this PR, where the unmarshalled value from the gRPC call isn't converted into the struct type -- so it'll remain a map. I am addressing this issue with the changes for prompt values.
It's also a little strange that these accept Input types - given that at least currently they will always be Outputs, and even once prompt values are possible, the user will always want to be clear here in the types which ones are prompt and which are Output. It will never be desirable for the implementor to see Input types vs. Output types. The implementation will always have to immediately turn the inputs back into outputs to work with them.
If this args struct used Outputs, the implementation here would also be much simpler and less "weird" in that it wouldn't have to discover To<>OutputWithContext to figure out what the output type was to create.
This is a good point. I was thinking you'd write the component implementation the same way you'd write it if you were releasing it as a regular Go library, where (ignoring prompt values for a moment) you would want to type inputs as the Input type to support either Input or Output values (and inside the implementation you'd immediately convert to Output to work with it).
But for the implementation of multi-lang components in Go, I could see how typing it as Output would simplify things here. We'd immediately know the Output type to use and it would save the user from having to convert the value to Output inside the component implementation.
I do still feel like we could support Input though... I could see supporting either Output or Input. If it's Output, great -- use it. If it's Input, one thing I could do is get its (Update: actually, I don't think this is possible), and finally falling back to looking for a ElementType() and then see if the output type has been registered with pulumi.RegisterOutputType (using it if so)To<>Output method.
discover To<>OutputWithContext
Note: after the recent discussion of possibly deprecating the <>WithContext methods, I've switched this to look for just To<>Output.
| } | ||
|
|
||
| // constructInputsSetArgs sets the inputs on the given args struct. | ||
| func constructInputsSetArgs(inputs map[string]interface{}, args interface{}) error { |
There was a problem hiding this comment.
Note: the implementation of this function isn't quite right, so it's not super important to review the details of it for this PR. I've reimplemented it for prompt values (separate PR on the way), and as part of that, it's leveraging rpc.unmarshalOutput to unmarshal the value.
Edit: Prompt value PR: #6790
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package provider |
There was a problem hiding this comment.
Any reason not to use plugin.providerServer? Looks like their functionality is similar.
There was a problem hiding this comment.
The Construct adapter func in this PR adapts the raw gRPC interface to/from the Pulumi Go SDK model.
pulumi/sdk/proto/go/provider.pb.go
Lines 2047 to 2048 in a8ea363
So the provider in this file implements the raw gRPC interface.
To use plugin.providerServer, Construct would have to be an adapter of plugin.providerServer's interface:
pulumi/sdk/go/common/resource/plugin/provider.go
Lines 78 to 80 in 664776e
Should Construct be an adapter for plugin.providerServer instead? Or should we have adapters for both the raw gRPC interface and plugin.providerServer?
The reason I made Construct an adapter for the raw gRPC interface is because 1) our existing provider boilerplate repo implements a provider using the raw gRPC interface rather than plugin.providerServer and 2) our own native providers also implement the raw gRPC interface over plugin.providerServer.
There was a problem hiding this comment.
Should Construct be an adapter for plugin.providerServer instead? Or should we have adapters for both the raw gRPC interface and plugin.providerServer?
The reason I made Construct an adapter for the raw gRPC interface is because 1) our existing provider boilerplate repo implements a provider using the raw gRPC interface rather than plugin.providerServer and 2) our own native providers also implement the raw gRPC interface over plugin.providerServer.
IMO we should adapt from plugin.providerServer, but I think that we can move forward with what you have. In the fullness of time, I'd love for all of our Go providers to be sitting on top of providerServer.
justinvp
force-pushed
the
justin/goprovider
branch
from
afe1096
to
e88d7a8
Compare
justinvp
force-pushed
the
justin/goprovider
branch
from
e88d7a8
to
9d02938
Compare
This makes it possible to create a resource provider with a
Constructimplementation in Go, similar to what we have for Node.Fixes #5489