From 203a103fc442994904212420abff186fd5630ce3 Mon Sep 17 00:00:00 2001 From: Carlisia Date: Mon, 14 Dec 2020 16:52:52 -0800 Subject: [PATCH] Minor refactor plus better documentation for naming Signed-off-by: Carlisia --- pkg/plugin/clientmgmt/manager.go | 42 ++++++++++++------------ pkg/plugin/clientmgmt/manager_test.go | 34 +++++++++++++++++++ site/content/docs/main/custom-plugins.md | 30 ++++++++++++++--- site/content/docs/v1.5/custom-plugins.md | 30 ++++++++++++++--- 4 files changed, 107 insertions(+), 29 deletions(-) diff --git a/pkg/plugin/clientmgmt/manager.go b/pkg/plugin/clientmgmt/manager.go index 18705bae4..411b3b0bd 100644 --- a/pkg/plugin/clientmgmt/manager.go +++ b/pkg/plugin/clientmgmt/manager.go @@ -131,10 +131,8 @@ func (m *manager) getRestartableProcess(kind framework.PluginKind, name string) // GetObjectStore returns a restartableObjectStore for name. func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) { - // Backwards compatibility with non-namespaced, built-in plugins. - if !strings.Contains(name, "/") { - name = "velero.io/" + name - } + name = sanitizeName(name) + restartableProcess, err := m.getRestartableProcess(framework.PluginKindObjectStore, name) if err != nil { return nil, err @@ -147,10 +145,8 @@ func (m *manager) GetObjectStore(name string) (velero.ObjectStore, error) { // GetVolumeSnapshotter returns a restartableVolumeSnapshotter for name. func (m *manager) GetVolumeSnapshotter(name string) (velero.VolumeSnapshotter, error) { - // Backwards compatibility with non-namespaced, built-in plugins. - if !strings.Contains(name, "/") { - name = "velero.io/" + name - } + name = sanitizeName(name) + restartableProcess, err := m.getRestartableProcess(framework.PluginKindVolumeSnapshotter, name) if err != nil { return nil, err @@ -183,10 +179,8 @@ func (m *manager) GetBackupItemActions() ([]velero.BackupItemAction, error) { // GetBackupItemAction returns a restartableBackupItemAction for name. func (m *manager) GetBackupItemAction(name string) (velero.BackupItemAction, error) { - // Backwards compatibility with non-namespaced, built-in plugins. - if !strings.Contains(name, "/") { - name = "velero.io/" + name - } + name = sanitizeName(name) + restartableProcess, err := m.getRestartableProcess(framework.PluginKindBackupItemAction, name) if err != nil { return nil, err @@ -218,10 +212,8 @@ func (m *manager) GetRestoreItemActions() ([]velero.RestoreItemAction, error) { // GetRestoreItemAction returns a restartableRestoreItemAction for name. func (m *manager) GetRestoreItemAction(name string) (velero.RestoreItemAction, error) { - // Backwards compatibility with non-namespaced, built-in plugins. - if !strings.Contains(name, "/") { - name = "velero.io/" + name - } + name = sanitizeName(name) + restartableProcess, err := m.getRestartableProcess(framework.PluginKindRestoreItemAction, name) if err != nil { return nil, err @@ -253,11 +245,8 @@ func (m *manager) GetDeleteItemActions() ([]velero.DeleteItemAction, error) { // GetDeleteItemAction returns a restartableDeleteItemAction for name. func (m *manager) GetDeleteItemAction(name string) (velero.DeleteItemAction, error) { - // Backwards compatibility with non-namespaced plugins, following principle of least surprise - // since DeleteItemActions were not bundled with Velero when plugins were non-namespaced. - if !strings.Contains(name, "/") { - name = "velero.io/" + name - } + name = sanitizeName(name) + restartableProcess, err := m.getRestartableProcess(framework.PluginKindDeleteItemAction, name) if err != nil { return nil, err @@ -266,3 +255,14 @@ func (m *manager) GetDeleteItemAction(name string) (velero.DeleteItemAction, err r := newRestartableDeleteItemAction(name, restartableProcess) return r, nil } + +// sanitizeName adds "velero.io" to legacy plugins that weren't namespaced. +func sanitizeName(name string) string { + // Backwards compatibility with non-namespaced Velero plugins, following principle of least surprise + // since DeleteItemActions were not bundled with Velero when plugins were non-namespaced. + if !strings.Contains(name, "/") { + name = "velero.io/" + name + } + + return name +} diff --git a/pkg/plugin/clientmgmt/manager_test.go b/pkg/plugin/clientmgmt/manager_test.go index 4be23b25a..cc36bc653 100644 --- a/pkg/plugin/clientmgmt/manager_test.go +++ b/pkg/plugin/clientmgmt/manager_test.go @@ -574,3 +574,37 @@ func TestGetDeleteItemActions(t *testing.T) { }) } } + +func TestSanitizeName(t *testing.T) { + tests := []struct { + name, pluginName, expectedName string + }{ + { + name: "Legacy, non-namespaced plugin", + pluginName: "aws", + expectedName: "velero.io/aws", + }, + { + name: "A Velero plugin", + pluginName: "velero.io/aws", + expectedName: "velero.io/aws", + }, + { + name: "A non-Velero plugin with a Velero namespace", + pluginName: "velero.io/plugin-for-velero", + expectedName: "velero.io/plugin-for-velero", + }, + { + name: "A non-Velero plugin with a non-Velero namespace", + pluginName: "digitalocean.com/velero", + expectedName: "digitalocean.com/velero", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + sanitizedName := sanitizeName(tc.pluginName) + assert.Equal(t, sanitizedName, tc.expectedName) + }) + } +} diff --git a/site/content/docs/main/custom-plugins.md b/site/content/docs/main/custom-plugins.md index 7d2ab9b14..1789f68f9 100644 --- a/site/content/docs/main/custom-plugins.md +++ b/site/content/docs/main/custom-plugins.md @@ -11,11 +11,33 @@ A fully-functional [sample plugin repository][1] is provided to serve as a conve ## Plugin Naming -When naming your plugin, keep in mind that the name needs to conform to these rules: -- have two parts separated by '/' +A plugin is identified by a prefix + name. + +**Note: Please don't use `velero.io` as the prefix for a plugin not supported by the Velero team.** The prefix should help users identify the entity developing the plugin, so please use a prefix that identify yourself. + +Whenever you define a Backup Storage Location or Volume Snapshot Location, this full name will be the value for the `provider` specification. + +For example: `oracle.io/oracle`. + +``` +apiVersion: velero.io/v1 +kind: BackupStorageLocation +spec: + provider: oracle.io/oracle +``` + +``` +apiVersion: velero.io/v1 +kind: VolumeSnapshotLocation +spec: + provider: oracle.io/oracle +``` + +When naming your plugin, keep in mind that the full name needs to conform to these rules: +- have two parts, prefix + name, separated by '/' - none of the above parts can be empty - the prefix is a valid DNS subdomain name -- a plugin with the same name cannot not already exist +- a plugin with the same prefix + name cannot not already exist ### Some examples: @@ -25,7 +47,7 @@ When naming your plugin, keep in mind that the name needs to conform to these ru - example-with-dash.io/azure ``` -You will need to give your plugin(s) a name when registering them by calling the appropriate `RegisterX` function: +You will need to give your plugin(s) the full name when registering them by calling the appropriate `RegisterX` function: ## Plugin Kinds diff --git a/site/content/docs/v1.5/custom-plugins.md b/site/content/docs/v1.5/custom-plugins.md index ff4f6acfd..36aa6eb07 100644 --- a/site/content/docs/v1.5/custom-plugins.md +++ b/site/content/docs/v1.5/custom-plugins.md @@ -11,11 +11,33 @@ A fully-functional [sample plugin repository][1] is provided to serve as a conve ## Plugin Naming -When naming your plugin, keep in mind that the name needs to conform to these rules: -- have two parts separated by '/' +A plugin is identified by a prefix + name. + +**Note: Please don't use `velero.io` as the prefix for a plugin not supported by the Velero team.** The prefix should help users identify the entity developing the plugin, so please use a prefix that identify yourself. + +Whenever you define a Backup Storage Location or Volume Snapshot Location, this full name will be the value for the `provider` specification. + +For example: `oracle.io/oracle`. + +``` +apiVersion: velero.io/v1 +kind: BackupStorageLocation +spec: + provider: oracle.io/oracle +``` + +``` +apiVersion: velero.io/v1 +kind: VolumeSnapshotLocation +spec: + provider: oracle.io/oracle +``` + +When naming your plugin, keep in mind that the full name needs to conform to these rules: +- have two parts, prefix + name, separated by '/' - none of the above parts can be empty - the prefix is a valid DNS subdomain name -- a plugin with the same name cannot not already exist +- a plugin with the same prefix + name cannot not already exist ### Some examples: @@ -25,7 +47,7 @@ When naming your plugin, keep in mind that the name needs to conform to these ru - example-with-dash.io/azure ``` -You will need to give your plugin(s) a name when registering them by calling the appropriate `RegisterX` function: +You will need to give your plugin(s) the full name when registering them by calling the appropriate `RegisterX` function: ## Plugin Kinds