plugin: new framework to implement plugins#580
Conversation
f84d0b5 to
0fbe2ac
Compare
AnomalRoil
left a comment
There was a problem hiding this comment.
I'm not entirely done with my tests but I can confirm both the standard and the interactive mode worked for me.
|
Okay, a first set of issues when trying to use
The rest of the library code is fairly agnostic of whether this is a plugin recipient or identity, so having an easy way of parsing plugin identities would be nice. Currently #518 is providing a way to detect "regular" identities from plugin identities through error handling, which could work as well but isn't as convenient I guess, although it's more in line with the rest of the currently exposed parsing functions (one is supposed to parse SSH vs age recipients and identities using the specific exported functions for each currently, meaning everyone ends up re-implementing this). In other notes:
|
|
It works great in my tests so far 🎉 One thing I noticed though is that the age client that communicates with the plugins currently still seems to spawn separate processes. For example, The debug messages I added show that each identity is handled in a different process/pid (each also gets the same stanza separately), so I think the recipients/identities arrays might currently not get longer than 1. It works, but sorting identities won't be possible like this. This is basically #526 , which I closed because it seemed to be intentional at the time. I think having only one plugin process with all the data could open more opportunities (more advance logic, ordering, maybe batching encrypts/decrypts in one request/operation, caching pins). |
|
After further testing in
On the plus side:
One last comment for now, it might be good in age.Decrypt, when ranging over stanzas/identities to first sort them to prioritize native identities over plugin ones? |
|
I'm not sure if it is relevant to the scope of this PR, but one thing I think would be useful would be plugin support for |
|
@FiloSottile
I think all of the "bumps on the road" I had using this framework can be solved by additive improvements that could come later on, such as:
|
| if s.Type == "fail" { | ||
| return "", fmt.Errorf("client failed to request value") | ||
| } | ||
| if err := expectStanzaWithBody(s, 0); err != nil { |
There was a problem hiding this comment.
I'd argue that a common pattern in a CLI is to say "Please provide XYZ or press enter to use the default value", so expecting a Stanza with a non-0 length body is not always practical.
At least it did bite me in my testing:
-> request-public
cGxlYXNlIHByb3ZpZGUgdGhlIGNoYWluaGFzaCBvZiB0aGUgbmV0d29yayB5b3Ug
d2FudCB0byB3b3JrIHdpdGggKGFuIGVtcHR5IHZhbHVlIHdpbGwgdXNlIHRoZSBk
ZWZhdWx0IG9uZSk
please provide the chainhash of the network you want to work with (an empty value will use the default one)
this felt safe, but isn't supported with the current implementation.
|
I'm looking to implement an age plugin but a bit confused about what state the support is in, the docs on pkg.dev don't show really how to implement one. Am I correct in saying this PR needs to be merged for support for implementing age plugins in Go? |
|
When I call Also, when I press enter without entering anything I get this: |
|
Apologies for letting this sit for so long.
That would have been a good idea, but ClientUI is already on main, and this does not feel worth a break.
This is intentional, to give the user more control over ordering. It's a tricky tradeoff which I am open to reconsidering. See str4d/rage#414 (comment) and maybe open an issue with a concrete use case description?
Opened #665 to track pluggable recipient/identity parsing, which is orthogonal to this PR.
I can't think of a good API to expose this, without depending on every package that implements an Identity, which anyway would be incomplete as there can be third-party ones. Somewhat related to #665.
Done, but Recipient.String returns
Good idea. This is not plugin-specific, we can sort identities we know require no user interaction and can't fail (the core native ones) before the rest. Done in a separate commit.
Plugin key generation can be extremely complex and involve hardware interaction, so we should leave it to the plugin. Adding an interface for
Fixed by #519. |
2214a55 to
d29d751
Compare
No description provided.