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

Use grpc for the handshake protocol #152

Closed
entropitor opened this issue Jul 20, 2020 · 5 comments
Closed

Use grpc for the handshake protocol #152

entropitor opened this issue Jul 20, 2020 · 5 comments

Comments

@entropitor
Copy link

entropitor commented Jul 20, 2020

Instead of the complicated and undocumented handshake protocol, use a grpc server that is hosted by the application accepting plugins and pass just a single env variable to those applications with the address of that server.

Ideally the address is served over TCP for both windows support and because the go implementation over UDS is not following the spec grpc/grpc-go#2628 at the moment (so plugins in rust / typescript / javascript might not work)

Process:

  1. Application starts plugin with env variable
  2. Plugin calls handshake GRPC service in application via address in env variable
  3. Server responds with the version it supports and the required port range (if any), this is cleaner then passing this over env variables. Potentially, it could also pass an application name (and that value is checked instead of the "MAGIC COOKIE" solution)
  4. The client figures out which version to use and registers itself to the server by calling a second GRPC endpoint, announcing on which endpoint the plugin server is running, ... (instead of putting this information on stdout, and having to block other output to stdout in the mean time and not being able to use newlines in e.g. SSL certificate)

For backwards compatibility reasons, it's probably possible to write a "adapter" binary that starts the real plugin and adapts between v1 and this v2 (this proposal) of the handshake protocol. Given this adapter application can be written once in 1 language, it would be much easier to write SDKs for other languages without necessarily breaking the protocol.

@mitchellh
Copy link
Contributor

The handshake is documented here: https://github.com/hashicorp/go-plugin/blob/master/docs/internals.md#handshake

I think if I were working on go-plugin from scratch today I would go with this route. At the moment, I don’t see a large enough benefit to warrant the change. You note that it would be easier for other languages to write SDKs and I do agree with that, but I also think that outputting the one line isn’t too bad. If the documentation is incorrect, its easier to update that.

This is one of those situations where the end goal is certainly cleaner, but the backwards compatibility in between is very painful as we have to support the two handshake mechanisms for at least many years since we’ve already committed to plugin compatibility across many of the HashiCorp products. Given there would likely be many years of N+1 support (https://xkcd.com/927/) I’m inclined to just stick with the one admittedly weird approach for now.

Your suggestion is reasonable but I hope this helps share my viewpoint. Thanks for the suggestion.

@entropitor
Copy link
Author

@mitchellh Okay, I understand! I think the env variables for supported app versions should maybe also be listed on that page then as well

What is unclear to me about that wiki page is, should the handshake line be the first line on the stdout, the only line on stdout or can it be any line on stdout (and go-plugin will pick the first one that matches)?
(I think it's especially here that it becomes a bit more cumbersome to write the SDK if it's "first line" as the SDK would have to block off the stdout from the plugin itself until the line is written and only then can stdout be used for real stuff)

(I was thinking an adapter application could do the translating between V1 and V2 and thus making it quite robust and backwards-compatible, outside of go-plugin, the applications or any of the SDKs) but given how e.g. terraform loads plugins, that might not be as easy to integrate. You would probably need a bash script as real extension then that loads the adapter application with an argument set to the real application you want to load and whether that application is V1 or V2)

@mitchellh
Copy link
Contributor

These are good things to update the doc with but to answer your questions:

What is unclear to me about that wiki page is, should the handshake line be the first line on the stdout, the only line on stdout or can it be any line on stdout (and go-plugin will pick the first one that matches)?

First line, preferably only but I think any later lines are presently ignored.

The expectation is that the plugin process takes over stdout/stderr immediately on startup. That is what the Go plugin framework does, it expects you to call plugin.Serve (or something like that) immediately on main which overrides os.Stdout so any loggers or whatever don't write to it.

(I was thinking an adapter application could do the translating between V1 and V2 and thus making it quite robust and backwards-compatible, outside of go-plugin, the applications or any of the SDKs)

Yeah its certainly possible no doubt. Its just one of those things that if the old way has to be maintained indefinitely, it feels a bit painful to introduce a second way. You went from one problem to two!

@entropitor
Copy link
Author

Okay I understand. Let's keep the current handshake!

Regarding the stdout/stderr, where should the output of plugins then go? Should it go to log files or just absorbed? I was thinking of potentially writing an SDK for another language and this is one of the last things that is unclear to me from reading the source code, what the SDK does exactly with stdout/stderr.

@mitchellh
Copy link
Contributor

If it goes to stderr then the host system will log it, so that is the recommended place for logs! Otherwise yes a file. Or sometimes what we do is launch our plugins with extra file descriptors to get access to the original TTY too.

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

No branches or pull requests

2 participants