secrets: add common secrets plugins impl #26335
Conversation
|
One thing I missed here is the ability to provide any sort of configuration to external plugins. I wonder if we want to allow a basic key/value config that is passed via environment variables to the plugin process. |
| @@ -21,13 +27,70 @@ func NewPluginsSecretsFingerprint(logger hclog.Logger) Fingerprint { | |||
|
|
|||
| func (s *SecretsPluginFingerprint) Fingerprint(request *FingerprintRequest, response *FingerprintResponse) error { | |||
| // Add builtin secrets providers | |||
| response.AddAttribute("plugins.secrets.nomad.version", "1.0.0") | |||
| response.AddAttribute("plugins.secrets.vault.version", "1.0.0") | |||
| defer func() { | |||
There was a problem hiding this comment.
I thought this was necessary for fingerprint reloading where we call RemoveAttribute later in this function.. But now that I look at it, we always pass in an empty FingerprintResponse even during reload, so this seems unnecessary?
There was a problem hiding this comment.
@tgross I know we are doing something similar with DHV fingerprinting, am I missing something here?
There was a problem hiding this comment.
Yeah it looks like the dynamic host volumes are the only caller of that RemoveAttribute method, which has been hanging around since 7+ years ago. It looks like you're right that we get a fresh FingerprintResponse, so there's no need to remove them at all, so we could remove this defer as well.
I can open a PR to remove it (and the unused RemoveAttribute method) from DHV fingerprinting.
There was a problem hiding this comment.
Oops, no, I'm wrong! 🤦 The RemoveAttribute doesn't remove it from the response object! It adds it to the response object with a value of "", which removes it from the Node when we merge the response back in. So this is what lets us remove plugins that have been removed!
Co-authored-by: Michael Schurter <[email protected]>
Co-authored-by: Michael Schurter <[email protected]>
|
In the spirit of not inflating this PR further, I will open a separate PR for exposing plugin config via environment variables. |
Description
This PR adds the ability to run executables as secrets plugins to fetch secrets from locations not builtin to Nomad. This PR uses the Dynamic Host Volumes plugins as reference, but adds a
commonpluginspackage which should be used as a single source for all "common plugins" in the future. Work to migrate DHV plugins to thecommonpluginspackage is out of scope of this PR.Testing & Reproduction steps
Links
Contributor Checklist
changelog entry using the
make clcommand.ensure regressions will be caught.
and job configuration, please update the Nomad website documentation to reflect this. Refer to
the website README for docs guidelines. Please also consider whether the
change requires notes within the upgrade guide.
Reviewer Checklist
backporting document.
in the majority of situations. The main exceptions are long-lived feature branches or merges where
history should be preserved.
within the public repository.