Skip to content

Commit

Permalink
Multiple plugins with the same name are now allowed by resolvePlugins…
Browse files Browse the repository at this point in the history 
…ByKey

to permit a CLI to understand multiple plugin versions. For example,
a config with "layout: go/v2.0" and another config with "layout: go/v2.1"
should both be understood by a particular CLI.

pkg/cli: remove validations requiring plugin names be unique within a CLI,
and resolve all matching plugins such that they can contain multiple
plugins with the same name. As long as there is only one plugin type,
ex. Init, per set of resolved plugins, no errors will occur.
  • Loading branch information
@estroz
estroz committed May 30, 2020
1 parent 9eb8673 commit eb9b0ee
Show file tree
Hide file tree
Showing 9 changed files with 452 additions and 156 deletions.
2 changes: 1 addition & 1 deletion pkg/cli/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (c cli) bindCreateAPI(ctx plugin.Context, cmd *cobra.Command) {
if isGetter {
if getter != nil {
err := fmt.Errorf("duplicate API creation plugins for project version %q: %s, %s",
c.projectVersion, getter.Name(), p.Name())
c.projectVersion, plugin.KeyFor(getter), plugin.KeyFor(p))
cmdErr(cmd, err)
return
}
Expand Down
88 changes: 10 additions & 78 deletions pkg/cli/cli.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ func WithPlugins(plugins ...plugin.Base) Option {
return func(c *cli) error {
for _, p := range plugins {
for _, version := range p.SupportedProjectVersions() {
if _, ok := c.pluginsFromOptions[version]; !ok {
c.pluginsFromOptions[version] = []plugin.Base{}
}
c.pluginsFromOptions[version] = append(c.pluginsFromOptions[version], p)
}
}
Expand All @@ -151,9 +148,6 @@ func WithDefaultPlugins(plugins ...plugin.Base) Option {
return func(c *cli) error {
for _, p := range plugins {
for _, version := range p.SupportedProjectVersions() {
if _, ok := c.defaultPluginsFromOptions[version]; !ok {
c.defaultPluginsFromOptions[version] = []plugin.Base{}
}
c.defaultPluginsFromOptions[version] = append(c.defaultPluginsFromOptions[version], p)
}
}
Expand Down Expand Up @@ -196,8 +190,8 @@ func (c *cli) initialize() error {
c.projectVersion = projectConfig.Version

if projectConfig.IsV1() {
return fmt.Errorf(noticeColor, "The v1 projects are no longer supported.\n"+
"See how to upgrade your project to v2: https://book.kubebuilder.io/migration/guide.html\n")
return fmt.Errorf(noticeColor, "project version 1 is no longer supported.\n"+
"See how to upgrade your project: https://book.kubebuilder.io/migration/guide.html\n")
}
} else {
return fmt.Errorf("failed to read config: %v", err)
Expand Down Expand Up @@ -303,14 +297,7 @@ func (c cli) validate() error {
if (!c.configured || !isLayoutSupported) && c.cliPluginKey == "" {
_, versionExists := c.defaultPluginsFromOptions[c.projectVersion]
if !versionExists {
return fmt.Errorf("no default plugins for project version %s", c.projectVersion)
}
}

// Validate plugin versions and name.
for _, versionedPlugins := range c.pluginsFromOptions {
if err := validatePlugins(versionedPlugins...); err != nil {
return err
return fmt.Errorf("no default plugins for project version %q", c.projectVersion)
}
}

Expand All @@ -334,7 +321,7 @@ func (c cli) validate() error {

// validatePlugins validates the name and versions of a list of plugins.
func validatePlugins(plugins ...plugin.Base) error {
pluginNameSet := make(map[string]struct{}, len(plugins))
pluginKeySet := make(map[string]struct{})
for _, p := range plugins {
pluginName := p.Name()
if err := plugin.ValidateName(pluginName); err != nil {
Expand All @@ -351,12 +338,13 @@ func validatePlugins(plugins ...plugin.Base) error {
pluginName, projectVersion, err)
}
}
// Check for duplicate plugin names. Names outside of a version can
// conflict because multiple project versions of a plugin may exist.
if _, seen := pluginNameSet[pluginName]; seen {
return fmt.Errorf("two plugins have the same name: %q", pluginName)

// Check for duplicate plugin keys.
key := plugin.KeyFor(p)
if _, hasKey := pluginKeySet[key]; hasKey {
return fmt.Errorf("two plugins have the same key: %q", key)
}
pluginNameSet[pluginName] = struct{}{}
pluginKeySet[key] = struct{}{}
}
return nil
}
Expand Down Expand Up @@ -389,62 +377,6 @@ func (c cli) buildRootCmd() *cobra.Command {
return rootCmd
}

// resolvePluginsByKey finds a plugin for pluginKey if it exactly matches
// some form of a known plugin's key. Those forms can be a:
// - Fully qualified key: "go.kubebuilder.io/v2.0.0"
// - Short key: "go/v2.0.0"
// - Fully qualified name: "go.kubebuilder.io"
// - Short name: "go"
// Some of these keys may conflict, ex. the fully-qualified and short names of
// "go.kubebuilder.io/v1.0.0" and "go.kubebuilder.io/v2.0.0" have ambiguous
// unversioned names "go.kubernetes.io" and "go". If pluginKey is ambiguous
// or does not match any known plugin's key, an error is returned.
//
// Note: resolvePluginsByKey returns a slice so initialize() can generalize
// setting default plugins if no pluginKey is set.
func resolvePluginsByKey(versionedPlugins []plugin.Base, pluginKey string) ([]plugin.Base, error) {
// Make a set of all possible key combinations to check pluginKey against.
// If the key is not ambiguous, set a valid pointer to the plugin for that
// key, otherwise set a tombstone so we know it is ambiguous. There will
// always be at least one key per plugin if their names are fully-qualified.
//
// Note: this isn't actually that inefficient compared to a memory-efficient
// solution since we're working with very small N's; it is also very simple.
allPluginKeyCombos := make(map[string]*plugin.Base)
for i, p := range versionedPlugins {
key := plugin.KeyFor(p)
// Short-circuit if we have an exact match.
if key == pluginKey {
return []plugin.Base{p}, nil
}
name := p.Name()
keys := []string{key, name}
if shortName := plugin.GetShortName(name); name != shortName {
keys = append(keys, shortName)
keys = append(keys, plugin.Key(shortName, p.Version()))
}

pp := &versionedPlugins[i]
for _, k := range keys {
if _, hasKey := allPluginKeyCombos[k]; hasKey {
allPluginKeyCombos[k] = nil
} else {
allPluginKeyCombos[k] = pp
}
}
}

pp, hasKey := allPluginKeyCombos[pluginKey]
if !hasKey {
return nil, fmt.Errorf("plugin key %q does not match a known plugin", pluginKey)
}
if pp == nil {
return nil, fmt.Errorf("plugin key %q matches more than one known plugin", pluginKey)
}

return []plugin.Base{*pp}, nil
}

// defaultCommand returns the root command without its subcommands.
func (c cli) defaultCommand() *cobra.Command {
return &cobra.Command{
Expand Down
65 changes: 65 additions & 0 deletions pkg/cli/cli_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,74 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/spf13/pflag"

internalconfig "sigs.k8s.io/kubebuilder/internal/config"
"sigs.k8s.io/kubebuilder/pkg/model/config"
"sigs.k8s.io/kubebuilder/pkg/plugin"
)

func TestCLI(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "CLI Suite")
}

// Test plugin types and constructors.
type mockPlugin struct {
name, version string
projectVersions []string
}

func (p mockPlugin) Name() string { return p.name }
func (p mockPlugin) Version() string { return p.version }
func (p mockPlugin) SupportedProjectVersions() []string { return p.projectVersions }

func (mockPlugin) UpdateContext(*plugin.Context) {}
func (mockPlugin) BindFlags(*pflag.FlagSet) {}
func (mockPlugin) InjectConfig(*config.Config) {}
func (mockPlugin) Run() error { return nil }

func makeBasePlugin(name, version string, projVers ...string) plugin.Base {
return mockPlugin{name, version, projVers}
}

func makePluginsForKeys(keys ...string) (plugins []plugin.Base) {
for _, key := range keys {
n, v := plugin.SplitKey(key)
plugins = append(plugins, makeBasePlugin(n, v, internalconfig.DefaultVersion))
}
return
}

func makeKeysForPlugins(plugins ...plugin.Base) (keys []string) {
for _, p := range plugins {
keys = append(keys, plugin.KeyFor(p))
}
return
}

type mockAllPlugin struct {
mockPlugin
mockInitPlugin
mockCreateAPIPlugin
mockCreateWebhookPlugin
}

type mockInitPlugin struct{ mockPlugin }
type mockCreateAPIPlugin struct{ mockPlugin }
type mockCreateWebhookPlugin struct{ mockPlugin }

func (p mockInitPlugin) GetInitPlugin() plugin.Init { return p }
func (p mockCreateAPIPlugin) GetCreateAPIPlugin() plugin.CreateAPI { return p }
func (p mockCreateWebhookPlugin) GetCreateWebhookPlugin() plugin.CreateWebhook { return p }

func makeAllPlugin(name, version string, projectVersions ...string) plugin.Base {
p := makeBasePlugin(name, version, projectVersions...).(mockPlugin)
return mockAllPlugin{
p,
mockInitPlugin{p},
mockCreateAPIPlugin{p},
mockCreateWebhookPlugin{p},
}
}

0 comments on commit eb9b0ee

Please sign in to comment.