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

feature: secrets/auth plugin multiplexing #14946

Merged
merged 69 commits into from Aug 30, 2022
Merged

Conversation

fairclothjm
Copy link
Contributor

@fairclothjm fairclothjm commented Apr 6, 2022

Description

This PR adds multiplexing support for secrets/auth plugins.

When multiplexed secrets/auth plugins are configured in Vault, each plugin will only have a single process managing all configurations for that plugin type. This single process will be multiplexed across all Vault namespaces for mounts of this type.

Some changes for the new v5 plugin set:

  • AutoMTLS will be used in favor of TLSProviderFunc
  • initial plugin lazy loading will not be done, i.e. the plugin will remain running after successful mount

To test multiplexing

  • build a secrets/auth plugin from main branch
  • build a multiplexed plugin from the feat/mux-secrets-auth branch
    • must change Serve to ServeMultiplex in the plugin's main.go file
  • build a vault binary from the multiplexing branch feat/mux-secrets-auth
  • start vault and register the plugins
  • write the configs and perform operations as normal

@vercel vercel bot temporarily deployed to Preview – vault April 6, 2022 21:39 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 6, 2022 21:39 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 7, 2022 20:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 7, 2022 20:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 8, 2022 16:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 8, 2022 16:44 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 8, 2022 16:46 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 8, 2022 16:46 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 8, 2022 19:26 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 8, 2022 19:26 Inactive
@calvn calvn self-requested a review April 11, 2022 20:45
@calvn calvn added this to the 1.11.0-rc1 milestone Apr 11, 2022
@vercel vercel bot temporarily deployed to Preview – vault April 13, 2022 18:49 Inactive
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 13, 2022 18:49 Inactive
@fairclothjm fairclothjm requested a review from a team April 15, 2022 18:39
@vercel vercel bot temporarily deployed to Preview – vault-storybook April 15, 2022 18:39 Inactive
@vercel vercel bot temporarily deployed to Preview – vault April 15, 2022 18:39 Inactive
sdk/plugin/grpc_backend_server.go Outdated Show resolved Hide resolved
sdk/plugin/backend.go Outdated Show resolved Hide resolved
sdk/helper/pluginutil/run_config.go Outdated Show resolved Hide resolved
@calvn calvn removed this from the 1.11.0-rc1 milestone May 20, 2022
if err != nil {
return err
}
b.Backend = nb
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Do we need to call initialize here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that gets called in the respective auth.go and mount.go

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in the case where a plugin crashes and is restarted i think we'd need to re-initialize, right?

Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few more suggestions / questions. Everything else LGTM. Will approve after 👍

sdk/plugin/grpc_backend_server.go Outdated Show resolved Hide resolved
sdk/plugin/grpc_backend_server.go Outdated Show resolved Hide resolved
sdk/plugin/grpc_backend_server.go Outdated Show resolved Hide resolved
sdk/plugin/plugin_v5.go Show resolved Hide resolved
vault/plugin_catalog.go Outdated Show resolved Hide resolved
vault/plugin_catalog.go Show resolved Hide resolved
vault/plugin_catalog.go Outdated Show resolved Hide resolved
builtin/plugin/backend.go Show resolved Hide resolved
builtin/plugin/v5/backend.go Outdated Show resolved Hide resolved
Copy link
Member

@austingebauer austingebauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice job on this, @fairclothjm!

@fairclothjm fairclothjm merged commit 07927e0 into main Aug 30, 2022
@fairclothjm fairclothjm deleted the feat/mux-secrets-auth branch August 30, 2022 02:42
@@ -0,0 +1,3 @@
```release-note:feature
**Secrets/auth plugin multiplexing**: manage multiple plugin configurations with a single plugin process
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was reviewing the changelog for 1.12 features, and wasn't sure if the formatting of just:

Plugin Multiplexing:

would be more consistent with other new feature entries from older releases?

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

Successfully merging this pull request may close these issues.

None yet

7 participants