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

plugincontainer: Support mlock #94

Merged
merged 6 commits into from
Sep 22, 2023

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Sep 21, 2023

When Vault's disable_mlock config setting is not set, it also tells plugins to call unix.Mlockall to ensure they don't swap any memory (with potentially sensitive data) out to disk. Without the IPC_LOCK capability, this is not possible inside a container, and previously meant containerized plugins would fail to start. hashicorp/vault#23215 is a WIP PR with the consumption of the new Config.CapIPCLock option.

To ensure better test coverage, I formalised the options we are interested in into a TestCompatibilityMatrix test. Some tests are skipped as they are known not to work, and some are skipped as TODO items, waiting to set up CI so they can pass. I'll be working to remove most/all of these skips over the coming weeks.

Also fixes a couple of small bugs and tidy-ups:

  • Config's Tag field was not correctly set, found in one of the new tests
  • The error message for a plugin that needs updating reported one of the wrong directories
  • Refactors most of the tests into an external plugincontainer_test package so they are forced to use the library like an external consumer (Vault) would.
  • A couple of errors are made more descriptive.

Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

Looks mostly good. One question in the main().

I appreciate all of the automated tests, as it is a pain to get all of the combinations working on my own Linux desktop :)

plugincontainer/compatibility_matrix_test.go Show resolved Hide resolved
plugincontainer/compatibility_matrix_test.go Show resolved Hide resolved
plugincontainer/examples/container/plugin-counter/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@swenson swenson left a comment

Choose a reason for hiding this comment

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

LGTM

@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 22, 2023

Thanks!

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.

2 participants