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

Multiplexing opt out flag #16972

Merged
merged 20 commits into from
Sep 8, 2022
Merged

Multiplexing opt out flag #16972

merged 20 commits into from
Sep 8, 2022

Conversation

maxcoulombe
Copy link
Contributor

@maxcoulombe maxcoulombe commented Sep 1, 2022

Added env var flag to allow users to opt out specific plugins from the multiplexing feature as a workaround if ever necessary. Flag is added as an "in case of emergency break glass" measure and should not be used unless a critical bug in the multiplexing feature is discovered for a given plugin.

Converted the MySQL plugin to multiplexing along the way as it was used for testing, Note that only external plugins can be multiplexed so for the time being this modification changes nothing.

Bumped plugin SDK to Go18 since Vault itself upgraded to v18 recently.

Manual tests performed:

  • Opted-out external mplexed plugin creates 1 process per instance
  • Non opted-out external mplxed plugin creates 1 process for any number of instances
  • External splexed plugins can be used normally along opted-out plugins
  • External mplexed plugins can be used normally along opted-out plugins
  • Opted-out splexed plugins are unaffected

@maxcoulombe maxcoulombe requested a review from a team September 1, 2022 02:02
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

Looking good! Couple of comments/questions :)

sdk/helper/pluginutil/env.go Outdated Show resolved Hide resolved
vault/plugin_catalog.go Show resolved Hide resolved
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@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.

Looking good! Just a couple of comments.

sdk/helper/pluginutil/multiplexing.go Outdated Show resolved Hide resolved
sdk/helper/pluginutil/multiplexing_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@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

sdk/helper/pluginutil/multiplexing_test.go Outdated Show resolved Hide resolved
@maxcoulombe maxcoulombe merged commit 8095da6 into main Sep 8, 2022
@maxcoulombe maxcoulombe deleted the vault-5199-mplexFlag branch September 8, 2022 15:32
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

3 participants