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

kubectl: Fix golint warnings for plugins/* #51684

Merged
merged 5 commits into from
Sep 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion hack/.golint_failures
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ pkg/kubectl/cmd/util/editor
pkg/kubectl/cmd/util/jsonmerge
pkg/kubectl/cmd/util/sanity
pkg/kubectl/metricsutil
pkg/kubectl/plugins
pkg/kubectl/resource
pkg/kubectl/testing
pkg/kubectl/util
Expand Down
4 changes: 2 additions & 2 deletions pkg/kubectl/cmd/util/factory_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,10 @@ func (f *ring2Factory) NewUnstructuredBuilder(allowRemoteCalls bool) (*resource.
// system directory structure spec for the given platform.
func (f *ring2Factory) PluginLoader() plugins.PluginLoader {
if len(os.Getenv("KUBECTL_PLUGINS_PATH")) > 0 {
return plugins.PluginsEnvVarPluginLoader()
return plugins.KubectlPluginsPathPluginLoader()
}
return plugins.TolerantMultiPluginLoader{
plugins.XDGDataPluginLoader(),
plugins.XDGDataDirsPluginLoader(),
plugins.UserDirPluginLoader(),
}
}
Expand Down
26 changes: 20 additions & 6 deletions pkg/kubectl/plugins/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,21 @@ import (
"github.com/spf13/pflag"
)

// Env represents an environment variable with its name and value
// Env represents an environment variable with its name and value.
type Env struct {
N string
V string
}

// String returns "name=value" string.
func (e Env) String() string {
return fmt.Sprintf("%s=%s", e.N, e.V)
}

// EnvList is a list of Env
// EnvList is a list of Env.
type EnvList []Env

// Slice returns a slice of "name=value" strings.
func (e EnvList) Slice() []string {
envs := []string{}
for _, env := range e {
Expand All @@ -45,6 +47,7 @@ func (e EnvList) Slice() []string {
return envs
}

// Merge converts "name=value" strings into Env values and merges them into e.
func (e EnvList) Merge(s ...string) EnvList {
newList := e
newList = append(newList, fromSlice(s)...)
Expand All @@ -53,12 +56,14 @@ func (e EnvList) Merge(s ...string) EnvList {

// EnvProvider provides the environment in which the plugin will run.
type EnvProvider interface {
// Env returns the env list.
Env() (EnvList, error)
}

// MultiEnvProvider is an EnvProvider for multiple env providers, returns on first error.
// MultiEnvProvider satisfies the EnvProvider interface for multiple env providers.
type MultiEnvProvider []EnvProvider

// Env returns the combined env list of multiple env providers, returns on first error.
func (p MultiEnvProvider) Env() (EnvList, error) {
env := EnvList{}
for _, provider := range p {
Expand All @@ -71,9 +76,10 @@ func (p MultiEnvProvider) Env() (EnvList, error) {
return env, nil
}

// PluginCallerEnvProvider provides env with the path to the caller binary (usually full path to 'kubectl').
// PluginCallerEnvProvider satisfies the EnvProvider interface.
type PluginCallerEnvProvider struct{}

// Env returns env with the path to the caller binary (usually full path to 'kubectl').
func (p *PluginCallerEnvProvider) Env() (EnvList, error) {
caller, err := os.Executable()
if err != nil {
Expand All @@ -84,11 +90,12 @@ func (p *PluginCallerEnvProvider) Env() (EnvList, error) {
}, nil
}

// PluginDescriptorEnvProvider provides env vars with information about the running plugin.
// PluginDescriptorEnvProvider satisfies the EnvProvider interface.
type PluginDescriptorEnvProvider struct {
Plugin *Plugin
}

// Env returns env with information about the running plugin.
func (p *PluginDescriptorEnvProvider) Env() (EnvList, error) {
if p.Plugin == nil {
return []Env{}, fmt.Errorf("plugin not present to extract env")
Expand All @@ -104,19 +111,24 @@ func (p *PluginDescriptorEnvProvider) Env() (EnvList, error) {
return env, nil
}

// OSEnvProvider provides current environment from the operating system.
// OSEnvProvider satisfies the EnvProvider interface.
type OSEnvProvider struct{}

// Env returns the current environment from the operating system.
func (p *OSEnvProvider) Env() (EnvList, error) {
return fromSlice(os.Environ()), nil
}

// EmptyEnvProvider satisfies the EnvProvider interface.
type EmptyEnvProvider struct{}

// Env returns an empty environment.
func (p *EmptyEnvProvider) Env() (EnvList, error) {
return EnvList{}, nil
}

// FlagToEnvName converts a flag string into a UNIX like environment variable name.
// e.g --some-flag => "PREFIX_SOME_FLAG"
func FlagToEnvName(flagName, prefix string) string {
envName := strings.TrimPrefix(flagName, "--")
envName = strings.ToUpper(envName)
Expand All @@ -125,6 +137,8 @@ func FlagToEnvName(flagName, prefix string) string {
return envName
}

// FlagToEnv converts a flag and its value into an Env.
// e.g --some-flag some-value => Env{N: "PREFIX_SOME_FLAG", V="SOME_VALUE"}
func FlagToEnv(flag *pflag.Flag, prefix string) Env {
envName := FlagToEnvName(flag.Name, prefix)
return Env{envName, flag.Value.String()}
Expand Down
19 changes: 12 additions & 7 deletions pkg/kubectl/plugins/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ import (
"k8s.io/client-go/tools/clientcmd"
)

// PluginDescriptorFilename is the default file name for plugin descriptions.
const PluginDescriptorFilename = "plugin.yaml"

// PluginLoader is capable of loading a list of plugin descriptions.
type PluginLoader interface {
// Load loads the plugin descriptions.
Load() (Plugins, error)
}

Expand Down Expand Up @@ -107,7 +109,7 @@ func (l *DirectoryPluginLoader) Load() (Plugins, error) {
return list, err
}

// UserDirPluginLoader is a PluginLoader that loads plugins from the
// UserDirPluginLoader returns a PluginLoader that loads plugins from the
// "plugins" directory under the user's kubeconfig dir (usually "~/.kube/plugins/").
func UserDirPluginLoader() PluginLoader {
dir := filepath.Join(clientcmd.RecommendedConfigDir, "plugins")
Expand All @@ -116,7 +118,7 @@ func UserDirPluginLoader() PluginLoader {
}
}

// PathFromEnvVarPluginLoader is a PluginLoader that loads plugins from one or more
// PathFromEnvVarPluginLoader returns a PluginLoader that loads plugins from one or more
// directories specified by the provided env var name. In case the env var is not
// set, the PluginLoader just loads nothing. A list of subdirectories can be provided,
// which will be appended to each path specified by the env var.
Expand All @@ -135,17 +137,17 @@ func PathFromEnvVarPluginLoader(envVarName string, subdirs ...string) PluginLoad
return loader
}

// PluginsEnvVarPluginLoader is a PluginLoader that loads plugins from one or more
// KubectlPluginsPathPluginLoader returns a PluginLoader that loads plugins from one or more
// directories specified by the KUBECTL_PLUGINS_PATH env var.
func PluginsEnvVarPluginLoader() PluginLoader {
func KubectlPluginsPathPluginLoader() PluginLoader {
return PathFromEnvVarPluginLoader("KUBECTL_PLUGINS_PATH")
}

// XDGDataPluginLoader is a PluginLoader that loads plugins from one or more
// XDGDataDirsPluginLoader returns a PluginLoader that loads plugins from one or more
// directories specified by the XDG system directory structure spec in the
// XDG_DATA_DIRS env var, plus the "kubectl/plugins/" suffix. According to the
// spec, if XDG_DATA_DIRS is not set it defaults to "/usr/local/share:/usr/share".
func XDGDataPluginLoader() PluginLoader {
func XDGDataDirsPluginLoader() PluginLoader {
envVarName := "XDG_DATA_DIRS"
if len(os.Getenv(envVarName)) > 0 {
return PathFromEnvVarPluginLoader(envVarName, "kubectl", "plugins")
Expand All @@ -164,6 +166,7 @@ func XDGDataPluginLoader() PluginLoader {
// a successful loading means every encapsulated loader was able to load without errors.
type MultiPluginLoader []PluginLoader

// Load calls Load() for each of the encapsulated Loaders.
func (l MultiPluginLoader) Load() (Plugins, error) {
plugins := Plugins{}
for _, loader := range l {
Expand All @@ -180,6 +183,7 @@ func (l MultiPluginLoader) Load() (Plugins, error) {
// but is tolerant to errors while loading from them.
type TolerantMultiPluginLoader []PluginLoader

// Load calls Load() for each of the encapsulated Loaders.
func (l TolerantMultiPluginLoader) Load() (Plugins, error) {
plugins := Plugins{}
for _, loader := range l {
Expand All @@ -191,9 +195,10 @@ func (l TolerantMultiPluginLoader) Load() (Plugins, error) {
return plugins, nil
}

// DummyPluginLoader loads nothing.
// DummyPluginLoader is a noop PluginLoader.
type DummyPluginLoader struct{}

// Load loads nothing.
func (l *DummyPluginLoader) Load() (Plugins, error) {
return Plugins{}, nil
}
4 changes: 2 additions & 2 deletions pkg/kubectl/plugins/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestUnexistentDirectoryPluginLoader(t *testing.T) {
}
}

func TestPluginsEnvVarPluginLoader(t *testing.T) {
func TestKubectlPluginsPathPluginLoader(t *testing.T) {
tmp, err := setupValidPlugins(1, 0)
if err != nil {
t.Fatalf("unexpected error: %v", err)
Expand All @@ -120,7 +120,7 @@ func TestPluginsEnvVarPluginLoader(t *testing.T) {
os.Setenv(env, tmp)
defer os.Unsetenv(env)

loader := PluginsEnvVarPluginLoader()
loader := KubectlPluginsPathPluginLoader()

plugins, err := loader.Load()
if err != nil {
Expand Down
34 changes: 22 additions & 12 deletions pkg/kubectl/plugins/plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,16 @@ import (
)

var (
IncompletePluginError = fmt.Errorf("incomplete plugin descriptor: name, shortDesc and command fields are required")
InvalidPluginNameError = fmt.Errorf("plugin name can't contain spaces")
IncompleteFlagError = fmt.Errorf("incomplete flag descriptor: name and desc fields are required")
InvalidFlagNameError = fmt.Errorf("flag name can't contain spaces")
InvalidFlagShorthandError = fmt.Errorf("flag shorthand must be only one letter")
// ErrIncompletePlugin indicates plugin is incomplete.
ErrIncompletePlugin = fmt.Errorf("incomplete plugin descriptor: name, shortDesc and command fields are required")
// ErrInvalidPluginName indicates plugin name is invalid.
ErrInvalidPluginName = fmt.Errorf("plugin name can't contain spaces")
// ErrIncompleteFlag indicates flag is incomplete.
ErrIncompleteFlag = fmt.Errorf("incomplete flag descriptor: name and desc fields are required")
// ErrInvalidFlagName indicates flag name is invalid.
ErrInvalidFlagName = fmt.Errorf("flag name can't contain spaces")
// ErrInvalidFlagShorthand indicates flag shorthand is invalid.
ErrInvalidFlagShorthand = fmt.Errorf("flag shorthand must be only one letter")
)

// Plugin is the representation of a CLI extension (plugin).
Expand All @@ -37,7 +42,7 @@ type Plugin struct {
Context RunningContext `json:"-"`
}

// PluginDescription holds everything needed to register a
// Description holds everything needed to register a
// plugin as a command. Usually comes from a descriptor file.
type Description struct {
Name string `json:"name"`
Expand All @@ -49,18 +54,19 @@ type Description struct {
Tree Plugins `json:"tree,omitempty"`
}

// PluginSource holds the location of a given plugin in the filesystem.
// Source holds the location of a given plugin in the filesystem.
type Source struct {
Dir string `json:"-"`
DescriptorName string `json:"-"`
}

// Validate validates plugin data.
func (p Plugin) Validate() error {
if len(p.Name) == 0 || len(p.ShortDesc) == 0 || (len(p.Command) == 0 && len(p.Tree) == 0) {
return IncompletePluginError
return ErrIncompletePlugin
}
if strings.Index(p.Name, " ") > -1 {
return InvalidPluginNameError
return ErrInvalidPluginName
}
for _, flag := range p.Flags {
if err := flag.Validate(); err != nil {
Expand All @@ -75,6 +81,7 @@ func (p Plugin) Validate() error {
return nil
}

// IsValid returns true if plugin data is valid.
func (p Plugin) IsValid() bool {
return p.Validate() == nil
}
Expand All @@ -90,24 +97,27 @@ type Flag struct {
DefValue string `json:"defValue,omitempty"`
}

// Validate validates flag data.
func (f Flag) Validate() error {
if len(f.Name) == 0 || len(f.Desc) == 0 {
return IncompleteFlagError
return ErrIncompleteFlag
}
if strings.Index(f.Name, " ") > -1 {
return InvalidFlagNameError
return ErrInvalidFlagName
}
return f.ValidateShorthand()
}

// ValidateShorthand validates flag shorthand data.
func (f Flag) ValidateShorthand() error {
length := len(f.Shorthand)
if length == 0 || (length == 1 && unicode.IsLetter(rune(f.Shorthand[0]))) {
return nil
}
return InvalidFlagShorthandError
return ErrInvalidFlagShorthand
}

// Shorthanded returns true if flag shorthand data is valid.
func (f Flag) Shorthanded() bool {
return f.ValidateShorthand() == nil
}
14 changes: 7 additions & 7 deletions pkg/kubectl/plugins/plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ func TestPlugin(t *testing.T) {
ShortDesc: "The test",
},
},
expectedErr: IncompletePluginError,
expectedErr: ErrIncompletePlugin,
},
{
plugin: &Plugin{},
expectedErr: IncompletePluginError,
expectedErr: ErrIncompletePlugin,
},
{
plugin: &Plugin{
Expand All @@ -53,7 +53,7 @@ func TestPlugin(t *testing.T) {
Command: "echo 1",
},
},
expectedErr: InvalidPluginNameError,
expectedErr: ErrInvalidPluginName,
},
{
plugin: &Plugin{
Expand All @@ -68,7 +68,7 @@ func TestPlugin(t *testing.T) {
},
},
},
expectedErr: IncompleteFlagError,
expectedErr: ErrIncompleteFlag,
},
{
plugin: &Plugin{
Expand All @@ -84,7 +84,7 @@ func TestPlugin(t *testing.T) {
},
},
},
expectedErr: InvalidFlagNameError,
expectedErr: ErrInvalidFlagName,
},
{
plugin: &Plugin{
Expand All @@ -101,7 +101,7 @@ func TestPlugin(t *testing.T) {
},
},
},
expectedErr: InvalidFlagShorthandError,
expectedErr: ErrInvalidFlagShorthand,
},
{
plugin: &Plugin{
Expand All @@ -118,7 +118,7 @@ func TestPlugin(t *testing.T) {
},
},
},
expectedErr: InvalidFlagShorthandError,
expectedErr: ErrInvalidFlagShorthand,
},
{
plugin: &Plugin{
Expand Down