-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fixes #3210: Enhanced Plugin Management Design #9352
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
Conversation
Signed-off-by: Lyndon-Li <[email protected]> Signed-off-by: Jake Abendroth <[email protected]>
Signed-off-by: Lyndon-Li <[email protected]> Signed-off-by: Jake Abendroth <[email protected]>
- Introduced 'Command' and 'BuiltIn' fields in PluginInfo for better plugin management. - Updated GetInstalledPluginInfo to mark built-in plugins. - Enhanced remove command to prevent removal of built-in plugins. - Modified plugin printer to display built-in status. Signed-off-by: Jake Abendroth <[email protected]>
- Introduced tests for GetInstalledPluginInfo to validate built-in plugin detection. - Added tests for plugin removal logic, ensuring built-in plugins cannot be removed. - Implemented heuristic matching for plugin names in various scenarios. Signed-off-by: Jake Abendroth <[email protected]>
- Add changelog for issue vmware-tanzu#3210 plugin management enhancements - Update overview-plugins.md to document new plugin name removal syntax - Add section explaining built-in vs external plugins - Document new BuiltIn column in 'velero plugin get' output Signed-off-by: Jake Abendroth <[email protected]>
| NAME KIND BuiltIn | ||
| velero.io/aws ObjectStore true | ||
| velero.io/pod BackupItemAction true | ||
| velero.io/gcp ObjectStore false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| NAME KIND BuiltIn | |
| velero.io/aws ObjectStore true | |
| velero.io/pod BackupItemAction true | |
| velero.io/gcp ObjectStore false | |
| NAME KIND Origin | |
| velero.io/aws ObjectStore Internal | |
| velero.io/pod BackupItemAction Internal | |
| velero.io/gcp ObjectStore External |
I like internal/external better imo.. but regardless both aws and gcp example here should match, they are both not built-in. note above is snippet is just for discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! We are mocking which plugins are external/internal in the test, but you're absolutely right - both AWS and GCP plugins are external in reality, not built-in.
I've updated the tests to use more realistic examples:
- Built-in plugins:
velero.io/pod,velero.io/pv - External plugins:
velero.io/aws,velero.io/gcp
The mock data was just to test the BuiltIn detection logic (Command == os.Args[0]), but it should reflect reality for clarity. Thanks for pointing that out!
…l plugins - Changed the command paths for the AWS plugin to reflect its status as an external plugin. - Updated test cases to ensure correct detection of built-in and external plugins. - Revised documentation to clarify the distinction between built-in and external plugins, including examples. Signed-off-by: Jake Abendroth <[email protected]>
…removal commands - Adjusted the output of the 'velero plugin get' command to correctly indicate the built-in status of the AWS and pod plugins. - Updated examples for the plugin removal command to demonstrate the inability to remove built-in plugins. Signed-off-by: Jake Abendroth <[email protected]>
|
@abendrothj Thanks for the PR. Could you write a separate PR for the design? After the design is reviewed and approved by the maintainers, we can merge the implementation. |
|
Abstract
This proposal enhances Velero's plugin management CLI to distinguish between built-in and external plugins, and allows removal of external plugins by plugin name.
The design addresses user confusion about which plugins are mandatory and provides intuitive plugin removal workflows.
Background
Currently,
velero plugin getshows all installed plugins without indicating which are built into the Velero server binary versus installed via init containers.Users cannot easily identify mandatory plugins that cannot be removed by modifying the Velero deployment.
Additionally,
velero plugin removeonly accepts init container names or images, requiring users to know the exact container details rather than the intuitive plugin names they see invelero plugin get.Goals
velero plugin getoutput to help users identify mandatory plugins.velero plugin remove <plugin-name>syntax for intuitive plugin removal.Non Goals
velero plugin remove <image>orvelero plugin remove <container-name>functionality.High-Level Design
The design extends the
ServerStatusRequestAPI to include plugin metadata (BuiltInflag andCommandfield) that the server populates based on plugin registration context.The CLI is enhanced to display this metadata and use it for intelligent plugin removal decisions.
Plugin name resolution uses heuristics to map user-provided plugin names to init containers in the Velero deployment.
Detailed Design
API Changes
Extend
PluginInfostruct inpkg/apis/velero/v1/server_status_request_types.go:Server-Side Changes
Modify
GetInstalledPluginInfoininternal/velero/serverstatusrequest.go:Commandfield with the plugin's registration command.BuiltIn = truewhenplugin.Command == os.Args[0](registered by Velero server binary).CLI Output Changes
Update
pkg/cmd/util/output/plugin_printer.go:Plugin Removal Enhancement
Enhance
pkg/cmd/cli/plugin/remove.go:Plugin Name Resolution Heuristics
When user provides plugin name (e.g.,
velero.io/aws):aws-plugin.velero/velero-plugin-for-aws.velero.io/awstoawsand match container names.awsin image or name.Alternatives Considered
Security Considerations
No security implications.
The changes only affect CLI display and plugin removal workflows.
No new attack surfaces are introduced.
Compatibility
velero plugin remove <image>andvelero plugin remove <container-name>syntax continues to work.PluginInfoare optional and won't break existing clients.Implementation
The implementation is complete as a proof of concept in the current PR.
The changes were implemented in the following sequence:
The author (Jake Abendroth) has implemented this design.
Open Issues
--forceflag to override built-in plugin protection for advanced users?Does your change fix a particular issue?
Fixes #3210
Please indicate you've done the following:
make new-changelog) or comment/kind changelog-not-requiredon this PR.site/content/docs/main.