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: Drop all capabilities from plugin containers #90

Merged
merged 3 commits into from
Sep 5, 2023

Conversation

tomhjp
Copy link
Contributor

@tomhjp tomhjp commented Sep 4, 2023

This makes several breaking API changes, so will necessitate a bump to v0.2.0, but should provide us with a better API in the longer term.

Builds on the additions in hashicorp/go-plugin#277 to enable dropping all default capabilities from the plugin container. We now let go-plugin set the PLUGIN_UNIX_SOCKET_GROUP env var, and override the value it sets for PLUGIN_UNIX_SOCKET_DIR because the plugin's view of that directory is different. As a result, renamed UnixSocketGroup as GroupAdd to more accurately reflect that it now only controls the container's GroupAdd setting.

As that requires a breaking change, I also updated the API for creating a RunnerFunc to make it a bit cleaner and reduce unnecessarily exported API surface like ContainerRunner which was useless anyway when directly created from outside the package - the best place to see the result of this is in container_runner_test.go or example/bidirectional/main.go.

Lastly this PR also removes the container's env from the Diagnose output, as it could reasonably include secrets. I'm planning to re-add that capability in a separate PR but behind a Debug configurable.

@tomhjp tomhjp changed the title Drop all capabilities from plugin containers plugincontainer: Drop all capabilities from plugin containers Sep 4, 2023
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

plugincontainer/container_runner.go Outdated Show resolved Hide resolved
@tomhjp tomhjp merged commit c947fbc into main Sep 5, 2023
18 checks passed
@tomhjp tomhjp deleted the drop-all-capabilities branch September 5, 2023 19:03
@tomhjp
Copy link
Contributor Author

tomhjp commented Sep 5, 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