Conversation
satoqz
force-pushed
the
plugins
branch
8 times, most recently
from
f34e25c to
4ba475f
Compare
| MagicCookieKey: "HASHICORP_GKW_PLUGIN", | ||
| MagicCookieValue: "wrapper", | ||
| // ServeOpts configures a KMS plugin server. | ||
| type ServeOpts struct { |
There was a problem hiding this comment.
Is the TLSProviderFunc intentionally omitted?
There was a problem hiding this comment.
IIUC TLSProviderFunc is only needed if the client doesn't support AutoMTLS, which we can assume OpenBao versions using this new plugin type do. While we could add it, not supporting it gives the plugin main functions one less thing to consider.
satoqz
force-pushed
the
plugins
branch
4 times, most recently
from
a536c4e to
e87fa75
Compare
satoqz
force-pushed
the
plugins
branch
4 times, most recently
from
ac917de to
bca6f5b
Compare
Signed-off-by: Jonas Köhnen <[email protected]>
The protobuf setup in this repo is so simple I don't believe dealing with an additional tool is worth it; protoc is fine. Additionally, move the protobuf input/output of the plugin package into a separate package as server/client definitions were polluting the package namespace. Signed-off-by: Jonas Köhnen <[email protected]>
Signed-off-by: Jonas Köhnen <[email protected]>
Not returning an unimplemented error from these seems more ergonomic, these are lifecycle hooks that can just default to no-ops without further complaints. Signed-off-by: Jonas Köhnen <[email protected]>
Signed-off-by: Jonas Köhnen <[email protected]>
Signed-off-by: Jonas Köhnen <[email protected]>
satoqz
changed the title
|
@cipherboy @mrclki okay, the semantics I've come up with here have reached a stable state that has been working well with my upcoming client-side work in the main repo, so opening this one up for review! For those curious, main repo work is here: https://github.com/Ki-Reply-GmbH/openbao/tree/auto-unseal-plugins. This already works as expected and even supports automatic recovery from plugin crashes. Just missing tests & docs. |
| if err := initFinalizer.Finalize( | ||
| ctx, | ||
| ); err != nil { | ||
| if err := initFinalizer.Init(ctx); err != nil { |
There was a problem hiding this comment.
We make use of the Options in InitRequest on the client side , but they don't apply here.
There was a problem hiding this comment.
I've omitted passing any options in Init and Finalize because there are no concrete options that any existing Init and Finalize calls accept. I've kept it on the clientside to reduce friction if this ever changes.
Specifically:
- For
SetConfig(), we want to passWithKeyIdandWithConfigMap - For
Encrypt()andDecrypt(), we want to passWithAadandWithKeyId - For
InitandFinalize, it's unclear but part of the interface. We also never pass any options to these in OpenBao.
| return &KeyBytesResponse{KeyBytes: keyBytes}, nil | ||
|
|
||
| // Call Finalize if the underlying implementation has it. | ||
| if initFinalizer, ok := wrapper.(wrapping.InitFinalizer); ok { |
There was a problem hiding this comment.
Same as InitRequest, we should make use of req.Options.
Signed-off-by: Jonas Köhnen <[email protected]>
2 participants
This PR is based on #69.
Part of openbao/openbao#2459.