Skip to content

Commit

Permalink
Merge pull request #1547 from estroz/chore/remove-plugin-patch-version
Browse files Browse the repository at this point in the history 
*: remove plugin minor/patch versions
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jun 4, 2020
2 parents 1972f4c + 4f890fc commit 977eb88
Show file tree
Hide file tree
Showing 13 changed files with 481 additions and 321 deletions.
44 changes: 3 additions & 41 deletions pkg/cli/cli.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (c *cli) initialize() error {
// In case 1, default plugins will be used to determine which plugin to use.
// In case 2, the value passed to --plugins is used.
// For all other commands, a config's 'layout' key is used. Since both
// layout and --plugins values can be short (ex. "go/v2.0.0") or unversioned
// layout and --plugins values can be short (ex. "go/v2") or unversioned
// (ex. "go.kubebuilder.io") keys or both, their values may need to be
// resolved to known plugins by key.
// Default plugins are checked first so any input key that has more than one
Expand Down Expand Up @@ -323,53 +323,15 @@ func (c cli) validate() error {
}
// CLI-set plugins do not have to contain a version.
if pluginVersion != "" {
if err := plugin.ValidateVersion(pluginVersion); err != nil {
return fmt.Errorf("invalid plugin %q version %q: %v",
pluginName, pluginVersion, err)
if _, err := plugin.ParseVersion(pluginVersion); err != nil {
return fmt.Errorf("invalid plugin version %q: %v", pluginVersion, err)
}
}
}

return nil
}

// validatePlugins validates the name and versions of a list of plugins.
func validatePlugins(plugins ...plugin.Base) error {
pluginKeySet := make(map[string]struct{}, len(plugins))
for _, p := range plugins {
if err := validatePlugin(p); err != nil {
return err
}
// Check for duplicate plugin keys.
pluginKey := plugin.KeyFor(p)
if _, seen := pluginKeySet[pluginKey]; seen {
return fmt.Errorf("two plugins have the same key: %q", pluginKey)
}
pluginKeySet[pluginKey] = struct{}{}
}
return nil
}

// validatePlugin validates the name and versions of a plugin.
func validatePlugin(p plugin.Base) error {
pluginName := p.Name()
if err := plugin.ValidateName(pluginName); err != nil {
return fmt.Errorf("invalid plugin name %q: %v", pluginName, err)
}
pluginVersion := p.Version()
if err := plugin.ValidateVersion(pluginVersion); err != nil {
return fmt.Errorf("invalid plugin %q version %q: %v",
pluginName, pluginVersion, err)
}
for _, projectVersion := range p.SupportedProjectVersions() {
if err := validation.ValidateProjectVersion(projectVersion); err != nil {
return fmt.Errorf("invalid plugin %q supported project version %q: %v",
pluginName, projectVersion, err)
}
}
return nil
}

// buildRootCmd returns a root command with a subcommand tree reflecting the
// current project's state.
func (c cli) buildRootCmd() *cobra.Command {
Expand Down
11 changes: 8 additions & 3 deletions pkg/cli/cli_suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ func TestCLI(t *testing.T) {

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

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

func (mockPlugin) UpdateContext(*plugin.Context) {}
Expand All @@ -50,7 +51,11 @@ 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}
v, err := plugin.ParseVersion(version)
if err != nil {
panic(err)
}
return mockPlugin{name, v, projVers}
}

func makePluginsForKeys(keys ...string) (plugins []plugin.Base) {
Expand Down
15 changes: 9 additions & 6 deletions pkg/cli/cli_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ var _ = Describe("CLI", func() {
pluginNameA = "go.example.com"
pluginNameB = "go.test.com"
projectVersions = []string{config.Version2, config.Version3Alpha}
pluginAV1 = makeAllPlugin(pluginNameA, "v1.0", projectVersions...)
pluginAV2 = makeAllPlugin(pluginNameA, "v2.0", projectVersions...)
pluginBV1 = makeAllPlugin(pluginNameB, "v1.0", projectVersions...)
pluginBV2 = makeAllPlugin(pluginNameB, "v2.0", projectVersions...)
pluginAV1 = makeAllPlugin(pluginNameA, "v1", projectVersions...)
pluginAV2 = makeAllPlugin(pluginNameA, "v2", projectVersions...)
pluginBV1 = makeAllPlugin(pluginNameB, "v1", projectVersions...)
pluginBV2 = makeAllPlugin(pluginNameB, "v2", projectVersions...)
allPlugins = []plugin.Base{pluginAV1, pluginAV2, pluginBV1, pluginBV2}
)

Expand Down Expand Up @@ -89,7 +89,7 @@ var _ = Describe("CLI", func() {

By("setting two plugins of the same name and version")
_, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV1))
Expect(err).To(MatchError(`broken pre-set plugins: two plugins have the same key: "go.example.com/v1.0"`))
Expect(err).To(MatchError(`broken pre-set plugins: two plugins have the same key: "go.example.com/v1"`))
})
})

Expand Down Expand Up @@ -145,7 +145,10 @@ var _ = Describe("CLI", func() {
By(`setting cliPluginKey to an non-existent key "foo"`)
setPluginsFlag("foo")
_, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV2))
Expect(err).To(MatchError(errAmbiguousPlugin{"foo", "no names match"}))
Expect(err).To(MatchError(errAmbiguousPlugin{
key: "foo",
msg: `no names match, possible plugins: ["go.example.com/v1" "go.example.com/v2"]`,
}))
})
})

Expand Down
173 changes: 66 additions & 107 deletions pkg/cli/plugins.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import (
"fmt"
"sort"

"github.com/blang/semver"

"sigs.k8s.io/kubebuilder/pkg/internal/validation"
"sigs.k8s.io/kubebuilder/pkg/plugin"
)

Expand All @@ -37,12 +36,12 @@ func (e errAmbiguousPlugin) Error() string {

// resolvePluginsByKey resolves versionedPlugins to a subset of plugins by
// matching keys to some form of pluginKey. Those forms can be a:
// - Fully qualified key: "go.kubebuilder.io/v2.0"
// - Short key: "go/v2.0"
// - Fully qualified key: "go.kubebuilder.io/v2"
// - Short key: "go/v2"
// - 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" and "go.kubebuilder.io/v2.0" have ambiguous
// "go.kubebuilder.io/v1" and "go.kubebuilder.io/v2" 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.
//
Expand All @@ -52,82 +51,53 @@ func resolvePluginsByKey(versionedPlugins []plugin.Base, pluginKey string) (reso

name, version := plugin.SplitKey(pluginKey)

// Compare versions first to narrow the list of name comparisons.
if version == "" {
// Case: if plugin key has no version, check all plugin names.
resolved = versionedPlugins
} else {
// Case: if plugin key has version, filter by version.
resolved = findPluginsMatchingVersion(versionedPlugins, version)
}

if len(resolved) == 0 {
return nil, errAmbiguousPlugin{pluginKey, "no versions match"}
}

// Compare names, taking into account whether name is fully-qualified or not.
shortName := plugin.GetShortName(name)
if name == shortName {
// Case: if plugin name is short, find matching short names.
resolved = findPluginsMatchingShortName(resolved, shortName)
resolved = findPluginsMatchingShortName(versionedPlugins, shortName)
} else {
// Case: if plugin name is fully-qualified, match only fully-qualified names.
resolved = findPluginsMatchingName(resolved, name)
resolved = findPluginsMatchingName(versionedPlugins, name)
}

if len(resolved) == 0 {
return nil, errAmbiguousPlugin{pluginKey, "no names match"}
}

// Since plugins has already been resolved by matching names and versions,
// it should only contain one matching value for a versionless pluginKey if
// it isn't ambiguous.
if version == "" {
if len(resolved) == 1 {
return resolved, nil
return nil, errAmbiguousPlugin{
key: pluginKey,
msg: fmt.Sprintf("no names match, possible plugins: %+q", makePluginKeySlice(versionedPlugins...)),
}
return nil, errAmbiguousPlugin{pluginKey, fmt.Sprintf("possible keys: %+q", makePluginKeySlice(resolved...))}
}

rp, err := resolveToPlugin(resolved)
if err != nil {
return nil, errAmbiguousPlugin{pluginKey, err.Error()}
}
return []plugin.Base{rp}, nil
}

// findPluginsMatchingVersion returns a set of plugins with Version() matching
// version. The set will contain plugins with either major and minor versions
// matching exactly or major versions matching exactly and greater minor versions,
// but not a mix of the two match types.
func findPluginsMatchingVersion(plugins []plugin.Base, version string) []plugin.Base {
// Assume versions have been validated already.
v := must(semver.ParseTolerant(version))

var equal, matchingMajor []plugin.Base
for _, p := range plugins {
pv := must(semver.ParseTolerant(p.Version()))
if v.Major == pv.Major {
if v.Minor == pv.Minor {
equal = append(equal, p)
} else if v.Minor < pv.Minor {
matchingMajor = append(matchingMajor, p)
if version != "" {
// Case: if plugin key has version, filter by version.
v, err := plugin.ParseVersion(version)
if err != nil {
return nil, err
}
keys := makePluginKeySlice(resolved...)
for i := 0; i < len(resolved); i++ {
if v.Compare(resolved[i].Version()) != 0 {
resolved = append(resolved[:i], resolved[i+1:]...)
i--
}
}
if len(resolved) == 0 {
return nil, errAmbiguousPlugin{
key: pluginKey,
msg: fmt.Sprintf("no versions match, possible plugins: %+q", keys),
}
}
}

if len(equal) != 0 {
return equal
}
return matchingMajor
}

// must wraps semver.Parse and panics if err is non-nil.
func must(v semver.Version, err error) semver.Version {
if err != nil {
panic(err)
// Since plugins has already been resolved by matching names and versions,
// it should only contain one matching value if it isn't ambiguous.
if len(resolved) != 1 {
return nil, errAmbiguousPlugin{
key: pluginKey,
msg: fmt.Sprintf("matching plugins: %+q", makePluginKeySlice(resolved...)),
}
}
return v
return resolved, nil
}

// findPluginsMatchingName returns a set of plugins with Name() exactly
Expand All @@ -152,56 +122,45 @@ func findPluginsMatchingShortName(plugins []plugin.Base, shortName string) (equa
return equal
}

// resolveToPlugin returns a single plugin from plugins given the following
// conditions about plugins:
// 1. len(plugins) > 0.
// 2. No two plugin names are different.
// An error is returned if either condition is invalidated.
func resolveToPlugin(plugins []plugin.Base) (rp plugin.Base, err error) {
// Versions are either an exact match or have greater minor versions, so
// we choose the last in a sorted list of versions to get the correct one.
versions := make([]semver.Version, len(plugins))
for i, p := range plugins {
versions[i] = must(semver.ParseTolerant(p.Version()))
}
if len(versions) == 0 {
return nil, fmt.Errorf("possible versions: %+q", versions)
// makePluginKeySlice returns a slice of all keys for each plugin in plugins.
func makePluginKeySlice(plugins ...plugin.Base) (keys []string) {
for _, p := range plugins {
keys = append(keys, plugin.KeyFor(p))
}
semver.Sort(versions)
useVersion := versions[len(versions)-1]
sort.Strings(keys)
return
}

// If more than one name in plugins exists, the name portion of pluginKey
// needs to be more specific.
nameSet := make(map[string]struct{})
// validatePlugins validates the name and versions of a list of plugins.
func validatePlugins(plugins ...plugin.Base) error {
pluginKeySet := make(map[string]struct{}, len(plugins))
for _, p := range plugins {
nameSet[p.Name()] = struct{}{}
// This condition will only be true once for an unambiguous plugin name,
// since plugin keys have been checked for duplicates already.
if must(semver.ParseTolerant(p.Version())).Equals(useVersion) {
rp = p
if err := validatePlugin(p); err != nil {
return err
}
// Check for duplicate plugin keys.
pluginKey := plugin.KeyFor(p)
if _, seen := pluginKeySet[pluginKey]; seen {
return fmt.Errorf("two plugins have the same key: %q", pluginKey)
}
pluginKeySet[pluginKey] = struct{}{}
}
if len(nameSet) != 1 {
return nil, fmt.Errorf("possible names: %+q", makeKeySlice(nameSet))
}

return rp, nil
return nil
}

// makeKeySlice returns a slice of all map keys in set.
func makeKeySlice(set map[string]struct{}) (keys []string) {
for key := range set {
keys = append(keys, key)
// validatePlugin validates the name and versions of a plugin.
func validatePlugin(p plugin.Base) error {
pluginName := p.Name()
if err := plugin.ValidateName(pluginName); err != nil {
return fmt.Errorf("invalid plugin name %q: %v", pluginName, err)
}
sort.Strings(keys)
return
}

// makePluginKeySlice returns a slice of all keys for each plugin in plugins.
func makePluginKeySlice(plugins ...plugin.Base) (keys []string) {
for _, p := range plugins {
keys = append(keys, plugin.KeyFor(p))
if err := p.Version().Validate(); err != nil {
return fmt.Errorf("invalid plugin version %q: %v", p.Version(), err)
}
sort.Strings(keys)
return
for _, projectVersion := range p.SupportedProjectVersions() {
if err := validation.ValidateProjectVersion(projectVersion); err != nil {
return fmt.Errorf("invalid project version %q: %v", projectVersion, err)
}
}
return nil
}
Loading

0 comments on commit 977eb88

Please sign in to comment.