Skip to content

Commit

Permalink
Fix issues found during review
Browse files Browse the repository at this point in the history
  • Loading branch information
pkosiec committed Jan 31, 2023
1 parent 6825658 commit b9fda3b
Show file tree
Hide file tree
Showing 13 changed files with 30 additions and 36 deletions.
4 changes: 2 additions & 2 deletions internal/plugin/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var pluginMap = map[string]plugin.Plugin{
type Manager struct {
isStarted atomic.Bool
log logrus.FieldLogger
cfg config.Plugins
cfg config.PluginManagement
httpClient *http.Client

executorsToEnable []string
Expand All @@ -57,7 +57,7 @@ type Manager struct {
}

// NewManager returns a new Manager instance.
func NewManager(logger logrus.FieldLogger, cfg config.Plugins, executors, sources []string) *Manager {
func NewManager(logger logrus.FieldLogger, cfg config.PluginManagement, executors, sources []string) *Manager {
return &Manager{
cfg: cfg,
httpClient: newHTTPClient(),
Expand Down
2 changes: 1 addition & 1 deletion internal/plugin/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestCollectEnabledRepositories(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// given
manager := NewManager(loggerx.NewNoop(), config.Plugins{
manager := NewManager(loggerx.NewNoop(), config.PluginManagement{
Repositories: tc.definedRepositories,
}, tc.enabledExecutors, tc.enabledSources)

Expand Down
24 changes: 12 additions & 12 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,15 @@ type Config struct {
Aliases Aliases `yaml:"aliases" validate:"dive"`
Communications map[string]Communications `yaml:"communications" validate:"required,min=1,dive"`

Filters Filters `yaml:"filters"`
Analytics Analytics `yaml:"analytics"`
Settings Settings `yaml:"settings"`
ConfigWatcher CfgWatcher `yaml:"configWatcher"`
Plugins Plugins `yaml:"plugins"`
Filters Filters `yaml:"filters"`
Analytics Analytics `yaml:"analytics"`
Settings Settings `yaml:"settings"`
ConfigWatcher CfgWatcher `yaml:"configWatcher"`
Plugins PluginManagement `yaml:"plugins"`
}

// Plugins holds Botkube plugins related configuration.
type Plugins struct {
// PluginManagement holds Botkube plugin management related configuration.
type PluginManagement struct {
CacheDir string `yaml:"cacheDir"`
Repositories map[string]PluginsRepositories `yaml:"repositories"`
}
Expand Down Expand Up @@ -210,7 +210,7 @@ type ActionBindings struct {
type Sources struct {
DisplayName string `yaml:"displayName"`
Kubernetes KubernetesSource `yaml:"kubernetes"`
Plugins PluginsMap `koanf:",remain"`
Plugins Plugins `koanf:",remain"`
}

// KubernetesSource contains configuration for Kubernetes sources.
Expand Down Expand Up @@ -290,8 +290,8 @@ type IngressRecommendations struct {
TLSSecretValid *bool `yaml:"tlsSecretValid,omitempty"`
}

// PluginsMap contains plugins configuration parameters defined in groups.
type PluginsMap map[string]Plugin
// Plugins contains plugins configuration parameters defined in groups.
type Plugins map[string]Plugin

// Plugin contains plugin specific configuration.
type Plugin struct {
Expand All @@ -301,8 +301,8 @@ type Plugin struct {

// Executors contains executors configuration parameters.
type Executors struct {
Kubectl Kubectl `yaml:"kubectl"`
Plugins PluginsMap `koanf:",remain"`
Kubectl Kubectl `yaml:"kubectl"`
Plugins Plugins `koanf:",remain"`
}

// Aliases contains aliases configuration.
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestLoadConfigSuccess(t *testing.T) {

func TestLoadConfigWithPlugins(t *testing.T) {
// given
expSourcePlugin := config.PluginsMap{
expSourcePlugin := config.Plugins{
"botkube/keptn": {
Enabled: true,
Config: map[string]interface{}{
Expand All @@ -62,7 +62,7 @@ func TestLoadConfigWithPlugins(t *testing.T) {
},
}

expExecutorPlugin := config.PluginsMap{
expExecutorPlugin := config.Plugins{
"botkube/echo": {
Enabled: true,
Config: map[string]interface{}{
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func validateBindPlugins(sl validator.StructLevel, enabledPluginsViaBindings []s
}
}

func validatePlugins(sl validator.StructLevel, pluginConfigs PluginsMap) {
func validatePlugins(sl validator.StructLevel, pluginConfigs Plugins) {
var enabledPluginsViaBindings []string
for pluginKey, plugin := range pluginConfigs {
if !plugin.Enabled {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ executors:
'kubectl-read-only':
kubectl:
enabled: true
helm:
'helm':
botkube/helm:
config: {}
enabled: true
Expand Down
4 changes: 0 additions & 4 deletions pkg/execute/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ func (e *AliasExecutor) getTabularOutput(bindings []string) string {
fmt.Fprintf(w, "%s\t%s\t%s\n", aliasName, aliasCfg.Command, aliasCfg.DisplayName)
}

if len(aliasesToDisplay) == 0 {
fmt.Fprintln(w, "No aliases found for current conversation.")
}

w.Flush()
return buf.String()
}
8 changes: 4 additions & 4 deletions pkg/execute/alias_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func fixAliasCfg() config.Config {
Kubectl: config.Kubectl{
Enabled: true,
},
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"gh": config.Plugin{
Enabled: false,
},
Expand All @@ -92,14 +92,14 @@ func fixAliasCfg() config.Config {
Kubectl: config.Kubectl{
Enabled: false,
},
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"gh": config.Plugin{
Enabled: true,
},
},
},
"plugins": {
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"botkube/helm": config.Plugin{
Enabled: true,
},
Expand All @@ -109,7 +109,7 @@ func fixAliasCfg() config.Config {
},
},
"other": {
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"botkube/[email protected]": config.Plugin{
Enabled: true,
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/execute/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ func TestExecutorBindingsExecutor(t *testing.T) {
},
},
"botkube/helm": {
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"botkube/helm": config.Plugin{
Enabled: true,
},
},
},
"botkube/[email protected]": {
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"botkube/[email protected]": config.Plugin{
Enabled: true,
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/execute/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (e *Kubectl) CanHandle(args []string) bool {

// make sure that verb is also specified
// empty `k|kc|kubectl` commands are handled by command builder
return len(args) >= 2 && args[0] == kubectlName
return len(args) >= 2 && args[0] == kubectlCommandName
}

// GetCommandPrefix gets verb command with k8s alias prefix.
Expand All @@ -95,7 +95,7 @@ func (e *Kubectl) getArgsWithoutAlias(msg string) ([]string, error) {
return nil, fmt.Errorf("while parsing the command message into args: %w", err)
}

if len(msgParts) >= 2 && msgParts[0] == kubectlName {
if len(msgParts) >= 2 && msgParts[0] == kubectlCommandName {
return msgParts[1:], nil
}

Expand Down
4 changes: 1 addition & 3 deletions pkg/execute/kubectl_cmd_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ var knownCmdPrefix = map[string]struct{}{
filterPlaintextInputCommand: {},
}

const kubectlName = "kubectl"

var errRequiredVerbDropdown = errors.New("verbs dropdown select cannot be empty")

type (
Expand Down Expand Up @@ -89,7 +87,7 @@ func (e *KubectlCmdBuilder) GetCommandPrefix(args []string) string {

case 1:
// check if it's only a kubectl command without arguments
if args[0] == kubectlName {
if args[0] == kubectlCommandName {
return args[0]
}
// it is a single arg which cannot start the kubectl command builder
Expand Down
2 changes: 1 addition & 1 deletion pkg/execute/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestSourceExecutor(t *testing.T) {
},
"plugins": {
DisplayName: "plugin-a",
Plugins: config.PluginsMap{
Plugins: config.Plugins{
"plugin-a": {
Enabled: true,
},
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/bots_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func runBotTest(t *testing.T,

// Those are a temporary tests. When we will extract kubectl and kubernetes as plugins
// they won't be needed anymore.
t.Run("Botkube Plugins", func(t *testing.T) {
t.Run("Botkube PluginManagement", func(t *testing.T) {
t.Run("Echo Executor success", func(t *testing.T) {
command := "echo test"
expectedBody := codeBlock(strings.ToUpper(command))
Expand Down

0 comments on commit b9fda3b

Please sign in to comment.